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.

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

Jump to bottom

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
Assignees
@topecongiro
Labels
a-comments poor-formatting

Comments

@mqudsi
Copy link

mqudsi commented Dec 16, 2018 •
edited
Loading

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();
}
This was referenced Jan 3, 2020
Closed
Closed
@topecongiro topecongiro removed the p-low label Jan 14, 2020
@topecongiro topecongiro self-assigned this Jan 14, 2020
@carols10cents carols10cents mentioned this issue Jan 24, 2020
Closed
@whizsid
Copy link
Contributor

whizsid commented Feb 21, 2021

I am working on this issue.

@whizsid whizsid mentioned this issue Feb 22, 2021
Closed
@whizsid whizsid linked a pull request Mar 8, 2021 that will close this issue
Open
@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

@RalfJung RalfJung mentioned this issue May 17, 2021
Closed
@dtolnay dtolnay mentioned this issue Jun 22, 2022
Merged
bors added a commit to rust-lang/miri that referenced this issue Jun 22, 2022
@bors
Auto merge of #2254 - dtolnay-contrib:rustfmt6, r=oli-obk
769c6c7 
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 `…{ //`.
@ytmimi ytmimi mentioned this issue Jan 8, 2023
Closed
@calebcartwright calebcartwright mentioned this issue Jul 22, 2023
Closed
@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

@ytmimi ytmimi mentioned this issue Aug 15, 2023
Closed
@mbartelsm
Copy link

mbartelsm commented Jan 3, 2024 •
edited
Loading

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.

This was referenced Jan 9, 2024
Merged
Merged
@RalfJung RalfJung mentioned this issue Jul 6, 2024
Merged
@RalfJung
Copy link
Member

RalfJung commented Jul 6, 2024 •
edited
Loading

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.

@RalfJung RalfJung linked a pull request Aug 4, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@topecongiro topecongiro

Labels
a-comments poor-formatting
Projects
None yet
Milestone
No milestone
7 participants
@RalfJung @mqudsi @mbartelsm @topecongiro @ytmimi @q2333gh @whizsid