Skip to content

Commit a08f41f

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

File tree

8 files changed

+327
-58
lines changed

8 files changed

+327
-58
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
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‎

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

‎clippy_lints/src/lib.rs‎

Lines changed: 5 additions & 0 deletions
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_lints/src/mem_replace.rs‎

Lines changed: 19 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::source::{snippet, snippet_with_applicability};
3-
use clippy_utils::{in_macro, is_diag_trait_item, is_lang_ctor, match_def_path, meets_msrv, msrvs, paths};
3+
use clippy_utils::{in_macro, is_default, is_lang_ctor, match_def_path, meets_msrv, msrvs, paths};
44
use if_chain::if_chain;
55
use rustc_errors::Applicability;
6-
use rustc_hir::def_id::DefId;
76
use rustc_hir::LangItem::OptionNone;
87
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, QPath};
98
use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -194,64 +193,26 @@ fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'
194193
}
195194
}
196195

197-
/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent"
198-
/// constructor from the std library
199-
fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool {
200-
let std_types_symbols = &[
201-
sym::string_type,
202-
sym::vec_type,
203-
sym::vecdeque_type,
204-
sym::LinkedList,
205-
sym::hashmap_type,
206-
sym::BTreeMap,
207-
sym::hashset_type,
208-
sym::BTreeSet,
209-
sym::BinaryHeap,
210-
];
211-
212-
if let QPath::TypeRelative(_, method) = path {
213-
if method.ident.name == sym::new {
214-
if let Some(impl_did) = cx.tcx.impl_of_method(def_id) {
215-
if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() {
216-
return std_types_symbols
217-
.iter()
218-
.any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did));
219-
}
220-
}
221-
}
222-
}
223-
false
224-
}
225-
226196
fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
227-
if_chain! {
228-
if let ExprKind::Call(repl_func, _) = src.kind;
229-
if !in_external_macro(cx.tcx.sess, expr_span);
230-
if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
231-
if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
232-
if is_diag_trait_item(cx, repl_def_id, sym::Default)
233-
|| is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
234-
235-
then {
236-
span_lint_and_then(
237-
cx,
238-
MEM_REPLACE_WITH_DEFAULT,
239-
expr_span,
240-
"replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`",
241-
|diag| {
242-
if !in_macro(expr_span) {
243-
let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, ""));
244-
245-
diag.span_suggestion(
246-
expr_span,
247-
"consider using",
248-
suggestion,
249-
Applicability::MachineApplicable
250-
);
251-
}
197+
if is_default(cx, src) && !in_external_macro(cx.tcx.sess, expr_span) {
198+
span_lint_and_then(
199+
cx,
200+
MEM_REPLACE_WITH_DEFAULT,
201+
expr_span,
202+
"replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`",
203+
|diag| {
204+
if !in_macro(expr_span) {
205+
let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, ""));
206+
207+
diag.span_suggestion(
208+
expr_span,
209+
"consider using",
210+
suggestion,
211+
Applicability::MachineApplicable,
212+
);
252213
}
253-
);
254-
}
214+
},
215+
);
255216
}
256217
}
257218

‎clippy_utils/src/lib.rs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,61 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
606606
false
607607
}
608608

609+
/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent"
610+
/// constructor from the std library
611+
fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool {
612+
let std_types_symbols = &[
613+
sym::string_type,
614+
sym::vec_type,
615+
sym::vecdeque_type,
616+
sym::LinkedList,
617+
sym::hashmap_type,
618+
sym::BTreeMap,
619+
sym::hashset_type,
620+
sym::BTreeSet,
621+
sym::BinaryHeap,
622+
];
623+
624+
if let QPath::TypeRelative(_, method) = path {
625+
if method.ident.name == sym::new {
626+
if let Some(impl_did) = cx.tcx.impl_of_method(def_id) {
627+
if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() {
628+
return std_types_symbols
629+
.iter()
630+
.any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did));
631+
}
632+
}
633+
}
634+
}
635+
false
636+
}
637+
638+
pub fn is_default(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
639+
match &e.kind {
640+
ExprKind::Lit(lit) => match lit.node {
641+
LitKind::Bool(x) => !x,
642+
LitKind::Int(x, _) => x == 0,
643+
LitKind::Str(s, _) => s.is_empty(),
644+
_ => false,
645+
},
646+
ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default(cx, x)),
647+
ExprKind::Repeat(x, _) => is_default(cx, x),
648+
ExprKind::Call(repl_func, _) => if_chain! {
649+
if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
650+
if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
651+
if is_diag_trait_item(cx, repl_def_id, sym::Default)
652+
|| is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
653+
then {
654+
true
655+
}
656+
else {
657+
false
658+
}
659+
},
660+
_ => false,
661+
}
662+
}
663+
609664
/// Checks if the top level expression can be moved into a closure as is.
610665
pub fn can_move_expr_to_closure_no_visit(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, jump_targets: &[HirId]) -> bool {
611666
match expr.kind {

‎clippy_utils/src/paths.rs‎

Lines changed: 1 addition & 0 deletions
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

0 commit comments

Comments
 (0)