From 86679a9a761ce53bc5d2e4d163fabd46faa88911 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Fri, 12 Aug 2022 12:35:24 +0300 Subject: [PATCH] Backport #4406 - Formatting pre and post cast comments enhancements --- src/expr.rs | 38 +++++-- src/pairs.rs | 65 +++++++++--- src/types.rs | 2 +- tests/source/issue-4406-cast-comments/one.rs | 100 ++++++++++++++++++ tests/source/issue-4406-cast-comments/two.rs | 100 ++++++++++++++++++ tests/target/issue-4406-cast-comments/one.rs | 101 +++++++++++++++++++ tests/target/issue-4406-cast-comments/two.rs | 101 +++++++++++++++++++ 7 files changed, 484 insertions(+), 23 deletions(-) create mode 100644 tests/source/issue-4406-cast-comments/one.rs create mode 100644 tests/source/issue-4406-cast-comments/two.rs create mode 100644 tests/target/issue-4406-cast-comments/one.rs create mode 100644 tests/target/issue-4406-cast-comments/two.rs diff --git a/src/expr.rs b/src/expr.rs index c9d704765c2..fecefcf1085 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -239,14 +239,34 @@ pub(crate) fn format_expr( ast::ExprKind::AddrOf(borrow_kind, mutability, ref expr) => { rewrite_expr_addrof(context, borrow_kind, mutability, expr, shape) } - ast::ExprKind::Cast(ref expr, ref ty) => rewrite_pair( - &**expr, - &**ty, - PairParts::infix(" as "), - context, - shape, - SeparatorPlace::Front, - ), + ast::ExprKind::Cast(ref subexpr, ref ty) => { + /* Retrieving the comments before and after cast */ + let prefix_span = mk_sp( + subexpr.span.hi(), + context.snippet_provider.span_before(expr.span, "as"), + ); + let suffix_span = mk_sp( + context.snippet_provider.span_after(expr.span, "as"), + ty.span.lo(), + ); + let infix_prefix_comments = rewrite_missing_comment(prefix_span, shape, context)?; + let infix_suffix_comments = rewrite_missing_comment(suffix_span, shape, context)?; + + rewrite_pair( + &**subexpr, + &**ty, + PairParts::new( + "", + &infix_prefix_comments, + " as ", + &infix_suffix_comments, + "", + ), + context, + shape, + SeparatorPlace::Front, + ) + } ast::ExprKind::Type(ref expr, ref ty) => rewrite_pair( &**expr, &**ty, @@ -261,7 +281,7 @@ pub(crate) fn format_expr( ast::ExprKind::Repeat(ref expr, ref repeats) => rewrite_pair( &**expr, &*repeats.value, - PairParts::new("[", "; ", "]"), + PairParts::new("[", "", "; ", "", "]"), context, shape, SeparatorPlace::Back, diff --git a/src/pairs.rs b/src/pairs.rs index d1c75126ea4..4e55114df2e 100644 --- a/src/pairs.rs +++ b/src/pairs.rs @@ -12,7 +12,9 @@ use crate::utils::{ #[derive(new, Clone, Copy)] pub(crate) struct PairParts<'a> { prefix: &'a str, + infix_prefix: &'a str, /* mainly for pre-infix comments */ infix: &'a str, + infix_suffix: &'a str, /* mainly for post-infix comments */ suffix: &'a str, } @@ -20,7 +22,9 @@ impl<'a> PairParts<'a> { pub(crate) fn infix(infix: &'a str) -> PairParts<'a> { PairParts { prefix: "", + infix_prefix: "", infix, + infix_suffix: "", suffix: "", } } @@ -163,22 +167,32 @@ where RHS: Rewrite, { let tab_spaces = context.config.tab_spaces(); + let infix_result = format!("{}{}", pp.infix, pp.infix_suffix); + let infix_suffix_separator = if pp.infix_suffix.is_empty() { "" } else { " " }; + let infix_prefix_separator = if pp.infix_prefix.is_empty() { "" } else { " " }; let lhs_overhead = match separator_place { - SeparatorPlace::Back => shape.used_width() + pp.prefix.len() + pp.infix.trim_end().len(), + SeparatorPlace::Back => { + shape.used_width() + pp.prefix.len() + pp.infix.trim_end().len() + pp.infix_prefix.len() + } SeparatorPlace::Front => shape.used_width(), }; let lhs_shape = Shape { width: context.budget(lhs_overhead), ..shape }; - let lhs_result = lhs - .rewrite(context, lhs_shape) - .map(|lhs_str| format!("{}{}", pp.prefix, lhs_str))?; + let lhs_result = lhs.rewrite(context, lhs_shape).map(|lhs_str| { + format!( + "{}{}{}{}", + pp.prefix, lhs_str, infix_prefix_separator, pp.infix_prefix + ) + })?; // Try to put both lhs and rhs on the same line. let rhs_orig_result = shape .offset_left(last_line_width(&lhs_result) + pp.infix.len()) - .and_then(|s| s.sub_width(pp.suffix.len())) + .and_then(|s| { + s.sub_width(pp.suffix.len() + pp.infix_suffix.len() + infix_suffix_separator.len()) + }) .and_then(|rhs_shape| rhs.rewrite(context, rhs_shape)); if let Some(ref rhs_result) = rhs_orig_result { // If the length of the lhs is equal to or shorter than the tab width or @@ -192,13 +206,13 @@ where .unwrap_or(false); if !rhs_result.contains('\n') || allow_same_line { let one_line_width = last_line_width(&lhs_result) - + pp.infix.len() + + infix_result.len() + first_line_width(rhs_result) + pp.suffix.len(); if one_line_width <= shape.width { return Some(format!( - "{}{}{}{}", - lhs_result, pp.infix, rhs_result, pp.suffix + "{}{}{}{}{}", + lhs_result, infix_result, infix_suffix_separator, rhs_result, pp.suffix )); } } @@ -219,20 +233,45 @@ where }; let infix = match separator_place { SeparatorPlace::Back => pp.infix.trim_end(), - SeparatorPlace::Front => pp.infix.trim_start(), + SeparatorPlace::Front => { + if pp.infix_suffix.is_empty() { + pp.infix.trim_start() + } else { + pp.infix + } + } + }; + let infix_suffix = if separator_place == SeparatorPlace::Front && !pp.infix_suffix.is_empty() { + pp.infix_suffix.trim_start() + } else { + pp.infix_suffix }; if separator_place == SeparatorPlace::Front { rhs_shape = rhs_shape.offset_left(infix.len())?; } let rhs_result = rhs.rewrite(context, rhs_shape)?; let indent_str = rhs_shape.indent.to_string_with_newline(context.config); - let infix_with_sep = match separator_place { - SeparatorPlace::Back => format!("{}{}", infix, indent_str), - SeparatorPlace::Front => format!("{}{}", indent_str, infix), + let mut infix_with_sep = match separator_place { + SeparatorPlace::Back => format!("{}{}{}", infix, infix_suffix.trim_end(), indent_str), + SeparatorPlace::Front => format!( + "{}{}{}{}", + indent_str, + infix.trim_start(), + infix_suffix, + infix_suffix_separator + ), + }; + let new_line_width = infix_with_sep.len() - 1 + rhs_result.len() + pp.suffix.len(); + let rhs_with_sep = if separator_place == SeparatorPlace::Front && new_line_width > shape.width { + let s: String = String::from(infix_with_sep); + infix_with_sep = s.trim_end().to_string(); + format!("{}{}", indent_str, rhs_result.trim_start()) + } else { + rhs_result }; Some(format!( "{}{}{}{}", - lhs_result, infix_with_sep, rhs_result, pp.suffix + lhs_result, infix_with_sep, rhs_with_sep, pp.suffix )) } diff --git a/src/types.rs b/src/types.rs index 25ad587ba85..026bb4c859a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -805,7 +805,7 @@ impl Rewrite for ast::Ty { ast::TyKind::Array(ref ty, ref repeats) => rewrite_pair( &**ty, &*repeats.value, - PairParts::new("[", "; ", "]"), + PairParts::new("[", "", "; ", "", "]"), context, shape, SeparatorPlace::Back, diff --git a/tests/source/issue-4406-cast-comments/one.rs b/tests/source/issue-4406-cast-comments/one.rs new file mode 100644 index 00000000000..795635dbb91 --- /dev/null +++ b/tests/source/issue-4406-cast-comments/one.rs @@ -0,0 +1,100 @@ +// rustfmt-version: One + +/***** + * Tests for proper formatting of pre and post cast ("as") comments + ******/ + +// Test 1 +fn main() { + let x = 0f64 /* x as */ as i32; +} + +// Test 2 +fn main() { +let x = 1 /* foo as */ as i32; +} + +// Test 3 +fn main() { +let x = 1 as /* bar as */ i32; +} + +// Test 4 +fn main() { +let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 5 +fn main() { +let x = 1 /* as foo */as/* as bar */ i32; +} + +// Test 6 +fn main() { +let x = 1 /* as foo */ +as/* as bar */ +i32; +} + +// Test 7 +fn main() { +let x = 1 /* as foo yyyyyyyyyyy */as/* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ i32; +} + +// Test 8 +fn main() { +let x = 1 /* as foo yyyyyyyyyyy */as/* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ i32; +} + +// Test 9 +fn main() { +let x = 1 /* as foo yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy */ +as/* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ +i32; +} + + +/***** + * Tests for not leaving trailing spaces related to cast comments (related to #2896?) + ******/ +// Test 10 - extra blank after the binary rhs at the 2nd line (comment followws at 3rd line) +fn main() { + if 0 == 1 + /* x */ as i32 {} } + +// Test 11 - extra blank after the binary rhs at the end of 2nd line +fn main() { + if 0 == ' ' + as i32 {} } + +// Test 12 - extra blank after the comment at the end of 2nd line +fn main() { + if 0 == ' ' /* x */ + as i32 {} } + + +/***** + * Tests for not moving "as" to new line unnecessarily - from #3528 + ******/ +fn get_old_backends(old_toml_config: &toml::Value) -> Option>> { + old_toml_config.as_table().and_then(|table| { + table + .get("backends") + .and_then(|backends| backends.as_table()) + .map(|backends| { + backends + .into_iter() + .filter_map(|(key, value)| match AvailableBackend::from(key.as_str()) { + AvailableBackend::Git => Some(Box::new(Git { + config: value.clone().try_into::().unwrap(), + }) + as Box), + AvailableBackend::Github => Some(Box::new(Github { + config: value.clone().try_into::().unwrap(), + }) + as Box), + }) + .collect() + }) + }) +} diff --git a/tests/source/issue-4406-cast-comments/two.rs b/tests/source/issue-4406-cast-comments/two.rs new file mode 100644 index 00000000000..7f3512f5e42 --- /dev/null +++ b/tests/source/issue-4406-cast-comments/two.rs @@ -0,0 +1,100 @@ +// rustfmt-version: Two + +/***** + * Tests for proper formatting of pre and post cast ("as") comments + ******/ + +// Test 1 +fn main() { + let x = 0f64 /* x as */ as i32; +} + +// Test 2 +fn main() { +let x = 1 /* foo as */ as i32; +} + +// Test 3 +fn main() { +let x = 1 as /* bar as */ i32; +} + +// Test 4 +fn main() { +let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 5 +fn main() { +let x = 1 /* as foo */as/* as bar */ i32; +} + +// Test 6 +fn main() { +let x = 1 /* as foo */ +as/* as bar */ +i32; +} + +// Test 7 +fn main() { +let x = 1 /* as foo yyyyyyyyyyy */as/* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ i32; +} + +// Test 8 +fn main() { +let x = 1 /* as foo yyyyyyyyyyy */as/* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ i32; +} + +// Test 9 +fn main() { +let x = 1 /* as foo yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy */ +as/* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ +i32; +} + + +/***** + * Tests for not leaving trailing spaces related to cast comments (related to #2896?) + ******/ +// Test 10 - extra blank after the binary rhs at the 2nd line (comment followws at 3rd line) +fn main() { + if 0 == 1 + /* x */ as i32 {} } + +// Test 11 - extra blank after the binary rhs at the end of 2nd line +fn main() { + if 0 == ' ' + as i32 {} } + +// Test 12 - extra blank after the comment at the end of 2nd line +fn main() { + if 0 == ' ' /* x */ + as i32 {} } + + +/***** + * Tests for not moving "as" to new line unnecessarily - from #3528 + ******/ +fn get_old_backends(old_toml_config: &toml::Value) -> Option>> { + old_toml_config.as_table().and_then(|table| { + table + .get("backends") + .and_then(|backends| backends.as_table()) + .map(|backends| { + backends + .into_iter() + .filter_map(|(key, value)| match AvailableBackend::from(key.as_str()) { + AvailableBackend::Git => Some(Box::new(Git { + config: value.clone().try_into::().unwrap(), + }) + as Box), + AvailableBackend::Github => Some(Box::new(Github { + config: value.clone().try_into::().unwrap(), + }) + as Box), + }) + .collect() + }) + }) +} diff --git a/tests/target/issue-4406-cast-comments/one.rs b/tests/target/issue-4406-cast-comments/one.rs new file mode 100644 index 00000000000..ac26f469aa3 --- /dev/null +++ b/tests/target/issue-4406-cast-comments/one.rs @@ -0,0 +1,101 @@ +// rustfmt-version: One + +/***** + * Tests for proper formatting of pre and post cast ("as") comments + ******/ + +// Test 1 +fn main() { + let x = 0f64 /* x as */ as i32; +} + +// Test 2 +fn main() { + let x = 1 /* foo as */ as i32; +} + +// Test 3 +fn main() { + let x = 1 as /* bar as */ i32; +} + +// Test 4 +fn main() { + let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 5 +fn main() { + let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 6 +fn main() { + let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 7 +fn main() { + let x = 1 /* as foo yyyyyyyyyyy */ + as /* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ i32; +} + +// Test 8 +fn main() { + let x = 1 /* as foo yyyyyyyyyyy */ + as /* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ + i32; +} + +// Test 9 +fn main() { + let x = 1 /* as foo yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy */ + as /* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ + i32; +} + +/***** + * Tests for not leaving trailing spaces related to cast comments (related to #2896?) + ******/ +// Test 10 - extra blank after the binary rhs at the 2nd line (comment followws at 3rd line) +fn main() { + if 0 == 1 /* x */ as i32 {} +} + +// Test 11 - extra blank after the binary rhs at the end of 2nd line +fn main() { + if 0 == ' ' as i32 {} +} + +// Test 12 - extra blank after the comment at the end of 2nd line +fn main() { + if 0 == ' ' /* x */ as i32 {} +} + +/***** + * Tests for not moving "as" to new line unnecessarily - from #3528 + ******/ +fn get_old_backends(old_toml_config: &toml::Value) -> Option>> { + old_toml_config.as_table().and_then(|table| { + table + .get("backends") + .and_then(|backends| backends.as_table()) + .map(|backends| { + backends + .into_iter() + .filter_map(|(key, value)| match AvailableBackend::from(key.as_str()) { + AvailableBackend::Git => { + Some(Box::new(Git { + config: value.clone().try_into::().unwrap(), + }) as Box) + } + AvailableBackend::Github => { + Some(Box::new(Github { + config: value.clone().try_into::().unwrap(), + }) as Box) + } + }) + .collect() + }) + }) +} diff --git a/tests/target/issue-4406-cast-comments/two.rs b/tests/target/issue-4406-cast-comments/two.rs new file mode 100644 index 00000000000..f47c7e230be --- /dev/null +++ b/tests/target/issue-4406-cast-comments/two.rs @@ -0,0 +1,101 @@ +// rustfmt-version: Two + +/***** + * Tests for proper formatting of pre and post cast ("as") comments + ******/ + +// Test 1 +fn main() { + let x = 0f64 /* x as */ as i32; +} + +// Test 2 +fn main() { + let x = 1 /* foo as */ as i32; +} + +// Test 3 +fn main() { + let x = 1 as /* bar as */ i32; +} + +// Test 4 +fn main() { + let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 5 +fn main() { + let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 6 +fn main() { + let x = 1 /* as foo */ as /* as bar */ i32; +} + +// Test 7 +fn main() { + let x = 1 /* as foo yyyyyyyyyyy */ + as /* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ i32; +} + +// Test 8 +fn main() { + let x = 1 /* as foo yyyyyyyyyyy */ + as /* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ + i32; +} + +// Test 9 +fn main() { + let x = 1 /* as foo yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy */ + as /* as bar xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/ + i32; +} + +/***** + * Tests for not leaving trailing spaces related to cast comments (related to #2896?) + ******/ +// Test 10 - extra blank after the binary rhs at the 2nd line (comment followws at 3rd line) +fn main() { + if 0 == 1 /* x */ as i32 {} +} + +// Test 11 - extra blank after the binary rhs at the end of 2nd line +fn main() { + if 0 == ' ' as i32 {} +} + +// Test 12 - extra blank after the comment at the end of 2nd line +fn main() { + if 0 == ' ' /* x */ as i32 {} +} + +/***** + * Tests for not moving "as" to new line unnecessarily - from #3528 + ******/ +fn get_old_backends(old_toml_config: &toml::Value) -> Option>> { + old_toml_config.as_table().and_then(|table| { + table + .get("backends") + .and_then(|backends| backends.as_table()) + .map(|backends| { + backends + .into_iter() + .filter_map(|(key, value)| match AvailableBackend::from(key.as_str()) { + AvailableBackend::Git => { + Some(Box::new(Git { + config: value.clone().try_into::().unwrap(), + }) as Box) + } + AvailableBackend::Github => { + Some(Box::new(Github { + config: value.clone().try_into::().unwrap(), + }) as Box) + } + }) + .collect() + }) + }) +}