Auto merge of #13654 - GnomedDev:early-exit-visitors, r=Alexendoo

bors · bors · commit 6631a2c1b707 · 2024-11-05T17:39:32.000Z
Swap Visitors to early exit, instead of storing poison flag

I noticed that a bunch of visitors had a `poison` or `success` field, when they could just be returning `ControlFlow`.

changelog: none
diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs
@@ -540,24 +540,22 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
             .iter()
             .filter(|&&(_, name)| !name.as_str().starts_with('_'))
             .any(|&(_, name)| {
-                let mut walker = ContainsName {
-                    name,
-                    result: false,
-                    cx,
-                };
+                let mut walker = ContainsName { name, cx };
 
                 // Scan block
-                block
+                let mut res = block
                     .stmts
                     .iter()
                     .filter(|stmt| !ignore_span.overlaps(stmt.span))
-                    .for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
+                    .try_for_each(|stmt| intravisit::walk_stmt(&mut walker, stmt));
 
                 if let Some(expr) = block.expr {
-                    intravisit::walk_expr(&mut walker, expr);
+                    if res.is_continue() {
+                        res = intravisit::walk_expr(&mut walker, expr);
+                    }
                 }
 
-                walker.result
+                res.is_break()
             })
     })
 }
diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs
@@ -1,3 +1,5 @@
+use std::ops::ControlFlow;
+
 use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then, span_lint_hir_and_then};
 use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy};
 use clippy_utils::{has_non_exhaustive_attr, is_lint_allowed, match_def_path, paths};
@@ -371,9 +373,8 @@ fn check_unsafe_derive_deserialize<'tcx>(
     ty: Ty<'tcx>,
 ) {
     fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
-        let mut visitor = UnsafeVisitor { cx, has_unsafe: false };
-        walk_item(&mut visitor, item);
-        visitor.has_unsafe
+        let mut visitor = UnsafeVisitor { cx };
+        walk_item(&mut visitor, item).is_break()
     }
 
     if let Some(trait_def_id) = trait_ref.trait_def_id()
@@ -406,38 +407,37 @@ fn check_unsafe_derive_deserialize<'tcx>(
 
 struct UnsafeVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
-    has_unsafe: bool,
 }
 
 impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
+    type Result = ControlFlow<()>;
     type NestedFilter = nested_filter::All;
 
-    fn visit_fn(&mut self, kind: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body_id: BodyId, _: Span, id: LocalDefId) {
-        if self.has_unsafe {
-            return;
-        }
-
+    fn visit_fn(
+        &mut self,
+        kind: FnKind<'tcx>,
+        decl: &'tcx FnDecl<'_>,
+        body_id: BodyId,
+        _: Span,
+        id: LocalDefId,
+    ) -> Self::Result {
         if let Some(header) = kind.header()
             && header.safety == Safety::Unsafe
         {
-            self.has_unsafe = true;
+            ControlFlow::Break(())
+        } else {
+            walk_fn(self, kind, decl, body_id, id)
         }
-
-        walk_fn(self, kind, decl, body_id, id);
     }
 
-    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
-        if self.has_unsafe {
-            return;
-        }
-
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> Self::Result {
         if let ExprKind::Block(block, _) = expr.kind {
             if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) {
-                self.has_unsafe = true;
+                return ControlFlow::Break(());
             }
         }
 
-        walk_expr(self, expr);
+        walk_expr(self, expr)
     }
 
     fn nested_visit_map(&mut self) -> Self::Map {
diff --git a/clippy_lints/src/from_over_into.rs b/clippy_lints/src/from_over_into.rs
@@ -1,3 +1,5 @@
+use std::ops::ControlFlow;
+
 use clippy_config::Conf;
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_then;
@@ -115,43 +117,46 @@ impl<'tcx> LateLintPass<'tcx> for FromOverInto {
 }
 
 /// Finds the occurrences of `Self` and `self`
+///
+/// Returns `ControlFlow::break` if any of the `self`/`Self` usages were from an expansion, or the
+/// body contained a binding already named `val`.
 struct SelfFinder<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     /// Occurrences of `Self`
     upper: Vec<Span>,
     /// Occurrences of `self`
     lower: Vec<Span>,
-    /// If any of the `self`/`Self` usages were from an expansion, or the body contained a binding
-    /// already named `val`
-    invalid: bool,
 }
 
 impl<'tcx> Visitor<'tcx> for SelfFinder<'_, 'tcx> {
+    type Result = ControlFlow<()>;
     type NestedFilter = OnlyBodies;
 
     fn nested_visit_map(&mut self) -> Self::Map {
         self.cx.tcx.hir()
     }
 
-    fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) {
+    fn visit_path(&mut self, path: &Path<'tcx>, _id: HirId) -> Self::Result {
         for segment in path.segments {
             match segment.ident.name {
                 kw::SelfLower => self.lower.push(segment.ident.span),
                 kw::SelfUpper => self.upper.push(segment.ident.span),
                 _ => continue,
             }
 
-            self.invalid |= segment.ident.span.from_expansion();
+            if segment.ident.span.from_expansion() {
+                return ControlFlow::Break(());
+            }
         }
 
-        if !self.invalid {
-            walk_path(self, path);
-        }
+        walk_path(self, path)
     }
 
-    fn visit_name(&mut self, name: Symbol) {
+    fn visit_name(&mut self, name: Symbol) -> Self::Result {
         if name == sym::val {
-            self.invalid = true;
+            ControlFlow::Break(())
+        } else {
+            ControlFlow::Continue(())
         }
     }
 }
@@ -209,11 +214,9 @@ fn convert_to_from(
         cx,
         upper: Vec::new(),
         lower: Vec::new(),
-        invalid: false,
     };
-    finder.visit_expr(body.value);
 
-    if finder.invalid {
+    if finder.visit_expr(body.value).is_break() {
         return None;
     }
 
diff --git a/clippy_lints/src/loops/while_immutable_condition.rs b/clippy_lints/src/loops/while_immutable_condition.rs
@@ -19,10 +19,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'
         cx,
         ids: HirIdSet::default(),
         def_ids: DefIdMap::default(),
-        skip: false,
     };
-    var_visitor.visit_expr(cond);
-    if var_visitor.skip {
+    if var_visitor.visit_expr(cond).is_break() {
         return;
     }
     let used_in_condition = &var_visitor.ids;
@@ -81,7 +79,6 @@ struct VarCollectorVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     ids: HirIdSet,
     def_ids: DefIdMap<bool>,
-    skip: bool,
 }
 
 impl<'tcx> VarCollectorVisitor<'_, 'tcx> {
@@ -104,11 +101,15 @@ impl<'tcx> VarCollectorVisitor<'_, 'tcx> {
 }
 
 impl<'tcx> Visitor<'tcx> for VarCollectorVisitor<'_, 'tcx> {
-    fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
+    type Result = ControlFlow<()>;
+    fn visit_expr(&mut self, ex: &'tcx Expr<'_>) -> Self::Result {
         match ex.kind {
-            ExprKind::Path(_) => self.insert_def_id(ex),
+            ExprKind::Path(_) => {
+                self.insert_def_id(ex);
+                ControlFlow::Continue(())
+            },
             // If there is any function/method call… we just stop analysis
-            ExprKind::Call(..) | ExprKind::MethodCall(..) => self.skip = true,
+            ExprKind::Call(..) | ExprKind::MethodCall(..) => ControlFlow::Break(()),
 
             _ => walk_expr(self, ex),
         }
diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs
@@ -1,3 +1,5 @@
+use std::ops::ControlFlow;
+
 use super::WHILE_LET_ON_ITERATOR;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
@@ -204,35 +206,32 @@ fn uses_iter<'tcx>(cx: &LateContext<'tcx>, iter_expr: &IterExpr, container: &'tc
     struct V<'a, 'b, 'tcx> {
         cx: &'a LateContext<'tcx>,
         iter_expr: &'b IterExpr,
-        uses_iter: bool,
     }
     impl<'tcx> Visitor<'tcx> for V<'_, '_, 'tcx> {
-        fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
-            if self.uses_iter {
-                // return
-            } else if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
-                self.uses_iter = true;
+        type Result = ControlFlow<()>;
+        fn visit_expr(&mut self, e: &'tcx Expr<'_>) -> Self::Result {
+            if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
+                ControlFlow::Break(())
             } else if let (e, true) = skip_fields_and_path(e) {
                 if let Some(e) = e {
-                    self.visit_expr(e);
+                    self.visit_expr(e)
+                } else {
+                    ControlFlow::Continue(())
                 }
             } else if let ExprKind::Closure(&Closure { body: id, .. }) = e.kind {
                 if is_res_used(self.cx, self.iter_expr.path, id) {
-                    self.uses_iter = true;
+                    ControlFlow::Break(())
+                } else {
+                    ControlFlow::Continue(())
                 }
             } else {
-                walk_expr(self, e);
+                walk_expr(self, e)
             }
         }
     }
 
-    let mut v = V {
-        cx,
-        iter_expr,
-        uses_iter: false,
-    };
-    v.visit_expr(container);
-    v.uses_iter
+    let mut v = V { cx, iter_expr };
+    v.visit_expr(container).is_break()
 }
 
 #[expect(clippy::too_many_lines)]
@@ -242,34 +241,38 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &
         iter_expr: &'b IterExpr,
         loop_id: HirId,
         after_loop: bool,
-        used_iter: bool,
     }
     impl<'tcx> Visitor<'tcx> for AfterLoopVisitor<'_, '_, 'tcx> {
         type NestedFilter = OnlyBodies;
+        type Result = ControlFlow<()>;
         fn nested_visit_map(&mut self) -> Self::Map {
             self.cx.tcx.hir()
         }
 
-        fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
-            if self.used_iter {
-                return;
-            }
+        fn visit_expr(&mut self, e: &'tcx Expr<'_>) -> Self::Result {
             if self.after_loop {
                 if is_expr_same_child_or_parent_field(self.cx, e, &self.iter_expr.fields, self.iter_expr.path) {
-                    self.used_iter = true;
+                    ControlFlow::Break(())
                 } else if let (e, true) = skip_fields_and_path(e) {
                     if let Some(e) = e {
-                        self.visit_expr(e);
+                        self.visit_expr(e)
+                    } else {
+                        ControlFlow::Continue(())
                     }
                 } else if let ExprKind::Closure(&Closure { body: id, .. }) = e.kind {
-                    self.used_iter = is_res_used(self.cx, self.iter_expr.path, id);
+                    if is_res_used(self.cx, self.iter_expr.path, id) {
+                        ControlFlow::Break(())
+                    } else {
+                        ControlFlow::Continue(())
+                    }
                 } else {
-                    walk_expr(self, e);
+                    walk_expr(self, e)
                 }
             } else if self.loop_id == e.hir_id {
                 self.after_loop = true;
+                ControlFlow::Continue(())
             } else {
-                walk_expr(self, e);
+                walk_expr(self, e)
             }
         }
     }
@@ -347,9 +350,8 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: &
             iter_expr,
             loop_id: loop_expr.hir_id,
             after_loop: false,
-            used_iter: false,
         };
-        v.visit_expr(cx.tcx.hir().body(cx.enclosing_body.unwrap()).value);
-        v.used_iter
+        v.visit_expr(cx.tcx.hir().body(cx.enclosing_body.unwrap()).value)
+            .is_break()
     }
 }
diff --git a/clippy_lints/src/matches/match_str_case_mismatch.rs b/clippy_lints/src/matches/match_str_case_mismatch.rs
@@ -1,3 +1,5 @@
+use std::ops::ControlFlow;
+
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::ty::is_type_lang_item;
 use rustc_ast::ast::LitKind;
@@ -23,11 +25,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>, arm
     if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(scrutinee).kind()
         && let ty::Str = ty.kind()
     {
-        let mut visitor = MatchExprVisitor { cx, case_method: None };
-
-        visitor.visit_expr(scrutinee);
-
-        if let Some(case_method) = visitor.case_method {
+        let mut visitor = MatchExprVisitor { cx };
+        if let ControlFlow::Break(case_method) = visitor.visit_expr(scrutinee) {
             if let Some((bad_case_span, bad_case_sym)) = verify_case(&case_method, arms) {
                 lint(cx, &case_method, bad_case_span, bad_case_sym.as_str());
             }
@@ -37,30 +36,33 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>, arm
 
 struct MatchExprVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
-    case_method: Option<CaseMethod>,
 }
 
 impl<'tcx> Visitor<'tcx> for MatchExprVisitor<'_, 'tcx> {
-    fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
-        match ex.kind {
-            ExprKind::MethodCall(segment, receiver, [], _) if self.case_altered(segment.ident.as_str(), receiver) => {},
-            _ => walk_expr(self, ex),
+    type Result = ControlFlow<CaseMethod>;
+    fn visit_expr(&mut self, ex: &'tcx Expr<'_>) -> Self::Result {
+        if let ExprKind::MethodCall(segment, receiver, [], _) = ex.kind {
+            let result = self.case_altered(segment.ident.as_str(), receiver);
+            if result.is_break() {
+                return result;
+            }
         }
+
+        walk_expr(self, ex)
     }
 }
 
 impl MatchExprVisitor<'_, '_> {
-    fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> bool {
+    fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> ControlFlow<CaseMethod> {
         if let Some(case_method) = get_case_method(segment_ident) {
             let ty = self.cx.typeck_results().expr_ty(receiver).peel_refs();
 
             if is_type_lang_item(self.cx, ty, LangItem::String) || ty.kind() == &ty::Str {
-                self.case_method = Some(case_method);
-                return true;
+                return ControlFlow::Break(case_method);
             }
         }
 
-        false
+        ControlFlow::Continue(())
     }
 }
 
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
diff --git a/clippy_utils/src/usage.rs b/clippy_utils/src/usage.rs
diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs
