bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Merge pull request #440 from Manishearth/map_clone

Browse source 
match .map(Clone::clone)
This commit is contained in:
Manish Goregaokar 2015-11-08 10:13:55 +05:30
commit 764791b83e
3 changed files with 56 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

91
src/map_clone.rs
View file

@ -1,8 +1,8 @@
use rustc::lint::*; use rustc::lint::*;
use rustc_front::hir::*; use rustc_front::hir::*;
use syntax::ast::Ident; use syntax::ast::Ident;
use utils::OPTION_PATH; use utils::{CLONE_PATH, OPTION_PATH};
use utils::{is_adjusted, match_trait_method, match_type, snippet, span_help_and_lint}; use utils::{is_adjusted, match_path, match_trait_method, match_type, snippet, span_help_and_lint};
use utils::{walk_ptrs_ty, walk_ptrs_ty_depth}; use utils::{walk_ptrs_ty, walk_ptrs_ty_depth};
declare_lint!(pub MAP_CLONE, Warn, declare_lint!(pub MAP_CLONE, Warn,
@ -14,43 +14,58 @@ pub struct MapClonePass;
impl LateLintPass for MapClonePass { impl LateLintPass for MapClonePass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if_let_chain! { // call to .map()
[ if let ExprMethodCall(name, _, ref args) = expr.node {
// call to .map() if name.node.as_str() == "map" && args.len() == 2 {
let ExprMethodCall(name, _, ref args) = expr.node, match args[1].node {
name.node.as_str() == "map" && args.len() == 2, ExprClosure(_, ref decl, ref blk) => {
let ExprClosure(_, ref decl, ref blk) = args[1].node, if_let_chain! {
// just one expression in the closure [
blk.stmts.is_empty(), // just one expression in the closure
let Some(ref closure_expr) = blk.expr, blk.stmts.is_empty(),
// nothing special in the argument, besides reference bindings let Some(ref closure_expr) = blk.expr,
// (e.g. .map(|&x| x) ) // nothing special in the argument, besides reference bindings
let Some(arg_ident) = get_arg_name(&*decl.inputs[0].pat), // (e.g. .map(|&x| x) )
// the method is being called on a known type (option or iterator) let Some(arg_ident) = get_arg_name(&*decl.inputs[0].pat),
let Some(type_name) = get_type_name(cx, expr, &args[0]) // 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(cx, &*closure_expr, arg_ident) && // look for derefs, for .map(|x| *x)
// .cloned() only removes one level of indirection, don't lint on more if only_derefs(cx, &*closure_expr, arg_ident) &&
walk_ptrs_ty_depth(cx.tcx.pat_ty(&*decl.inputs[0].pat)).1 == 1 // .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 \ span_help_and_lint(cx, MAP_CLONE, expr.span, &format!(
using `.cloned()`", type_name), "you seem to be using .map() to clone the contents of an {}, consider \
&format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); 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 { // explicit clone() calls ( .map(|x| x.clone()) )
if clone_call.node.as_str() == "clone" && else if let ExprMethodCall(clone_call, _, ref clone_args) = closure_expr.node {
clone_args.len() == 1 && if clone_call.node.as_str() == "clone" &&
match_trait_method(cx, closure_expr, &["core", "clone", "Clone"]) && clone_args.len() == 1 &&
expr_eq_ident(&clone_args[0], arg_ident) 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 \ span_help_and_lint(cx, MAP_CLONE, expr.span, &format!(
using `.cloned()`", type_name), "you seem to be using .map() to clone the contents of an {}, consider \
&format!("try\n{}.cloned()", snippet(cx, args[0].span, ".."))); using `.cloned()`", type_name),
&format!("try\n{}.cloned()", snippet(cx, args[0].span, "..")));
}
}
}
}
},
ExprPath(_, ref path) => {
if match_path(path, &CLONE_PATH) {
let type_name = get_type_name(cx, expr, &args[0]).unwrap_or("_");
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, "..")));
}
} }
_ => (),
} }
} }
} }

1
src/utils.rs
View file

@ -17,6 +17,7 @@ pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"];
pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; pub const LL_PATH: [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"]; pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"];
pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const MUTEX_PATH: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"];
pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"];
/// Produce a nested chain of if-lets and ifs from the patterns: /// Produce a nested chain of if-lets and ifs from the patterns:
/// ///

2
tests/compile-fail/map_clone.rs
View file

@ -15,6 +15,8 @@ fn map_clone_iter() {
//~^ HELP try //~^ HELP try
x.iter().map(|y| *y); //~ ERROR you seem to be using .map() x.iter().map(|y| *y); //~ ERROR you seem to be using .map()
//~^ HELP try //~^ HELP try
x.iter().map(Clone::clone); //~ ERROR you seem to be using .map()
//~^ HELP try
} }
fn map_clone_option() { fn map_clone_option() {