Check expr usage for manual_flatten
#7566
Conversation
|
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| @@ -47,6 +48,9 @@ pub(super) fn check<'tcx>( | |||
| let some_ctor = is_lang_ctor(cx, qpath, OptionSome); | |||
| let ok_ctor = is_lang_ctor(cx, qpath, ResultOk); | |||
| if some_ctor || ok_ctor; | |||
| // Ensure epxr in `if let` is not used afterwards | |||
| let mut used_visitor = LocalUsedVisitor::new(cx, pat_hir_id); | |||
| if !match_arms.iter().any(|arm| used_visitor.check_arm(arm)); | |||
There was a problem hiding this comment.
Small NIT: The usage of an iterator can be avoided here by deconstructing the slice in line 40ff like this:
if let ExprKind::Match(
match_expr, [true_arm, _else_arm], MatchSource::IfLetDesugar{ contains_else_clause: false }
) = inner_expr.kind;Then it should be possible to directly call if !LocalUsedVisitor::new(cx, pat_hir_id).check_arm(true_arm); and the direct index call in line 47 can also be avoided.
The rest looks good to me. I'm happy to merge this afterwards 🙃
There was a problem hiding this comment.
Ah that was a nice one. I should re-emphasize Rust's powerful pattern matching in my bible :)
There was a problem hiding this comment.
Looking at this, it's actually surprising that we don't have a lint for this. I'll create am issue, even if this might ignore macros as most lints do 🙃
There was a problem hiding this comment.
Created #7569 and also liked this PR as an example how this improves code quality. I also suggested to lint inside if_chain! macros to catch these cases inside Clippy. Thank you again for the contributing and inspiration 🙃
|
r? @xFrednet |
dswij
changed the title
manual_flatenmanual_flatten
Add a scenario where `manual_flatten` is triggered when match expression will still be used after the match in `if let`.
`manual_flatten` should not trigger when match expression in `if let` is going to be used.
dswij
force-pushed
the
manual-flatten-use
branch
from
8a29f65
to
a09bc1b
Compare
|
@bors r+ |
|
📌 Commit a09bc1b has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #6784
Fixes #7538
manual_flattenshould not trigger whenif letmatch expression will be used.changelog: [
manual_flatten] checks for expr usage afterif let