-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix single_match
lint being emitted when it should not
#13765
Conversation
@@ -1059,7 +1061,28 @@ impl<'tcx> LateLintPass<'tcx> for Matches { | |||
} | |||
|
|||
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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 InnerSpan
s here since we parse a snippet and not use AST comments.
@rustbot author |
436be6a
to
771b0ec
Compare
@rustbot ready |
771b0ec
to
efe3fe9
Compare
Thank you! |
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 ofmatch
arms are taken into account before emitting the lint.changelog: Fix
single_match
lint being emitted when it should not