diff --git a/CHANGELOG.md b/CHANGELOG.md index 25f3b5da198a..36ea07cedfc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1906,6 +1906,7 @@ Released 2018-09-13 [`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero [`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer +[`rebind_fn_arg_as_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#rebind_fn_arg_as_mut [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3be8bc0e36d6..3c0d0adc64b1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -288,6 +288,7 @@ mod ptr_eq; mod ptr_offset_with_cast; mod question_mark; mod ranges; +mod rebind_fn_arg_as_mut; mod redundant_clone; mod redundant_closure_call; mod redundant_field_names; @@ -798,6 +799,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &ranges::RANGE_PLUS_ONE, &ranges::RANGE_ZIP_WITH_LEN, &ranges::REVERSED_EMPTY_RANGES, + &rebind_fn_arg_as_mut::REBIND_FN_ARG_AS_MUT, &redundant_clone::REDUNDANT_CLONE, &redundant_closure_call::REDUNDANT_CLOSURE_CALL, &redundant_field_names::REDUNDANT_FIELD_NAMES, @@ -959,6 +961,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box lifetimes::Lifetimes); store.register_late_pass(|| box entry::HashMapPass); store.register_late_pass(|| box ranges::Ranges); + store.register_late_pass(|| box rebind_fn_arg_as_mut::RebindFnArgAsMut); store.register_late_pass(|| box types::Casts); let type_complexity_threshold = conf.type_complexity_threshold; store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold)); @@ -1489,6 +1492,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&ranges::MANUAL_RANGE_CONTAINS), LintId::of(&ranges::RANGE_ZIP_WITH_LEN), LintId::of(&ranges::REVERSED_EMPTY_RANGES), + LintId::of(&rebind_fn_arg_as_mut::REBIND_FN_ARG_AS_MUT), LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&redundant_closure_call::REDUNDANT_CLOSURE_CALL), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), @@ -1646,6 +1650,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&ptr_eq::PTR_EQ), LintId::of(&question_mark::QUESTION_MARK), LintId::of(&ranges::MANUAL_RANGE_CONTAINS), + LintId::of(&rebind_fn_arg_as_mut::REBIND_FN_ARG_AS_MUT), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), LintId::of(®ex::TRIVIAL_REGEX), diff --git a/clippy_lints/src/rebind_fn_arg_as_mut.rs b/clippy_lints/src/rebind_fn_arg_as_mut.rs new file mode 100644 index 000000000000..fbf53167ae9d --- /dev/null +++ b/clippy_lints/src/rebind_fn_arg_as_mut.rs @@ -0,0 +1,92 @@ +use crate::utils::{snippet, span_lint_and_then}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{ + BindingAnnotation, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, Local, Node, PatKind, QPath, TraitFn, + TraitItem, TraitItemKind, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +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. + /// + /// **Known problems:** The function argument might have been shadowed by another + /// value before the assignment. + /// + /// **Example:** + /// + /// ```rust + /// fn foo_bad(bar: Vec) -> Vec { + /// let mut bar = bar; + /// bar.push(42); + /// bar + /// } + /// ``` + /// Use instead: + /// ```rust + /// fn foo(mut bar: Vec) -> Vec { + /// bar.push(42); + /// bar + /// } + /// ``` + pub REBIND_FN_ARG_AS_MUT, + style, + "non-mutable function argument rebound as mutable" +} + +declare_lint_pass!(RebindFnArgAsMut => [REBIND_FN_ARG_AS_MUT]); + +impl LateLintPass<'_> for RebindFnArgAsMut { + fn check_local(&mut self, cx: &LateContext<'tcx>, local: &Local<'tcx>) { + if_chain! { + if !in_external_macro(cx.tcx.sess, local.span); + + // LHS + if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.kind; + + // RHS + if let Some(init) = local.init; + if let ExprKind::Path(QPath::Resolved(_, path)) = init.kind; + if path.segments.len() == 1; + if path.segments[0].ident == name; + + if let rustc_hir::def::Res::Local(id) = path.res; + + // Fn + let parent_id = cx.tcx.hir().get_parent_item(id); + + if let Node::Item(&Item { kind: ItemKind::Fn(_, _, body_id), .. }) + | Node::ImplItem(&ImplItem { kind: ImplItemKind::Fn(_, body_id), .. }) + | Node::TraitItem(&TraitItem { kind: TraitItemKind::Fn(_, TraitFn::Provided(body_id)), .. }) + = cx.tcx.hir().get(parent_id); + + let body = cx.tcx.hir().body(body_id); + if let Some(param) = body.params.iter().find(|param| param.pat.hir_id == id); + if let PatKind::Binding(BindingAnnotation::Unannotated, ..) = param.pat.kind; + + then { + span_lint_and_then( + cx, + REBIND_FN_ARG_AS_MUT, + param.pat.span, + &format!("argument `{}` is declared as not mutable, and later rebound as mutable", name), + |diag| { + diag.span_suggestion( + param.pat.span, + "consider just declaring as mutable", + format!("mut {}", snippet(cx, param.pat.span, "_")), + Applicability::MaybeIncorrect, + ); + diag.span_help(local.span, "and remove this"); + }, + ); + } + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 6272ce45efbc..9a49b425bef3 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1936,6 +1936,13 @@ vec![ deprecation: None, module: "types", }, + Lint { + name: "rebind_fn_arg_as_mut", + group: "style", + desc: "non-mutable function argument rebound as mutable", + deprecation: None, + module: "rebind_fn_arg_as_mut", + }, Lint { name: "redundant_allocation", group: "perf", diff --git a/tests/ui/rebind_fn_arg_as_mut.rs b/tests/ui/rebind_fn_arg_as_mut.rs new file mode 100644 index 000000000000..06be096ab5f9 --- /dev/null +++ b/tests/ui/rebind_fn_arg_as_mut.rs @@ -0,0 +1,36 @@ +// edition:2018 + +#![warn(clippy::rebind_fn_arg_as_mut)] + +fn f(x: bool) { + let mut x = x; +} + +trait T { + fn tm1(x: bool) { + let mut x = x; + } + fn tm2(x: bool); +} + +struct S; + +impl S { + fn m(x: bool) { + let mut x = x; + } +} + +impl T for S { + fn tm2(x: bool) { + let mut x = x; + } +} + +fn f_no_lint(mut x: bool) { + let mut x = x; +} + +async fn expansion(_: T) {} + +fn main() {} diff --git a/tests/ui/rebind_fn_arg_as_mut.stderr b/tests/ui/rebind_fn_arg_as_mut.stderr new file mode 100644 index 000000000000..b432b3231285 --- /dev/null +++ b/tests/ui/rebind_fn_arg_as_mut.stderr @@ -0,0 +1,51 @@ +error: argument `x` is declared as not mutable, and later rebound as mutable + --> $DIR/rebind_fn_arg_as_mut.rs:5:6 + | +LL | fn f(x: bool) { + | ^ help: consider just declaring as mutable: `mut x` + | + = note: `-D clippy::rebind-fn-arg-as-mut` implied by `-D warnings` +help: and remove this + --> $DIR/rebind_fn_arg_as_mut.rs:6:5 + | +LL | let mut x = x; + | ^^^^^^^^^^^^^^ + +error: argument `x` is declared as not mutable, and later rebound as mutable + --> $DIR/rebind_fn_arg_as_mut.rs:10:12 + | +LL | fn tm1(x: bool) { + | ^ help: consider just declaring as mutable: `mut x` + | +help: and remove this + --> $DIR/rebind_fn_arg_as_mut.rs:11:9 + | +LL | let mut x = x; + | ^^^^^^^^^^^^^^ + +error: argument `x` is declared as not mutable, and later rebound as mutable + --> $DIR/rebind_fn_arg_as_mut.rs:19:10 + | +LL | fn m(x: bool) { + | ^ help: consider just declaring as mutable: `mut x` + | +help: and remove this + --> $DIR/rebind_fn_arg_as_mut.rs:20:9 + | +LL | let mut x = x; + | ^^^^^^^^^^^^^^ + +error: argument `x` is declared as not mutable, and later rebound as mutable + --> $DIR/rebind_fn_arg_as_mut.rs:25:12 + | +LL | fn tm2(x: bool) { + | ^ help: consider just declaring as mutable: `mut x` + | +help: and remove this + --> $DIR/rebind_fn_arg_as_mut.rs:26:9 + | +LL | let mut x = x; + | ^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors +