Fix #5533: Prefer light_rewrite_comment if it is not a doccomment

[pan93412]

This pull request adapts the plan 2 of @ytmimi's comment, and it won't touch any comment which is not a documentation comment now.

However, it did not fixed the root cause (which is partially indicated in Plan 1): block_comment is true if the length is too long…?. As it seems not related to #5533, it may be fixed in another PR.

[pan93412]

cc @ytmimi . Thanks!

[pan93412]

Thanks for the follow up!

I still believe is_doc_comment is the only boolean we need to include in our check to force code execution to move to light_rewrite_comment. I could be missing something, and if we need to check both is_doc_comment and style.is_doc_comment could you please explain why. The comment is helpful and I liked that you kept it 👍🏼

I have reverted it. However, why did L384 do such a double check?

https://github.com/rust-lang/rustfmt/blob/8919fd39d13b60a2c2e70e4f768cbd083b318de4/src/comment.rs#L378-L385

Seems like this code had been refactored before, and I don't know why they wrote it. Besides, even when I removed the determination, all the tests were still passed.

[ytmimi]

why did L384 do such a double check?

That decision predates my time with the project. My best guess is that there was a time when both were necessary to be absolutely sure we were rewriting a doc comment. Now that we have rewrite_comment and rewrite_doc_comment as our top level comment rewriting functions it's easier to differentiate what kind of comment we're rewriting.

Seems like this code had been refactored before, and I don't know why they wrote it. Besides, even when I removed the determination, all the tests were still passed.

It's might be the case that style.is_doc_comment() is no longer needed, but it's not hurting us right now, so we'll keep it for the time being.

[ytmimi]

@pan93412 I noticed that this PR also resolves #5568 so I added a test case on your branch to validate that

[pan93412]

@pan93412 I noticed that this PR also resolves #5568 so I added a test case on your branch to validate that

Thanks!

[lopopolo]

Hey folks, I think this PR has the ready to merge label and it resolves several issues with formatting comments. What else needs to be done to get this merged?

[ytmimi]

@pan93412 thanks again for your help on this one. This fixes a few issues so I'm going to merge.

Diff check job ran successfully ✅

[mightyiam]

Thank you.