From 25c23826fbdddc841484c98a149b0d74146ebf24 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Aug 2024 13:28:15 +0200 Subject: [PATCH 1/3] avoid inserting newline between curly brace and comment --- src/parse/session.rs | 10 ++++++ src/visitor.rs | 35 +++++++++++++++++++ tests/source/issue-3255.rs | 26 ++++++++++++++ tests/target/async_fn.rs | 3 +- .../control-brace-style-always-next-line.rs | 3 +- .../control-brace-style-always-same-line.rs | 3 +- tests/target/issue-3255.rs | 25 +++++++++++++ tests/target/label_break.rs | 3 +- tests/target/match.rs | 3 +- 9 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 tests/source/issue-3255.rs create mode 100644 tests/target/issue-3255.rs diff --git a/src/parse/session.rs b/src/parse/session.rs index 05cf467167c..7684b6de8c1 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -1,3 +1,4 @@ +use std::ops::Range; use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; @@ -237,6 +238,15 @@ impl ParseSess { } } + pub(crate) fn line_bounds(&self, pos: BytePos) -> Option> { + let line = self.raw_psess.source_map().lookup_line(pos).ok(); + + match line { + Some(line_info) => Some(line_info.sf.line_bounds(line_info.line)), + None => None, + } + } + pub(crate) fn line_of_byte_pos(&self, pos: BytePos) -> usize { self.raw_psess.source_map().lookup_char_pos(pos).line } diff --git a/src/visitor.rs b/src/visitor.rs index b08aa35f8f8..dbae9e5f569 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -223,6 +223,41 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_str("{"); self.trim_spaces_after_opening_brace(b, inner_attrs); + // Try to detect comments that refer to the block, not the first statement in the block. + if has_braces { + let block_line_range = self.psess.lookup_line_range(b.span); + if block_line_range.lo != block_line_range.hi { + // Skipping if a single line block + // Make sure there is no code on the first line we have to worry about. + // That would also be ambiguous: what should `if { /* comment */ statement;` + // even do -- should the comment be attached to the `if` or the `statement`? + // The current logic answers this with "statement". + let first_line_contains_stmt = if let Some(first_stmt) = b.stmts.first() { + self.psess.lookup_line_range(first_stmt.span).lo == block_line_range.lo + } else { + false + }; + if !first_line_contains_stmt { + let first_line_bounds = self.psess.line_bounds(self.last_pos).unwrap(); + let first_line_snip = self + .snippet(mk_sp(self.last_pos, first_line_bounds.end)) + .trim(); + + if contains_comment(first_line_snip) { + if let Ok(comment) = + rewrite_comment(first_line_snip, false, self.shape(), self.config) + { + self.push_str(" "); + self.push_str(&comment); + // This line has no statements, so we can jump to the end of it + // (but *before* the newline). + self.last_pos = first_line_bounds.end - BytePos::from_usize(1); + } + } + } + } + } + // Format inner attributes if available. if let Some(attrs) = inner_attrs { self.visit_attrs(attrs, ast::AttrStyle::Inner); diff --git a/tests/source/issue-3255.rs b/tests/source/issue-3255.rs new file mode 100644 index 00000000000..7f28cc5dec6 --- /dev/null +++ b/tests/source/issue-3255.rs @@ -0,0 +1,26 @@ +fn foo(){ + if true { // Sample comment + 1 + } +} + + +fn foo(){ + if true { + // Sample comment + 1 + } +} + +fn foo(){ + if true { /* Sample comment */ + 1 + } +} + +fn foo(){ + if true { /* Sample + comment */ + 1 + } +} diff --git a/tests/target/async_fn.rs b/tests/target/async_fn.rs index ac151dddb25..1bca39d5114 100644 --- a/tests/target/async_fn.rs +++ b/tests/target/async_fn.rs @@ -13,8 +13,7 @@ async unsafe fn foo() { } async unsafe fn rust() { - async move { - // comment + async move { // comment Ok(()) } } diff --git a/tests/target/control-brace-style-always-next-line.rs b/tests/target/control-brace-style-always-next-line.rs index 054a3075ca0..cb82dff904b 100644 --- a/tests/target/control-brace-style-always-next-line.rs +++ b/tests/target/control-brace-style-always-next-line.rs @@ -20,8 +20,7 @@ fn main() { } 'while_label: while cond - { - // while comment + { // while comment (); } diff --git a/tests/target/control-brace-style-always-same-line.rs b/tests/target/control-brace-style-always-same-line.rs index cf3f82dfcf1..1b90505c9fb 100644 --- a/tests/target/control-brace-style-always-same-line.rs +++ b/tests/target/control-brace-style-always-same-line.rs @@ -15,8 +15,7 @@ fn main() { (); } - 'while_label: while cond { - // while comment + 'while_label: while cond { // while comment (); } diff --git a/tests/target/issue-3255.rs b/tests/target/issue-3255.rs new file mode 100644 index 00000000000..9ba9c381a3d --- /dev/null +++ b/tests/target/issue-3255.rs @@ -0,0 +1,25 @@ +fn foo() { + if true { // Sample comment + 1 + } +} + +fn foo() { + if true { + // Sample comment + 1 + } +} + +fn foo() { + if true { /* Sample comment */ + 1 + } +} + +fn foo() { + if true { /* Sample + comment */ + 1 + } +} diff --git a/tests/target/label_break.rs b/tests/target/label_break.rs index 728d78137c9..6f47f2a1b8c 100644 --- a/tests/target/label_break.rs +++ b/tests/target/label_break.rs @@ -19,8 +19,7 @@ fn main() { // comment break 'block 1; } - if bar() { - /* comment */ + if bar() { /* comment */ break 'block 2; } 3 diff --git a/tests/target/match.rs b/tests/target/match.rs index 0e7815a814d..e50554625bc 100644 --- a/tests/target/match.rs +++ b/tests/target/match.rs @@ -7,8 +7,7 @@ fn foo() { // Some comment. a => foo(), b if 0 < 42 => foo(), - c => { - // Another comment. + c => { // Another comment. // Comment. an_expression; foo() From c2fe0d8223ea5fe91eabd91c732300d6490347f4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2024 17:35:41 +0200 Subject: [PATCH 2/3] extend tests --- tests/source/issue-3255.rs | 12 ++++++++++++ tests/target/issue-3255.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/source/issue-3255.rs b/tests/source/issue-3255.rs index 7f28cc5dec6..313e211ad61 100644 --- a/tests/source/issue-3255.rs +++ b/tests/source/issue-3255.rs @@ -1,5 +1,6 @@ fn foo(){ if true { // Sample comment + // second-line comment 1 } } @@ -24,3 +25,14 @@ fn foo(){ 1 } } + +// This isn't ideal. +fn foo(){ + if true { /* Sample + * another line + * another line + * end + */ + 1 + } +} diff --git a/tests/target/issue-3255.rs b/tests/target/issue-3255.rs index 9ba9c381a3d..a74a198fce2 100644 --- a/tests/target/issue-3255.rs +++ b/tests/target/issue-3255.rs @@ -1,5 +1,6 @@ fn foo() { if true { // Sample comment + // second-line comment 1 } } @@ -23,3 +24,14 @@ fn foo() { 1 } } + +// This isn't ideal. +fn foo() { + if true { /* Sample + * another line + * another line + * end + */ + 1 + } +} From 8595fbb191aedbbdc9f53ae7eab75f163d98c7af Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2024 17:39:14 +0200 Subject: [PATCH 3/3] gate the change on style edition 2024 --- src/visitor.rs | 2 +- tests/source/issue-3255.rs | 2 ++ tests/target/async_fn.rs | 3 ++- tests/target/control-brace-style-always-next-line.rs | 3 ++- tests/target/control-brace-style-always-same-line.rs | 3 ++- tests/target/issue-3255.rs | 2 ++ tests/target/label_break.rs | 3 ++- tests/target/match.rs | 3 ++- 8 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/visitor.rs b/src/visitor.rs index dbae9e5f569..b57a49e502f 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -224,7 +224,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.trim_spaces_after_opening_brace(b, inner_attrs); // Try to detect comments that refer to the block, not the first statement in the block. - if has_braces { + if has_braces && self.config.style_edition() >= StyleEdition::Edition2024 { let block_line_range = self.psess.lookup_line_range(b.span); if block_line_range.lo != block_line_range.hi { // Skipping if a single line block diff --git a/tests/source/issue-3255.rs b/tests/source/issue-3255.rs index 313e211ad61..fc54c0c5996 100644 --- a/tests/source/issue-3255.rs +++ b/tests/source/issue-3255.rs @@ -1,3 +1,5 @@ +// rustfmt-style_edition: 2024 + fn foo(){ if true { // Sample comment // second-line comment diff --git a/tests/target/async_fn.rs b/tests/target/async_fn.rs index 1bca39d5114..ac151dddb25 100644 --- a/tests/target/async_fn.rs +++ b/tests/target/async_fn.rs @@ -13,7 +13,8 @@ async unsafe fn foo() { } async unsafe fn rust() { - async move { // comment + async move { + // comment Ok(()) } } diff --git a/tests/target/control-brace-style-always-next-line.rs b/tests/target/control-brace-style-always-next-line.rs index cb82dff904b..054a3075ca0 100644 --- a/tests/target/control-brace-style-always-next-line.rs +++ b/tests/target/control-brace-style-always-next-line.rs @@ -20,7 +20,8 @@ fn main() { } 'while_label: while cond - { // while comment + { + // while comment (); } diff --git a/tests/target/control-brace-style-always-same-line.rs b/tests/target/control-brace-style-always-same-line.rs index 1b90505c9fb..cf3f82dfcf1 100644 --- a/tests/target/control-brace-style-always-same-line.rs +++ b/tests/target/control-brace-style-always-same-line.rs @@ -15,7 +15,8 @@ fn main() { (); } - 'while_label: while cond { // while comment + 'while_label: while cond { + // while comment (); } diff --git a/tests/target/issue-3255.rs b/tests/target/issue-3255.rs index a74a198fce2..1277a7f3751 100644 --- a/tests/target/issue-3255.rs +++ b/tests/target/issue-3255.rs @@ -1,3 +1,5 @@ +// rustfmt-style_edition: 2024 + fn foo() { if true { // Sample comment // second-line comment diff --git a/tests/target/label_break.rs b/tests/target/label_break.rs index 6f47f2a1b8c..728d78137c9 100644 --- a/tests/target/label_break.rs +++ b/tests/target/label_break.rs @@ -19,7 +19,8 @@ fn main() { // comment break 'block 1; } - if bar() { /* comment */ + if bar() { + /* comment */ break 'block 2; } 3 diff --git a/tests/target/match.rs b/tests/target/match.rs index e50554625bc..0e7815a814d 100644 --- a/tests/target/match.rs +++ b/tests/target/match.rs @@ -7,7 +7,8 @@ fn foo() { // Some comment. a => foo(), b if 0 < 42 => foo(), - c => { // Another comment. + c => { + // Another comment. // Comment. an_expression; foo()