Skip to content

Commit 5564158

Browse files
borsbors
committed
Auto merge of #9429 - kraktus:deriv_ma, r=xFrednet
Make `derivable_impls` machine applicable changelog: [`derivable_impls`]: Now machine applicable
2 parents 2e55b42 + 6f13203 commit 5564158

File tree

4 files changed

+278
-19
lines changed

4 files changed

+278
-19
lines changed

‎clippy_lints/src/derivable_impls.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
use clippy_utils::diagnostics::span_lint_and_help;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::{is_default_equivalent, peel_blocks};
3+
use rustc_errors::Applicability;
34
use rustc_hir::{
45
def::{DefKind, Res},
56
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
@@ -100,15 +101,28 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
100101
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
101102
_ => false,
102103
};
104+
103105
if should_emit {
104-
let path_string = cx.tcx.def_path_str(adt_def.did());
105-
span_lint_and_help(
106+
let struct_span = cx.tcx.def_span(adt_def.did());
107+
span_lint_and_then(
106108
cx,
107109
DERIVABLE_IMPLS,
108110
item.span,
109111
"this `impl` can be derived",
110-
None,
111-
&format!("try annotating `{}` with `#[derive(Default)]`", path_string),
112+
|diag| {
113+
diag.span_suggestion_hidden(
114+
item.span,
115+
"remove the manual implementation...",
116+
String::new(),
117+
Applicability::MachineApplicable
118+
);
119+
diag.span_suggestion(
120+
struct_span.shrink_to_lo(),
121+
"...and instead derive it",
122+
"#[derive(Default)]\n".to_string(),
123+
Applicability::MachineApplicable
124+
);
125+
}
112126
);
113127
}
114128
}

‎tests/ui/derivable_impls.fixed

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
// run-rustfix
2+
3+
#![allow(dead_code)]
4+
5+
use std::collections::HashMap;
6+
7+
#[derive(Default)]
8+
struct FooDefault<'a> {
9+
a: bool,
10+
b: i32,
11+
c: u64,
12+
d: Vec<i32>,
13+
e: FooND1,
14+
f: FooND2,
15+
g: HashMap<i32, i32>,
16+
h: (i32, Vec<i32>),
17+
i: [Vec<i32>; 3],
18+
j: [i32; 5],
19+
k: Option<i32>,
20+
l: &'a [i32],
21+
}
22+
23+
24+
25+
#[derive(Default)]
26+
struct TupleDefault(bool, i32, u64);
27+
28+
29+
30+
struct FooND1 {
31+
a: bool,
32+
}
33+
34+
impl std::default::Default for FooND1 {
35+
fn default() -> Self {
36+
Self { a: true }
37+
}
38+
}
39+
40+
struct FooND2 {
41+
a: i32,
42+
}
43+
44+
impl std::default::Default for FooND2 {
45+
fn default() -> Self {
46+
Self { a: 5 }
47+
}
48+
}
49+
50+
struct FooNDNew {
51+
a: bool,
52+
}
53+
54+
impl FooNDNew {
55+
fn new() -> Self {
56+
Self { a: true }
57+
}
58+
}
59+
60+
impl Default for FooNDNew {
61+
fn default() -> Self {
62+
Self::new()
63+
}
64+
}
65+
66+
struct FooNDVec(Vec<i32>);
67+
68+
impl Default for FooNDVec {
69+
fn default() -> Self {
70+
Self(vec![5, 12])
71+
}
72+
}
73+
74+
#[derive(Default)]
75+
struct StrDefault<'a>(&'a str);
76+
77+
78+
79+
#[derive(Default)]
80+
struct AlreadyDerived(i32, bool);
81+
82+
macro_rules! mac {
83+
() => {
84+
0
85+
};
86+
($e:expr) => {
87+
struct X(u32);
88+
impl Default for X {
89+
fn default() -> Self {
90+
Self($e)
91+
}
92+
}
93+
};
94+
}
95+
96+
mac!(0);
97+
98+
#[derive(Default)]
99+
struct Y(u32);
100+
101+
102+
struct RustIssue26925<T> {
103+
a: Option<T>,
104+
}
105+
106+
// We should watch out for cases where a manual impl is needed because a
107+
// derive adds different type bounds (https://github.com/rust-lang/rust/issues/26925).
108+
// For example, a struct with Option<T> does not require T: Default, but a derive adds
109+
// that type bound anyways. So until #26925 get fixed we should disable lint
110+
// for the following case
111+
impl<T> Default for RustIssue26925<T> {
112+
fn default() -> Self {
113+
Self { a: None }
114+
}
115+
}
116+
117+
struct SpecializedImpl<A, B> {
118+
a: A,
119+
b: B,
120+
}
121+
122+
impl<T: Default> Default for SpecializedImpl<T, T> {
123+
fn default() -> Self {
124+
Self {
125+
a: T::default(),
126+
b: T::default(),
127+
}
128+
}
129+
}
130+
131+
#[derive(Default)]
132+
struct WithoutSelfCurly {
133+
a: bool,
134+
}
135+
136+
137+
138+
#[derive(Default)]
139+
struct WithoutSelfParan(bool);
140+
141+
142+
143+
// https://github.com/rust-lang/rust-clippy/issues/7655
144+
145+
pub struct SpecializedImpl2<T> {
146+
v: Vec<T>,
147+
}
148+
149+
impl Default for SpecializedImpl2<String> {
150+
fn default() -> Self {
151+
Self { v: Vec::new() }
152+
}
153+
}
154+
155+
// https://github.com/rust-lang/rust-clippy/issues/7654
156+
157+
pub struct Color {
158+
pub r: u8,
159+
pub g: u8,
160+
pub b: u8,
161+
}
162+
163+
/// `#000000`
164+
impl Default for Color {
165+
fn default() -> Self {
166+
Color { r: 0, g: 0, b: 0 }
167+
}
168+
}
169+
170+
pub struct Color2 {
171+
pub r: u8,
172+
pub g: u8,
173+
pub b: u8,
174+
}
175+
176+
impl Default for Color2 {
177+
/// `#000000`
178+
fn default() -> Self {
179+
Self { r: 0, g: 0, b: 0 }
180+
}
181+
}
182+
183+
#[derive(Default)]
184+
pub struct RepeatDefault1 {
185+
a: [i8; 32],
186+
}
187+
188+
189+
190+
pub struct RepeatDefault2 {
191+
a: [i8; 33],
192+
}
193+
194+
impl Default for RepeatDefault2 {
195+
fn default() -> Self {
196+
RepeatDefault2 { a: [0; 33] }
197+
}
198+
}
199+
200+
// https://github.com/rust-lang/rust-clippy/issues/7753
201+
202+
pub enum IntOrString {
203+
Int(i32),
204+
String(String),
205+
}
206+
207+
impl Default for IntOrString {
208+
fn default() -> Self {
209+
IntOrString::Int(0)
210+
}
211+
}
212+
213+
fn main() {}

‎tests/ui/derivable_impls.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
// run-rustfix
2+
3+
#![allow(dead_code)]
4+
15
use std::collections::HashMap;
26

37
struct FooDefault<'a> {

‎tests/ui/derivable_impls.stderr

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `impl` can be derived
2-
--> $DIR/derivable_impls.rs:18:1
2+
--> $DIR/derivable_impls.rs:22:1
33
|
44
LL | / impl std::default::Default for FooDefault<'_> {
55
LL | | fn default() -> Self {
@@ -11,10 +11,14 @@ LL | | }
1111
| |_^
1212
|
1313
= note: `-D clippy::derivable-impls` implied by `-D warnings`
14-
= help: try annotating `FooDefault` with `#[derive(Default)]`
14+
= help: remove the manual implementation...
15+
help: ...and instead derive it
16+
|
17+
LL | #[derive(Default)]
18+
|
1519

