From 4328079c615f1898ca7a530a5a91748206898a96 Mon Sep 17 00:00:00 2001 From: Karol Zwolak Date: Tue, 25 Feb 2025 21:31:20 +0100 Subject: [PATCH 1/4] add tests and config spec for new option `remove_nested_blocks` --- Configurations.md | 33 +++++++++ src/config/mod.rs | 3 + src/config/options.rs | 1 + tests/source/remove_nested_blocks.rs | 102 +++++++++++++++++++++++++++ tests/target/remove_nested_blocks.rs | 99 ++++++++++++++++++++++++++ 5 files changed, 238 insertions(+) create mode 100644 tests/source/remove_nested_blocks.rs create mode 100644 tests/target/remove_nested_blocks.rs diff --git a/Configurations.md b/Configurations.md index 8e04ddd325e..a4b9ee76ed7 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2227,6 +2227,39 @@ fn main() { } ``` +## `remove_nested_blocks` + +Works similarly to [`remove_nested_parens`](#remove_nested_parens), but removes nested blocks. + +- **Default value**: `false`, +- **Possible values**: `true`, `false` +- **Stable**: No + + +#### `true`: +```rust +fn main() { + { + foo(); + } +} +``` + +#### `false` (default): +```rust +fn main() { + { + { + { + { + foo(); + } + } + } + } +} +``` + ## `reorder_impl_items` diff --git a/src/config/mod.rs b/src/config/mod.rs index 742b99c15ef..009506465e8 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -117,6 +117,7 @@ create_config! { // Misc. remove_nested_parens: RemoveNestedParens, true, "Remove nested parens"; + remove_nested_blocks: RemoveNestedBlocks, false, "Remove nested blocks"; combine_control_expr: CombineControlExpr, false, "Combine control expressions with function \ calls"; short_array_element_width_threshold: ShortArrayElementWidthThreshold, true, @@ -796,6 +797,7 @@ space_after_colon = true spaces_around_ranges = false binop_separator = "Front" remove_nested_parens = true +remove_nested_blocks = false combine_control_expr = true short_array_element_width_threshold = 10 overflow_delimited_expr = false @@ -887,6 +889,7 @@ space_after_colon = true spaces_around_ranges = false binop_separator = "Front" remove_nested_parens = true +remove_nested_blocks = false combine_control_expr = true short_array_element_width_threshold = 10 overflow_delimited_expr = true diff --git a/src/config/options.rs b/src/config/options.rs index 3f86999c30b..e7bf20dfaff 100644 --- a/src/config/options.rs +++ b/src/config/options.rs @@ -658,6 +658,7 @@ config_option_with_style_edition_default!( // Misc. RemoveNestedParens, bool, _ => true; + RemoveNestedBlocks, bool, _ => false; CombineControlExpr, bool, _ => true; ShortArrayElementWidthThreshold, usize, _ => 10; OverflowDelimitedExpr, bool, Edition2024 => true, _ => false; diff --git a/tests/source/remove_nested_blocks.rs b/tests/source/remove_nested_blocks.rs new file mode 100644 index 00000000000..4cd92c8cae3 --- /dev/null +++ b/tests/source/remove_nested_blocks.rs @@ -0,0 +1,102 @@ +// rustfmt-remove_nested_blocks: true +fn foo() {} + +// The way it is implemented right now, only 'naked' blocks are removed. +// This means unsafe blocks, const blocks, blocks with labels and attributes, +// blocks belonging to other constructs such as loops are not removed. +fn main() { + { + { + { + foo(); + } + } + } + // The next block will be removed + { + { + // Blocks with comments are not removed + { + { + /* second comment */ + { + foo(); + } + } + } + } + } + { + { + /* + * multi-line comment + */ + foo(); + } + } + {{{{{{{foo();}}}}}}} + {{/*comment*/{{{foo();}}}}} + {{/*comment*/{{/*comment*/{foo();}}}}} + { + const { const {} } + } + const { const {} } + unsafe { unsafe {} } + // As const and unsafe blocks are not 'naked' blocks, they are not removed. + unsafe { + unsafe { + { + const { + const { + { + foo(); + } + } + } + } + } + } + const { + unsafe { + { + unsafe { + const { + { + foo(); + } + } + } + } + } + } + { + 'label1: { + 'label2: { + foo(); + } + } + } + 'outer: loop { + { + 'inner: loop { + { + foo(); + } + } + } + } + if let Some(x) = 5f64.map(|x| Ok(x)) { + { + if let Some(x) = 5f64.map(|x| Ok(x)) { + foo(); + } + } + } + if false { + { {} } + } + #[cfg(debug)] + { + { {} } + } +} diff --git a/tests/target/remove_nested_blocks.rs b/tests/target/remove_nested_blocks.rs new file mode 100644 index 00000000000..779665adbb6 --- /dev/null +++ b/tests/target/remove_nested_blocks.rs @@ -0,0 +1,99 @@ +// rustfmt-remove_nested_blocks: true +fn foo() {} + +// The way it is implemented right now, only 'naked' blocks are removed. +// This means unsafe blocks, const blocks, blocks with labels and attributes, +// blocks belonging to other constructs such as loops are not removed. +fn main() { + { + foo(); + } + // The next block will be removed + { + // Blocks with comments are not removed + { + /* second comment */ + { + foo(); + } + } + } + { + /* + * multi-line comment + */ + foo(); + } + { + foo(); + } + { + /*comment*/ + { + foo(); + } + } + { + /*comment*/ + { + /*comment*/ + { + foo(); + } + } + } + const { const {} } + const { const {} } + unsafe { unsafe {} } + // As const and unsafe blocks are not 'naked' blocks, they are not removed. + unsafe { + unsafe { + const { + const { + { + foo(); + } + } + } + } + } + const { + unsafe { + unsafe { + const { + { + foo(); + } + } + } + } + } + 'label1: { + 'label2: { + foo(); + } + } + 'outer: loop { + { + 'inner: loop { + { + foo(); + } + } + } + } + if let Some(x) = 5f64.map(|x| Ok(x)) { + { + if let Some(x) = 5f64.map(|x| Ok(x)) { + foo(); + } + } + } + if false { + {} + } + #[cfg(debug)] + { + {} + } +} From 6b99762bb694e5e025e7030f73ffed2dc855689b Mon Sep 17 00:00:00 2001 From: Karol Zwolak Date: Wed, 26 Feb 2025 10:46:11 +0100 Subject: [PATCH 2/4] feat: implement the `remove_nested_blocks` option --- src/expr.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 72 insertions(+), 6 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 0c5752380f6..3ab1b0443a2 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -192,16 +192,13 @@ pub(crate) fn format_expr( // Rewrite block without trying to put it in a single line. Ok(rw) } else { - let prefix = block_prefix(context, block, shape)?; - - rewrite_block_with_visitor( - context, - &prefix, + rewrite_block_inner( block, Some(&expr.attrs), opt_label, + false, + context, shape, - true, ) } } @@ -634,6 +631,31 @@ fn rewrite_block( rewrite_block_inner(block, attrs, label, true, context, shape) } +fn remove_nested_block( + block: &ast::Block, + inner_label: Option, + block_expr: &ast::Expr, + inner_block: &ast::Block, + context: &RewriteContext<'_>, + shape: Shape, +) -> Option { + let pre_inner_block_span = mk_sp(block.span.lo(), block_expr.span.lo()); + let post_inner_block_span = mk_sp(block_expr.span.hi(), block.span.hi()); + + let immdiately_contains_comment = contains_comment(context.snippet(pre_inner_block_span)) + || contains_comment(context.snippet(post_inner_block_span)); + if immdiately_contains_comment { + return None; + } + Some(rewrite_block( + inner_block, + Some(&block_expr.attrs), + inner_label, + context, + shape, + )) +} + fn rewrite_block_inner( block: &ast::Block, attrs: Option<&[ast::Attribute]>, @@ -642,8 +664,52 @@ fn rewrite_block_inner( context: &RewriteContext<'_>, shape: Shape, ) -> RewriteResult { + debug!("rewrite_block : {:?}", context.snippet(block.span)); let prefix = block_prefix(context, block, shape)?; + let no_attrs = attrs.is_none() || attrs.unwrap().is_empty(); + + if context.config.remove_nested_blocks() + && !is_unsafe_block(block) + && no_attrs + && label.is_none() + && block.stmts.len() == 1 + { + if let ast::StmtKind::Expr(ref block_expr) = &block.stmts[0].kind { + match block_expr.kind { + ast::ExprKind::Block(ref inner_block, inner_label) => { + if let Some(rw) = remove_nested_block( + block, + inner_label, + block_expr, + inner_block, + context, + shape, + ) { + return rw; + } + } + + ast::ExprKind::ConstBlock(ref anon_const) => { + if let ast::ExprKind::Block(ref inner_block, inner_label) = + anon_const.value.kind + { + if let Some(rw) = remove_nested_block( + block, + inner_label, + block_expr, + inner_block, + context, + shape, + ) { + return Ok(format!("const {}", rw?)); + } + } + } + _ => {} + } + } + } // shape.width is used only for the single line case: either the empty block `{}`, // or an unsafe expression `unsafe { e }`. if let Some(rw_str) = rewrite_empty_block(context, block, attrs, label, &prefix, shape) { From b31bf3ae598126be4a0490b8150af905e757d635 Mon Sep 17 00:00:00 2001 From: Karol Zwolak Date: Wed, 26 Feb 2025 21:27:57 +0100 Subject: [PATCH 3/4] fix: fix remove nested parens for const blocks --- src/expr.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 3ab1b0443a2..5d007483a38 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -175,7 +175,15 @@ pub(crate) fn format_expr( // not the `ast::Block` node we're about to rewrite. To prevent dropping inner // attributes call `rewrite_block` directly. // See https://github.com/rust-lang/rustfmt/issues/6158 - rewrite_block(block, Some(&expr.attrs), opt_label, context, shape)? + rewrite_block_inner( + block, + Some(&expr.attrs), + opt_label, + true, + context, + shape, + false, + )? } _ => anon_const.rewrite_result(context, shape)?, }; @@ -199,6 +207,7 @@ pub(crate) fn format_expr( false, context, shape, + true, ) } } @@ -628,7 +637,7 @@ fn rewrite_block( context: &RewriteContext<'_>, shape: Shape, ) -> RewriteResult { - rewrite_block_inner(block, attrs, label, true, context, shape) + rewrite_block_inner(block, attrs, label, true, context, shape, true) } fn remove_nested_block( @@ -638,6 +647,7 @@ fn remove_nested_block( inner_block: &ast::Block, context: &RewriteContext<'_>, shape: Shape, + can_inner_be_removed: bool, ) -> Option { let pre_inner_block_span = mk_sp(block.span.lo(), block_expr.span.lo()); let post_inner_block_span = mk_sp(block_expr.span.hi(), block.span.hi()); @@ -647,12 +657,14 @@ fn remove_nested_block( if immdiately_contains_comment { return None; } - Some(rewrite_block( + Some(rewrite_block_inner( inner_block, Some(&block_expr.attrs), inner_label, + true, context, shape, + can_inner_be_removed, )) } @@ -663,6 +675,7 @@ fn rewrite_block_inner( allow_single_line: bool, context: &RewriteContext<'_>, shape: Shape, + can_be_removed: bool, ) -> RewriteResult { debug!("rewrite_block : {:?}", context.snippet(block.span)); let prefix = block_prefix(context, block, shape)?; @@ -670,6 +683,8 @@ fn rewrite_block_inner( let no_attrs = attrs.is_none() || attrs.unwrap().is_empty(); if context.config.remove_nested_blocks() + && can_be_removed + && prefix.is_empty() && !is_unsafe_block(block) && no_attrs && label.is_none() @@ -685,6 +700,7 @@ fn rewrite_block_inner( inner_block, context, shape, + true, ) { return rw; } @@ -701,6 +717,7 @@ fn rewrite_block_inner( inner_block, context, shape, + false, ) { return Ok(format!("const {}", rw?)); } @@ -734,7 +751,7 @@ pub(crate) fn rewrite_let_else_block( context: &RewriteContext<'_>, shape: Shape, ) -> RewriteResult { - rewrite_block_inner(block, None, None, allow_single_line, context, shape) + rewrite_block_inner(block, None, None, allow_single_line, context, shape, false) } // Rewrite condition if the given expression has one. From eaab0dedab818ef53995e864cff82bdea7184987 Mon Sep 17 00:00:00 2001 From: Karol Zwolak Date: Wed, 26 Feb 2025 22:16:22 +0100 Subject: [PATCH 4/4] docs: improve documentation of `remove_nested_blocks` --- Configurations.md | 20 +++++++++++--------- src/expr.rs | 3 +++ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/Configurations.md b/Configurations.md index a4b9ee76ed7..401375c2cb6 100644 --- a/Configurations.md +++ b/Configurations.md @@ -2235,32 +2235,34 @@ Works similarly to [`remove_nested_parens`](#remove_nested_parens), but removes - **Possible values**: `true`, `false` - **Stable**: No +Blocks with any sort of comments or attributes, `unsafe` and `const` blocks will not be removed. -#### `true`: +#### `false` (default): ```rust fn main() { { - foo(); + { + // comment + { + foo(); + } + } } } ``` -#### `false` (default): +#### `true`: ```rust fn main() { { + // comment { - { - { - foo(); - } - } + foo(); } } } ``` - ## `reorder_impl_items` Reorder impl items. `type` and `const` are put first, then macros and methods. diff --git a/src/expr.rs b/src/expr.rs index 5d007483a38..483dc0a9cb9 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -676,12 +676,15 @@ fn rewrite_block_inner( context: &RewriteContext<'_>, shape: Shape, can_be_removed: bool, + // ^ this is a fix for const blocks, which are not removed, but are passed identically to blocks ) -> RewriteResult { debug!("rewrite_block : {:?}", context.snippet(block.span)); let prefix = block_prefix(context, block, shape)?; let no_attrs = attrs.is_none() || attrs.unwrap().is_empty(); + // If the option `remove_nested_blocks` is enabled, we remove all unnecessary nested blocks. + // Blocks with any sort of comments or attributes, unsafe and const blocks will not be removed. if context.config.remove_nested_blocks() && can_be_removed && prefix.is_empty()