Merge pull request #354 from Pyriphlegethon/master

Add "unnecessary mut passed" lint

Author: Manishearth

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

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,

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<Expr>], 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));
+                    }
+                },
+                _ => {}
+            }
+        }
+    }
+}

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];

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);
+    
+}