Skip to content

Add "unnecessary mut passed" lint #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 6, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
74 changes: 74 additions & 0 deletions src/mut_reference.rs
Original file line number Diff line number Diff line change
@@ -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));
}
},
_ => {}
}
}
}
}
2 changes: 1 addition & 1 deletion tests/compile-fail/for_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
46 changes: 46 additions & 0 deletions tests/compile-fail/mut_reference.rs
Original file line number Diff line number Diff line change
@@ -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);

}