Add a lint about using clone on Copy types

This commit is contained in:
mcarton 2016-02-02 22:35:01 +01:00
parent 47e26ab287
commit db205c82a4
7 changed files with 50 additions and 11 deletions

View file

@ -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 107 lints included in this crate:
There are 108 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -22,6 +22,7 @@ name
[cast_sign_loss](https://github.com/Manishearth/rust-clippy/wiki#cast_sign_loss) | allow | casts from signed types to unsigned types, e.g `x as u32` where `x: i32`
[char_lit_as_u8](https://github.com/Manishearth/rust-clippy/wiki#char_lit_as_u8) | warn | Casting a character literal to u8
[chars_next_cmp](https://github.com/Manishearth/rust-clippy/wiki#chars_next_cmp) | warn | using `.chars().next()` to check if a string starts with a char
[clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#clone_on_copy) | warn | using `clone` on a `Copy` type
[cmp_nan](https://github.com/Manishearth/rust-clippy/wiki#cmp_nan) | deny | comparisons to NAN (which will always return false, which is probably not intended)
[cmp_owned](https://github.com/Manishearth/rust-clippy/wiki#cmp_owned) | warn | creating owned instances for comparing with others, e.g. `x == "foo".to_string()`
[collapsible_if](https://github.com/Manishearth/rust-clippy/wiki#collapsible_if) | warn | two nested `if`-expressions can be collapsed into one, e.g. `if x { if y { foo() } }` can be written as `if x && y { foo() }` and an `else { if .. } expression can be collapsed to `else if`

View file

@ -214,6 +214,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
matches::MATCH_REF_PATS,
matches::SINGLE_MATCH,
methods::CHARS_NEXT_CMP,
methods::CLONE_ON_COPY,
methods::EXTEND_FROM_SLICE,
methods::FILTER_NEXT,
methods::OK_EXPECT,

View file

@ -475,7 +475,7 @@ fn check_for_loop_explicit_counter(cx: &LateContext, arg: &Expr, body: &Expr, ex
let mut visitor2 = InitializeVisitor {
cx: cx,
end_expr: expr,
var_id: id.clone(),
var_id: *id,
state: VarState::IncrOnce,
name: None,
depth: 0,

View file

@ -219,6 +219,17 @@ declare_lint!(pub OR_FUN_CALL, Warn,
declare_lint!(pub EXTEND_FROM_SLICE, Warn,
"`.extend_from_slice(_)` is a faster way to extend a Vec by a slice");
/// **What it does:** This lint warns on using `.clone()` on a `Copy` type.
///
/// **Why is this bad?** The only reason `Copy` types implement `Clone` is for generics, not for
/// using the `clone` method on a concrete type.
///
/// **Known problems:** None.
///
/// **Example:** `42u64.clone()`
declare_lint!(pub CLONE_ON_COPY, Warn,
"using `clone` on a `Copy` type");
impl LintPass for MethodsPass {
fn get_lints(&self) -> LintArray {
lint_array!(EXTEND_FROM_SLICE,
@ -233,7 +244,8 @@ impl LintPass for MethodsPass {
OPTION_MAP_UNWRAP_OR,
OPTION_MAP_UNWRAP_OR_ELSE,
OR_FUN_CALL,
CHARS_NEXT_CMP)
CHARS_NEXT_CMP,
CLONE_ON_COPY)
}
}
@ -269,6 +281,7 @@ impl LateLintPass for MethodsPass {
}
lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
lint_clone_on_copy(cx, expr, &name.node.as_str(), &args);
}
ExprBinary(op, ref lhs, ref rhs) if op.node == BiEq || op.node == BiNe => {
if !lint_chars_next(cx, expr, lhs, rhs, op.node == BiEq) {
@ -439,6 +452,19 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P<Expr>])
}
}
/// Checks for the `CLONE_ON_COPY` lint.
fn lint_clone_on_copy(cx: &LateContext, expr: &Expr, name: &str, args: &[P<Expr>]) {
if args.len() == 1 && name == "clone" {
let ty = cx.tcx.expr_ty(expr);
let parent = cx.tcx.map.get_parent(expr.id);
let parameter_environment = ty::ParameterEnvironment::for_item(cx.tcx, parent);
if !ty.moves_by_default(&parameter_environment, expr.span) {
span_lint(cx, CLONE_ON_COPY, expr.span, "using `clone` on a `Copy` type");
}
}
}
fn lint_extend(cx: &LateContext, expr: &Expr, args: &MethodArgs) {
let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
if !match_type(cx, obj_ty, &VEC_PATH) {
@ -701,7 +727,7 @@ fn lint_chars_next(cx: &LateContext, expr: &Expr, chain: &Expr, other: &Expr, eq
false
}
// Given a `Result<T, E>` type, return its error type (`E`)
/// Given a `Result<T, E>` type, return its error type (`E`).
fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
if !match_type(cx, ty, &RESULT_PATH) {
return None;
@ -714,9 +740,9 @@ fn get_error_type<'a>(cx: &LateContext, ty: ty::Ty<'a>) -> Option<ty::Ty<'a>> {
None
}
// This checks whether a given type is known to implement Debug. It's
// conservative, i.e. it should not return false positives, but will return
// false negatives.
/// This checks whether a given type is known to implement Debug. It's
/// conservative, i.e. it should not return false positives, but will return
/// false negatives.
fn has_debug_impl<'a, 'b>(ty: ty::Ty<'a>, cx: &LateContext<'b, 'a>) -> bool {
let no_ref_ty = walk_ptrs_ty(ty);
let debug = match cx.tcx.lang_items.debug_trait() {

View file

@ -110,7 +110,7 @@ impl EarlyLintPass for MiscEarly {
arg_name[1..].to_owned()));
}
} else {
registered_names.insert(arg_name, arg.pat.span.clone());
registered_names.insert(arg_name, arg.pat.span);
}
}
}

View file

@ -3,7 +3,7 @@
#![plugin(clippy)]
#![deny(map_clone)]
#![allow(unused)]
#![allow(clone_on_copy, unused)]
use std::ops::Deref;

View file

@ -323,3 +323,14 @@ fn use_extend_from_slice() {
//~| HELP try this
//~| SUGGESTION v.extend_from_slice(&["But", "this"]);
}
fn clone_on_copy() {
42.clone(); //~ERROR using `clone` on a `Copy` type
vec![1].clone(); // ok, not a Copy type
Some(vec![1]).clone(); // ok, not a Copy type
}
fn clone_on_copy_generic<T: Copy>(t: T) {
t.clone(); //~ERROR using `clone` on a `Copy` type
Some(t).clone(); //~ERROR using `clone` on a `Copy` type
}