Skip to content

Commit 2288ca5

Browse files
HKalbasiHKalbasi
committed
add derivable impls lint
1 parent 6d36d1a commit 2288ca5

File tree

6 files changed

+221
-0
lines changed

6 files changed

+221
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2483,6 +2483,7 @@ Released 2018-09-13
24832483
[`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
24842484
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
24852485
[`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
2486+
[`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls
24862487
[`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
24872488
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
24882489
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method

‎clippy_lints/src/derivable_impls.rs

+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::{is_automatically_derived, match_def_path, paths};
3+
use rustc_ast::LitKind;
4+
use rustc_hir::{Block, BlockCheckMode, BodyId, Expr, ExprField, ExprKind, FnDecl, HirId, Impl, ImplItemKind, Item, ItemKind, Node, QPath, TraitRef, TyKind, UnsafeSource, Unsafety};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Checks for double comparisons that could be simplified to a single expression.
12+
///
13+
///
14+
/// ### Why is this bad?
15+
/// Readability.
16+
///
17+
/// ### Example
18+
/// ```rust
19+
/// # let x = 1;
20+
/// # let y = 2;
21+
/// if x == y || x < y {}
22+
/// ```
23+
///
24+
/// Could be written as:
25+
///
26+
/// ```rust
27+
/// # let x = 1;
28+
/// # let y = 2;
29+
/// if x <= y {}
30+
/// ```
31+
pub DERIVABLE_IMPLS,
32+
complexity,
33+
"manual implementation of a trait which is equal to a derive"
34+
}
35+
36+
declare_lint_pass!(DerivableImpls => [DERIVABLE_IMPLS]);
37+
38+
fn struct_fields<'hir>(e: &'hir Expr<'hir>) -> Option<&'hir [ExprField]> {
39+
match e.kind {
40+
ExprKind::Struct(_, fields, _) => Some(fields),
41+
ExprKind::Block(Block { expr, .. }, _) => expr.and_then(struct_fields),
42+
_ => None,
43+
}
44+
}
45+
46+
fn is_path_self(e: &Expr<'_>) -> bool {
47+
if let ExprKind::Path(QPath::Resolved(_, p)) = e.kind {
48+
if p.segments.len() != 1 {
49+
return false;
50+
}
51+
if p.segments[0].ident.name == rustc_span::symbol::kw::SelfUpper {
52+
return true;
53+
}
54+
}
55+
false
56+
}
57+
58+
fn tuple_fields<'hir>(e: &'hir Expr<'hir>) -> Option<&'hir [Expr<'hir>]> {
59+
match e.kind {
60+
ExprKind::Tup(fields) => Some(fields),
61+
ExprKind::Call(callee, args) => if is_path_self(callee) {
62+
Some(args)
63+
} else {
64+
None
65+
},
66+
ExprKind::Block(Block { expr, .. }, _) => expr.and_then(tuple_fields),
67+
_ => None,
68+
}
69+
}
70+
71+
fn is_default(_cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
72+
if let ExprKind::Lit(lit) = &e.kind {
73+
match lit.node {
74+
LitKind::Bool(x) => !x,
75+
LitKind::Int(x, _) => x == 0,
76+
_ => false,
77+
}
78+
} else {
79+
false
80+
}
81+
}
82+
83+
impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
84+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
85+
if let ItemKind::Impl(Impl {
86+
of_trait: Some(ref trait_ref),
87+
self_ty,
88+
items,
89+
..
90+
}) = item.kind
91+
{
92+
let ty = cx.tcx.type_of(item.def_id);
93+
let attrs = cx.tcx.hir().attrs(item.hir_id());
94+
if is_automatically_derived(attrs) {
95+
return;
96+
}
97+
if let Some(def_id) = trait_ref.trait_def_id() {
98+
if !match_def_path(cx, def_id, &paths::DEFAULT_TRAIT) {
99+
return;
100+
}
101+
}
102+
let impl_item_hir = items[0].id.hir_id();
103+
let func_expr = match cx.tcx.hir().find(impl_item_hir) {
104+
Some(Node::ImplItem(x)) => match &x.kind {
105+
ImplItemKind::Fn(_, b) => match cx.tcx.hir().find(b.hir_id) {
106+
Some(Node::Expr(e)) => e,
107+
_ => return,
108+
},
109+
_ => return,
110+
},
111+
_ => return,
112+
};
113+
let should_emit = if let Some(s) = struct_fields(func_expr) {
114+
s.iter().all(|ef| is_default(cx, ef.expr))
115+
} else if let Some(s) = tuple_fields(func_expr) {
116+
s.iter().all(|e| is_default(cx, e))
117+
} else {
118+
return;
119+
};
120+
if should_emit {
121+
span_lint(cx, DERIVABLE_IMPLS, item.span, "derivable impl of trait Default:");
122+
}
123+
}
124+
}
125+
}

‎clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ mod dbg_macro;
189189
mod default;
190190
mod default_numeric_fallback;
191191
mod dereference;
192+
mod derivable_impls;
192193
mod derive;
193194
mod disallowed_method;
194195
mod disallowed_script_idents;
@@ -587,6 +588,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
587588
default::FIELD_REASSIGN_WITH_DEFAULT,
588589
default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK,
589590
dereference::EXPLICIT_DEREF_METHODS,
591+
derivable_impls::DERIVABLE_IMPLS,
590592
derive::DERIVE_HASH_XOR_EQ,
591593
derive::DERIVE_ORD_XOR_PARTIAL_ORD,
592594
derive::EXPL_IMPL_CLONE_ON_COPY,
@@ -1200,6 +1202,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12001202
LintId::of(copies::IFS_SAME_COND),
12011203
LintId::of(copies::IF_SAME_THEN_ELSE),
12021204
LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
1205+
LintId::of(derivable_impls::DERIVABLE_IMPLS),
12031206
LintId::of(derive::DERIVE_HASH_XOR_EQ),
12041207
LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD),
12051208
LintId::of(doc::MISSING_SAFETY_DOC),
@@ -1583,6 +1586,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15831586
LintId::of(casts::CHAR_LIT_AS_U8),
15841587
LintId::of(casts::UNNECESSARY_CAST),
15851588
LintId::of(copies::BRANCHES_SHARING_CODE),
1589+
LintId::of(derivable_impls::DERIVABLE_IMPLS),
15861590
LintId::of(double_comparison::DOUBLE_COMPARISONS),
15871591
LintId::of(double_parens::DOUBLE_PARENS),
15881592
LintId::of(duration_subsec::DURATION_SUBSEC),
@@ -1922,6 +1926,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
19221926
store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented);
19231927
store.register_late_pass(|| box strings::StringLitAsBytes);
19241928
store.register_late_pass(|| box derive::Derive);
1929+
store.register_late_pass(|| box derivable_impls::DerivableImpls);
19251930
store.register_late_pass(|| box get_last_with_len::GetLastWithLen);
19261931
store.register_late_pass(|| box drop_forget_ref::DropForgetRef);
19271932
store.register_late_pass(|| box empty_enum::EmptyEnum);

‎clippy_utils/src/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"];
3131
pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"];
3232
pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
3333
pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"];
34+
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
3435
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
3536
pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
3637
/// Preferably use the diagnostic item `sym::deref_method` where possible

‎tests/ui/derivable_impls.rs

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
#[deny(clippy::derivable_impls)]
2+
3+
struct FooDefault {
4+
a: bool,
5+
b: i32,
6+
c: u64,
7+
}
8+
9+
struct TupleDefault(bool, i32, u64);
10+
11+
struct FooND1 {
12+
a: bool,
13+
}
14+
15+
struct FooND2 {
16+
a: i32,
17+
}
18+
19+
struct FooNDNew {
20+
a: bool,
21+
}
22+
23+
impl std::default::Default for FooDefault {
24+
fn default() -> Self {
25+
Self {
26+
a: false,
27+
b: 0,
28+
c: 0u64,
29+
}
30+
}
31+
}
32+
33+
impl std::default::Default for TupleDefault {
34+
fn default() -> Self {
35+
Self(false, 0, 0u64)
36+
}
37+
}
38+
39+
impl std::default::Default for FooND1 {
40+
fn default() -> Self {
41+
Self { a: true }
42+
}
43+
}
44+
45+
impl std::default::Default for FooND2 {
46+
fn default() -> Self {
47+
Self { a: 5 }
48+
}
49+
}
50+
51+
impl FooNDNew {
52+
fn new() -> Self {
53+
Self { a: true }
54+
}
55+
}
56+
57+
impl Default for FooNDNew {
58+
fn default() -> Self {
59+
Self::new()
60+
}
61+
}
62+
63+
fn main() {}

‎tests/ui/derivable_impls.stderr

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: derivable impl of trait Default:
2+
--> $DIR/derivable_impls.rs:23:1
3+
|
4+
LL | / impl std::default::Default for FooDefault {
5+
LL | | fn default() -> Self {
6+
LL | | Self {
7+
LL | | a: false,
8+
... |
9+
LL | | }
10+
LL | | }
11+
| |_^
12+
|
13+
= note: `-D clippy::derivable-impls` implied by `-D warnings`
14+
15+
error: derivable impl of trait Default:
16+
--> $DIR/derivable_impls.rs:33:1
17+
|
18+
LL | / impl std::default::Default for TupleDefault {
19+
LL | | fn default() -> Self {
20+
LL | | Self(false, 0, 0u64)
21+
LL | | }
22+
LL | | }
23+
| |_^
24+
25+
error: aborting due to 2 previous errors
26+

0 commit comments

Comments
 (0)