Skip to content

Conversation

@GuillaumeGomez
Copy link
Member

We realized when running clippy --fix on rustdoc (PR comment here) that some comments were removed, which is problematic. This PR checks that comments outside of match arms are taken into account before emitting the lint.

changelog: Fix single_match lint being emitted when it should not

@rustbot
Copy link
Collaborator

rustbot commented Nov 30, 2024

r? @llogiq

rustbot has assigned @llogiq.
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 rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 30, 2024
redundant_pattern_match::check_match(cx, expr, ex, arms);
single_match::check(cx, ex, arms, expr);
let source_map = cx.tcx.sess.source_map();
let mut match_comments = span_extract_comments(source_map, expr.span);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to extract the comment spans instead of the text to compare?

What if we have the same comment text twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a Vec, not a map. So if a comment is present more than once, not an issue since it'll still remain one at the end. Gonna add a test for this case though.

As for using spans, it's possible but would likely make the code much less readable as we manipulate InnerSpans here since we parse a snippet and not use AST comments.

@llogiq
Copy link
Contributor

llogiq commented Dec 7, 2024

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 7, 2024
@GuillaumeGomez
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 13, 2024
@llogiq
Copy link
Contributor

llogiq commented Dec 13, 2024

Thank you!

@llogiq llogiq added this pull request to the merge queue Dec 13, 2024
Merged via the queue into rust-lang:master with commit c607408 Dec 13, 2024
9 checks passed
@GuillaumeGomez GuillaumeGomez deleted the fix-single_match branch December 13, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants