From ca552cf6c3da4e893f3a9b9b3e504146a3e0f9f1 Mon Sep 17 00:00:00 2001 From: Denis Pakhomov Date: Sun, 1 Feb 2026 16:55:04 +0100 Subject: [PATCH] implement fn_arg_mut_rebindings lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/fn_arg_mut_rebindings.rs | 117 ++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/fn_arg_mut_rebindings.rs | 74 ++++++++++++++ tests/ui/fn_arg_mut_rebindings.stderr | 40 ++++++++ tests/ui/unused_io_amount.rs | 2 +- 7 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/fn_arg_mut_rebindings.rs create mode 100644 tests/ui/fn_arg_mut_rebindings.rs create mode 100644 tests/ui/fn_arg_mut_rebindings.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 24b91932567a..84aec956fdf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6737,6 +6737,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 79ed199147f1..327b4344e2d8 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -168,6 +168,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, diff --git a/clippy_lints/src/fn_arg_mut_rebindings.rs b/clippy_lints/src/fn_arg_mut_rebindings.rs new file mode 100644 index 000000000000..7b59aacf6a14 --- /dev/null +++ b/clippy_lints/src/fn_arg_mut_rebindings.rs @@ -0,0 +1,117 @@ +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) -> Vec { + /// let mut bar = bar; + /// bar.push(42); + /// bar + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn foo(mut bar: Vec) -> Vec { + /// bar.push(42); + /// bar + /// } + /// ``` + #[clippy::version = "1.96.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, cx: &LateContext<'_>, st: &'_ LetStmt<'_>) { + if !st.span.in_external_macro(cx.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)) = cx.tcx.hir_parent_owner_iter(st.hir_id).next() + && let Some(body_id) = fn_body_id(&owner) + && let &Body { params, value } = cx.tcx.hir_body(body_id) + && let ExprKind::Block(fn_block, _) = value.kind + && let Some(st_block) = get_enclosing_block(cx, 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( + cx, + 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(cx, pat_span, "_")), + Applicability::MaybeIncorrect, + ); + diag.span_help(st.span, "and remove this"); + }, + ); + } + } +} + +fn fn_body_id(node: &OwnerNode<'_>) -> Option { + 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, + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0875982f3bbf..e80f9096ad6a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -135,6 +135,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; @@ -869,6 +870,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))), Box::new(|_| Box::new(byte_char_slices::ByteCharSlice)), Box::new(|_| Box::new(manual_assert_eq::ManualAssertEq)), + 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); diff --git a/tests/ui/fn_arg_mut_rebindings.rs b/tests/ui/fn_arg_mut_rebindings.rs new file mode 100644 index 000000000000..866fe4cbd2f9 --- /dev/null +++ b/tests/ui/fn_arg_mut_rebindings.rs @@ -0,0 +1,74 @@ +//@aux-build:proc_macros.rs +//@no-rustfix + +#![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, 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 +} diff --git a/tests/ui/fn_arg_mut_rebindings.stderr b/tests/ui/fn_arg_mut_rebindings.stderr new file mode 100644 index 000000000000..3146dff66fd7 --- /dev/null +++ b/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:15: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:17: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:49: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:51: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:60: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:62:9 + | +LL | let mut x = x; + | ^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/unused_io_amount.rs b/tests/ui/unused_io_amount.rs index 32a50375806a..5b0e7a12c9c9 100644 --- a/tests/ui/unused_io_amount.rs +++ b/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)]