Fix `println_empty_string` suggestion caused error when there is a `,` after arg

[relaxcn]

Closes: rust-lang/rust-clippy#16167

changelog: [println_empty_string]: fix suggestion caused error when there is a comma after arg.

[rustbot]

r? @samueltardieu

rustbot has assigned @samueltardieu. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[relaxcn]

@rustbot ready

[relaxcn]

Ensure that there is no comment inside the macro call and don't provide a suggestion in this case.

It's reasonable. But how about using rustc_lexer to skip comment, whitespace to find the following span of ,?

Or we just skip linting the situation you mentioned above.

[relaxcn]

@rustbot ready

[samueltardieu]

Ensure that there is no comment inside the macro call and don't provide a suggestion in this case.

It's reasonable. But how about using rustc_lexer to skip comment, whitespace to find the following span of ,?

Or we just skip linting the situation you mentioned above.

Note that I'm not proposing not to lint, but to not provide a suggestion (as in "autofix") in this case.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[relaxcn]

This is a lot of complex code for practically no gain. What about my proposal to just not have an autofix if there are comments inside the macro call? You can still lint, you just have to replace the suggestion by a help message saying "remove the empty string" and let the user deal with it.

Thank you. I think using simpler but more robust way to compute span is good as you mentioned before.

So I adopted your suggested code.

However, I still think that using rustc_lexer is a more perfect solution.

At least for writeln_empty_string line, there must be a comma in the macro. We can't use a way that determines whether to use MachineApplicable by checking for the presence of commas in the macro, so we also need to come up with another "compromise" at that time.

Compared to carefully handling strings, operating on tokens is more reassuring.

Just my two cents :)

@rustbot ready