From 0f03552052debaf5d4edda7dbf837a12f3812f22 Mon Sep 17 00:00:00 2001 From: yanglinshu Date: Wed, 13 Mar 2024 12:12:06 +0800 Subject: [PATCH 1/5] Add test case for #6040 --- tests/source/issue_6040.rs | 21 +++++++++++++++++++++ tests/target/issue_6040.rs | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tests/source/issue_6040.rs create mode 100644 tests/target/issue_6040.rs diff --git a/tests/source/issue_6040.rs b/tests/source/issue_6040.rs new file mode 100644 index 00000000000..a95104a7e12 --- /dev/null +++ b/tests/source/issue_6040.rs @@ -0,0 +1,21 @@ +fn main() { + match e { // this line still gets formatted + MyStruct { + field_a, + .. // this comment here apparently causes trouble + } => (), + _ => (), // this line is no longer formatted + }; +} + +fn main() { + match e { // this line still gets formatted + MyStruct { + field_a, + field_b, // this should be aligned with field_a + field_c, // this should be aligned with field_a + } => (), + + _ => (), // this line is no longer formatted + }; +} diff --git a/tests/target/issue_6040.rs b/tests/target/issue_6040.rs new file mode 100644 index 00000000000..ffce4f93f1c --- /dev/null +++ b/tests/target/issue_6040.rs @@ -0,0 +1,23 @@ +fn main() { + match e { + // this line still gets formatted + MyStruct { + field_a, + .. // this comment here apparently causes trouble + } => (), + _ => (), // this line is no longer formatted + }; +} + +fn main() { + match e { + // this line still gets formatted + MyStruct { + field_a, + field_b, // this should be aligned with field_a + field_c, // this should be aligned with field_a + } => (), + + _ => (), // this line is no longer formatted + }; +} From 22b219086ab06b4147c10ee37a6d6098c428826b Mon Sep 17 00:00:00 2001 From: yanglinshu Date: Wed, 13 Mar 2024 23:47:30 +0800 Subject: [PATCH 2/5] Adopt rewrite_struct_lit to rewrite_struct_pat --- src/patterns.rs | 168 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 122 insertions(+), 46 deletions(-) diff --git a/src/patterns.rs b/src/patterns.rs index d8cb26a20f1..2dc19bba127 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -5,7 +5,7 @@ use rustc_span::{BytePos, Span}; use crate::comment::{combine_strs_with_missing_comments, FindUncommented}; use crate::config::lists::*; use crate::config::Version; -use crate::expr::{can_be_overflowed_expr, rewrite_unary_prefix, wrap_struct_field}; +use crate::expr::{can_be_overflowed_expr, rewrite_unary_prefix, span_ends_with_comma, wrap_struct_field}; use crate::lists::{ definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape, struct_lit_tactic, write_list, ListFormatting, ListItem, Separator, @@ -313,35 +313,68 @@ fn rewrite_struct_pat( context: &RewriteContext<'_>, shape: Shape, ) -> Option { + debug!("rewrite_struct_pat: shape: {:?}", shape); + + enum StructPatField<'a> { + Regular(&'a ast::PatField), + Rest(Span), + } + // 2 = ` {` let path_shape = shape.sub_width(2)?; let path_str = rewrite_path(context, PathContext::Expr, qself, path, path_shape)?; - if fields.is_empty() && !ellipsis { - return Some(format!("{path_str} {{}}")); - } + if fields.is_empty() { + if ellipsis { + return Some(format!("{} {{ .. }}", path_str)); + } - let (ellipsis_str, terminator) = if ellipsis { (", ..", "..") } else { ("", "}") }; + return Some(format!("{} {{}}", path_str)); + } // 3 = ` { `, 2 = ` }`. let (h_shape, v_shape) = - struct_lit_shape(shape, context, path_str.len() + 3, ellipsis_str.len() + 2)?; + struct_lit_shape(shape, context, path_str.len() + 3, 2)?; + + let one_line_width = h_shape.map_or(0, |shape| shape.width); + let body_lo = context.snippet_provider.span_after(span, "{"); + + let field_iter = fields + .iter() + .map(StructPatField::Regular) + .chain(match ellipsis { + false => None, + true => { // 2 = `..`.len() + let last_field_hi = fields.last().map_or(body_lo, |field| field.span.hi()); + let snippet = context.snippet(mk_sp(last_field_hi, span.hi())); + let rest_lo = last_field_hi + BytePos(snippet.find_uncommented("..").unwrap() as u32); + let rest_hi = rest_lo + BytePos(2); + Some(StructPatField::Rest(mk_sp(rest_lo, rest_hi))) + } + }); + + let span_lo = |item: &StructPatField<'_>| match *item { + StructPatField::Regular(f) => f.span.lo(), + StructPatField::Rest(span) => span.lo(), + }; + let span_hi = |item: &StructPatField<'_>| match *item { + StructPatField::Regular(f) => f.span.hi(), + StructPatField::Rest(span) => span.hi(), + }; + let rewrite = |item: &StructPatField<'_>| match *item { + StructPatField::Regular(f) => f.rewrite(context, v_shape), + StructPatField::Rest(_) => Some("..".to_owned()), + }; let items = itemize_list( context.snippet_provider, - fields.iter(), - terminator, + field_iter, + "}", ",", - |f| { - if f.attrs.is_empty() { - f.span.lo() - } else { - f.attrs.first().unwrap().span.lo() - } - }, - |f| f.span.hi(), - |f| f.rewrite(context, v_shape), - context.snippet_provider.span_after(span, "{"), + span_lo, + span_hi, + rewrite, + body_lo, span.hi(), false, ); @@ -349,37 +382,80 @@ fn rewrite_struct_pat( let tactic = struct_lit_tactic(h_shape, context, &item_vec); let nested_shape = shape_for_tactic(tactic, h_shape, v_shape); - let fmt = struct_lit_formatting(nested_shape, tactic, context, false); - - let mut fields_str = write_list(&item_vec, &fmt)?; - let one_line_width = h_shape.map_or(0, |shape| shape.width); - - let has_trailing_comma = fmt.needs_trailing_separator(); - if ellipsis { - if fields_str.contains('\n') || fields_str.len() > one_line_width { - // Add a missing trailing comma. - if !has_trailing_comma { - fields_str.push(','); - } - fields_str.push('\n'); - fields_str.push_str(&nested_shape.indent.to_string(context.config)); - } else { - if !fields_str.is_empty() { - // there are preceding struct fields being matched on - if has_trailing_comma { - fields_str.push(' '); - } else { - fields_str.push_str(", "); - } - } - } - fields_str.push_str(".."); - } + let fmt = struct_lit_formatting(nested_shape, tactic, context, ellipsis); + let fields_str = write_list(&item_vec, &fmt)?; - // ast::Pat doesn't have attrs so use &[] let fields_str = wrap_struct_field(context, &[], &fields_str, shape, v_shape, one_line_width)?; - Some(format!("{path_str} {{{fields_str}}}")) + Some(format!("{} {{{}}}", path_str, fields_str)) + + // // 2 = ` {` + // let path_shape = shape.sub_width(2)?; + // let path_str = rewrite_path(context, PathContext::Expr, qself, path, path_shape)?; + + // if fields.is_empty() && !ellipsis { + // return Some(format!("{path_str} {{}}")); + // } + + // let (ellipsis_str, terminator) = if ellipsis { (", ..", "..") } else { ("", "}") }; + + // // 3 = ` { `, 2 = ` }`. + // let (h_shape, v_shape) = + // struct_lit_shape(shape, context, path_str.len() + 3, ellipsis_str.len() + 2)?; + + // let items = itemize_list( + // context.snippet_provider, + // fields.iter(), + // terminator, + // ",", + // |f| { + // if f.attrs.is_empty() { + // f.span.lo() + // } else { + // f.attrs.first().unwrap().span.lo() + // } + // }, + // |f| f.span.hi(), + // |f| f.rewrite(context, v_shape), + // context.snippet_provider.span_after(span, "{"), + // span.hi(), + // false, + // ); + // let item_vec = items.collect::>(); + + // let tactic = struct_lit_tactic(h_shape, context, &item_vec); + // let nested_shape = shape_for_tactic(tactic, h_shape, v_shape); + // let fmt = struct_lit_formatting(nested_shape, tactic, context, false); + + // let mut fields_str = write_list(&item_vec, &fmt)?; + // let one_line_width = h_shape.map_or(0, |shape| shape.width); + + // let has_trailing_comma = fmt.needs_trailing_separator(); + + // if ellipsis { + // if fields_str.contains('\n') || fields_str.len() > one_line_width { + // // Add a missing trailing comma. + // if !has_trailing_comma { + // fields_str.push(','); + // } + // fields_str.push('\n'); + // fields_str.push_str(&nested_shape.indent.to_string(context.config)); + // } else { + // if !fields_str.is_empty() { + // // there are preceding struct fields being matched on + // if has_trailing_comma { + // fields_str.push(' '); + // } else { + // fields_str.push_str(", "); + // } + // } + // } + // fields_str.push_str(".."); + // } + + // // ast::Pat doesn't have attrs so use &[] + // let fields_str = wrap_struct_field(context, &[], &fields_str, shape, v_shape, one_line_width)?; + // Some(format!("{path_str} {{{fields_str}}}")) } impl Rewrite for PatField { From 52a93f3c0c831faf5b96cfe893ad15fa3bbc387d Mon Sep 17 00:00:00 2001 From: yanglinshu Date: Wed, 13 Mar 2024 23:48:12 +0800 Subject: [PATCH 3/5] Handle attributes in fields --- src/patterns.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/patterns.rs b/src/patterns.rs index 2dc19bba127..104207d5fac 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -333,8 +333,7 @@ fn rewrite_struct_pat( } // 3 = ` { `, 2 = ` }`. - let (h_shape, v_shape) = - struct_lit_shape(shape, context, path_str.len() + 3, 2)?; + let (h_shape, v_shape) = struct_lit_shape(shape, context, path_str.len() + 3, 2)?; let one_line_width = h_shape.map_or(0, |shape| shape.width); let body_lo = context.snippet_provider.span_after(span, "{"); @@ -344,17 +343,25 @@ fn rewrite_struct_pat( .map(StructPatField::Regular) .chain(match ellipsis { false => None, - true => { // 2 = `..`.len() + true => { + // 2 = `..`.len() let last_field_hi = fields.last().map_or(body_lo, |field| field.span.hi()); let snippet = context.snippet(mk_sp(last_field_hi, span.hi())); - let rest_lo = last_field_hi + BytePos(snippet.find_uncommented("..").unwrap() as u32); + let rest_lo = + last_field_hi + BytePos(snippet.find_uncommented("..").unwrap() as u32); let rest_hi = rest_lo + BytePos(2); Some(StructPatField::Rest(mk_sp(rest_lo, rest_hi))) } }); let span_lo = |item: &StructPatField<'_>| match *item { - StructPatField::Regular(f) => f.span.lo(), + StructPatField::Regular(f) => { + if f.attrs.is_empty() { + f.pat.span.lo() + } else { + f.attrs.first().unwrap().span.lo() + } + } StructPatField::Rest(span) => span.lo(), }; let span_hi = |item: &StructPatField<'_>| match *item { From daeec140e0cf0adc0ad568a85674eb5a8942ba1a Mon Sep 17 00:00:00 2001 From: yanglinshu Date: Thu, 14 Mar 2024 14:47:07 +0800 Subject: [PATCH 4/5] Recreate the behavior of mixed tactic from the old version --- src/patterns.rs | 87 ++++++++++--------------------------------------- 1 file changed, 17 insertions(+), 70 deletions(-) diff --git a/src/patterns.rs b/src/patterns.rs index 104207d5fac..3189c799cd7 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -5,7 +5,7 @@ use rustc_span::{BytePos, Span}; use crate::comment::{combine_strs_with_missing_comments, FindUncommented}; use crate::config::lists::*; use crate::config::Version; -use crate::expr::{can_be_overflowed_expr, rewrite_unary_prefix, span_ends_with_comma, wrap_struct_field}; +use crate::expr::{can_be_overflowed_expr, rewrite_unary_prefix, wrap_struct_field}; use crate::lists::{ definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape, struct_lit_tactic, write_list, ListFormatting, ListItem, Separator, @@ -387,7 +387,22 @@ fn rewrite_struct_pat( ); let item_vec = items.collect::>(); - let tactic = struct_lit_tactic(h_shape, context, &item_vec); + // FIXME: Figure out the way to determine when to use mixed tactic. The old + // code ignored it and directly uses the tactic for the fields. + let tactic = if ellipsis { + let field_item_vec = &item_vec[..item_vec.len() - 1]; + let field_tactic = struct_lit_tactic(h_shape, context, field_item_vec); + let rest_item_vec = &item_vec[item_vec.len() - 1..]; + let rest_tactic = struct_lit_tactic(h_shape, context, rest_item_vec); + match (field_tactic, rest_tactic) { + (DefinitiveListTactic::Vertical, _) | (_, DefinitiveListTactic::Vertical) => { + DefinitiveListTactic::Vertical + } + _ => field_tactic, + } + } else { + struct_lit_tactic(h_shape, context, &item_vec) + }; let nested_shape = shape_for_tactic(tactic, h_shape, v_shape); let fmt = struct_lit_formatting(nested_shape, tactic, context, ellipsis); @@ -395,74 +410,6 @@ fn rewrite_struct_pat( let fields_str = wrap_struct_field(context, &[], &fields_str, shape, v_shape, one_line_width)?; Some(format!("{} {{{}}}", path_str, fields_str)) - - // // 2 = ` {` - // let path_shape = shape.sub_width(2)?; - // let path_str = rewrite_path(context, PathContext::Expr, qself, path, path_shape)?; - - // if fields.is_empty() && !ellipsis { - // return Some(format!("{path_str} {{}}")); - // } - - // let (ellipsis_str, terminator) = if ellipsis { (", ..", "..") } else { ("", "}") }; - - // // 3 = ` { `, 2 = ` }`. - // let (h_shape, v_shape) = - // struct_lit_shape(shape, context, path_str.len() + 3, ellipsis_str.len() + 2)?; - - // let items = itemize_list( - // context.snippet_provider, - // fields.iter(), - // terminator, - // ",", - // |f| { - // if f.attrs.is_empty() { - // f.span.lo() - // } else { - // f.attrs.first().unwrap().span.lo() - // } - // }, - // |f| f.span.hi(), - // |f| f.rewrite(context, v_shape), - // context.snippet_provider.span_after(span, "{"), - // span.hi(), - // false, - // ); - // let item_vec = items.collect::>(); - - // let tactic = struct_lit_tactic(h_shape, context, &item_vec); - // let nested_shape = shape_for_tactic(tactic, h_shape, v_shape); - // let fmt = struct_lit_formatting(nested_shape, tactic, context, false); - - // let mut fields_str = write_list(&item_vec, &fmt)?; - // let one_line_width = h_shape.map_or(0, |shape| shape.width); - - // let has_trailing_comma = fmt.needs_trailing_separator(); - - // if ellipsis { - // if fields_str.contains('\n') || fields_str.len() > one_line_width { - // // Add a missing trailing comma. - // if !has_trailing_comma { - // fields_str.push(','); - // } - // fields_str.push('\n'); - // fields_str.push_str(&nested_shape.indent.to_string(context.config)); - // } else { - // if !fields_str.is_empty() { - // // there are preceding struct fields being matched on - // if has_trailing_comma { - // fields_str.push(' '); - // } else { - // fields_str.push_str(", "); - // } - // } - // } - // fields_str.push_str(".."); - // } - - // // ast::Pat doesn't have attrs so use &[] - // let fields_str = wrap_struct_field(context, &[], &fields_str, shape, v_shape, one_line_width)?; - // Some(format!("{path_str} {{{fields_str}}}")) } impl Rewrite for PatField { From a24689d8ee196508fe0844659b705a0d4b172d65 Mon Sep 17 00:00:00 2001 From: yanglinshu Date: Thu, 14 Mar 2024 23:40:14 +0800 Subject: [PATCH 5/5] Merge vertical lists whenever possible --- src/patterns.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/patterns.rs b/src/patterns.rs index 3189c799cd7..e552e0640ca 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -3,8 +3,8 @@ use rustc_ast::ptr; use rustc_span::{BytePos, Span}; use crate::comment::{combine_strs_with_missing_comments, FindUncommented}; -use crate::config::lists::*; use crate::config::Version; +use crate::config::{lists::*, IndentStyle}; use crate::expr::{can_be_overflowed_expr, rewrite_unary_prefix, wrap_struct_field}; use crate::lists::{ definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape, @@ -391,9 +391,25 @@ fn rewrite_struct_pat( // code ignored it and directly uses the tactic for the fields. let tactic = if ellipsis { let field_item_vec = &item_vec[..item_vec.len() - 1]; - let field_tactic = struct_lit_tactic(h_shape, context, field_item_vec); - let rest_item_vec = &item_vec[item_vec.len() - 1..]; - let rest_tactic = struct_lit_tactic(h_shape, context, rest_item_vec); + let mut field_tactic = struct_lit_tactic(h_shape, context, field_item_vec); + let rest_item_vec = &item_vec[item_vec.len() - 1..]; + let mut rest_tactic = struct_lit_tactic(h_shape, context, rest_item_vec); + + if !(context.config.struct_lit_single_line() + || context.config.indent_style() == IndentStyle::Visual && field_item_vec.len() == 1) + { + // FIXME: Figure out the exact behavior of mixed tactic. The old + // produces mixed tactic by ignoring the `..` when determining the + // tactic. See #5066 for an example. + let nested_shape = shape_for_tactic(field_tactic, h_shape, v_shape); + let fmt = struct_lit_formatting(nested_shape, field_tactic, context, false); + let fields_str = write_list(field_item_vec, &fmt)?; + if !(fields_str.contains('\n') || fields_str.len() > one_line_width) { + field_tactic = DefinitiveListTactic::Horizontal; + rest_tactic = DefinitiveListTactic::Horizontal; + } + } + match (field_tactic, rest_tactic) { (DefinitiveListTactic::Vertical, _) | (_, DefinitiveListTactic::Vertical) => { DefinitiveListTactic::Vertical