Skip to content
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.

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

Comment indentation on opening line of a block is incorrect #3255

Open
mqudsi opened this issue Dec 16, 2018 · 6 comments · May be fixed by #4745 or #6265
Open

Comment indentation on opening line of a block is incorrect #3255

mqudsi opened this issue Dec 16, 2018 · 6 comments · May be fixed by #4745 or #6265

Comments

@mqudsi
Copy link

mqudsi commented Dec 16, 2018

Similar to the just-filed #3254 but a different case and with a possibly different resolution, the following comment is re-indented incorrectly:

if foo { // this comment describes the entire block
    let x = y();
}

Which incorrectly interprets the comment as belonging within the if statement, which is
technically correct only in the most pedantic sense, as to human programmers (an admittedly slowly
dying species) it is fairly clear that the comment (regardless of its contents) describes the preceding
contents of the line and does not belong inside the block:

if foo {
    // this comment describes the entire block
    let x = y();
}

As now the comment appears to describe the first line of the block (let x = y();) and not the
block itself. This should either not be reformatted or it should be reformatted to the following:

// this comment describes the entire block
if foo {
    let x = y();
}
@whizsid
Copy link
Contributor

whizsid commented Feb 21, 2021

I am working on this issue.

@RalfJung
Copy link
Member

This issue still exists, and it is quite a problem IMO. Putting the comment on the line after the if changes the semantic meaning of what the comment explains! Consider the following code:

                if flags != 2 { // BCRYPT_USE_SYSTEM_PREFERRED_RNG
                    throw_unsup_format!(
                        "BCryptGenRandom is supported only with the BCRYPT_USE_SYSTEM_PREFERRED_RNG flag"
                    );
                }

The comment is crucial to understand what the conditional check itself is doing.
However, rustfmt changes this to

                if flags != 2 {
                    // BCRYPT_USE_SYSTEM_PREFERRED_RNG
                    throw_unsup_format!(
                        "BCryptGenRandom is supported only with the BCRYPT_USE_SYSTEM_PREFERRED_RNG flag"
                    );
                }

This is bad because it changes the semantic comment of the comment! Now it looks like the comment is explaining the throw_unsup_format invocation.

I think this kind of re-ordering of comments is not something a tool should do automatically. I also found no way, including using /*...*/ comments, to actually have a satisfying formatting for this code that rustfmt accept.

Is there any way to disable this comment-changing behavior?

Cc #4573

bors added a commit to rust-lang/miri that referenced this issue Jun 22, 2022
Format tests with rustfmt (276-287 of 299)

Extracted from #2097.

This is one half of the last 24 files (left for last because they require more unique attention than the first 275 "easy" files).

I'll comment below to call attention to cases where I exercised my own judgement in how to format the test.

rust-lang/rustfmt#3255 is especially annoying: rustfmt does not like `…( //` and `…{ //`.
@q2333gh
Copy link

q2333gh commented Jul 23, 2023

in year 2023. still not find any solution . I got the same problem too, #5856. the pr may fix the problem has not accept yet: pull/4745

@mbartelsm
Copy link

mbartelsm commented Jan 3, 2024

2024, let's go.

If I can't document my code correctly without the formatter changing the meaning of my comments, then that means the formatter is unusable for me.

@RalfJung
Copy link
Member

RalfJung commented Jul 6, 2024

Seems like the current state of this is that a PR exists at #4745, but it targets the wrong branch and has to be rebased to master. I don't know how much rustfmt-2.0.0-rc.2 differs from master -- how hard would such a rebase be?

@ytmimi
Copy link
Contributor

ytmimi commented Jul 6, 2024

Tough to say how much things have diverged for this change in particular, but we might get lucky and it could be a simple cherry-pick of the commits to a new branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment