1

Check expr usage for `manual_flatten` by dswij · Pull Request #7566 · rust-lang...

 2 years ago
source link: https://github.com/rust-lang/rust-clippy/pull/7566
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Copy link

Collaborator

rust-highfive commented 5 days ago

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@@ -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));

xFrednet 4 days ago

edited

Member

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 upside_down_face

dswij 3 days ago

Author

Contributor

Ah that was a nice one. I should re-emphasize Rust's powerful pattern matching in my bible :)

xFrednet 3 days ago

Member

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 upside_down_face

xFrednet 3 days ago

Member

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 upside_down_face


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK