Skip to content

Commit 96a0663

Browse files
estebankestebank
committed
Introduce default_could_be_derived lint
New lint that detects when an enum with no type paremeters has a manual `Default` impl that uses a single unit variant and suggests deriving instead. ``` error: `impl Default` that could be derived --> $DIR/manual-default-impl-could-be-derived.rs:58:1 | LL | / impl Default for F { LL | | fn default() -> Self { LL | | F::Unit LL | | } LL | | } | |_^ | note: the lint level is defined here --> $DIR/manual-default-impl-could-be-derived.rs:4:9 | LL | #![deny(default_could_be_derived)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: you don't need to manually `impl Default`, you can derive it | LL ~ #[derive(Default)] enum F { LL ~ #[default] Unit, | ``` The rendering of the suggestion doesn't include the deletion, but it is there and gets machine applied.
1 parent dd17de0 commit 96a0663

File tree

5 files changed

+285
-0
lines changed

5 files changed

+285
-0
lines changed

‎compiler/rustc_lint/src/default_could_be_derived.rs

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
use rustc_errors::Applicability;
2+
use rustc_hir as hir;
3+
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
4+
use rustc_session::{declare_lint, declare_lint_pass};
5+
use rustc_span::symbol::{kw, sym};
6+
7+
use crate::{LateContext, LateLintPass};
8+
9+
declare_lint! {
10+
/// The `default_could_be_derived` lint checks for manual `impl` blocks
11+
/// of the `Default` trait that could have been derived.
12+
///
13+
/// ### Example
14+
///
15+
/// ```rust,compile_fail
16+
/// enum Foo {
17+
/// Bar,
18+
/// }
19+
///
20+
/// #[deny(default_could_be_derived)]
21+
/// impl Default for Foo {
22+
/// fn default() -> Foo {
23+
/// Foo::Bar
24+
/// }
25+
/// }
26+
/// ```
27+
///
28+
/// {{produces}}
29+
///
30+
pub DEFAULT_COULD_BE_DERIVED,
31+
Warn,
32+
"detect `Default` impl that could be derived"
33+
}
34+
35+
declare_lint_pass!(DefaultCouldBeDerived => [DEFAULT_COULD_BE_DERIVED]);
36+
37+
impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
38+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
39+
let hir::ItemKind::Impl(data) = item.kind else { return };
40+
let Some(trait_ref) = data.of_trait else { return };
41+
let Res::Def(DefKind::Trait, def_id) = trait_ref.path.res else { return };
42+
if Some(def_id) != cx.tcx.get_diagnostic_item(sym::Default) {
43+
return;
44+
}
45+
if cx.tcx.has_attr(def_id, sym::automatically_derived) {
46+
return;
47+
}
48+
let hir_self_ty = data.self_ty;
49+
let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = hir_self_ty.kind else { return };
50+
let Res::Def(_, type_def_id) = path.res else { return };
51+
let generics = cx.tcx.generics_of(type_def_id);
52+
if !generics.own_params.is_empty() {
53+
return;
54+
}
55+
// We have a manual `impl Default for Ty {}` item, where `Ty` has no type parameters.
56+
57+
let hir = cx.tcx.hir();
58+
for assoc in data.items {
59+
let hir::AssocItemKind::Fn { has_self: false } = assoc.kind else { continue };
60+
if assoc.ident.name != kw::Default {
61+
continue;
62+
}
63+
let assoc = hir.impl_item(assoc.id);
64+
let hir::ImplItemKind::Fn(_ty, body) = assoc.kind else { continue };
65+
let body = hir.body(body);
66+
let hir::ExprKind::Block(hir::Block { stmts: [], expr: Some(expr), .. }, None) =
67+
body.value.kind
68+
else {
69+
continue;
70+
};
71+
72+
match expr.kind {
73+
hir::ExprKind::Path(hir::QPath::Resolved(_, path))
74+
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), def_id) =
75+
path.res =>
76+
{
77+
// We have a unit variant as the default of an enum in a manual impl.
78+
//
79+
// enum Foo {
80+
// Bar,
81+
// }
82+
//
83+
// impl Default for Foo {
84+
// fn default() -> Foo {
85+
// Foo::Bar
86+
// }
87+
// }
88+
//
89+
// We suggest
90+
//
91+
// #[derive(Default)] enum Foo {
92+
// #[default] Bar,
93+
// }
94+
cx.tcx.node_span_lint(
95+
DEFAULT_COULD_BE_DERIVED,
96+
item.hir_id(),
97+
item.span,
98+
|diag| {
99+
diag.primary_message("`impl Default` that could be derived");
100+
diag.multipart_suggestion_verbose(
101+
"you don't need to manually `impl Default`, you can derive it",
102+
vec![
103+
(
104+
cx.tcx.def_span(type_def_id).shrink_to_lo(),
105+
"#[derive(Default)] ".to_string(),
106+
),
107+
(
108+
cx.tcx.def_span(def_id).shrink_to_lo(),
109+
"#[default] ".to_string(),
110+
),
111+
(item.span, String::new()),
112+
],
113+
Applicability::MachineApplicable,
114+
);
115+
},
116+
);
117+
}
118+
_ => {}
119+
}
120+
}
121+
}
122+
}

