Skip to content

Commit 5167f10

Browse files
HKalbasiHKalbasi
committed
add derivable impls lint
1 parent 15b1c6f commit 5167f10

9 files changed

+360
-57
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2613,6 +2613,7 @@ Released 2018-09-13
26132613
[`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
26142614
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
26152615
[`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
2616+
[`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls
26162617
[`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
26172618
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
26182619
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method

‎clippy_lints/src/derivable_impls.rs

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::{in_macro, is_automatically_derived, is_default_equivalent, remove_blocks};
3+
use rustc_hir::{Expr, 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+
/// Detects custom `std::default::Default` implementations that are identical to a derived implementation.
10+
///
11+
///
12+
/// ### Why is this bad?
13+
/// It is more concise.
14+
///
15+
/// ### Example
16+
/// ```rust
17+
/// struct Foo {
18+
/// bar: bool
19+
/// }
20+
///
21+
/// impl std::default::Default for Foo {
22+
/// fn default() -> Self {
23+
/// Self {
24+
/// bar: false
25+
/// }
26+
/// }
27+
/// }
28+
/// ```
29+
///
30+
/// Could be written as:
31+
///
32+
/// ```rust
33+
/// #[derive(Default)]
34+
/// struct Foo {
35+
/// bar: bool
36+
/// }
37+
/// ```
38+
pub DERIVABLE_IMPLS,
39+
complexity,
40+
"manual implementation of the default trait which is equal to a derive"
41+
}
42+
43+
declare_lint_pass!(DerivableImpls => [DERIVABLE_IMPLS]);
44+
45+
fn is_path_self(e: &Expr<'_>) -> bool {
46+
if let ExprKind::Path(QPath::Resolved(_, p)) = e.kind {
47+
if p.segments.len() != 1 {
48+
return false;
49+
}
50+
if p.segments[0].ident.name == rustc_span::symbol::kw::SelfUpper {
51+
return true;
52+
}
53+
}
54+
false
55+
}
56+
57+
impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
58+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
59+
if_chain! {
60+
if let ItemKind::Impl(Impl {
61+
of_trait: Some(ref trait_ref),
62+
items,
63+
..
64+
}) = item.kind;
65+
if let attrs = cx.tcx.hir().attrs(item.hir_id());
66+
if !is_automatically_derived(attrs);
67+
if !in_macro(item.span);
68+
if let Some(def_id) = trait_ref.trait_def_id();
69+
if cx.tcx.is_diagnostic_item(rustc_span::sym::Default, def_id);
70+
if let impl_item_hir = items[0].id.hir_id();
71+
if let Some(Node::ImplItem(impl_item)) = cx.tcx.hir().find(impl_item_hir);
72+
if let ImplItemKind::Fn(_, b) = &impl_item.kind;
73+
if let Some(Node::Expr(func_expr)) = cx.tcx.hir().find(b.hir_id);
74+
then {
75+
let should_emit = match remove_blocks(func_expr).kind {
76+
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
77+
ExprKind::Call(callee, args)
78+
if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),
79+
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
80+
_ => false,
81+
};
82+
if should_emit {
83+
span_lint(cx, DERIVABLE_IMPLS, item.span, "derivable impl of trait Default:");
84+
}
85+
}
86+
}
87+
}
88+
}

‎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;
@@ -588,6 +589,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
588589
default::FIELD_REASSIGN_WITH_DEFAULT,
589590
default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK,
590591
dereference::EXPLICIT_DEREF_METHODS,
592+
derivable_impls::DERIVABLE_IMPLS,
591593
derive::DERIVE_HASH_XOR_EQ,
592594
derive::DERIVE_ORD_XOR_PARTIAL_ORD,
593595
derive::EXPL_IMPL_CLONE_ON_COPY,
@@ -1203,6 +1205,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12031205
LintId::of(copies::IFS_SAME_COND),
12041206
LintId::of(copies::IF_SAME_THEN_ELSE),
12051207
LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
1208+
LintId::of(derivable_impls::DERIVABLE_IMPLS),
12061209
LintId::of(derive::DERIVE_HASH_XOR_EQ),
12071210
LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD),
12081211
LintId::of(doc::MISSING_SAFETY_DOC),
@@ -1588,6 +1591,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15881591
LintId::of(booleans::NONMINIMAL_BOOL),
15891592
LintId::of(casts::CHAR_LIT_AS_U8),
15901593
LintId::of(casts::UNNECESSARY_CAST),
1594+
LintId::of(derivable_impls::DERIVABLE_IMPLS),
15911595
LintId::of(double_comparison::DOUBLE_COMPARISONS),
15921596
LintId::of(double_parens::DOUBLE_PARENS),
15931597
LintId::of(duration_subsec::DURATION_SUBSEC),
@@ -1937,6 +1941,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
19371941
store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented);
19381942
store.register_late_pass(|| box strings::StringLitAsBytes);
19391943
store.register_late_pass(|| box derive::Derive);
1944+
store.register_late_pass(|| box derivable_impls::DerivableImpls);
19401945
store.register_late_pass(|| box get_last_with_len::GetLastWithLen);
19411946
store.register_late_pass(|| box drop_forget_ref::DropForgetRef);
19421947
store.register_late_pass(|| box empty_enum::EmptyEnum);

‎clippy_lints/src/mem_replace.rs

+27-56
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_equivalent, 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,36 @@ 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-
}
196+
fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
197+
// disable lint for primitives
198+
if let ExprKind::Lit(_) = src.kind {
199+
return;
200+
}
201+
// disable lint for Option since it is covered in another lint
202+
if let ExprKind::Path(q) = &src.kind {
203+
if is_lang_ctor(cx, q, OptionNone) {
204+
return;
221205
}
222206
}
223-
false
224-
}
207+
if is_default_equivalent(cx, src) && !in_external_macro(cx.tcx.sess, expr_span) {
208+
span_lint_and_then(
209+
cx,
210+
MEM_REPLACE_WITH_DEFAULT,
211+
expr_span,
212+
"replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`",
213+
|diag| {
214+
if !in_macro(expr_span) {
215+
let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, ""));
225216

226-
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-
}
217+
diag.span_suggestion(
218+
expr_span,
219+
"consider using",
220+
suggestion,
221+
Applicability::MachineApplicable,
222+
);
252223
}
253-
);
254-
}
224+
},
225+
);
255226
}
256227
}
257228

‎clippy_utils/src/lib.rs

+60-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ use rustc_hir::def::{DefKind, Res};
7070
use rustc_hir::def_id::DefId;
7171
use rustc_hir::hir_id::{HirIdMap, HirIdSet};
7272
use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor};
73-
use rustc_hir::LangItem::{ResultErr, ResultOk};
73+
use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk};
7474
use rustc_hir::{
7575
def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl,
7676
ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat,
@@ -630,6 +630,65 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
630630
false
631631
}
632632

633+
/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent"
634+
/// constructor from the std library
635+
fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool {
636+
let std_types_symbols = &[
637+
sym::string_type,
638+
sym::vec_type,
639+
sym::vecdeque_type,
640+
sym::LinkedList,
641+
sym::hashmap_type,
642+
sym::BTreeMap,
643+
sym::hashset_type,
644+
sym::BTreeSet,
645+
sym::BinaryHeap,
646+
];
647+
648+
if let QPath::TypeRelative(_, method) = path {
649+
if method.ident.name == sym::new {
650+
if let Some(impl_did) = cx.tcx.impl_of_method(def_id) {
651+
if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() {
652+
return std_types_symbols
653+
.iter()
654+
.any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did));
655+
}
656+
}
657+
}
658+
}
659+
false
660+
}
661+
662+
/// Returns true if the expr is equal to `Default::default()` of it's type when evaluated.
663+
/// It doesn't cover all cases, for example indirect function calls (some of std
664+
/// functions are supported) but it is the best we have.
665+
pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
666+
match &e.kind {
667+
ExprKind::Lit(lit) => match lit.node {
668+
LitKind::Bool(false) | LitKind::Int(0, _) => true,
669+
LitKind::Str(s, _) => s.is_empty(),
670+
_ => false,
671+
},
672+
ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default_equivalent(cx, x)),
673+
ExprKind::Repeat(x, _) => is_default_equivalent(cx, x),
674+
ExprKind::Call(repl_func, _) => if_chain! {
675+
if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
676+
if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
677+
if is_diag_trait_item(cx, repl_def_id, sym::Default)
678+
|| is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
679+
then {
680+
true
681+
}
682+
else {
683+
false
684+
}
685+
},
686+
ExprKind::Path(qpath) => is_lang_ctor(cx, qpath, OptionNone),
687+
ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])),
688+
_ => false,
689+
}
690+
}
691+
633692
/// Checks if the top level expression can be moved into a closure as is.
634693
/// Currently checks for:
635694
/// * Break/Continue outside the given loop HIR ids.

0 commit comments

Comments
 (0)