nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-12-18 19:43:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #580 from mcarton/derive

Browse Source 
Lint explicit Clone implementations on Copy type
This commit is contained in:
llogiq 2016-01-24 15:53:16 +01:00
commit 5f17b38bf2
5 changed files with 178 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
README.md
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 97 lints included in this crate:
There are 98 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -30,6 +30,7 @@ name
[duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | Function arguments having names which only differ by an underscore
[empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop) | warn | empty `loop {}` detected
[eq_op](https://github.com/Manishearth/rust-clippy/wiki#eq_op) | warn | equal operands on both sides of a comparison or bitwise combination (e.g. `x == x`)
[expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy) | warn | implementing `Clone` explicitly on `Copy` types
[explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do
[explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop) | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do
[filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next) | warn | using `filter(p).next()`, which is more succinctly expressed as `.find(p)`

183
src/derive.rs
View File

@ -1,13 +1,15 @@
use rustc::lint::*;
use rustc::middle::ty::fast_reject::simplify_type;
use rustc::middle::ty;
use rustc_front::hir::*;
use syntax::ast::{Attribute, MetaItem_};
use syntax::codemap::Span;
use utils::{CLONE_TRAIT_PATH, HASH_PATH};
use utils::{match_path, span_lint_and_then};
use utils::HASH_PATH;
use rustc::middle::ty::fast_reject::simplify_type;
use rustc::middle::ty::TypeVariants;
/// **What it does:** This lint warns about deriving `Hash` but implementing `PartialEq`
/// explicitely.
/// explicitly.
///
/// **Why is this bad?** The implementation of these traits must agree (for example for use with
/// `HashMap`) so it’s probably a bad idea to use a default-generated `Hash` implementation with
@ -33,66 +35,145 @@ declare_lint! {
"deriving `Hash` but implementing `PartialEq` explicitly"
}
/// **What it does:** This lint warns about explicit `Clone` implementation for `Copy` types.
///
/// **Why is this bad?** To avoid surprising behaviour, these traits should agree and the behaviour
/// of `Copy` cannot be overridden. In almost all situations a `Copy` type should have a `Clone`
/// implementation that does nothing more than copy the object, which is what
/// `#[derive(Copy, Clone)]` gets you.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// #[derive(Copy)]
/// struct Foo;
///
/// impl Clone for Foo {
/// ..
/// }
declare_lint! {
pub EXPL_IMPL_CLONE_ON_COPY,
Warn,
"implementing `Clone` explicitly on `Copy` types"
}
pub struct Derive;
impl LintPass for Derive {
fn get_lints(&self) -> LintArray {
lint_array!(DERIVE_HASH_NOT_EQ)
lint_array!(EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_NOT_EQ)
}
}
impl LateLintPass for Derive {
fn check_item(&mut self, cx: &LateContext, item: &Item) {
/// A `#[derive]`d implementation has a `#[automatically_derived]` attribute.
fn is_automatically_derived(attr: &Attribute) -> bool {
if let MetaItem_::MetaWord(ref word) = attr.node.value.node {
word == &"automatically_derived"
}
else {
false
}
}
let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow();
// If `item` is an automatically derived `Hash` implementation
if_let_chain! {[
let ItemImpl(_, _, _, Some(ref trait_ref), ref ast_ty, _) = item.node,
match_path(&trait_ref.path, &HASH_PATH),
item.attrs.iter().any(is_automatically_derived),
let Some(peq_trait_def_id) = cx.tcx.lang_items.eq_trait()
let Some(&ty) = ast_ty_to_ty_cache.get(&ast_ty.id)
], {
let peq_trait_def = cx.tcx.lookup_trait_def(peq_trait_def_id);
cx.tcx.populate_implementations_for_trait_if_necessary(peq_trait_def.trait_ref.def_id);
let peq_impls = peq_trait_def.borrow_impl_lists(cx.tcx).1;
let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow();
// Look for the PartialEq implementations for `ty`
if_let_chain! {[
let Some(ty) = ast_ty_to_ty_cache.get(&ast_ty.id),
let Some(simpl_ty) = simplify_type(cx.tcx, ty, false),
let Some(impl_ids) = peq_impls.get(&simpl_ty)
], {
for &impl_id in impl_ids {
let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
// Only care about `impl PartialEq<Foo> for Foo`
if trait_ref.input_types()[0] == *ty &&
!cx.tcx.get_attrs(impl_id).iter().any(is_automatically_derived) {
span_lint_and_then(
cx, DERIVE_HASH_NOT_EQ, item.span,
&format!("you are deriving `Hash` but have implemented \
`PartialEq` explicitely"), |db| {
if let Some(node_id) = cx.tcx.map.as_local_node_id(impl_id) {
db.span_note(
cx.tcx.map.span(node_id),
"`PartialEq` implemented here"
);
}
});
}
}
}}
if item.attrs.iter().any(is_automatically_derived) {
check_hash_peq(cx, item.span, trait_ref, ty);
}
else {
check_copy_clone(cx, item.span, trait_ref, ty);
}
}}
}
}
/// Implementation of the `DERIVE_HASH_NOT_EQ` lint.
fn check_hash_peq(cx: &LateContext, span: Span, trait_ref: &TraitRef, ty: ty::Ty) {
// If `item` is an automatically derived `Hash` implementation
if_let_chain! {[
match_path(&trait_ref.path, &HASH_PATH),
let Some(peq_trait_def_id) = cx.tcx.lang_items.eq_trait()
], {
let peq_trait_def = cx.tcx.lookup_trait_def(peq_trait_def_id);
cx.tcx.populate_implementations_for_trait_if_necessary(peq_trait_def.trait_ref.def_id);
let peq_impls = peq_trait_def.borrow_impl_lists(cx.tcx).1;
// Look for the PartialEq implementations for `ty`
if_let_chain! {[
let Some(simpl_ty) = simplify_type(cx.tcx, ty, false),
let Some(impl_ids) = peq_impls.get(&simpl_ty)
], {
for &impl_id in impl_ids {
let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
// Only care about `impl PartialEq<Foo> for Foo`
if trait_ref.input_types()[0] == ty &&
!cx.tcx.get_attrs(impl_id).iter().any(is_automatically_derived) {
span_lint_and_then(
cx, DERIVE_HASH_NOT_EQ, span,
"you are deriving `Hash` but have implemented `PartialEq` explicitly",
|db| {
if let Some(node_id) = cx.tcx.map.as_local_node_id(impl_id) {
db.span_note(
cx.tcx.map.span(node_id),
"`PartialEq` implemented here"
);
}
});
}
}
}}
}}
}
/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, span: Span, trait_ref: &TraitRef, ty: ty::Ty<'tcx>) {
if match_path(&trait_ref.path, &CLONE_TRAIT_PATH) {
let parameter_environment = cx.tcx.empty_parameter_environment();
if ty.moves_by_default(&parameter_environment, span) {
return; // ty is not Copy
}
// Some types are not Clone by default but could be cloned `by hand` if necessary
match ty.sty {
TypeVariants::TyEnum(def, substs) | TypeVariants::TyStruct(def, substs) => {
for variant in &def.variants {
for field in &variant.fields {
match field.ty(cx.tcx, substs).sty {
TypeVariants::TyArray(_, size) if size > 32 => {
return;
}
TypeVariants::TyBareFn(..) => {
return;
}
TypeVariants::TyTuple(ref tys) if tys.len() > 12 => {
return;
}
_ => (),
}
}
}
}
_ => (),
}
span_lint_and_then(
cx, DERIVE_HASH_NOT_EQ, span,
"you are implementing `Clone` explicitly on a `Copy` type",
|db| {
db.span_note(
span,
"consider deriving `Clone` or removing `Copy`"
);
});
}
}
/// Checks for the `#[automatically_derived]` attribute all `#[derive]`d implementations have.
fn is_automatically_derived(attr: &Attribute) -> bool {
if let MetaItem_::MetaWord(ref word) = attr.node.value.node {
word == &"automatically_derived"
}
else {
false
}
}

1
src/lib.rs
View File

@ -173,6 +173,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
collapsible_if::COLLAPSIBLE_IF,
cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
derive::DERIVE_HASH_NOT_EQ,
derive::EXPL_IMPL_CLONE_ON_COPY,
entry::MAP_ENTRY,
eq_op::EQ_OP,
escape::BOXED_LOCAL,

3
src/utils.rs
View File

@ -22,7 +22,8 @@ pub type MethodArgs = HirVec<P<Expr>>;
pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"];
pub const BTREEMAP_ENTRY_PATH: [&'static str; 4] = ["collections", "btree", "map", "Entry"];
pub const BTREEMAP_PATH: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"];
pub const CLONE_PATH: [&'static str; 2] = ["Clone", "clone"];
pub const CLONE_PATH: [&'static str; 3] = ["clone", "Clone", "clone"];
pub const CLONE_TRAIT_PATH: [&'static str; 2] = ["clone", "Clone"];
pub const COW_PATH: [&'static str; 3] = ["collections", "borrow", "Cow"];
pub const DEFAULT_TRAIT_PATH: [&'static str; 3] = ["core", "default", "Default"];
pub const HASHMAP_ENTRY_PATH: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"];

43
tests/compile-fail/derive.rs
View File

@ -2,6 +2,7 @@
#![plugin(clippy)]
#![deny(warnings)]
#![allow(dead_code)]
#[derive(PartialEq, Hash)]
struct Foo;
@ -11,7 +12,7 @@ impl PartialEq<u64> for Foo {
}
#[derive(Hash)]
//~^ ERROR you are deriving `Hash` but have implemented `PartialEq` explicitely
//~^ ERROR you are deriving `Hash` but have implemented `PartialEq` explicitly
struct Bar;
impl PartialEq for Bar {
@ -19,11 +20,49 @@ impl PartialEq for Bar {
}
#[derive(Hash)]
//~^ ERROR you are deriving `Hash` but have implemented `PartialEq` explicitely
//~^ ERROR you are deriving `Hash` but have implemented `PartialEq` explicitly
struct Baz;
impl PartialEq<Baz> for Baz {
fn eq(&self, _: &Baz) -> bool { true }
}
#[derive(Copy)]
struct Qux;
impl Clone for Qux {
//~^ ERROR you are implementing `Clone` explicitly on a `Copy` type
fn clone(&self) -> Self { Qux }
}
// Ok, `Clone` cannot be derived because of the big array
#[derive(Copy)]
struct BigArray {
a: [u8; 65],
}
impl Clone for BigArray {
fn clone(&self) -> Self { unimplemented!() }
}
// Ok, function pointers are not always Clone
#[derive(Copy)]
struct FnPtr {
a: fn() -> !,
}
impl Clone for FnPtr {
fn clone(&self) -> Self { unimplemented!() }
}
// Ok, generics
#[derive(Copy)]
struct Generic<T> {
a: T,
}
impl<T> Clone for Generic<T> {
fn clone(&self) -> Self { unimplemented!() }
}
fn main() {}