add only_used_in_recursion lint

- fix code that have variables that is "only used in recursion"
- add test

CHANGELOG.md

@@ -3349,6 +3349,7 @@ Released 2018-09-13
 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
 [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
 [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
+[`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
 [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
 [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap

clippy_lints/src/lib.register_all.rs

@@ -226,6 +226,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
     LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
     LintId::of(octal_escapes::OCTAL_ESCAPES),
+    LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
     LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
     LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
     LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),

clippy_lints/src/lib.register_complexity.rs

@@ -63,6 +63,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
     LintId::of(no_effect::NO_EFFECT),
     LintId::of(no_effect::UNNECESSARY_OPERATION),
+    LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
     LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
     LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
     LintId::of(precedence::PRECEDENCE),

clippy_lints/src/lib.register_lints.rs

@@ -388,6 +388,7 @@ store.register_lints(&[
     non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY,
     nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES,
     octal_escapes::OCTAL_ESCAPES,
+    only_used_in_recursion::ONLY_USED_IN_RECURSION,
     open_options::NONSENSICAL_OPEN_OPTIONS,
     option_env_unwrap::OPTION_ENV_UNWRAP,
     option_if_let_else::OPTION_IF_LET_ELSE,

clippy_lints/src/lib.rs

@@ -316,6 +316,7 @@ mod non_octal_unix_permissions;
 mod non_send_fields_in_send_ty;
 mod nonstandard_macro_braces;
 mod octal_escapes;
+mod only_used_in_recursion;
 mod open_options;
 mod option_env_unwrap;
 mod option_if_let_else;
@@ -862,6 +863,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv)));
     store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv)));
     store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation));
+    store.register_late_pass(|| Box::new(only_used_in_recursion::OnlyUsedInRecursion));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/methods/option_map_or_none.rs

@@ -14,10 +14,7 @@ use super::RESULT_MAP_OR_INTO_OPTION;
 
 // The expression inside a closure may or may not have surrounding braces
 // which causes problems when generating a suggestion.
-fn reduce_unit_expression<'a>(
+fn reduce_unit_expression<'a>(expr: &'a hir::Expr<'_>) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> {
-    cx: &LateContext<'_>,
-    expr: &'a hir::Expr<'_>,
-) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> {
     match expr.kind {
         hir::ExprKind::Call(func, arg_char) => Some((func, arg_char)),
         hir::ExprKind::Block(block, _) => {
@@ -25,7 +22,7 @@ fn reduce_unit_expression<'a>(
                 (&[], Some(inner_expr)) => {
                     // If block only contains an expression,
                     // reduce `|x| { x + 1 }` to `|x| x + 1`
-                    reduce_unit_expression(cx, inner_expr)
+                    reduce_unit_expression(inner_expr)
                 },
                 _ => None,
             }
@@ -77,7 +74,7 @@ pub(super) fn check<'tcx>(
         if let hir::ExprKind::Closure(_, _, id, span, _) = map_arg.kind;
             let arg_snippet = snippet(cx, span, "..");
             let body = cx.tcx.hir().body(id);
-                if let Some((func, [arg_char])) = reduce_unit_expression(cx, &body.value);
+                if let Some((func, [arg_char])) = reduce_unit_expression(&body.value);
                 if let Some(id) = path_def_id(cx, func).and_then(|ctor_id| cx.tcx.parent(ctor_id));
                 if Some(id) == cx.tcx.lang_items().option_some_variant();
                 then {

clippy_lints/src/only_used_in_recursion.rs

@@ -0,0 +1,563 @@
+use std::collections::VecDeque;
+
+use clippy_utils::diagnostics::span_lint_and_sugg;
+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::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,
+};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_middle::ty::{Ty, TyCtxt, TypeckResults};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::kw;
+use rustc_span::symbol::Ident;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for arguments that is only used in recursion with no side-effects.
+    /// The arguments can be involved in calculations and assignments but as long as
+    /// the calculations have no side-effects (function calls or mutating dereference)
+    /// and the assigned variables are also only in recursion, it is useless.
+    ///
+    /// ### Why is this bad?
+    /// The could contain a useless calculation and can make function simpler.
+    ///
+    /// ### Known Issues
+    /// It could not catch the variable that has no side effects but only used in recursion.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn f(a: usize, b: usize) -> usize {
+    ///     if a == 0 {
+    ///         1
+    ///     } else {
+    ///         f(a - 1, b + 1)
+    ///     }
+    /// }
+    /// # fn main() {
+    /// #     print!("{}", f(1, 1));
+    /// # }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn f(a: usize) -> usize {
+    ///     if a == 0 {
+    ///         1
+    ///     } else {
+    ///         f(a - 1)
+    ///     }
+    /// }
+    /// # fn main() {
+    /// #     print!("{}", f(1));
+    /// # }
+    /// ```
+    #[clippy::version = "1.60.0"]
+    pub ONLY_USED_IN_RECURSION,
+    complexity,
+    "default lint description"
+}
+declare_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
+
+impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        kind: FnKind<'tcx>,
+        _: &'tcx rustc_hir::FnDecl<'tcx>,
+        body: &'tcx Body<'tcx>,
+        _: Span,
+        _: HirId,
+    ) {
+        if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind {
+            let ty_res = cx.typeck_results();
+            let param_span = body
+                .params
+                .iter()
+                .flat_map(|param| {
+                    let mut v = Vec::new();
+                    param.pat.each_binding(|_, hir_id, span, ident| {
+                        v.push((hir_id, span, ident));
+                    });
+                    v
+                })
+                .skip(match kind {
+                    FnKind::Method(..) => 1,
+                    _ => 0,
+                })
+                .filter(|(_, _, ident)| !ident.name.as_str().starts_with('_'))
+                .collect_vec();
+
+            let params = body.params.iter().map(|param| param.pat).collect();
+
+            let mut visitor = SideEffectVisit {
+                graph: FxHashMap::default(),
+                has_side_effect: FxHashSet::default(),
+                ret_vars: Vec::new(),
+                contains_side_effect: false,
+                break_vars: FxHashMap::default(),
+                params,
+                fn_ident: ident,
+                is_method: matches!(kind, FnKind::Method(..)),
+                ty_res,
+                ty_ctx: cx.tcx,
+            };
+
+            visitor.visit_expr(&body.value);
+            let vars = std::mem::take(&mut visitor.ret_vars);
+            visitor.add_side_effect(vars);
+
+            let mut queue = visitor.has_side_effect.iter().copied().collect::<VecDeque<_>>();
+
+            while let Some(id) = queue.pop_front() {
+                if let Some(next) = visitor.graph.get(&id) {
+                    for i in next {
+                        if !visitor.has_side_effect.contains(i) {
+                            visitor.has_side_effect.insert(*i);
+                            queue.push_back(*i);
+                        }
+                    }
+                }
+            }
+
+            for (id, span, ident) in param_span {
+                if !visitor.has_side_effect.contains(&id) {
+                    span_lint_and_sugg(
+                        cx,
+                        ONLY_USED_IN_RECURSION,
+                        span,
+                        "parameter is only used in recursion with no side-effects",
+                        "if this is intentional, prefix with an underscore",
+                        format!("_{}", ident.name.as_str()),
+                        Applicability::MaybeIncorrect,
+                    );
+                }
+            }
+        }
+    }
+}
+
+pub fn is_primitive(ty: Ty<'_>) -> bool {
+    match ty.kind() {
+        ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => true,
+        ty::Ref(_, t, _) => is_primitive(t),
+        _ => false,
+    }
+}
+
+pub fn is_array(ty: Ty<'_>) -> bool {
+    match ty.kind() {
+        ty::Array(..) | ty::Slice(..) => true,
+        ty::Ref(_, t, _) => is_array(t),
+        _ => false,
+    }
+}
+
+pub struct SideEffectVisit<'tcx> {
+    graph: FxHashMap<HirId, FxHashSet<HirId>>,
+    has_side_effect: FxHashSet<HirId>,
+    // bool for if the variable was dereferenced from mutable reference
+    ret_vars: Vec<(HirId, bool)>,
+    contains_side_effect: bool,
+    // break label
+    break_vars: FxHashMap<Ident, Vec<(HirId, bool)>>,
+    params: Vec<&'tcx Pat<'tcx>>,
+    fn_ident: Ident,
+    is_method: bool,
+    ty_res: &'tcx TypeckResults<'tcx>,
+    ty_ctx: TyCtxt<'tcx>,
+}
+
+impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
+    fn visit_block(&mut self, b: &'tcx Block<'tcx>) {
+        b.stmts.iter().for_each(|stmt| {
+            self.visit_stmt(stmt);
+            self.ret_vars.clear();
+        });
+        walk_list!(self, visit_expr, b.expr);
+    }
+
+    fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) {
+        match s.kind {
+            StmtKind::Local(Local {
+                pat, init: Some(init), ..
+            }) => {
+                self.visit_pat_expr(pat, init);
+            },
+            StmtKind::Item(i) => {
+                let item = self.ty_ctx.hir().item(i);
+                self.visit_item(item);
+            },
+            StmtKind::Expr(e) | StmtKind::Semi(e) => self.visit_expr(e),
+            StmtKind::Local(_) => {},
+        }
+    }
+
+    fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
+        debug_assert!(self.ret_vars.is_empty());
+        match ex.kind {
+            ExprKind::Array(exprs) | ExprKind::Tup(exprs) => {
+                self.ret_vars = exprs
+                    .iter()
+                    .flat_map(|expr| {
+                        self.visit_expr(expr);
+                        std::mem::take(&mut self.ret_vars)
+                    })
+                    .collect();
+            },
+            ExprKind::Call(callee, args) => self.visit_fn(callee, args),
+            ExprKind::MethodCall(path, args, _) => self.visit_method_call(path, args),
+            ExprKind::Binary(_, lhs, rhs) => {
+                self.visit_bin_op(lhs, rhs);
+            },
+            ExprKind::Unary(op, expr) => self.visit_un_op(op, expr),
+            ExprKind::Let(Let { pat, init, .. }) => self.visit_pat_expr(pat, init),
+            ExprKind::If(bind, then_expr, else_expr) => {
+                self.visit_if(bind, then_expr, else_expr);
+            },
+            ExprKind::Match(expr, arms, _) => self.visit_match(expr, arms),
+            ExprKind::Closure(_, _, body_id, _, _) => {
+                let body = self.ty_ctx.hir().body(body_id);
+                self.visit_body(body);
+                let vars = std::mem::take(&mut self.ret_vars);
+                self.add_side_effect(vars);
+            },
+            ExprKind::Loop(block, label, _, _) | ExprKind::Block(block, label) => {
+                self.visit_block_label(block, label);
+            },
+            ExprKind::Assign(bind, expr, _) => {
+                self.visit_assign(bind, expr);
+            },
+            ExprKind::AssignOp(_, bind, expr) => {
+                self.visit_assign(bind, expr);
+                self.visit_bin_op(bind, expr);
+            },
+            ExprKind::Field(expr, _) => {
+                self.visit_expr(expr);
+                if matches!(self.ty_res.expr_ty(expr).kind(), ty::Ref(_, _, Mutability::Mut)) {
+                    self.ret_vars.iter_mut().for_each(|(_, b)| *b = true);
+                }
+            },
+            ExprKind::Index(expr, index) => {
+                self.visit_expr(expr);
+                let mut vars = std::mem::take(&mut self.ret_vars);
+                self.visit_expr(index);
+                self.ret_vars.append(&mut vars);
+
+                if !is_array(self.ty_res.expr_ty(expr)) {
+                    self.add_side_effect(self.ret_vars.clone());
+                } else if matches!(self.ty_res.expr_ty(expr).kind(), ty::Ref(_, _, Mutability::Mut)) {
+                    self.ret_vars.iter_mut().for_each(|(_, b)| *b = true);
+                }
+            },
+            ExprKind::Break(dest, Some(expr)) => {
+                self.visit_expr(expr);
+                if let Some(label) = dest.label {
+                    self.break_vars
+                        .entry(label.ident)
+                        .or_insert(Vec::new())
+                        .append(&mut self.ret_vars);
+                }
+                self.contains_side_effect = true;
+            },
+            ExprKind::Ret(Some(expr)) => {
+                self.visit_expr(expr);
+                let vars = std::mem::take(&mut self.ret_vars);
+                self.add_side_effect(vars);
+                self.contains_side_effect = true;
+            },
+            ExprKind::Break(_, None) | ExprKind::Continue(_) | ExprKind::Ret(None) => {
+                self.contains_side_effect = true;
+            },
+            ExprKind::Struct(_, exprs, expr) => {
+                let mut ret_vars = exprs
+                    .iter()
+                    .flat_map(|field| {
+                        self.visit_expr(field.expr);
+                        std::mem::take(&mut self.ret_vars)
+                    })
+                    .collect();
+
+                walk_list!(self, visit_expr, expr);
+                self.ret_vars.append(&mut ret_vars);
+            },
+            _ => walk_expr(self, ex),
+        }
+    }
+
+    fn visit_path(&mut self, path: &'tcx Path<'tcx>, _id: HirId) {
+        if let Res::Local(id) = path.res {
+            self.ret_vars.push((id, false));
+        }
+    }
+}
+
+impl<'tcx> SideEffectVisit<'tcx> {
+    fn visit_assign(&mut self, lhs: &'tcx Expr<'tcx>, rhs: &'tcx Expr<'tcx>) {
+        // Just support array and tuple unwrapping for now.
+        //
+        // ex) `(a, b) = (c, d);`
+        // The graph would look like this:
+        //   a -> c
+        //   b -> d
+        //
+        // This would minimize the connection of the side-effect graph.
+        match (&lhs.kind, &rhs.kind) {
+            (ExprKind::Array(lhs), ExprKind::Array(rhs)) | (ExprKind::Tup(lhs), ExprKind::Tup(rhs)) => {
+                // if not, it is a compile error
+                debug_assert!(lhs.len() == rhs.len());
+                izip!(*lhs, *rhs).for_each(|(lhs, rhs)| self.visit_assign(lhs, rhs));
+            },
+            // in other assigns, we have to connect all each other
+            // because they can be connected somehow
+            _ => {
+                self.visit_expr(lhs);
+                let lhs_vars = std::mem::take(&mut self.ret_vars);
+                self.visit_expr(rhs);
+                let rhs_vars = std::mem::take(&mut self.ret_vars);
+                self.connect_assign(&lhs_vars, &rhs_vars);
+            },
+        }
+    }
+
+    fn visit_block_label(&mut self, block: &'tcx Block<'tcx>, label: Option<Label>) {
+        self.visit_block(block);
+        let _ = label.and_then(|label| {
+            self.break_vars
+                .remove(&label.ident)
+                .map(|mut break_vars| self.ret_vars.append(&mut break_vars))
+        });
+    }
+
+    fn visit_bin_op(&mut self, lhs: &'tcx Expr<'tcx>, rhs: &'tcx Expr<'tcx>) {
+        self.visit_expr(lhs);
+        let mut ret_vars = std::mem::take(&mut self.ret_vars);
+        self.visit_expr(rhs);
+        self.ret_vars.append(&mut ret_vars);
+        if !is_primitive(self.ty_res.expr_ty(lhs)) || !is_primitive(self.ty_res.expr_ty(rhs)) {
+            self.ret_vars.iter().for_each(|id| {
+                self.has_side_effect.insert(id.0);
+            });
+            self.contains_side_effect = true;
+        }
+    }
+
+    fn visit_un_op(&mut self, op: UnOp, expr: &'tcx Expr<'tcx>) {
+        self.visit_expr(expr);
+        let ty = self.ty_res.expr_ty(expr);
+        // dereferencing a reference has no side-effect
+        if !is_primitive(ty) && !matches!((op, ty.kind()), (UnOp::Deref, ty::Ref(..))) {
+            self.add_side_effect(self.ret_vars.clone());
+        }
+
+        if matches!((op, ty.kind()), (UnOp::Deref, ty::Ref(_, _, Mutability::Mut))) {
+            self.ret_vars.iter_mut().for_each(|(_, b)| *b = true);
+        }
+    }
+
+    fn visit_pat_expr(&mut self, pat: &'tcx Pat<'tcx>, expr: &'tcx Expr<'tcx>) {
+        match (&pat.kind, &expr.kind) {
+            (PatKind::Tuple(pats, _), ExprKind::Tup(exprs)) => {
+                self.ret_vars = izip!(*pats, *exprs)
+                    .flat_map(|(pat, expr)| {
+                        self.visit_pat_expr(pat, expr);
+                        std::mem::take(&mut self.ret_vars)
+                    })
+                    .collect();
+            },
+            (PatKind::Slice(front_exprs, _, back_exprs), ExprKind::Array(exprs)) => {
+                let mut vars = izip!(*front_exprs, *exprs)
+                    .flat_map(|(pat, expr)| {
+                        self.visit_pat_expr(pat, expr);
+                        std::mem::take(&mut self.ret_vars)
+                    })
+                    .collect();
+                self.ret_vars = izip!(back_exprs.iter().rev(), exprs.iter().rev())
+                    .flat_map(|(pat, expr)| {
+                        self.visit_pat_expr(pat, expr);
+                        std::mem::take(&mut self.ret_vars)
+                    })
+                    .collect();
+                self.ret_vars.append(&mut vars);
+            },
+            _ => {
+                let mut lhs_vars = Vec::new();
+                pat.each_binding(|_, id, _, _| lhs_vars.push((id, false)));
+                self.visit_expr(expr);
+                let rhs_vars = std::mem::take(&mut self.ret_vars);
+                self.connect_assign(&lhs_vars, &rhs_vars);
+                self.ret_vars = rhs_vars;
+            },
+        }
+    }
+
+    fn visit_fn(&mut self, callee: &'tcx Expr<'tcx>, args: &'tcx [Expr<'tcx>]) {
+        self.visit_expr(callee);
+        let mut ret_vars = std::mem::take(&mut self.ret_vars);
+        self.add_side_effect(ret_vars.clone());
+
+        if_chain! {
+            if !self.is_method;
+            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;
+            then {
+                izip!(self.params.clone(), args)
+                    .for_each(|(pat, expr)| {
+                        self.visit_pat_expr(pat, expr);
+                        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);
+    }
+
+    fn visit_method_call(&mut self, path: &'tcx PathSegment<'tcx>, args: &'tcx [Expr<'tcx>]) {
+        if_chain! {
+            if self.is_method;
+            if path.ident == self.fn_ident;
+            if let ExprKind::Path(QPath::Resolved(_, path)) = args.first().unwrap().kind;
+            if let Res::Local(..) = path.res;
+            let ident = path.segments.last().unwrap().ident;
+            if ident.name == kw::SelfLower;
+            then {
+                izip!(self.params.clone(), args.iter())
+                    .for_each(|(pat, expr)| {
+                        self.visit_pat_expr(pat, expr);
+                        self.ret_vars.clear();
+                    });
+            } else {
+                self.ret_vars = args
+                    .iter()
+                    .flat_map(|expr| {
+                        self.visit_expr(expr);
+                        std::mem::take(&mut self.ret_vars)
+                    })
+                    .collect_vec()
+                    .into_iter()
+                    .map(|a| {
+                        self.has_side_effect.insert(a.0);
+                        a
+                    })
+                    .collect();
+                self.contains_side_effect = true;
+            }
+        }
+    }
+
+    fn visit_if(&mut self, bind: &'tcx Expr<'tcx>, then_expr: &'tcx Expr<'tcx>, else_expr: Option<&'tcx Expr<'tcx>>) {
+        let contains_side_effect = self.contains_side_effect;
+        self.contains_side_effect = false;
+        self.visit_expr(bind);
+        let mut vars = std::mem::take(&mut self.ret_vars);
+        self.visit_expr(then_expr);
+        let mut then_vars = std::mem::take(&mut self.ret_vars);
+        walk_list!(self, visit_expr, else_expr);
+        if self.contains_side_effect {
+            self.add_side_effect(vars.clone());
+        }
+        self.contains_side_effect |= contains_side_effect;
+        self.ret_vars.append(&mut vars);
+        self.ret_vars.append(&mut then_vars);
+    }
+
+    fn visit_match(&mut self, expr: &'tcx Expr<'tcx>, arms: &'tcx [Arm<'tcx>]) {
+        self.visit_expr(expr);
+        let mut expr_vars = std::mem::take(&mut self.ret_vars);
+        self.ret_vars = arms
+            .iter()
+            .flat_map(|arm| {
+                let contains_side_effect = self.contains_side_effect;
+                self.contains_side_effect = false;
+                // this would visit `expr` multiple times
+                // but couldn't think of a better way
+                self.visit_pat_expr(arm.pat, expr);
+                let mut vars = std::mem::take(&mut self.ret_vars);
+                let _ = arm.guard.as_ref().map(|guard| {
+                    self.visit_expr(match guard {
+                        Guard::If(expr) | Guard::IfLet(_, expr) => expr,
+                    });
+                    vars.append(&mut self.ret_vars);
+                });
+                self.visit_expr(arm.body);
+                if self.contains_side_effect {
+                    self.add_side_effect(vars.clone());
+                    self.add_side_effect(expr_vars.clone());
+                }
+                self.contains_side_effect |= contains_side_effect;
+                vars.append(&mut self.ret_vars);
+                vars
+            })
+            .collect();
+        self.ret_vars.append(&mut expr_vars);
+    }
+
+    fn connect_assign(&mut self, lhs: &[(HirId, bool)], rhs: &[(HirId, bool)]) {
+        // if mutable dereference is on assignment it can have side-effect
+        // (this can lead to parameter mutable dereference and change the original value)
+        // too hard to detect whether this value is from parameter, so this would all
+        // check mutable dereference assignment to side effect
+        lhs.iter().filter(|(_, b)| *b).for_each(|(id, _)| {
+            self.has_side_effect.insert(*id);
+            self.contains_side_effect = true;
+        });
+
+        // there is no connection
+        if lhs.is_empty() || rhs.is_empty() {
+            return;
+        }
+
+        // by connected rhs in cycle, the connections would decrease
+        // from `n * m` to `n + m`
+        // where `n` and `m` are length of `lhs` and `rhs`.
+
+        // unwrap is possible since rhs is not empty
+        let rhs_first = rhs.first().unwrap();
+        for (id, _) in lhs.iter() {
+            self.graph
+                .entry(*id)
+                .or_insert_with(FxHashSet::default)
+                .insert(rhs_first.0);
+        }
+
+        let rhs = rhs.iter();
+        izip!(rhs.clone().cycle().skip(1), rhs).for_each(|(from, to)| {
+            self.graph.entry(from.0).or_insert_with(FxHashSet::default).insert(to.0);
+        });
+    }
+
+    fn add_side_effect(&mut self, v: Vec<(HirId, bool)>) {
+        for (id, _) in v {
+            self.has_side_effect.insert(id);
+            self.contains_side_effect = true;
+        }
+    }
+}

clippy_lints/src/unnecessary_sort_by.rs

@@ -67,59 +67,51 @@ struct SortByKeyDetection {
 
 /// Detect if the two expressions are mirrored (identical, except one
 /// contains a and the other replaces it with b)
-fn mirrored_exprs(
+fn mirrored_exprs(a_expr: &Expr<'_>, a_ident: &Ident, b_expr: &Expr<'_>, b_ident: &Ident) -> bool {
-    cx: &LateContext<'_>,
-    a_expr: &Expr<'_>,
-    a_ident: &Ident,
-    b_expr: &Expr<'_>,
-    b_ident: &Ident,
-) -> bool {
     match (&a_expr.kind, &b_expr.kind) {
         // Two boxes with mirrored contents
         (ExprKind::Box(left_expr), ExprKind::Box(right_expr)) => {
-            mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
+            mirrored_exprs(left_expr, a_ident, right_expr, b_ident)
         },
         // Two arrays with mirrored contents
         (ExprKind::Array(left_exprs), ExprKind::Array(right_exprs)) => {
-            iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
+            iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident))
         },
         // The two exprs are function calls.
         // Check to see that the function itself and its arguments are mirrored
         (ExprKind::Call(left_expr, left_args), ExprKind::Call(right_expr, right_args)) => {
-            mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
+            mirrored_exprs(left_expr, a_ident, right_expr, b_ident)
-                && iter::zip(*left_args, *right_args)
+                && iter::zip(*left_args, *right_args).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident))
-                    .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
         },
         // The two exprs are method calls.
         // Check to see that the function is the same and the arguments are mirrored
         // This is enough because the receiver of the method is listed in the arguments
         (ExprKind::MethodCall(left_segment, left_args, _), ExprKind::MethodCall(right_segment, right_args, _)) => {
             left_segment.ident == right_segment.ident
-                && iter::zip(*left_args, *right_args)
+                && iter::zip(*left_args, *right_args).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident))
-                    .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
         },
         // Two tuples with mirrored contents
         (ExprKind::Tup(left_exprs), ExprKind::Tup(right_exprs)) => {
-            iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident))
+            iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident))
         },
         // Two binary ops, which are the same operation and which have mirrored arguments
         (ExprKind::Binary(left_op, left_left, left_right), ExprKind::Binary(right_op, right_left, right_right)) => {
             left_op.node == right_op.node
-                && mirrored_exprs(cx, left_left, a_ident, right_left, b_ident)
+                && mirrored_exprs(left_left, a_ident, right_left, b_ident)
-                && mirrored_exprs(cx, left_right, a_ident, right_right, b_ident)
+                && mirrored_exprs(left_right, a_ident, right_right, b_ident)
         },
         // Two unary ops, which are the same operation and which have the same argument
         (ExprKind::Unary(left_op, left_expr), ExprKind::Unary(right_op, right_expr)) => {
-            left_op == right_op && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident)
+            left_op == right_op && mirrored_exprs(left_expr, a_ident, right_expr, b_ident)
         },
         // The two exprs are literals of some kind
         (ExprKind::Lit(left_lit), ExprKind::Lit(right_lit)) => left_lit.node == right_lit.node,
-        (ExprKind::Cast(left, _), ExprKind::Cast(right, _)) => mirrored_exprs(cx, left, a_ident, right, b_ident),
+        (ExprKind::Cast(left, _), ExprKind::Cast(right, _)) => mirrored_exprs(left, a_ident, right, b_ident),
         (ExprKind::DropTemps(left_block), ExprKind::DropTemps(right_block)) => {
-            mirrored_exprs(cx, left_block, a_ident, right_block, b_ident)
+            mirrored_exprs(left_block, a_ident, right_block, b_ident)
         },
         (ExprKind::Field(left_expr, left_ident), ExprKind::Field(right_expr, right_ident)) => {
-            left_ident.name == right_ident.name && mirrored_exprs(cx, left_expr, a_ident, right_expr, right_ident)
+            left_ident.name == right_ident.name && mirrored_exprs(left_expr, a_ident, right_expr, right_ident)
         },
         // Two paths: either one is a and the other is b, or they're identical to each other
         (
@@ -151,11 +143,9 @@ fn mirrored_exprs(
         (
             ExprKind::AddrOf(left_kind, Mutability::Not, left_expr),
             ExprKind::AddrOf(right_kind, Mutability::Not, right_expr),
-        ) => left_kind == right_kind && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident),
+        ) => left_kind == right_kind && mirrored_exprs(left_expr, a_ident, right_expr, b_ident),
-        (_, ExprKind::AddrOf(_, Mutability::Not, right_expr)) => {
+        (_, ExprKind::AddrOf(_, Mutability::Not, right_expr)) => mirrored_exprs(a_expr, a_ident, right_expr, b_ident),
-            mirrored_exprs(cx, a_expr, a_ident, right_expr, b_ident)
+        (ExprKind::AddrOf(_, Mutability::Not, left_expr), _) => mirrored_exprs(left_expr, a_ident, b_expr, b_ident),
-        },
-        (ExprKind::AddrOf(_, Mutability::Not, left_expr), _) => mirrored_exprs(cx, left_expr, a_ident, b_expr, b_ident),
         _ => false,
     }
 }
@@ -176,14 +166,13 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<LintTrigger> {
         if method_path.ident.name == sym::cmp;
         then {
             let (closure_body, closure_arg, reverse) = if mirrored_exprs(
-                cx,
                 left_expr,
                 left_ident,
                 right_expr,
                 right_ident
             ) {
                 (Sugg::hir(cx, left_expr, "..").to_string(), left_ident.name.to_string(), false)
-            } else if mirrored_exprs(cx, left_expr, right_ident, right_expr, left_ident) {
+            } else if mirrored_exprs(left_expr, right_ident, right_expr, left_ident) {
                 (Sugg::hir(cx, left_expr, "..").to_string(), right_ident.name.to_string(), true)
             } else {
                 return None;

tests/ui/only_used_in_recursion.rs

@@ -0,0 +1,73 @@
+#![warn(clippy::only_used_in_recursion)]
+
+fn simple(a: usize, b: usize) -> usize {
+    if a == 0 { 1 } else { simple(a - 1, b) }
+}
+
+fn with_calc(a: usize, b: isize) -> usize {
+    if a == 0 { 1 } else { with_calc(a - 1, -b + 1) }
+}
+
+fn tuple((a, b): (usize, usize)) -> usize {
+    if a == 0 { 1 } else { tuple((a - 1, b + 1)) }
+}
+
+fn let_tuple(a: usize, b: usize) -> usize {
+    let (c, d) = (a, b);
+    if c == 0 { 1 } else { let_tuple(c - 1, d + 1) }
+}
+
+fn array([a, b]: [usize; 2]) -> usize {
+    if a == 0 { 1 } else { array([a - 1, b + 1]) }
+}
+
+fn index(a: usize, mut b: &[usize], c: usize) -> usize {
+    if a == 0 { 1 } else { index(a - 1, b, c + b[0]) }
+}
+
+fn break_(a: usize, mut b: usize, mut c: usize) -> usize {
+    let c = loop {
+        b += 1;
+        c += 1;
+        if c == 10 {
+            break b;
+        }
+    };
+
+    if a == 0 { 1 } else { break_(a - 1, c, c) }
+}
+
+// this has a side effect
+fn mut_ref(a: usize, b: &mut usize) -> usize {
+    *b = 1;
+    if a == 0 { 1 } else { mut_ref(a - 1, b) }
+}
+
+fn mut_ref2(a: usize, b: &mut usize) -> usize {
+    let mut c = *b;
+    if a == 0 { 1 } else { mut_ref2(a - 1, &mut c) }
+}
+
+fn not_primitive(a: usize, b: String) -> usize {
+    if a == 0 { 1 } else { not_primitive(a - 1, b) }
+}
+
+// this doesn't have a side effect,
+// but `String` is not primitive.
+fn not_primitive_op(a: usize, b: String, c: &str) -> usize {
+    if a == 1 { 1 } else { not_primitive_op(a, b + c, c) }
+}
+
+struct A;
+
+impl A {
+    fn method(&self, a: usize, b: usize) -> usize {
+        if a == 0 { 1 } else { self.method(a - 1, b + 1) }
+    }
+}
+
+fn ignore(a: usize, _: usize) -> usize {
+    if a == 1 { 1 } else { ignore(a - 1, 0) }
+}
+
+fn main() {}

tests/ui/only_used_in_recursion.stderr

@@ -0,0 +1,70 @@
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:3:21
+   |
+LL | fn simple(a: usize, b: usize) -> usize {
+   |                     ^ help: if this is intentional, prefix with an underscore: `_b`
+   |
+   = note: `-D clippy::only-used-in-recursion` implied by `-D warnings`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:7:24
+   |
+LL | fn with_calc(a: usize, b: isize) -> usize {
+   |                        ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:11:14
+   |
+LL | fn tuple((a, b): (usize, usize)) -> usize {
+   |              ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:15:24
+   |
+LL | fn let_tuple(a: usize, b: usize) -> usize {
+   |                        ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:20:14
+   |
+LL | fn array([a, b]: [usize; 2]) -> usize {
+   |              ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:24:20
+   |
+LL | fn index(a: usize, mut b: &[usize], c: usize) -> usize {
+   |                    ^^^^^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:24:37
+   |
+LL | fn index(a: usize, mut b: &[usize], c: usize) -> usize {
+   |                                     ^ help: if this is intentional, prefix with an underscore: `_c`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:28:21
+   |
+LL | fn break_(a: usize, mut b: usize, mut c: usize) -> usize {
+   |                     ^^^^^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:46:23
+   |
+LL | fn mut_ref2(a: usize, b: &mut usize) -> usize {
+   |                       ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:51:28
+   |
+LL | fn not_primitive(a: usize, b: String) -> usize {
+   |                            ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: parameter is only used in recursion with no side-effects
+  --> $DIR/only_used_in_recursion.rs:64:32
+   |
+LL |     fn method(&self, a: usize, b: usize) -> usize {
+   |                                ^ help: if this is intentional, prefix with an underscore: `_b`
+
+error: aborting due to 11 previous errors
+