‎compiler/rustc_lint/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ mod async_fn_in_trait;
4141
pub mod builtin;
4242
mod context;
4343
mod dangling;
44+
mod default_could_be_derived;
4445
mod deref_into_dyn_supertrait;
4546
mod drop_forget_useless;
4647
mod early;
@@ -85,6 +86,7 @@ use async_closures::AsyncClosureUsage;
8586
use async_fn_in_trait::AsyncFnInTrait;
8687
use builtin::*;
8788
use dangling::*;
89+
use default_could_be_derived::DefaultCouldBeDerived;
8890
use deref_into_dyn_supertrait::*;
8991
use drop_forget_useless::*;
9092
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
@@ -189,6 +191,7 @@ late_lint_methods!(
189191
BuiltinCombinedModuleLateLintPass,
190192
[
191193
ForLoopsOverFallibles: ForLoopsOverFallibles,
194+
DefaultCouldBeDerived: DefaultCouldBeDerived,
192195
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
193196
DropForgetUseless: DropForgetUseless,
194197
ImproperCTypesDeclarations: ImproperCTypesDeclarations,

‎tests/ui/structs/manual-default-impl-could-be-derived.fixed

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Warn when we encounter a manual `Default` impl that could be derived.
2+
//@ run-rustfix
3+
#![allow(dead_code)]
4+
#![deny(default_could_be_derived)]
5+
6+
// #[derive(Debug)]
7+
// struct A;
8+
//
9+
// impl Default for A {
10+
// fn default() -> Self { A }
11+
// }
12+
//
13+
// #[derive(Debug)]
14+
// struct B(Option<i32>);
15+
//
16+
// impl Default for B {
17+
// fn default() -> Self { B(Default::default()) }
18+
// }
19+
//
20+
// #[derive(Debug)]
21+
// struct C(Option<i32>);
22+
//
23+
// impl Default for C {
24+
// fn default() -> Self { C(None) }
25+
// }
26+
//
27+
// #[derive(Debug)]
28+
// struct D {
29+
// x: Option<i32>,
30+
// }
31+
//
32+
// impl Default for D {
33+
// fn default() -> Self {
34+
// D {
35+
// x: Default::default(),
36+
// }
37+
// }
38+
// }
39+
//
40+
// #[derive(Debug)]
41+
// struct E {
42+
// x: Option<i32>,
43+
// }
44+
//
45+
// impl Default for E {
46+
// fn default() -> Self {
47+
// E {
48+
// x: None,
49+
// }
50+
// }
51+
// }
52+
53+
#[derive(Default)] enum F {
54+
#[default] Unit,
55+
Tuple(i32),
56+
}
57+
58+
59+
fn main() {
60+
// let _ = A::default();
61+
// let _ = B::default();
62+
// let _ = C::default();
63+
// let _ = D::default();
64+
// let _ = E::default();
65+
let _ = F::default();
66+
}

‎tests/ui/structs/manual-default-impl-could-be-derived.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Warn when we encounter a manual `Default` impl that could be derived.
2+
//@ run-rustfix
3+
#![allow(dead_code)]
4+
#![deny(default_could_be_derived)]
5+
6+
// #[derive(Debug)]
7+
// struct A;
8+
//
9+
// impl Default for A {
10+
// fn default() -> Self { A }
11+
// }
12+
//
13+
// #[derive(Debug)]
14+
// struct B(Option<i32>);
15+
//
16+
// impl Default for B {
17+
// fn default() -> Self { B(Default::default()) }
18+
// }
19+
//
20+
// #[derive(Debug)]
21+
// struct C(Option<i32>);
22+
//
23+
// impl Default for C {
24+
// fn default() -> Self { C(None) }
25+
// }
26+
//
27+
// #[derive(Debug)]
28+
// struct D {
29+
// x: Option<i32>,
30+
// }
31+
//
32+
// impl Default for D {
33+
// fn default() -> Self {
34+
// D {
35+
// x: Default::default(),
36+
// }
37+
// }
38+
// }
39+
//
40+
// #[derive(Debug)]
41+
// struct E {
42+
// x: Option<i32>,
43+
// }
44+
//
45+
// impl Default for E {
46+
// fn default() -> Self {
47+
// E {
48+
// x: None,
49+
// }
50+
// }
51+
// }
52+
53+
enum F {
54+
Unit,
55+
Tuple(i32),
56+
}
57+
58+
impl Default for F { //~ ERROR
59+
fn default() -> Self {
60+
F::Unit
61+
}
62+
}
63+
64+
fn main() {
65+
// let _ = A::default();
66+
// let _ = B::default();
67+
// let _ = C::default();
68+
// let _ = D::default();
69+
// let _ = E::default();
70+
let _ = F::default();
71+
}

‎tests/ui/structs/manual-default-impl-could-be-derived.stderr

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
error: `impl Default` that could be derived
2+
--> $DIR/manual-default-impl-could-be-derived.rs:58:1
3+
|
4+
LL | / impl Default for F {
5+
LL | | fn default() -> Self {
6+
LL | | F::Unit
7+
LL | | }
8+
LL | | }
9+
| |_^
10+
|
11+
note: the lint level is defined here
12+
--> $DIR/manual-default-impl-could-be-derived.rs:4:9
13+
|
14+
LL | #![deny(default_could_be_derived)]
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^
16+
help: you don't need to manually `impl Default`, you can derive it
17+
|
18+
LL ~ #[derive(Default)] enum F {
19+
LL ~ #[default] Unit,
20+
|
21+
22+
error: aborting due to 1 previous error
23+

0 commit comments

Comments
 (0)