1620
error: this `impl` can be derived
17-
--> $DIR/derivable_impls.rs:39:1
21+
--> $DIR/derivable_impls.rs:43:1
1822
|
1923
LL | / impl std::default::Default for TupleDefault {
2024
LL | | fn default() -> Self {
@@ -23,10 +27,14 @@ LL | | }
2327
LL | | }
2428
| |_^
2529
|
26-
= help: try annotating `TupleDefault` with `#[derive(Default)]`
30+
= help: remove the manual implementation...
31+
help: ...and instead derive it
32+
|
33+
LL | #[derive(Default)]
34+
|
2735

2836
error: this `impl` can be derived
29-
--> $DIR/derivable_impls.rs:91:1
37+
--> $DIR/derivable_impls.rs:95:1
3038
|
3139
LL | / impl Default for StrDefault<'_> {
3240
LL | | fn default() -> Self {
@@ -35,10 +43,14 @@ LL | | }
3543
LL | | }
3644
| |_^
3745
|
38-
= help: try annotating `StrDefault` with `#[derive(Default)]`
46+
= help: remove the manual implementation...
47+
help: ...and instead derive it
48+
|
49+
LL | #[derive(Default)]
50+
|
3951

4052
error: this `impl` can be derived
41-
--> $DIR/derivable_impls.rs:117:1
53+
--> $DIR/derivable_impls.rs:121:1
4254
|
4355
LL | / impl Default for Y {
4456
LL | | fn default() -> Self {
@@ -47,10 +59,14 @@ LL | | }
4759
LL | | }
4860
| |_^
4961
|
50-
= help: try annotating `Y` with `#[derive(Default)]`
62+
= help: remove the manual implementation...
63+
help: ...and instead derive it
64+
|
65+
LL | #[derive(Default)]
66+
|
5167

5268
error: this `impl` can be derived
53-
--> $DIR/derivable_impls.rs:156:1
69+
--> $DIR/derivable_impls.rs:160:1
5470
|
5571
LL | / impl Default for WithoutSelfCurly {
5672
LL | | fn default() -> Self {
@@ -59,10 +75,14 @@ LL | | }
5975
LL | | }
6076
| |_^
6177
|
62-
= help: try annotating `WithoutSelfCurly` with `#[derive(Default)]`
78+
= help: remove the manual implementation...
79+
help: ...and instead derive it
80+
|
81+
LL | #[derive(Default)]
82+
|
6383

6484
error: this `impl` can be derived
65-
--> $DIR/derivable_impls.rs:164:1
85+
--> $DIR/derivable_impls.rs:168:1
6686
|
6787
LL | / impl Default for WithoutSelfParan {
6888
LL | | fn default() -> Self {
@@ -71,10 +91,14 @@ LL | | }
7191
LL | | }
7292
| |_^
7393
|
74-
= help: try annotating `WithoutSelfParan` with `#[derive(Default)]`
94+
= help: remove the manual implementation...
95+
help: ...and instead derive it
96+
|
97+
LL | #[derive(Default)]
98+
|
7599

76100
error: this `impl` can be derived
77-
--> $DIR/derivable_impls.rs:214:1
101+
--> $DIR/derivable_impls.rs:218:1
78102
|
79103
LL | / impl Default for RepeatDefault1 {
80104
LL | | fn default() -> Self {
@@ -83,7 +107,11 @@ LL | | }
83107
LL | | }
84108
| |_^
85109
|
86-
= help: try annotating `RepeatDefault1` with `#[derive(Default)]`
110+
= help: remove the manual implementation...
111+
help: ...and instead derive it
112+
|
113+
LL | #[derive(Default)]
114+
|
87115

88116
error: aborting due to 7 previous errors
89117

0 commit comments

Comments
 (0)