diff --git a/README.md b/README.md index 2147530ff57..a5ab856fc21 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 58 lints included in this crate: +There are 59 lints included in this crate: name | default | meaning -------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -63,6 +63,7 @@ name [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions [unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc) | allow | using a unicode literal not in NFC normal form (see http://www.unicode.org/reports/tr15/ for further information) [unit_cmp](https://github.com/Manishearth/rust-clippy/wiki#unit_cmp) | warn | comparing unit values (which is always `true` or `false`, respectively) +[unnecessary_mut_passed](https://github.com/Manishearth/rust-clippy/wiki#unnecessary_mut_passed) | warn | an argument is passed as a mutable reference although the function/method only demands an immutable reference [unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect) | warn | `collect()`ing an iterator without using the result; this is usually better written as a for loop [while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop [wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention diff --git a/src/lib.rs b/src/lib.rs index 3c2d870aee2..2f71d8cc9df 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,6 +33,7 @@ pub mod eta_reduction; pub mod identity_op; pub mod minmax; pub mod mut_mut; +pub mod mut_reference; pub mod len_zero; pub mod attrs; pub mod collapsible_if; @@ -66,6 +67,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box eta_reduction::EtaPass); reg.register_late_lint_pass(box identity_op::IdentityOp); reg.register_late_lint_pass(box mut_mut::MutMut); + reg.register_late_lint_pass(box mut_reference::UnnecessaryMutPassed); reg.register_late_lint_pass(box len_zero::LenZero); reg.register_late_lint_pass(box misc::CmpOwned); reg.register_late_lint_pass(box attrs::AttrPass); @@ -138,6 +140,7 @@ pub fn plugin_registrar(reg: &mut Registry) { misc::MODULO_ONE, misc::REDUNDANT_PATTERN, misc::TOPLEVEL_REF_ARG, + mut_reference::UNNECESSARY_MUT_PASSED, needless_bool::NEEDLESS_BOOL, precedence::PRECEDENCE, ranges::RANGE_STEP_BY_ZERO, diff --git a/src/mut_reference.rs b/src/mut_reference.rs new file mode 100644 index 00000000000..1cc04e096ba --- /dev/null +++ b/src/mut_reference.rs @@ -0,0 +1,74 @@ +use rustc::lint::*; +use rustc_front::hir::*; +use utils::span_lint; +use rustc::middle::ty::{TypeAndMut, TypeVariants, MethodCall, TyS}; +use syntax::ptr::P; + +declare_lint! { + pub UNNECESSARY_MUT_PASSED, + Warn, + "an argument is passed as a mutable reference although the function/method only demands an \ + immutable reference" +} + + +#[derive(Copy,Clone)] +pub struct UnnecessaryMutPassed; + +impl LintPass for UnnecessaryMutPassed { + fn get_lints(&self) -> LintArray { + lint_array!(UNNECESSARY_MUT_PASSED) + } +} + +impl LateLintPass for UnnecessaryMutPassed { + fn check_expr(&mut self, cx: &LateContext, e: &Expr) { + let borrowed_table = cx.tcx.tables.borrow(); + match e.node { + ExprCall(ref fn_expr, ref arguments) => { + match borrowed_table.node_types.get(&fn_expr.id) { + Some(function_type) => { + if let ExprPath(_, ref path) = fn_expr.node { + check_arguments(cx, &arguments, function_type, + &format!("{}", path)); + } + }, + None => unreachable!(), // A function with unknown type is called. + // If this happened the compiler would have aborted the + // compilation long ago. + }; + + + }, + ExprMethodCall(ref name, _, ref arguments) => { + let method_call = MethodCall::expr(e.id); + match borrowed_table.method_map.get(&method_call) { + Some(method_type) => check_arguments(cx, &arguments, method_type.ty, + &format!("{}", name.node.as_str())), + None => unreachable!(), // Just like above, this should never happen. + }; + }, + _ => {} + } + } +} + +fn check_arguments(cx: &LateContext, arguments: &[P], type_definition: &TyS, name: &str) { + if let TypeVariants::TyBareFn(_, ref fn_type) = type_definition.sty { + let parameters = &fn_type.sig.skip_binder().inputs; + for (argument, parameter) in arguments.iter().zip(parameters.iter()) { + match parameter.sty { + TypeVariants::TyRef(_, TypeAndMut {ty: _, mutbl: MutImmutable}) | + TypeVariants::TyRawPtr(TypeAndMut {ty: _, mutbl: MutImmutable}) => { + if let Expr_::ExprAddrOf(MutMutable, _) = argument.node { + span_lint(cx, UNNECESSARY_MUT_PASSED, + argument.span, &format!("The function/method \"{}\" \ + doesn't need a mutable reference", + name)); + } + }, + _ => {} + } + } + } +} diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index d6d73db3c18..11810242a88 100755 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -16,7 +16,7 @@ impl Unrelated { #[deny(needless_range_loop, explicit_iter_loop, iter_next_loop, reverse_range_loop, explicit_counter_loop)] #[deny(unused_collect)] -#[allow(linkedlist,shadow_unrelated)] +#[allow(linkedlist,shadow_unrelated,unnecessary_mut_passed)] fn main() { let mut vec = vec![1, 2, 3, 4]; let vec2 = vec![1, 2, 3, 4]; diff --git a/tests/compile-fail/mut_reference.rs b/tests/compile-fail/mut_reference.rs new file mode 100644 index 00000000000..7480add8e68 --- /dev/null +++ b/tests/compile-fail/mut_reference.rs @@ -0,0 +1,46 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![allow(unused_variables)] + +fn takes_an_immutable_reference(a: &i32) { +} + + +fn takes_a_mutable_reference(a: &mut i32) { +} + +struct MyStruct; + +impl MyStruct { + fn takes_an_immutable_reference(&self, a: &i32) { + } + + fn takes_a_mutable_reference(&self, a: &mut i32) { + } +} + +#[deny(unnecessary_mut_passed)] +fn main() { + // Functions + takes_an_immutable_reference(&mut 42); //~ERROR The function/method "takes_an_immutable_reference" doesn't need a mutable reference + + // Methods + let my_struct = MyStruct; + my_struct.takes_an_immutable_reference(&mut 42); //~ERROR The function/method "takes_an_immutable_reference" doesn't need a mutable reference + + + // No error + + // Functions + takes_an_immutable_reference(&42); + takes_a_mutable_reference(&mut 42); + let a = &mut 42; + takes_an_immutable_reference(a); + + // Methods + my_struct.takes_an_immutable_reference(&42); + my_struct.takes_a_mutable_reference(&mut 42); + my_struct.takes_an_immutable_reference(a); + +}