diff options
| -rw-r--r-- | clippy_lints/src/lib.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lifetimes.rs | 18 | ||||
| -rw-r--r-- | clippy_lints/src/loops/mut_range_bound.rs | 15 | ||||
| -rw-r--r-- | clippy_lints/src/loops/while_immutable_condition.rs | 24 | ||||
| -rw-r--r-- | clippy_lints/src/methods/option_map_unwrap_or.rs | 32 | ||||
| -rw-r--r-- | clippy_lints/src/redundant_closure_call.rs | 25 | ||||
| -rw-r--r-- | clippy_lints/src/unconditional_recursion.rs | 14 | ||||
| -rw-r--r-- | clippy_lints/src/unused_peekable.rs | 61 | ||||
| -rw-r--r-- | clippy_utils/src/visitors.rs | 29 |
9 files changed, 100 insertions, 119 deletions
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5b40e362bca..ecfb6cdbdf0 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,6 +1,7 @@ #![feature(array_windows)] #![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] +#![feature(control_flow_enum)] #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 443d6189c1f..9f063ced313 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -22,6 +22,7 @@ use rustc_session::declare_lint_pass; use rustc_span::def_id::LocalDefId; use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::Span; +use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does @@ -380,11 +381,8 @@ fn could_use_elision<'tcx>( return None; } - let mut checker = BodyLifetimeChecker { - lifetimes_used_in_body: false, - }; - checker.visit_expr(body.value); - if checker.lifetimes_used_in_body { + let mut checker = BodyLifetimeChecker; + if checker.visit_expr(body.value).is_break() { return None; } } @@ -694,15 +692,15 @@ fn report_extra_impl_lifetimes<'tcx>(cx: &LateContext<'tcx>, impl_: &'tcx Impl<' } } -struct BodyLifetimeChecker { - lifetimes_used_in_body: bool, -} +struct BodyLifetimeChecker; impl<'tcx> Visitor<'tcx> for BodyLifetimeChecker { + type Result = ControlFlow<()>; // for lifetimes as parameters of generics - fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) { + fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) -> ControlFlow<()> { if !lifetime.is_anonymous() && lifetime.ident.name != kw::StaticLifetime { - self.lifetimes_used_in_body = true; + return ControlFlow::Break(()); } + ControlFlow::Continue(()) } } diff --git a/clippy_lints/src/loops/mut_range_bound.rs b/clippy_lints/src/loops/mut_range_bound.rs index 6c6a9a1a2e0..21f9a71f2c5 100644 --- a/clippy_lints/src/loops/mut_range_bound.rs +++ b/clippy_lints/src/loops/mut_range_bound.rs @@ -8,6 +8,7 @@ use rustc_lint::LateContext; use rustc_middle::mir::FakeReadCause; use rustc_middle::ty; use rustc_span::Span; +use std::ops::ControlFlow; pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) { if let Some(higher::Range { @@ -114,7 +115,6 @@ impl MutatePairDelegate<'_, '_> { struct BreakAfterExprVisitor { hir_id: HirId, past_expr: bool, - past_candidate: bool, break_after_expr: bool, } @@ -123,7 +123,6 @@ impl BreakAfterExprVisitor { let mut visitor = BreakAfterExprVisitor { hir_id, past_expr: false, - past_candidate: false, break_after_expr: false, }; @@ -135,21 +134,19 @@ impl BreakAfterExprVisitor { } impl<'tcx> Visitor<'tcx> for BreakAfterExprVisitor { - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if self.past_candidate { - return; - } - + type Result = ControlFlow<()>; + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> { if expr.hir_id == self.hir_id { self.past_expr = true; + ControlFlow::Continue(()) } else if self.past_expr { if matches!(&expr.kind, ExprKind::Break(..)) { self.break_after_expr = true; } - self.past_candidate = true; + ControlFlow::Break(()) } else { - intravisit::walk_expr(self, expr); + intravisit::walk_expr(self, expr) } } } diff --git a/clippy_lints/src/loops/while_immutable_condition.rs b/clippy_lints/src/loops/while_immutable_condition.rs index 3dff826cb85..e7b3a2c4973 100644 --- a/clippy_lints/src/loops/while_immutable_condition.rs +++ b/clippy_lints/src/loops/while_immutable_condition.rs @@ -7,6 +7,7 @@ use rustc_hir::def_id::DefIdMap; use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::{Expr, ExprKind, HirIdSet, QPath}; use rustc_lint::LateContext; +use std::ops::ControlFlow; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) { if constant(cx, cx.typeck_results(), cond).is_some() { @@ -35,11 +36,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &' }; let mutable_static_in_cond = var_visitor.def_ids.items().any(|(_, v)| *v); - let mut has_break_or_return_visitor = HasBreakOrReturnVisitor { - has_break_or_return: false, - }; - has_break_or_return_visitor.visit_expr(expr); - let has_break_or_return = has_break_or_return_visitor.has_break_or_return; + let mut has_break_or_return_visitor = HasBreakOrReturnVisitor; + let has_break_or_return = has_break_or_return_visitor.visit_expr(expr).is_break(); if no_cond_variable_mutated && !mutable_static_in_cond { span_lint_and_then( @@ -59,25 +57,19 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &' } } -struct HasBreakOrReturnVisitor { - has_break_or_return: bool, -} +struct HasBreakOrReturnVisitor; impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor { - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if self.has_break_or_return { - return; - } - + type Result = ControlFlow<()>; + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> ControlFlow<()> { match expr.kind { ExprKind::Ret(_) | ExprKind::Break(_, _) => { - self.has_break_or_return = true; - return; + return ControlFlow::Break(()); }, _ => {}, } - walk_expr(self, expr); + walk_expr(self, expr) } } diff --git a/clippy_lints/src/methods/option_map_unwrap_or.rs b/clippy_lints/src/methods/option_map_unwrap_or.rs index 3d326bc99f9..ed3bed42eb3 100644 --- a/clippy_lints/src/methods/option_map_unwrap_or.rs +++ b/clippy_lints/src/methods/option_map_unwrap_or.rs @@ -10,6 +10,7 @@ use rustc_hir::{ExprKind, HirId, Node, PatKind, Path, QPath}; use rustc_lint::LateContext; use rustc_middle::hir::nested_filter; use rustc_span::{sym, Span}; +use std::ops::ControlFlow; use super::MAP_UNWRAP_OR; @@ -54,15 +55,14 @@ pub(super) fn check<'tcx>( let mut reference_visitor = ReferenceVisitor { cx, identifiers: unwrap_visitor.identifiers, - found_reference: false, unwrap_or_span: unwrap_arg.span, }; let map = cx.tcx.hir(); let body = map.body_owned_by(map.enclosing_body_owner(expr.hir_id)); - reference_visitor.visit_body(body); - if reference_visitor.found_reference { + // Visit the body, and return if we've found a reference + if reference_visitor.visit_body(body).is_break() { return; } } @@ -151,29 +151,27 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrapVisitor<'a, 'tcx> { struct ReferenceVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, identifiers: FxHashSet<HirId>, - found_reference: bool, unwrap_or_span: Span, } impl<'a, 'tcx> Visitor<'tcx> for ReferenceVisitor<'a, 'tcx> { type NestedFilter = nested_filter::All; - fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) { + type Result = ControlFlow<()>; + fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) -> ControlFlow<()> { // If we haven't found a reference yet, check if this references // one of the locals that was moved in the `unwrap_or` argument. // We are only interested in exprs that appear before the `unwrap_or` call. - if !self.found_reference { - if expr.span < self.unwrap_or_span - && let ExprKind::Path(ref path) = expr.kind - && let QPath::Resolved(_, path) = path - && let Res::Local(local_id) = path.res - && let Node::Pat(pat) = self.cx.tcx.hir_node(local_id) - && let PatKind::Binding(_, local_id, ..) = pat.kind - && self.identifiers.contains(&local_id) - { - self.found_reference = true; - } - rustc_hir::intravisit::walk_expr(self, expr); + if expr.span < self.unwrap_or_span + && let ExprKind::Path(ref path) = expr.kind + && let QPath::Resolved(_, path) = path + && let Res::Local(local_id) = path.res + && let Node::Pat(pat) = self.cx.tcx.hir_node(local_id) + && let PatKind::Binding(_, local_id, ..) = pat.kind + && self.identifiers.contains(&local_id) + { + return ControlFlow::Break(()); } + rustc_hir::intravisit::walk_expr(self, expr) } fn nested_visit_map(&mut self) -> Self::Map { diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index 47d3ed08b8e..bad9b979203 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -14,6 +14,7 @@ use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_session::declare_lint_pass; use rustc_span::ExpnKind; +use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does @@ -42,24 +43,15 @@ declare_clippy_lint! { declare_lint_pass!(RedundantClosureCall => [REDUNDANT_CLOSURE_CALL]); // Used to find `return` statements or equivalents e.g., `?` -struct ReturnVisitor { - found_return: bool, -} - -impl ReturnVisitor { - #[must_use] - fn new() -> Self { - Self { found_return: false } - } -} +struct ReturnVisitor; impl<'tcx> Visitor<'tcx> for ReturnVisitor { - fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { + type Result = ControlFlow<()>; + fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) -> ControlFlow<()> { if let ExprKind::Ret(_) | ExprKind::Match(.., hir::MatchSource::TryDesugar(_)) = ex.kind { - self.found_return = true; - } else { - hir_visit::walk_expr(self, ex); + return ControlFlow::Break(()); } + hir_visit::walk_expr(self, ex) } } @@ -101,9 +93,8 @@ fn find_innermost_closure<'tcx>( while let ExprKind::Closure(closure) = expr.kind && let body = cx.tcx.hir().body(closure.body) && { - let mut visitor = ReturnVisitor::new(); - visitor.visit_expr(body.value); - !visitor.found_return + let mut visitor = ReturnVisitor; + !visitor.visit_expr(body.value).is_break() } && steps > 0 { diff --git a/clippy_lints/src/unconditional_recursion.rs b/clippy_lints/src/unconditional_recursion.rs index 6ba98a92423..42100e1d755 100644 --- a/clippy_lints/src/unconditional_recursion.rs +++ b/clippy_lints/src/unconditional_recursion.rs @@ -16,6 +16,7 @@ use rustc_session::impl_lint_pass; use rustc_span::symbol::{kw, Ident}; use rustc_span::{sym, Span}; use rustc_trait_selection::error_reporting::traits::suggestions::ReturnsVisitor; +use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does @@ -276,7 +277,6 @@ struct CheckCalls<'a, 'tcx> { cx: &'a LateContext<'tcx>, map: Map<'tcx>, implemented_ty_id: DefId, - found_default_call: bool, method_span: Span, } @@ -285,16 +285,14 @@ where 'tcx: 'a, { type NestedFilter = nested_filter::OnlyBodies; + type Result = ControlFlow<()>; fn nested_visit_map(&mut self) -> Self::Map { self.map } - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if self.found_default_call { - return; - } - walk_expr(self, expr); + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> { + walk_expr(self, expr)?; if let ExprKind::Call(f, _) = expr.kind && let ExprKind::Path(qpath) = f.kind @@ -303,9 +301,10 @@ where && let Some(trait_def_id) = self.cx.tcx.trait_of_item(method_def_id) && self.cx.tcx.is_diagnostic_item(sym::Default, trait_def_id) { - self.found_default_call = true; span_error(self.cx, self.method_span, expr); + return ControlFlow::Break(()); } + ControlFlow::Continue(()) } } @@ -383,7 +382,6 @@ impl UnconditionalRecursion { cx, map: cx.tcx.hir(), implemented_ty_id, - found_default_call: false, method_span, }; walk_body(&mut c, body); diff --git a/clippy_lints/src/unused_peekable.rs b/clippy_lints/src/unused_peekable.rs index e6f799335d7..86a811e17ca 100644 --- a/clippy_lints/src/unused_peekable.rs +++ b/clippy_lints/src/unused_peekable.rs @@ -8,6 +8,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_session::declare_lint_pass; use rustc_span::sym; +use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does @@ -70,15 +71,16 @@ impl<'tcx> LateLintPass<'tcx> for UnusedPeekable { return; } - for stmt in &block.stmts[idx..] { - vis.visit_stmt(stmt); - } + let mut found_peek_call = block.stmts[idx..].iter().any(|stmt| vis.visit_stmt(stmt).is_break()); - if let Some(expr) = block.expr { - vis.visit_expr(expr); + if !found_peek_call + && let Some(expr) = block.expr + && vis.visit_expr(expr).is_break() + { + found_peek_call = true; } - if !vis.found_peek_call { + if !found_peek_call { span_lint_hir_and_then( cx, UNUSED_PEEKABLE, @@ -98,31 +100,23 @@ impl<'tcx> LateLintPass<'tcx> for UnusedPeekable { struct PeekableVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, expected_hir_id: HirId, - found_peek_call: bool, } impl<'a, 'tcx> PeekableVisitor<'a, 'tcx> { fn new(cx: &'a LateContext<'tcx>, expected_hir_id: HirId) -> Self { - Self { - cx, - expected_hir_id, - found_peek_call: false, - } + Self { cx, expected_hir_id } } } impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { type NestedFilter = OnlyBodies; + type Result = ControlFlow<()>; fn nested_visit_map(&mut self) -> Self::Map { self.cx.tcx.hir() } - fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { - if self.found_peek_call { - return; - } - + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> ControlFlow<()> { if path_to_local_id(ex, self.expected_hir_id) { for (_, node) in self.cx.tcx.hir().parent_iter(ex.hir_id) { match node { @@ -137,14 +131,14 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { && func_did == into_iter_did { // Probably a for loop desugar, stop searching - return; + return ControlFlow::Continue(()); } if args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) { - self.found_peek_call = true; + return ControlFlow::Break(()); } - return; + return ControlFlow::Continue(()); }, // Catch anything taking a Peekable mutably ExprKind::MethodCall( @@ -162,16 +156,14 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { if matches!(method_name, "peek" | "peek_mut" | "next_if" | "next_if_eq") && arg_is_mut_peekable(self.cx, self_arg) { - self.found_peek_call = true; - return; + return ControlFlow::Break(()); } // foo.some_method() excluding Iterator methods if remaining_args.iter().any(|arg| arg_is_mut_peekable(self.cx, arg)) && !is_trait_method(self.cx, expr, sym::Iterator) { - self.found_peek_call = true; - return; + return ControlFlow::Break(()); } // foo.by_ref(), keep checking for `peek` @@ -179,41 +171,42 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { continue; } - return; + return ControlFlow::Continue(()); }, ExprKind::AddrOf(_, Mutability::Mut, _) | ExprKind::Unary(..) | ExprKind::DropTemps(_) => { }, - ExprKind::AddrOf(_, Mutability::Not, _) => return, + ExprKind::AddrOf(_, Mutability::Not, _) => return ControlFlow::Continue(()), _ => { - self.found_peek_call = true; - return; + return ControlFlow::Break(()); }, } }, Node::LetStmt(LetStmt { init: Some(init), .. }) => { if arg_is_mut_peekable(self.cx, init) { - self.found_peek_call = true; + return ControlFlow::Break(()); } - return; + return ControlFlow::Continue(()); }, Node::Stmt(stmt) => { match stmt.kind { - StmtKind::Let(_) | StmtKind::Item(_) => self.found_peek_call = true, + StmtKind::Let(_) | StmtKind::Item(_) => { + return ControlFlow::Break(()); + }, StmtKind::Expr(_) | StmtKind::Semi(_) => {}, } - return; + return ControlFlow::Continue(()); }, Node::Block(_) | Node::ExprField(_) => {}, _ => { - return; + return ControlFlow::Continue(()); }, } } } - walk_expr(self, ex); + walk_expr(self, ex) } } diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs index 3a39e178515..7066c9ad2b9 100644 --- a/clippy_utils/src/visitors.rs +++ b/clippy_utils/src/visitors.rs @@ -109,23 +109,36 @@ pub fn for_each_expr_without_closures<'tcx, B, C: Continue>( res: Option<B>, } impl<'tcx, B, C: Continue, F: FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B, C>> Visitor<'tcx> for V<B, F> { - fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) { + type Result = ControlFlow<()>; + + fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) -> ControlFlow<()> { if self.res.is_some() { - return; + return ControlFlow::Break(()); } match (self.f)(e) { ControlFlow::Continue(c) if c.descend() => walk_expr(self, e), - ControlFlow::Break(b) => self.res = Some(b), - ControlFlow::Continue(_) => (), + ControlFlow::Break(b) => { + self.res = Some(b); + ControlFlow::Break(()) + }, + ControlFlow::Continue(_) => ControlFlow::Continue(()), } } // Avoid unnecessary `walk_*` calls. - fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) {} - fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) {} - fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) {} + fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) -> ControlFlow<()> { + ControlFlow::Continue(()) + } + fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) -> ControlFlow<()> { + ControlFlow::Continue(()) + } + fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) -> ControlFlow<()> { + ControlFlow::Continue(()) + } // Avoid monomorphising all `visit_*` functions. - fn visit_nested_item(&mut self, _: ItemId) {} + fn visit_nested_item(&mut self, _: ItemId) -> ControlFlow<()> { + ControlFlow::Continue(()) + } } let mut v = V { f, res: None }; node.visit(&mut v); |