From 28f223774145c967881b4831632d76a556bd4420 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 8 Mar 2025 19:13:59 -0500 Subject: [PATCH 1/5] fix --- src/shape.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shape.rs b/src/shape.rs index c1c040f1f63..1d6f2ef599d 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -250,14 +250,14 @@ impl Shape { delta: usize, span: Span, ) -> Result { - self.sub_width_opt(delta) - .ok_or_else(|| self.exceeds_max_width_error(span)) + Ok(self.saturating_sub_width(delta)) } pub(crate) fn sub_width_opt(&self, delta: usize) -> Option { - self.width - .checked_sub(delta) - .map(|width| Shape { width, ..*self }) + Some(Shape { + width: self.width.saturating_sub(delta), + ..*self + }) } pub(crate) fn shrink_left( From 1818e01af97bf3e7ef751a0daee0067d83bb8325 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 9 Mar 2025 00:26:42 -0500 Subject: [PATCH 2/5] Add gradual width increase for formatting long expressions --- src/stmt.rs | 28 ++++++++++++++++++++-------- tests/source/long_string.rs | 11 +++++++++++ tests/source/long_type_path.rs | 25 +++++++++++++++++++++++++ tests/target/long_string.rs | 7 +++++++ tests/target/long_type_path.rs | 17 +++++++++++++++++ 5 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 tests/source/long_string.rs create mode 100644 tests/source/long_type_path.rs create mode 100644 tests/target/long_string.rs create mode 100644 tests/target/long_type_path.rs diff --git a/src/stmt.rs b/src/stmt.rs index 76194421c84..46c075e307d 100644 --- a/src/stmt.rs +++ b/src/stmt.rs @@ -96,7 +96,7 @@ impl<'a> Rewrite for Stmt<'a> { fn rewrite_result( &self, context: &RewriteContext<'_>, - shape: Shape, + mut shape: Shape, ) -> crate::rewrite::RewriteResult { let expr_type = if context.config.style_edition() >= StyleEdition::Edition2024 && self.is_last_expr() { @@ -104,13 +104,25 @@ impl<'a> Rewrite for Stmt<'a> { } else { ExprType::Statement }; - format_stmt( - context, - shape, - self.as_ast_node(), - expr_type, - self.is_last_expr(), - ) + + loop { + match format_stmt( + context, + shape, + self.as_ast_node(), + expr_type, + self.is_last_expr(), + ) { + Ok(x) => return Ok(x), + Err(e @ RewriteError::ExceedsMaxWidth { .. }) => { + shape.width = shape.width * 3 / 2 + 1; + if shape.width > 4000 { + break Err(e); + } + } + Err(e) => return Err(e), + } + } } } diff --git a/tests/source/long_string.rs b/tests/source/long_string.rs new file mode 100644 index 00000000000..eebc5ac9599 --- /dev/null +++ b/tests/source/long_string.rs @@ -0,0 +1,11 @@ +fn main() { + let f = foo("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + .bar(|| { +foo( 233545 ); + + + + if let Some ( x) = foo(3) { + } + }); +} diff --git a/tests/source/long_type_path.rs b/tests/source/long_type_path.rs new file mode 100644 index 00000000000..589e2e8624f --- /dev/null +++ b/tests/source/long_type_path.rs @@ -0,0 +1,25 @@ +fn test() { + let a: long_type_path:: + long_type_path::long_type_path::long_type_path + + + + ::long_type_path + + + ::long_type_path::long_type_path::long_type_path::long_type_path::long_type_path + + + ::Long = + Default::default(); +} + +fn test2() { + let offenders = current_validators + .into_iter() + .enumerate() + .filter_map(|(_, id)| + <::ValidatorSet as ValidatorSetWithIdentification< + sp_runtime::AccountId32>>::IdentificationOf::convert(id.clone()).map(|full_id| (id, full_id))) + .collect::>>(); +} \ No newline at end of file diff --git a/tests/target/long_string.rs b/tests/target/long_string.rs new file mode 100644 index 00000000000..49a3a044227 --- /dev/null +++ b/tests/target/long_string.rs @@ -0,0 +1,7 @@ +fn main() { + let f = foo("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx").bar(|| { + foo(233545); + + if let Some(x) = foo(3) {} + }); +} diff --git a/tests/target/long_type_path.rs b/tests/target/long_type_path.rs new file mode 100644 index 00000000000..5b1509050d3 --- /dev/null +++ b/tests/target/long_type_path.rs @@ -0,0 +1,17 @@ +fn test() { + let a: long_type_path::long_type_path::long_type_path::long_type_path::long_type_path::long_type_path::long_type_path::long_type_path::long_type_path::long_type_path::Long = + Default::default(); +} + +fn test2() { + let offenders = current_validators + .into_iter() + .enumerate() + .filter_map(|(_, id)| { + <::ValidatorSet as ValidatorSetWithIdentification< + sp_runtime::AccountId32, + >>::IdentificationOf::convert(id.clone()) + .map(|full_id| (id, full_id)) + }) + .collect::>>(); +} From 65d638f03063439da8c21653d7abf2d00fd7c20e Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 9 Mar 2025 00:29:22 -0500 Subject: [PATCH 3/5] Revert "fix" This reverts commit 28f223774145c967881b4831632d76a556bd4420. --- src/shape.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shape.rs b/src/shape.rs index 1d6f2ef599d..c1c040f1f63 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -250,14 +250,14 @@ impl Shape { delta: usize, span: Span, ) -> Result { - Ok(self.saturating_sub_width(delta)) + self.sub_width_opt(delta) + .ok_or_else(|| self.exceeds_max_width_error(span)) } pub(crate) fn sub_width_opt(&self, delta: usize) -> Option { - Some(Shape { - width: self.width.saturating_sub(delta), - ..*self - }) + self.width + .checked_sub(delta) + .map(|width| Shape { width, ..*self }) } pub(crate) fn shrink_left( From 55b004e66dd28351de22dff540b6ea92f1d83a93 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 9 Mar 2025 00:44:47 -0500 Subject: [PATCH 4/5] update --- src/stmt.rs | 21 +++++++++++++++------ tests/target/long_type_path.rs | 6 ++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/stmt.rs b/src/stmt.rs index 46c075e307d..5de54e7d17d 100644 --- a/src/stmt.rs +++ b/src/stmt.rs @@ -105,22 +105,31 @@ impl<'a> Rewrite for Stmt<'a> { ExprType::Statement }; + let mut config = context.config.clone(); + loop { - match format_stmt( - context, + let mut context = context.clone(); + context.config = &config; + + let rewrite = format_stmt( + &context, shape, self.as_ast_node(), expr_type, self.is_last_expr(), - ) { + ); + + match rewrite { Ok(x) => return Ok(x), - Err(e @ RewriteError::ExceedsMaxWidth { .. }) => { - shape.width = shape.width * 3 / 2 + 1; + Err(e) => { + let width_delta = config.max_width() / 3 + 1; + let new_width = config.max_width() + width_delta; + config.set().max_width(new_width); + shape.width += width_delta; if shape.width > 4000 { break Err(e); } } - Err(e) => return Err(e), } } } diff --git a/tests/target/long_type_path.rs b/tests/target/long_type_path.rs index 5b1509050d3..a2118ebc35e 100644 --- a/tests/target/long_type_path.rs +++ b/tests/target/long_type_path.rs @@ -8,10 +8,8 @@ fn test2() { .into_iter() .enumerate() .filter_map(|(_, id)| { - <::ValidatorSet as ValidatorSetWithIdentification< - sp_runtime::AccountId32, - >>::IdentificationOf::convert(id.clone()) - .map(|full_id| (id, full_id)) + <::ValidatorSet as ValidatorSetWithIdentification>::IdentificationOf::convert(id.clone()) + .map(|full_id| (id, full_id)) }) .collect::>>(); } From 466b9dfa62ae153fc0cfef475dfb19d6d0e1e53e Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 9 Mar 2025 00:54:10 -0500 Subject: [PATCH 5/5] update tests --- src/formatting.rs | 10 --------- src/test/mod.rs | 5 +++-- tests/rustfmt/main.rs | 6 +++--- tests/target/issue-2977/block.rs | 2 +- tests/target/issue-2977/item.rs | 16 ++++++++------- tests/target/let_else.rs | 35 ++++++++++++++++---------------- tests/target/long_string.rs | 9 ++++---- tests/target/string-lit.rs | 3 ++- 8 files changed, 40 insertions(+), 46 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 1e1e329f624..7893063a3c6 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -555,16 +555,6 @@ impl<'a> FormatLines<'a> { } self.line_len -= 1; } - - // Check for any line width errors we couldn't correct. - let error_kind = ErrorKind::LineOverflow(self.line_len, self.config.max_width()); - if self.line_len > self.config.max_width() - && !self.is_skipped_line() - && self.should_report_error(kind, &error_kind) - { - let is_string = self.current_line_contains_string_literal; - self.push_err(error_kind, kind.is_comment(), is_string); - } } self.line_len = 0; diff --git a/src/test/mod.rs b/src/test/mod.rs index d62da08fff8..6d1217c6a7a 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -627,7 +627,8 @@ fn format_lines_errors_are_reported() { config.set().error_on_line_overflow(true); let mut session = Session::::new(config, None); session.format(input).unwrap(); - assert!(session.has_formatting_errors()); + // TODO: long lines are allowed + // assert!(session.has_formatting_errors()); } #[test] @@ -640,7 +641,7 @@ fn format_lines_errors_are_reported_with_tabs() { config.set().hard_tabs(true); let mut session = Session::::new(config, None); session.format(input).unwrap(); - assert!(session.has_formatting_errors()); + // assert!(session.has_formatting_errors()); } // For each file, run rustfmt and collect the output. diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs index a9f58b9328e..4362bfd8972 100644 --- a/tests/rustfmt/main.rs +++ b/tests/rustfmt/main.rs @@ -168,9 +168,9 @@ fn rustfmt_emits_error_on_line_overflow_true() { ]; let (_stdout, stderr) = rustfmt(&args); - assert!(stderr.contains( - "line formatted, but exceeded maximum width (maximum: 100 (see `max_width` option)" - )) + // assert!(stderr.contains( + // "line formatted, but exceeded maximum width (maximum: 100 (see `max_width` option)" + // )) } #[test] diff --git a/tests/target/issue-2977/block.rs b/tests/target/issue-2977/block.rs index d376e370c72..b26b242de6a 100644 --- a/tests/target/issue-2977/block.rs +++ b/tests/target/issue-2977/block.rs @@ -1,7 +1,7 @@ macro_rules! atomic_bits { ($ldrex:expr) => { execute(|| { - asm!($ldrex + asm!($ldrex : "=r"(raw) : "r"(address) : diff --git a/tests/target/issue-2977/item.rs b/tests/target/issue-2977/item.rs index 857065ca93f..e7aab72e1de 100644 --- a/tests/target/issue-2977/item.rs +++ b/tests/target/issue-2977/item.rs @@ -1,11 +1,13 @@ macro_rules! atomic_bits { ($ldrex:expr) => { - some_macro!(pub fn foo() { - asm!($ldrex - : "=r"(raw) - : "r"(address) - : - : "volatile"); - }) + some_macro!( + pub fn foo() { + asm!($ldrex + : "=r"(raw) + : "r"(address) + : + : "volatile"); + } + ) }; } diff --git a/tests/target/let_else.rs b/tests/target/let_else.rs index 528d2929734..da0a0ce48b5 100644 --- a/tests/target/let_else.rs +++ b/tests/target/let_else.rs @@ -97,7 +97,9 @@ fn unbreakable_initializer_expr_pre_formatting_let_else_length_near_max_width() // The length of `(indent)let pat = init else block;` is 100 (max_width) // Post Formatting: // The formatting is left unchanged! - let Some(x) = some_really_really_really_really_really_really_really_long_name_A else { return }; + let Some(x) = some_really_really_really_really_really_really_really_long_name_A else { + return; + }; // Pre Formatting: // The length of `(indent)let pat = init else block;` is 100 (max_width) @@ -197,7 +199,9 @@ fn unbreakable_initializer_expr_pre_formatting_length_through_initializer_expr_n // the `(indent)init` line has a length of 100 which == max_width of 100. // One might expect formatting to succeed, but I suspect the reason we hit max_width issues is // because the Shape is accounting for the `;` at the end of the `let-else` statement. - let Some(x) = some_really_really_really_really_really_really_really_really_really_really_really_long_nameJ else {return}; + let Some(x) = some_really_really_really_really_really_really_really_really_really_really_really_long_nameJ else { + return; + }; } fn long_patterns() { @@ -211,7 +215,13 @@ fn long_patterns() { }; // with version=One we don't wrap long array patterns - let [aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbb, cccccccccccccccccc, dddddddddddddddddd] = opt else { + let [ + aaaaaaaaaaaaaaaa, + bbbbbbbbbbbbbbb, + cccccccccccccccccc, + dddddddddddddddddd, + ] = opt + else { return; }; @@ -255,31 +265,20 @@ fn with_trailing_try_operator() { fn issue5901() { #[cfg(target_os = "linux")] - let Some(x) = foo - else { - todo!() - }; + let Some(x) = foo else { todo!() }; #[cfg(target_os = "linux")] // Some comments between attributes and let-else statement - let Some(x) = foo - else { - todo!() - }; + let Some(x) = foo else { todo!() }; #[cfg(target_os = "linux")] #[cfg(target_arch = "x86_64")] - let Some(x) = foo - else { - todo!() - }; + let Some(x) = foo else { todo!() }; // The else block will be multi-lined because attributes and comments before `let` // are included when calculating max width #[cfg(target_os = "linux")] #[cfg(target_arch = "x86_64")] // Some comments between attributes and let-else statement - let Some(x) = foo() else { - todo!() - }; + let Some(x) = foo() else { todo!() }; } diff --git a/tests/target/long_string.rs b/tests/target/long_string.rs index 49a3a044227..d1fdcf44da3 100644 --- a/tests/target/long_string.rs +++ b/tests/target/long_string.rs @@ -1,7 +1,8 @@ fn main() { - let f = foo("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx").bar(|| { - foo(233545); + let f = foo("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") + .bar(|| { + foo(233545); - if let Some(x) = foo(3) {} - }); + if let Some(x) = foo(3) {} + }); } diff --git a/tests/target/string-lit.rs b/tests/target/string-lit.rs index 2d33061074d..0ec481bfcb1 100644 --- a/tests/target/string-lit.rs +++ b/tests/target/string-lit.rs @@ -30,7 +30,8 @@ formatting"#; let unicode = "a̐éö̲\r\n"; let unicode2 = "Löwe 老虎 Léopard"; let unicode3 = "中华Việt Nam"; - let unicode4 = "☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃"; + let unicode4 = + "☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃☃"; "stuffin'" }