implement fn_arg_mut_rebindings lint

Author: faeroon

CHANGELOG.md

@@ -6523,6 +6523,7 @@ Released 2018-09-13
 [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const
 [`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_equality_without_abs
 [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons
+[`fn_arg_mut_rebindings`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_arg_mut_rebindings
 [`fn_null_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_null_check
 [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
 [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast

clippy_lints/src/declared_lints.rs

@@ -167,6 +167,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::float_literal::LOSSY_FLOAT_LITERAL_INFO,
     crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO,
     crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO,
+    crate::fn_arg_mut_rebindings::FN_ARG_MUT_REBINDINGS_INFO,
     crate::format::USELESS_FORMAT_INFO,
     crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO,
     crate::format_args::POINTER_FORMAT_INFO,

clippy_lints/src/fn_arg_mut_rebindings.rs

@@ -0,0 +1,116 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::get_enclosing_block;
+use clippy_utils::source::snippet;
+use rustc_errors::Applicability;
+use rustc_hir::def::Res;
+use rustc_hir::{
+    BindingMode, Body, BodyId, ExprKind, ImplItem, ImplItemImplKind, ImplItemKind, Item, ItemKind, LetStmt, OwnerNode,
+    Param, Pat, PatKind, QPath, TraitFn, TraitItem, TraitItemKind,
+};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for function arguments declared as not mutable and later rebound as mutable.
+    ///
+    /// ### Why is this bad?
+    /// It can be easily improved by just declaring the function argument as mutable and
+    /// removing the unnecessary assignment.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn foo_bad(bar: Vec<i32>) -> Vec<i32> {
+    ///     let mut bar = bar;
+    ///     bar.push(42);
+    ///     bar
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn foo(mut bar: Vec<i32>) -> Vec<i32> {
+    ///     bar.push(42);
+    ///     bar
+    /// }
+    /// ```
+    #[clippy::version = "1.95.0"]
+    pub FN_ARG_MUT_REBINDINGS,
+    style,
+    "non-mutable function argument rebound as mutable"
+}
+declare_lint_pass!(FnArgMutRebindings => [FN_ARG_MUT_REBINDINGS]);
+
+impl LateLintPass<'_> for FnArgMutRebindings {
+    fn check_local(&mut self, ctx: &LateContext<'_>, st: &'_ LetStmt<'_>) {
+        if !st.span.in_external_macro(ctx.tcx.sess.source_map())
+
+            // check let statement binds as mutable
+            && let PatKind::Binding(BindingMode::MUT, _, ident, None) = st.pat.kind
+            && let Some(init) = st.init
+
+            // check let statement binds to variable with same name
+            && let ExprKind::Path(QPath::Resolved(_, path)) = init.kind
+            && path.segments.len() == 1
+            && path.segments[0].ident == ident
+            && let Res::Local(id) = path.res
+
+            // check let statement scope is whole function body
+            && let Some((_, owner)) = ctx.tcx.hir_parent_owner_iter(st.hir_id).next()
+            && let Some(body_id) = fn_body_id(&owner)
+            && let &Body { params, value } = ctx.tcx.hir_body(body_id)
+            && let ExprKind::Block(fn_block, _) = value.kind
+            && let Some(st_block) = get_enclosing_block(ctx, st.hir_id)
+            && fn_block.hir_id == st_block.hir_id
+
+            // check param declares as immutable
+            && let Some(&Param {
+                span: pat_span,
+                pat:
+                    Pat {
+                        kind: PatKind::Binding(BindingMode::NONE, ..),
+                        ..
+                    },
+                ..
+            }) = params.iter().find(|p| p.pat.hir_id == id)
+        {
+            span_lint_and_then(
+                ctx,
+                FN_ARG_MUT_REBINDINGS,
+                pat_span,
+                format!(
+                    "argument `{}` is declared as not mutable, and later rebound as mutable",
+                    ident.name
+                ),
+                |diag| {
+                    diag.span_suggestion(
+                        pat_span,
+                        "consider just declaring as mutable",
+                        format!("mut {}", snippet(ctx, pat_span, "_")),
+                        Applicability::MaybeIncorrect,
+                    );
+                    diag.span_help(st.span, "and remove this");
+                },
+            );
+        }
+    }
+}
+
+fn fn_body_id(node: &OwnerNode<'_>) -> Option<BodyId> {
+    match node {
+        OwnerNode::Item(Item {
+            kind: ItemKind::Fn { body: body_id, .. },
+            ..
+        })
+        | OwnerNode::ImplItem(ImplItem {
+            kind: ImplItemKind::Fn(_, body_id),
+            // avoid false-positive: trait-fn can come from external crate
+            impl_kind: ImplItemImplKind::Inherent { .. },
+            ..
+        })
+        | OwnerNode::TraitItem(TraitItem {
+            kind: TraitItemKind::Fn(_, TraitFn::Provided(body_id)),
+            ..
+        }) => Some(*body_id),
+        _ => None,
+    }
+}

clippy_lints/src/lib.rs

@@ -136,6 +136,7 @@ mod fallible_impl_from;
 mod field_scoped_visibility_modifiers;
 mod float_literal;
 mod floating_point_arithmetic;
+mod fn_arg_mut_rebindings;
 mod format;
 mod format_args;
 mod format_impl;
@@ -863,6 +864,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))),
         Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))),
         Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)),
+        Box::new(|_| Box::new(fn_arg_mut_rebindings::FnArgMutRebindings)),
         // add late passes here, used by `cargo dev new_lint`
     ];
     store.late_passes.extend(late_lints);

tests/ui/fn_arg_mut_rebindings.fixed

@@ -0,0 +1,73 @@
+//@aux-build:proc_macros.rs
+
+#![warn(clippy::fn_arg_mut_rebindings)]
+
+extern crate proc_macros;
+use proc_macros::external;
+
+external! {
+    fn external_macros_rebinding(x: bool, y: bool) {
+        let mut x = x;
+    }
+}
+
+fn fn_body_rebinding(mut x: bool) {
+    //~^ fn_arg_mut_rebindings
+    let mut x = x;
+}
+
+fn branch_rebinding(x: bool, m: u32) {
+    if x {
+        let mut m = m;
+    }
+}
+
+fn arm_rebinding(x: Option<u32>, m: u32) {
+    match x {
+        Some(_) => {
+            let mut m = m;
+        },
+        None => {
+            let mut m = 1;
+        },
+    }
+}
+
+fn inner_block_rebinding(x: bool) {
+    {
+        let mut x = x;
+    }
+}
+
+fn shadowed_rebinding(x: bool) {
+    let x = 0;
+    let mut x = x;
+}
+
+trait MyTrait {
+    fn provided_fn_rebinding(&self, mut x: bool) {
+        //~^ fn_arg_mut_rebindings
+        let mut x = x;
+    }
+
+    fn inherent_fn_rebinding(&self, x: bool);
+}
+
+struct MyStruct;
+
+impl MyStruct {
+    fn impl_fn_rebinding(&self, mut x: bool) {
+        //~^ fn_arg_mut_rebindings
+        let mut x = x;
+    }
+}
+
+impl MyTrait for MyStruct {
+    fn inherent_fn_rebinding(&self, x: bool) {
+        let mut x = x;
+    }
+}
+
+fn main() {
+    // test code goes here
+}

tests/ui/fn_arg_mut_rebindings.rs

@@ -0,0 +1,73 @@
+//@aux-build:proc_macros.rs
+
+#![warn(clippy::fn_arg_mut_rebindings)]
+
+extern crate proc_macros;
+use proc_macros::external;
+
+external! {
+    fn external_macros_rebinding(x: bool, y: bool) {
+        let mut x = x;
+    }
+}
+
+fn fn_body_rebinding(x: bool) {
+    //~^ fn_arg_mut_rebindings
+    let mut x = x;
+}
+
+fn branch_rebinding(x: bool, m: u32) {
+    if x {
+        let mut m = m;
+    }
+}
+
+fn arm_rebinding(x: Option<u32>, m: u32) {
+    match x {
+        Some(_) => {
+            let mut m = m;
+        },
+        None => {
+            let mut m = 1;
+        },
+    }
+}
+
+fn inner_block_rebinding(x: bool) {
+    {
+        let mut x = x;
+    }
+}
+
+fn shadowed_rebinding(x: bool) {
+    let x = 0;
+    let mut x = x;
+}
+
+trait MyTrait {
+    fn provided_fn_rebinding(&self, x: bool) {
+        //~^ fn_arg_mut_rebindings
+        let mut x = x;
+    }
+
+    fn inherent_fn_rebinding(&self, x: bool);
+}
+
+struct MyStruct;
+
+impl MyStruct {
+    fn impl_fn_rebinding(&self, x: bool) {
+        //~^ fn_arg_mut_rebindings
+        let mut x = x;
+    }
+}
+
+impl MyTrait for MyStruct {
+    fn inherent_fn_rebinding(&self, x: bool) {
+        let mut x = x;
+    }
+}
+
+fn main() {
+    // test code goes here
+}

tests/ui/fn_arg_mut_rebindings.stderr

@@ -0,0 +1,40 @@
+error: argument `x` is declared as not mutable, and later rebound as mutable
+  --> tests/ui/fn_arg_mut_rebindings.rs:14:22
+   |
+LL | fn fn_body_rebinding(x: bool) {
+   |                      ^^^^^^^ help: consider just declaring as mutable: `mut x: bool`
+   |
+help: and remove this
+  --> tests/ui/fn_arg_mut_rebindings.rs:16:5
+   |
+LL |     let mut x = x;
+   |     ^^^^^^^^^^^^^^
+   = note: `-D clippy::fn-arg-mut-rebindings` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::fn_arg_mut_rebindings)]`
+
+error: argument `x` is declared as not mutable, and later rebound as mutable
+  --> tests/ui/fn_arg_mut_rebindings.rs:48:37
+   |
+LL |     fn provided_fn_rebinding(&self, x: bool) {
+   |                                     ^^^^^^^ help: consider just declaring as mutable: `mut x: bool`
+   |
+help: and remove this
+  --> tests/ui/fn_arg_mut_rebindings.rs:50:9
+   |
+LL |         let mut x = x;
+   |         ^^^^^^^^^^^^^^
+
+error: argument `x` is declared as not mutable, and later rebound as mutable
+  --> tests/ui/fn_arg_mut_rebindings.rs:59:33
+   |
+LL |     fn impl_fn_rebinding(&self, x: bool) {
+   |                                 ^^^^^^^ help: consider just declaring as mutable: `mut x: bool`
+   |
+help: and remove this
+  --> tests/ui/fn_arg_mut_rebindings.rs:61:9
+   |
+LL |         let mut x = x;
+   |         ^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+

tests/ui/unused_io_amount.rs

@@ -1,4 +1,4 @@
-#![allow(dead_code, clippy::needless_pass_by_ref_mut)]
+#![allow(dead_code, clippy::needless_pass_by_ref_mut, clippy::fn_arg_mut_rebindings)]
 #![allow(clippy::redundant_pattern_matching)]
 #![warn(clippy::unused_io_amount)]