OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

New lint for using .cloned()

Browse Source
This commit is contained in:
wartman4404 2015-10-30 23:58:37 -05:00
parent 4b1409684a
commit 414c0d20f7
4 changed files with 175 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
README.md
View File

@ -34,6 +34,7 @@ name
[let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block
[let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards
[linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist) | warn | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque
[map_clone](https://github.com/Manishearth/rust-clippy/wiki#map_clone) | warn | using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends `.cloned()` instead)
[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool) | warn | a match on boolean expression; recommends `if..else` block instead
[match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match has all arms prefixed with `&`; the match expression can be dereferenced instead
[min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant

3
src/lib.rs
View File

@ -45,6 +45,7 @@ pub mod returns;
pub mod lifetimes;
pub mod loops;
pub mod ranges;
pub mod map_clone;
pub mod matches;
pub mod precedence;
pub mod mutex_atomic;
@ -100,6 +101,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box needless_features::NeedlessFeaturesPass);
reg.register_late_lint_pass(box needless_update::NeedlessUpdatePass);
reg.register_late_lint_pass(box no_effect::NoEffectPass);
reg.register_late_lint_pass(box map_clone::MapClonePass);
reg.register_lint_group("clippy_pedantic", vec![
methods::OPTION_UNWRAP_USED,
@ -141,6 +143,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
loops::UNUSED_COLLECT,
loops::WHILE_LET_LOOP,
loops::WHILE_LET_ON_ITERATOR,
map_clone::MAP_CLONE,
matches::MATCH_BOOL,
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,

102
src/map_clone.rs Normal file
View File

@ -0,0 +1,102 @@
use rustc::lint::*;
use rustc_front::hir::*;
use syntax::ast::Ident;
use utils::OPTION_PATH;
use utils::{match_trait_method, match_type, snippet, span_help_and_lint};
use utils::{walk_ptrs_ty, walk_ptrs_ty_depth};
declare_lint!(pub MAP_CLONE, Warn,
"using `.map(|x| x.clone())` to clone an iterator or option's contents (recommends \
`.cloned()` instead)");
#[derive(Copy, Clone)]
pub struct MapClonePass;
impl LateLintPass for MapClonePass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if_let_chain! {
[
// call to .map()
let ExprMethodCall(name, _, ref args) = expr.node,
name.node.as_str() == "map" && args.len() == 2,
let ExprClosure(_, ref decl, ref blk) = args[1].node,
// just one expression in the closure
blk.stmts.is_empty(),
let Some(ref closure_expr) = blk.expr,
// nothing special in the argument, besides reference bindings
// (e.g. .map(|&x| x) )
let Some(arg_ident) = get_arg_name(&*decl.inputs[0].pat),
// the method is being called on a known type (option or iterator)
let Some(type_name) = get_type_name(cx, expr, &args[0])
], {
// look for derefs, for .map(|x| *x)
if only_derefs(&*closure_expr, arg_ident) &&
// .cloned() only removes one level of indirection, don't lint on more
walk_ptrs_ty_depth(cx.tcx.pat_ty(&*decl.inputs[0].pat)).1 == 1
{
span_help_and_lint(cx, MAP_CLONE, expr.span, &format!(
"you seem to be using .map() to clone the contents of an {}, consider \
using `.cloned()`", type_name),
&format!("try\n{}.cloned()", snippet(cx, args[0].span, "..")));
}
// explicit clone() calls ( .map(|x| x.clone()) )
else if let ExprMethodCall(clone_call, _, ref clone_args) = closure_expr.node {
if clone_call.node.as_str() == "clone" &&
clone_args.len() == 1 &&
match_trait_method(cx, closure_expr, &["core", "clone", "Clone"]) &&
expr_eq_ident(&clone_args[0], arg_ident)
{
span_help_and_lint(cx, MAP_CLONE, expr.span, &format!(
"you seem to be using .map() to clone the contents of an {}, consider \
using `.cloned()`", type_name),
&format!("try\n{}.cloned()", snippet(cx, args[0].span, "..")));
}
}
}
}
}
}
fn expr_eq_ident(expr: &Expr, id: Ident) -> bool {
match expr.node {
ExprPath(None, ref path) => {
let arg_segment = [PathSegment { identifier: id, parameters: PathParameters::none() }];
!path.global && path.segments == arg_segment
},
_ => false,
}
}
fn get_type_name(cx: &LateContext, expr: &Expr, arg: &Expr) -> Option<&'static str> {
if match_trait_method(cx, expr, &["core", "iter", "Iterator"]) {
Some("iterator")
} else if match_type(cx, walk_ptrs_ty(cx.tcx.expr_ty(arg)), &OPTION_PATH) {
Some("Option")
} else {
None
}
}
fn get_arg_name(pat: &Pat) -> Option<Ident> {
match pat.node {
PatIdent(_, ident, None) => Some(ident.node),
PatRegion(ref subpat, _) => get_arg_name(subpat),
_ => None,
}
}
fn only_derefs(expr: &Expr, id: Ident) -> bool {
if expr_eq_ident(expr, id) {
true
} else if let ExprUnary(UnDeref, ref subexpr) = expr.node {
only_derefs(subexpr, id)
} else {
false
}
}
impl LintPass for MapClonePass {
fn get_lints(&self) -> LintArray {
lint_array!(MAP_CLONE)
}
}

69
tests/compile-fail/map_clone.rs Normal file
View File

@ -0,0 +1,69 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(map_clone)]
#![allow(unused)]
fn map_clone_iter() {
let x = [1,2,3];
x.iter().map(|y| y.clone()); //~ ERROR you seem to be using .map()
//~^ HELP try
x.iter().map(|&y| y); //~ ERROR you seem to be using .map()
//~^ HELP try
x.iter().map(|y| *y); //~ ERROR you seem to be using .map()
//~^ HELP try
}
fn map_clone_option() {
let x = Some(4);
x.as_ref().map(|y| y.clone()); //~ ERROR you seem to be using .map()
//~^ HELP try
x.as_ref().map(|&y| y); //~ ERROR you seem to be using .map()
//~^ HELP try
x.as_ref().map(|y| *y); //~ ERROR you seem to be using .map()
//~^ HELP try
}
fn not_linted_option() {
let x = Some(5);
// Not linted: other statements
x.as_ref().map(|y| {
println!("y: {}", y);
y.clone()
});
// Not linted: argument bindings
let x = Some((6, 7));
x.map(|(y, _)| y.clone());
// Not linted: cloning something else
x.map(|y| y.0.clone());
// Not linted: no dereferences
x.map(|y| y);
// Not linted: multiple dereferences
let _: Option<(i32, i32)> = x.as_ref().as_ref().map(|&&x| x);
}
#[derive(Copy, Clone)]
struct Wrapper<T>(T);
impl<T> Wrapper<T> {
fn map<U, F: FnOnce(T) -> U>(self, f: F) -> Wrapper<U> {
Wrapper(f(self.0))
}
}
fn map_clone_other() {
let eight = 8;
let x = Wrapper(&eight);
// Not linted: not a linted type
x.map(|y| y.clone());
x.map(|&y| y);
x.map(|y| *y);
}
fn main() { }