diff --git a/clippy_lints/src/blocks_in_if_conditions.rs b/clippy_lints/src/blocks_in_if_conditions.rs index 4df4d6ddf416..569bf27c3e71 100644 --- a/clippy_lints/src/blocks_in_if_conditions.rs +++ b/clippy_lints/src/blocks_in_if_conditions.rs @@ -3,10 +3,11 @@ use clippy_utils::get_parent_expr; use clippy_utils::higher; use clippy_utils::source::snippet_block_with_applicability; use clippy_utils::ty::implements_trait; +use clippy_utils::visitors::{for_each_expr, Descend}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{BlockCheckMode, Closure, Expr, ExprKind}; +use rustc_hir::{BlockCheckMode, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -44,39 +45,6 @@ declare_clippy_lint! { declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]); -struct ExVisitor<'a, 'tcx> { - found_block: Option<&'tcx Expr<'tcx>>, - cx: &'a LateContext<'tcx>, -} - -impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if let ExprKind::Closure(&Closure { body, .. }) = expr.kind { - // do not lint if the closure is called using an iterator (see #1141) - if_chain! { - if let Some(parent) = get_parent_expr(self.cx, expr); - if let ExprKind::MethodCall(_, self_arg, ..) = &parent.kind; - let caller = self.cx.typeck_results().expr_ty(self_arg); - if let Some(iter_id) = self.cx.tcx.get_diagnostic_item(sym::Iterator); - if implements_trait(self.cx, caller, iter_id, &[]); - then { - return; - } - } - - let body = self.cx.tcx.hir().body(body); - let ex = &body.value; - if let ExprKind::Block(block, _) = ex.kind { - if !body.value.span.from_expansion() && !block.stmts.is_empty() { - self.found_block = Some(ex); - return; - } - } - } - walk_expr(self, expr); - } -} - const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition"; const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \ instead, move the block or closure higher and bind it with a `let`"; @@ -144,11 +112,31 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions { } } } else { - let mut visitor = ExVisitor { found_block: None, cx }; - walk_expr(&mut visitor, cond); - if let Some(block) = visitor.found_block { - span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE); - } + let _: Option = for_each_expr(cond, |e| { + if let ExprKind::Closure(closure) = e.kind { + // do not lint if the closure is called using an iterator (see #1141) + if_chain! { + if let Some(parent) = get_parent_expr(cx, e); + if let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind; + let caller = cx.typeck_results().expr_ty(self_arg); + if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator); + if implements_trait(cx, caller, iter_id, &[]); + then { + return ControlFlow::Continue(Descend::No); + } + } + + let body = cx.tcx.hir().body(closure.body); + let ex = &body.value; + if let ExprKind::Block(block, _) = ex.kind { + if !body.value.span.from_expansion() && !block.stmts.is_empty() { + span_lint(cx, BLOCKS_IN_IF_CONDITIONS, ex.span, COMPLEX_BLOCK_MESSAGE); + return ControlFlow::Continue(Descend::No); + } + } + } + ControlFlow::Continue(Descend::Yes) + }); } } } diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index fed04ae7f3d5..77af3b53d633 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -3,10 +3,12 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::source::snippet_opt; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::for_each_expr; use clippy_utils::LimitStack; +use core::ops::ControlFlow; use rustc_ast::ast::Attribute; -use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, ExprKind, FnDecl, HirId}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -61,11 +63,27 @@ impl CognitiveComplexity { return; } - let expr = &body.value; + let expr = body.value; + + let mut cc = 1u64; + let mut returns = 0u64; + let _: Option = for_each_expr(expr, |e| { + match e.kind { + ExprKind::If(_, _, _) => { + cc += 1; + }, + ExprKind::Match(_, arms, _) => { + if arms.len() > 1 { + cc += 1; + } + cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64; + }, + ExprKind::Ret(_) => returns += 1, + _ => {}, + } + ControlFlow::Continue(()) + }); - let mut helper = CcHelper { cc: 1, returns: 0 }; - helper.visit_expr(expr); - let CcHelper { cc, returns } = helper; let ret_ty = cx.typeck_results().node_type(expr.hir_id); let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::Result) { returns @@ -74,13 +92,12 @@ impl CognitiveComplexity { (returns / 2) }; - let mut rust_cc = cc; // prevent degenerate cases where unreachable code contains `return` statements - if rust_cc >= ret_adjust { - rust_cc -= ret_adjust; + if cc >= ret_adjust { + cc -= ret_adjust; } - if rust_cc > self.limit.limit() { + if cc > self.limit.limit() { let fn_span = match kind { FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span, FnKind::Closure => { @@ -107,7 +124,7 @@ impl CognitiveComplexity { COGNITIVE_COMPLEXITY, fn_span, &format!( - "the function has a cognitive complexity of ({rust_cc}/{})", + "the function has a cognitive complexity of ({cc}/{})", self.limit.limit() ), None, @@ -140,27 +157,3 @@ impl<'tcx> LateLintPass<'tcx> for CognitiveComplexity { self.limit.pop_attrs(cx.sess(), attrs, "cognitive_complexity"); } } - -struct CcHelper { - cc: u64, - returns: u64, -} - -impl<'tcx> Visitor<'tcx> for CcHelper { - fn visit_expr(&mut self, e: &'tcx Expr<'_>) { - walk_expr(self, e); - match e.kind { - ExprKind::If(_, _, _) => { - self.cc += 1; - }, - ExprKind::Match(_, arms, _) => { - if arms.len() > 1 { - self.cc += 1; - } - self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64; - }, - ExprKind::Ret(_) => self.returns += 1, - _ => {}, - } - } -} diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index 977c8ee594f4..6e85f6af5448 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -1,7 +1,7 @@ use rustc_ast::ast::Attribute; use rustc_errors::Applicability; use rustc_hir::def_id::{DefIdSet, LocalDefId}; -use rustc_hir::{self as hir, def::Res, intravisit, QPath}; +use rustc_hir::{self as hir, def::Res, QPath}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::{ lint::in_external_macro, @@ -13,8 +13,11 @@ use clippy_utils::attrs::is_proc_macro; use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::is_must_use_ty; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{match_def_path, return_ty, trait_ref_of_method}; +use core::ops::ControlFlow; + use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT}; pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { @@ -200,79 +203,55 @@ fn is_mutable_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, tys: &m } } -struct StaticMutVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - mutates_static: bool, +fn is_mutated_static(e: &hir::Expr<'_>) -> bool { + use hir::ExprKind::{Field, Index, Path}; + + match e.kind { + Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)), + Path(_) => true, + Field(inner, _) | Index(inner, _) => is_mutated_static(inner), + _ => false, + } } -impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { +fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool { + for_each_expr(body.value, |e| { use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall}; - if self.mutates_static { - return; - } - match expr.kind { + match e.kind { Call(_, args) => { let mut tys = DefIdSet::default(); for arg in args { - if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) - && is_mutable_ty( - self.cx, - self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), - arg.span, - &mut tys, - ) + if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) + && is_mutable_ty(cx, cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), arg.span, &mut tys) && is_mutated_static(arg) { - self.mutates_static = true; - return; + return ControlFlow::Break(()); } tys.clear(); } + ControlFlow::Continue(()) }, MethodCall(_, receiver, args, _) => { let mut tys = DefIdSet::default(); for arg in std::iter::once(receiver).chain(args.iter()) { - if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) - && is_mutable_ty( - self.cx, - self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), - arg.span, - &mut tys, - ) + if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) + && is_mutable_ty(cx, cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), arg.span, &mut tys) && is_mutated_static(arg) { - self.mutates_static = true; - return; + return ControlFlow::Break(()); } tys.clear(); } + ControlFlow::Continue(()) }, - Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) => { - self.mutates_static |= is_mutated_static(target); + Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) + if is_mutated_static(target) => + { + ControlFlow::Break(()) }, - _ => {}, + _ => ControlFlow::Continue(()), } - } -} - -fn is_mutated_static(e: &hir::Expr<'_>) -> bool { - use hir::ExprKind::{Field, Index, Path}; - - match e.kind { - Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)), - Path(_) => true, - Field(inner, _) | Index(inner, _) => is_mutated_static(inner), - _ => false, - } -} - -fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool { - let mut v = StaticMutVisitor { - cx, - mutates_static: false, - }; - intravisit::walk_expr(&mut v, body.value); - v.mutates_static + }) + .is_some() } diff --git a/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs index 0b50431fbaab..b7595d101e0f 100644 --- a/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs +++ b/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs @@ -5,8 +5,11 @@ use rustc_span::def_id::LocalDefId; use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::type_is_unsafe_function; +use clippy_utils::visitors::for_each_expr_with_closures; use clippy_utils::{iter_input_pats, path_to_local}; +use core::ops::ControlFlow; + use super::NOT_UNSAFE_PTR_ARG_DEREF; pub(super) fn check_fn<'tcx>( @@ -39,21 +42,34 @@ fn check_raw_ptr<'tcx>( body: &'tcx hir::Body<'tcx>, def_id: LocalDefId, ) { - let expr = &body.value; if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) { let raw_ptrs = iter_input_pats(decl, body) .filter_map(|arg| raw_ptr_arg(cx, arg)) .collect::(); if !raw_ptrs.is_empty() { - let typeck_results = cx.tcx.typeck_body(body.id()); - let mut v = DerefVisitor { - cx, - ptrs: raw_ptrs, - typeck_results, - }; - - intravisit::walk_expr(&mut v, expr); + let typeck = cx.tcx.typeck_body(body.id()); + let _: Option = for_each_expr_with_closures(cx, body.value, |e| { + match e.kind { + hir::ExprKind::Call(f, args) if type_is_unsafe_function(cx, typeck.expr_ty(f)) => { + for arg in args { + check_arg(cx, &raw_ptrs, arg); + } + }, + hir::ExprKind::MethodCall(_, recv, args, _) => { + let def_id = typeck.type_dependent_def_id(e.hir_id).unwrap(); + if cx.tcx.fn_sig(def_id).skip_binder().unsafety == hir::Unsafety::Unsafe { + check_arg(cx, &raw_ptrs, recv); + for arg in args { + check_arg(cx, &raw_ptrs, arg); + } + } + }, + hir::ExprKind::Unary(hir::UnOp::Deref, ptr) => check_arg(cx, &raw_ptrs, ptr), + _ => (), + } + ControlFlow::Continue(()) + }); } } } @@ -70,54 +86,13 @@ fn raw_ptr_arg(cx: &LateContext<'_>, arg: &hir::Param<'_>) -> Option } } -struct DerefVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - ptrs: HirIdSet, - typeck_results: &'a ty::TypeckResults<'tcx>, -} - -impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - match expr.kind { - hir::ExprKind::Call(f, args) => { - let ty = self.typeck_results.expr_ty(f); - - if type_is_unsafe_function(self.cx, ty) { - for arg in args { - self.check_arg(arg); - } - } - }, - hir::ExprKind::MethodCall(_, receiver, args, _) => { - let def_id = self.typeck_results.type_dependent_def_id(expr.hir_id).unwrap(); - let base_type = self.cx.tcx.type_of(def_id); - - if type_is_unsafe_function(self.cx, base_type) { - self.check_arg(receiver); - for arg in args { - self.check_arg(arg); - } - } - }, - hir::ExprKind::Unary(hir::UnOp::Deref, ptr) => self.check_arg(ptr), - _ => (), - } - - intravisit::walk_expr(self, expr); - } -} - -impl<'a, 'tcx> DerefVisitor<'a, 'tcx> { - fn check_arg(&self, ptr: &hir::Expr<'_>) { - if let Some(id) = path_to_local(ptr) { - if self.ptrs.contains(&id) { - span_lint( - self.cx, - NOT_UNSAFE_PTR_ARG_DEREF, - ptr.span, - "this public function might dereference a raw pointer but is not marked `unsafe`", - ); - } - } +fn check_arg(cx: &LateContext<'_>, raw_ptrs: &HirIdSet, arg: &hir::Expr<'_>) { + if path_to_local(arg).map_or(false, |id| raw_ptrs.contains(&id)) { + span_lint( + cx, + NOT_UNSAFE_PTR_ARG_DEREF, + arg.span, + "this public function might dereference a raw pointer but is not marked `unsafe`", + ); } } diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index ee16982d5248..98a3d8ff982d 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -2,9 +2,10 @@ use super::utils::clone_or_copy_needed; use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::is_copy; use clippy_utils::usage::mutated_variables; +use clippy_utils::visitors::{for_each_expr, Descend}; use clippy_utils::{is_lang_ctor, is_trait_method, path_to_local_id}; +use core::ops::ControlFlow; use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_lint::LateContext; use rustc_middle::ty; @@ -13,7 +14,7 @@ use rustc_span::sym; use super::UNNECESSARY_FILTER_MAP; use super::UNNECESSARY_FIND_MAP; -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>, name: &str) { if !is_trait_method(cx, expr, sym::Iterator) { return; } @@ -26,10 +27,16 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< let (mut found_mapping, mut found_filtering) = check_expression(cx, arg_id, body.value); - let mut return_visitor = ReturnVisitor::new(cx, arg_id); - return_visitor.visit_expr(body.value); - found_mapping |= return_visitor.found_mapping; - found_filtering |= return_visitor.found_filtering; + let _: Option = for_each_expr(body.value, |e| { + if let hir::ExprKind::Ret(Some(e)) = &e.kind { + let (found_mapping_res, found_filtering_res) = check_expression(cx, arg_id, e); + found_mapping |= found_mapping_res; + found_filtering |= found_filtering_res; + ControlFlow::Continue(Descend::No) + } else { + ControlFlow::Continue(Descend::Yes) + } + }); let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); let sugg = if !found_filtering { @@ -97,35 +104,3 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc _ => (true, true), } } - -struct ReturnVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - arg_id: hir::HirId, - // Found a non-None return that isn't Some(input) - found_mapping: bool, - // Found a return that isn't Some - found_filtering: bool, -} - -impl<'a, 'tcx> ReturnVisitor<'a, 'tcx> { - fn new(cx: &'a LateContext<'tcx>, arg_id: hir::HirId) -> ReturnVisitor<'a, 'tcx> { - ReturnVisitor { - cx, - arg_id, - found_mapping: false, - found_filtering: false, - } - } -} - -impl<'a, 'tcx> Visitor<'tcx> for ReturnVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - if let hir::ExprKind::Ret(Some(expr)) = &expr.kind { - let (found_mapping, found_filtering) = check_expression(self.cx, self.arg_id, expr); - self.found_mapping |= found_mapping; - self.found_filtering |= found_filtering; - } else { - walk_expr(self, expr); - } - } -} diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs index 2d5d5d143ffa..26bca7c306a8 100644 --- a/clippy_lints/src/operators/assign_op_pattern.rs +++ b/clippy_lints/src/operators/assign_op_pattern.rs @@ -2,11 +2,12 @@ use clippy_utils::binop_traits; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{eq_expr_value, trait_ref_of_method}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir_analysis::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_lint::LateContext; use rustc_middle::mir::FakeReadCause; @@ -65,15 +66,19 @@ pub(super) fn check<'tcx>( } }; - let mut visitor = ExprVisitor { - assignee, - counter: 0, - cx, - }; - - walk_expr(&mut visitor, e); + let mut found = false; + let found_multiple = for_each_expr(e, |e| { + if eq_expr_value(cx, assignee, e) { + if found { + return ControlFlow::Break(()); + } + found = true; + } + ControlFlow::Continue(()) + }) + .is_some(); - if visitor.counter == 1 { + if found && !found_multiple { // a = a op b if eq_expr_value(cx, assignee, l) { lint(assignee, r); @@ -98,22 +103,6 @@ pub(super) fn check<'tcx>( } } -struct ExprVisitor<'a, 'tcx> { - assignee: &'a hir::Expr<'a>, - counter: u8, - cx: &'a LateContext<'tcx>, -} - -impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - if eq_expr_value(self.cx, self.assignee, expr) { - self.counter += 1; - } - - walk_expr(self, expr); - } -} - fn imm_borrows_in_expr(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> hir::HirIdSet { struct S(hir::HirIdSet); impl Delegate<'_> for S { diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index f758f4cff8ba..2b2a41d16011 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -1,9 +1,11 @@ use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::source::{snippet_opt, snippet_with_context}; +use clippy_utils::visitors::{for_each_expr, Descend}; use clippy_utils::{fn_def_id, path_to_local_id}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; +use rustc_hir::intravisit::FnKind; use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; @@ -270,33 +272,20 @@ fn emit_return_lint( } fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { - let mut visitor = BorrowVisitor { cx, borrows: false }; - walk_expr(&mut visitor, expr); - visitor.borrows -} - -struct BorrowVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - borrows: bool, -} - -impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if self.borrows || expr.span.from_expansion() { - return; - } - - if let Some(def_id) = fn_def_id(self.cx, expr) { - self.borrows = self - .cx + for_each_expr(expr, |e| { + if let Some(def_id) = fn_def_id(cx, e) + && cx .tcx .fn_sig(def_id) - .output() .skip_binder() + .output() .walk() - .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_))); + .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_))) + { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(Descend::from(!expr.span.from_expansion())) } - - walk_expr(self, expr); - } + }) + .is_some() } diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index d47ed459387e..b57b484bdc89 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -1,8 +1,9 @@ use clippy_utils::diagnostics::span_lint; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{binop_traits, trait_ref_of_method, BINOP_TRAITS, OP_ASSIGN_TRAITS}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -92,25 +93,17 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { } fn count_binops(expr: &hir::Expr<'_>) -> u32 { - let mut visitor = BinaryExprVisitor::default(); - visitor.visit_expr(expr); - visitor.nb_binops -} - -#[derive(Default)] -struct BinaryExprVisitor { - nb_binops: u32, -} - -impl<'tcx> Visitor<'tcx> for BinaryExprVisitor { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - match expr.kind { + let mut count = 0u32; + let _: Option = for_each_expr(expr, |e| { + if matches!( + e.kind, hir::ExprKind::Binary(..) - | hir::ExprKind::Unary(hir::UnOp::Not | hir::UnOp::Neg, _) - | hir::ExprKind::AssignOp(..) => self.nb_binops += 1, - _ => {}, + | hir::ExprKind::Unary(hir::UnOp::Not | hir::UnOp::Neg, _) + | hir::ExprKind::AssignOp(..) + ) { + count += 1; } - - walk_expr(self, expr); - } + ControlFlow::Continue(()) + }); + count } diff --git a/clippy_lints/src/unwrap_in_result.rs b/clippy_lints/src/unwrap_in_result.rs index baa53ba664f6..a69719b127b2 100644 --- a/clippy_lints/src/unwrap_in_result.rs +++ b/clippy_lints/src/unwrap_in_result.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{method_chain_args, return_ty}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_hir as hir; -use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{Expr, ImplItemKind}; +use rustc_hir::ImplItemKind; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span}; @@ -73,51 +73,37 @@ impl<'tcx> LateLintPass<'tcx> for UnwrapInResult { } } -struct FindExpectUnwrap<'a, 'tcx> { - lcx: &'a LateContext<'tcx>, - typeck_results: &'tcx ty::TypeckResults<'tcx>, - result: Vec, -} - -impl<'a, 'tcx> Visitor<'tcx> for FindExpectUnwrap<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - // check for `expect` - if let Some(arglists) = method_chain_args(expr, &["expect"]) { - let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs(); - if is_type_diagnostic_item(self.lcx, receiver_ty, sym::Option) - || is_type_diagnostic_item(self.lcx, receiver_ty, sym::Result) - { - self.result.push(expr.span); +fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) { + if let ImplItemKind::Fn(_, body_id) = impl_item.kind { + let body = cx.tcx.hir().body(body_id); + let typeck = cx.tcx.typeck(impl_item.def_id.def_id); + let mut result = Vec::new(); + let _: Option = for_each_expr(body.value, |e| { + // check for `expect` + if let Some(arglists) = method_chain_args(e, &["expect"]) { + let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs(); + if is_type_diagnostic_item(cx, receiver_ty, sym::Option) + || is_type_diagnostic_item(cx, receiver_ty, sym::Result) + { + result.push(e.span); + } } - } - // check for `unwrap` - if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { - let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs(); - if is_type_diagnostic_item(self.lcx, receiver_ty, sym::Option) - || is_type_diagnostic_item(self.lcx, receiver_ty, sym::Result) - { - self.result.push(expr.span); + // check for `unwrap` + if let Some(arglists) = method_chain_args(e, &["unwrap"]) { + let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs(); + if is_type_diagnostic_item(cx, receiver_ty, sym::Option) + || is_type_diagnostic_item(cx, receiver_ty, sym::Result) + { + result.push(e.span); + } } - } - // and check sub-expressions - intravisit::walk_expr(self, expr); - } -} - -fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) { - if let ImplItemKind::Fn(_, body_id) = impl_item.kind { - let body = cx.tcx.hir().body(body_id); - let mut fpu = FindExpectUnwrap { - lcx: cx, - typeck_results: cx.tcx.typeck(impl_item.def_id.def_id), - result: Vec::new(), - }; - fpu.visit_expr(body.value); + ControlFlow::Continue(()) + }); // if we've found one, lint - if !fpu.result.is_empty() { + if !result.is_empty() { span_lint_and_then( cx, UNWRAP_IN_RESULT, @@ -125,7 +111,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tc "used unwrap or expect in a function that returns result or option", move |diag| { diag.help("unwrap and expect should not be used in a function that returns result or option"); - diag.span_note(fpu.result, "potential non-recoverable error(s)"); + diag.span_note(result, "potential non-recoverable error(s)"); }, ); }