Skip to content

Fix misidentification of indented comments #6617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
matthewhughes934 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix misidentification of indented comments #6617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/comment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ fn identify_comment(
config: &Config,
is_doc_comment: bool,
) -> RewriteResult {
let style = comment_style(orig, false);
Copy link
Member

@jieyouxu jieyouxu Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: can you leave a brief comment here with an example, i.e.

// Account for leading whitespace, e.g. `  //#`

Copy link
Contributor Author

@matthewhughes934 matthewhughes934 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: can you leave a brief comment here with an example, i.e.

// Account for leading whitespace, e.g. `  //#`

🤔 let me actually think over this change a bit more: I'm wondering if this is too naive and comments with different indentation levels are now accidentally match on the same style.

let style = comment_style(orig.trim_start(), false);

// Computes the byte length of line taking into account a newline if the line is part of a
// paragraph.
Expand Down
8 changes: 8 additions & 0 deletions tests/source/issue-6612/hidden_code_in_doc_comment.rs
View file
Open in desktop
Copy link
Member

@jieyouxu jieyouxu Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question [IDEMPOTENCE 2/2]: ditto on the "format as expected"

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// rustfmt-format_code_in_doc_comments: true
// rustfmt-edition: 2024

/// ```
/// println!("1"); // comment
/// # println!("2")
/// ```
struct S;
12 changes: 12 additions & 0 deletions tests/source/issue-6612/mixed_comment_types.rs
View file
Open in desktop
Copy link
Member

@jieyouxu jieyouxu Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question [IDEMPOTENCE 1/2]: is this a "rustfmt doesn't change stable formatting" test? If so, can you remove the source/ copy and leave only the target/ copy (as idempotence check is done against the target/ copy)?

Copy link
Contributor Author

@matthewhughes934 matthewhughes934 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks, I wasn't aware of a standalone file in target meant it is run as an idempotentcy test, I'll double check that's the case and update accordingly

Copy link
Member

@jieyouxu jieyouxu Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some more trial and error myself, I missed that there is specifically a case where you would want both a source/ and target/ copy, which is to ensure idempotence. That is, having both source/ and target/ even if identical helps you catch the case where the target copy is already in canonical form, but that it make take more than 1 rustfmt runs to get from the source copy to target copy.

When there's only a target/ copy, the test is only good for "the format is indeed canonical" and implies no guarantees on idempotency actually, sorry.

(IOW, leaving the source copies as-is is probably good, because other stuff might have strange interations with these examples.)

I'll update the contributing docs because this wasn't super obvious to me at first, but became obvious on hindsight
EDIT: #6821

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-edition: 2024

fn wat1() {
println!("1"); // double slash comment
/* another comment type */
}

fn wat1() {
println!("1"); // double slash comment
/* another comment type */
//# yet another type
}
8 changes: 8 additions & 0 deletions tests/target/issue-6612/hidden_code_in_doc_comment.rs
View file
Open in desktop
Copy link
Member

@jieyouxu jieyouxu Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: can you also add coverage for:

  1. Doc comment but without the code fenses, i.e.
/// println!("1"); // comment
/// # println!("2")
struct S;
  1. Code block in doc comment but no space between /// and #, i.e.
/// ```
/// println!("1"); // comment
///# println!("2")
/// ```
struct S;
/// ```
/// println!("1"); // comment
///#println!("2")
/// ```
struct S;

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// rustfmt-format_code_in_doc_comments: true
// rustfmt-edition: 2024

/// ```
/// println!("1"); // comment
/// # println!("2")
/// ```
struct S;
12 changes: 12 additions & 0 deletions tests/target/issue-6612/mixed_comment_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-edition: 2024

fn wat1() {
println!("1"); // double slash comment
/* another comment type */
}

fn wat1() {
println!("1"); // double slash comment
/* another comment type */
//# yet another type
}
Loading