diff options
| -rw-r--r-- | clippy_lints/src/only_used_in_recursion.rs | 96 | ||||
| -rw-r--r-- | tests/ui/only_used_in_recursion.rs | 15 | ||||
| -rw-r--r-- | tests/ui/only_used_in_recursion.stderr | 8 |
3 files changed, 80 insertions, 39 deletions
diff --git a/clippy_lints/src/only_used_in_recursion.rs b/clippy_lints/src/only_used_in_recursion.rs index 0d7e52e1d62..7ec110295dc 100644 --- a/clippy_lints/src/only_used_in_recursion.rs +++ b/clippy_lints/src/only_used_in_recursion.rs @@ -5,12 +5,13 @@ use itertools::{izip, Itertools}; use rustc_ast::{walk_list, Label, Mutability}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; -use rustc_hir::def::Res; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::{ - Arm, Block, Body, Expr, ExprKind, Guard, HirId, Let, Local, Pat, PatKind, Path, PathSegment, QPath, Stmt, StmtKind, - UnOp, + Arm, Block, Body, Expr, ExprKind, Guard, HirId, ImplicitSelfKind, Let, Local, Pat, PatKind, Path, PathSegment, + QPath, Stmt, StmtKind, TyKind, UnOp, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; @@ -94,13 +95,15 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { &mut self, cx: &LateContext<'tcx>, kind: FnKind<'tcx>, - _: &'tcx rustc_hir::FnDecl<'tcx>, + decl: &'tcx rustc_hir::FnDecl<'tcx>, body: &'tcx Body<'tcx>, _: Span, id: HirId, ) { if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind { - let data = cx.tcx.def_path(cx.tcx.hir().local_def_id(id).to_def_id()).data; + let def_id = id.owner.to_def_id(); + let data = cx.tcx.def_path(def_id).data; + if data.len() > 1 { match data.get(data.len() - 2) { Some(DisambiguatedDefPathData { @@ -111,6 +114,8 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { } } + let has_self = !matches!(decl.implicit_self, ImplicitSelfKind::None); + let ty_res = cx.typeck_results(); let param_span = body .params @@ -122,10 +127,7 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { }); v }) - .skip(match kind { - FnKind::Method(..) => 1, - _ => 0, - }) + .skip(if has_self { 1 } else { 0 }) .filter(|(_, _, ident)| !ident.name.as_str().starts_with('_')) .collect_vec(); @@ -139,7 +141,9 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { break_vars: FxHashMap::default(), params, fn_ident: ident, + fn_def_id: def_id, is_method: matches!(kind, FnKind::Method(..)), + has_self, ty_res, ty_ctx: cx.tcx, }; @@ -242,7 +246,9 @@ pub struct SideEffectVisit<'tcx> { break_vars: FxHashMap<Ident, Vec<(HirId, bool)>>, params: Vec<&'tcx Pat<'tcx>>, fn_ident: Ident, + fn_def_id: DefId, is_method: bool, + has_self: bool, ty_res: &'tcx TypeckResults<'tcx>, ty_ctx: TyCtxt<'tcx>, } @@ -479,41 +485,55 @@ impl<'tcx> SideEffectVisit<'tcx> { let mut ret_vars = std::mem::take(&mut self.ret_vars); self.add_side_effect(ret_vars.clone()); + let mut is_recursive = false; + if_chain! { - if !self.is_method; + if !self.has_self; if let ExprKind::Path(QPath::Resolved(_, path)) = callee.kind; - if let Res::Def(..) = path.res; - if path.segments.len() == 1; - let ident = path.segments.last().unwrap().ident; - if ident == self.fn_ident; + if let Res::Def(DefKind::Fn, def_id) = path.res; + if self.fn_def_id == def_id; then { - izip!(self.params.clone(), args) - .for_each(|(pat, expr)| { - self.visit_pat_expr(pat, expr, true); - self.ret_vars.clear(); - }); - } else { - // This would set arguments used in closure that does not have side-effect. - // Closure itself can be detected whether there is a side-effect, but the - // value of variable that is holding closure can change. - // So, we just check the variables. - self.ret_vars = args - .iter() - .flat_map(|expr| { - self.visit_expr(expr); - std::mem::take(&mut self.ret_vars) - }) - .collect_vec() - .into_iter() - .map(|id| { - self.has_side_effect.insert(id.0); - id - }) - .collect(); - self.contains_side_effect = true; + is_recursive = true; + } + } + + if_chain! { + if !self.has_self && self.is_method; + if let ExprKind::Path(QPath::TypeRelative(ty, segment)) = callee.kind; + if segment.ident == self.fn_ident; + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind; + if let Res::SelfTy(..) = path.res; + then { + is_recursive = true; } } + if is_recursive { + izip!(self.params.clone(), args).for_each(|(pat, expr)| { + self.visit_pat_expr(pat, expr, true); + self.ret_vars.clear(); + }); + } else { + // This would set arguments used in closure that does not have side-effect. + // Closure itself can be detected whether there is a side-effect, but the + // value of variable that is holding closure can change. + // So, we just check the variables. + self.ret_vars = args + .iter() + .flat_map(|expr| { + self.visit_expr(expr); + std::mem::take(&mut self.ret_vars) + }) + .collect_vec() + .into_iter() + .map(|id| { + self.has_side_effect.insert(id.0); + id + }) + .collect(); + self.contains_side_effect = true; + } + self.ret_vars.append(&mut ret_vars); } diff --git a/tests/ui/only_used_in_recursion.rs b/tests/ui/only_used_in_recursion.rs index 0f49fc071bf..5768434f988 100644 --- a/tests/ui/only_used_in_recursion.rs +++ b/tests/ui/only_used_in_recursion.rs @@ -104,4 +104,19 @@ fn ignore2(a: usize, _b: usize) -> usize { if a == 1 { 1 } else { ignore2(a - 1, _b) } } +fn f1(a: u32) -> u32 { + a +} + +fn f2(a: u32) -> u32 { + f1(a) +} + +fn inner_fn(a: u32) -> u32 { + fn inner_fn(a: u32) -> u32 { + a + } + inner_fn(a) +} + fn main() {} diff --git a/tests/ui/only_used_in_recursion.stderr b/tests/ui/only_used_in_recursion.stderr index c24a357cd20..6fe9361bf5f 100644 --- a/tests/ui/only_used_in_recursion.stderr +++ b/tests/ui/only_used_in_recursion.stderr @@ -67,10 +67,16 @@ LL | fn method2(&self, a: usize, b: usize) -> usize { | ^ help: if this is intentional, prefix with an underscore: `_b` error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:90:24 + | +LL | fn hello(a: usize, b: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion --> $DIR/only_used_in_recursion.rs:94:32 | LL | fn hello2(&self, a: usize, b: usize) -> usize { | ^ help: if this is intentional, prefix with an underscore: `_b` -error: aborting due to 12 previous errors +error: aborting due to 13 previous errors |