Skip to content

Commit 3e33c84

Browse files
nyurikada4a
authored andcommitted
Implement default_mismatches_new lint
If a type has an auto-derived `Default` trait and a `fn new() -> Self`, this lint checks if the `new()` method performs custom logic rather than simply calling the `default()` method. Users expect the `new()` method to be equivalent to `default()`, so if the `Default` trait is auto-derived, the `new()` method should not perform custom logic. Otherwise, there is a risk of different behavior between the two instantiation methods. ```rust struct MyStruct(i32); impl MyStruct { fn new() -> Self { Self(42) } } ``` Users are unlikely to notice that `MyStruct::new()` and `MyStruct::default()` would produce different results. The `new()` method should use auto-derived `default()` instead to be consistent: ```rust struct MyStruct(i32); impl MyStruct { fn new() -> Self { Self::default() } } ``` Alternatively, if the `new()` method requires a non-default initialization, implement a custom `Default`. This also allows you to mark the `new()` implementation as `const`: ```rust struct MyStruct(i32); impl MyStruct { const fn new() -> Self { Self(42) } } impl Default for MyStruct { fn default() -> Self { Self::new() } } ```
1 parent 67aed03 commit 3e33c84

12 files changed

Lines changed: 738 additions & 109 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6727,6 +6727,7 @@ Released 2018-09-13
67276727
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
67286728
[`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs
67296729
[`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty
6730+
[`default_mismatches_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_mismatches_new
67306731
[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback
67316732
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
67326733
[`default_union_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_union_representation

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
572572
crate::needless_update::NEEDLESS_UPDATE_INFO,
573573
crate::neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD_INFO,
574574
crate::neg_multiply::NEG_MULTIPLY_INFO,
575+
crate::new_vs_default::DEFAULT_MISMATCHES_NEW_INFO,
575576
crate::new_vs_default::NEW_WITHOUT_DEFAULT_INFO,
576577
crate::no_effect::NO_EFFECT_INFO,
577578
crate::no_effect::NO_EFFECT_UNDERSCORE_BINDING_INFO,

clippy_lints/src/new_vs_default.rs

Lines changed: 252 additions & 85 deletions
Large diffs are not rendered by default.

tests/ui/default_constructed_unit_structs.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::default_mismatches_new)]
12
#![warn(clippy::default_constructed_unit_structs)]
23
use std::marker::PhantomData;
34

tests/ui/default_constructed_unit_structs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![allow(clippy::default_mismatches_new)]
12
#![warn(clippy::default_constructed_unit_structs)]
23
use std::marker::PhantomData;
34

tests/ui/default_constructed_unit_structs.stderr

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: use of `default` to create a unit struct
2-
--> tests/ui/default_constructed_unit_structs.rs:10:9
2+
--> tests/ui/default_constructed_unit_structs.rs:11:9
33
|
44
LL | Self::default()
55
| ^^^^^^^^^^^^^^^
@@ -13,7 +13,7 @@ LL + Self
1313
|
1414

1515
error: use of `default` to create a unit struct
16-
--> tests/ui/default_constructed_unit_structs.rs:53:20
16+
--> tests/ui/default_constructed_unit_structs.rs:54:20
1717
|
1818
LL | inner: PhantomData::default(),
1919
| ^^^^^^^^^^^^^^^^^^^^^^
@@ -25,7 +25,7 @@ LL + inner: PhantomData,
2525
|
2626

2727
error: use of `default` to create a unit struct
28-
--> tests/ui/default_constructed_unit_structs.rs:127:13
28+
--> tests/ui/default_constructed_unit_structs.rs:128:13
2929
|
3030
LL | let _ = PhantomData::<usize>::default();
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -37,7 +37,7 @@ LL + let _ = PhantomData::<usize>;
3737
|
3838

3939
error: use of `default` to create a unit struct
40-
--> tests/ui/default_constructed_unit_structs.rs:129:31
40+
--> tests/ui/default_constructed_unit_structs.rs:130:31
4141
|
4242
LL | let _: PhantomData<i32> = PhantomData::default();
4343
| ^^^^^^^^^^^^^^^^^^^^^^
@@ -49,7 +49,7 @@ LL + let _: PhantomData<i32> = PhantomData;
4949
|
5050

5151
error: use of `default` to create a unit struct
52-
--> tests/ui/default_constructed_unit_structs.rs:131:31
52+
--> tests/ui/default_constructed_unit_structs.rs:132:31
5353
|
5454
LL | let _: PhantomData<i32> = std::marker::PhantomData::default();
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -61,7 +61,7 @@ LL + let _: PhantomData<i32> = std::marker::PhantomData;
6161
|
6262

6363
error: use of `default` to create a unit struct
64-
--> tests/ui/default_constructed_unit_structs.rs:133:13
64+
--> tests/ui/default_constructed_unit_structs.rs:134:13
6565
|
6666
LL | let _ = UnitStruct::default();
6767
| ^^^^^^^^^^^^^^^^^^^^^
@@ -73,7 +73,7 @@ LL + let _ = UnitStruct;
7373
|
7474

7575
error: use of `default` to create a unit struct
76-
--> tests/ui/default_constructed_unit_structs.rs:171:7
76+
--> tests/ui/default_constructed_unit_structs.rs:172:7
7777
|
7878
LL | f(<G>::default());
7979
| ^^^^^^^^^^^^^^
Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
#![allow(clippy::needless_return, clippy::diverging_sub_expression)]
2+
#![warn(clippy::default_mismatches_new)]
3+
4+
fn main() {}
5+
6+
// Don't warn: Default impl is manual
7+
struct ManualDefault(i32);
8+
impl ManualDefault {
9+
fn new() -> Self {
10+
Self(42)
11+
}
12+
}
13+
impl Default for ManualDefault {
14+
fn default() -> Self {
15+
Self(42)
16+
}
17+
}
18+
19+
// Don't warn: new() calls automatic Default impl
20+
#[derive(Default)]
21+
struct CallToDefaultDefault(i32);
22+
impl CallToDefaultDefault {
23+
fn new() -> Self {
24+
Default::default()
25+
}
26+
}
27+
28+
#[derive(Default)]
29+
struct CallToSelfDefault(i32);
30+
impl CallToSelfDefault {
31+
fn new() -> Self {
32+
Self::default()
33+
}
34+
}
35+
36+
#[derive(Default)]
37+
struct CallToTypeDefault(i32);
38+
impl CallToTypeDefault {
39+
fn new() -> Self {
40+
CallToTypeDefault::default()
41+
}
42+
}
43+
44+
#[derive(Default)]
45+
struct CallToFullTypeDefault(i32);
46+
impl CallToFullTypeDefault {
47+
fn new() -> Self {
48+
crate::CallToFullTypeDefault::default()
49+
}
50+
}
51+
52+
#[derive(Default)]
53+
struct ReturnCallToSelfDefault(i32);
54+
impl ReturnCallToSelfDefault {
55+
fn new() -> Self {
56+
return Self::default();
57+
}
58+
}
59+
60+
// Don't warn: new() returns Result, so it can't return Self::default()
61+
#[derive(Default)]
62+
struct MakeResultSelf(i32);
63+
impl MakeResultSelf {
64+
fn new() -> Result<Self, ()> {
65+
Ok(Self(10))
66+
}
67+
}
68+
69+
// Don't warn: new() takes parameters, so it is semantically different from default()
70+
#[derive(Default)]
71+
struct WithParams(i32);
72+
impl WithParams {
73+
fn new(val: i32) -> Self {
74+
Self(val)
75+
}
76+
}
77+
78+
// Don't warn: new() is async
79+
#[derive(Default)]
80+
struct Async(i32);
81+
impl Async {
82+
async fn new() -> Self {
83+
Self(42)
84+
}
85+
}
86+
87+
#[derive(Default)]
88+
struct DeriveDefault;
89+
impl DeriveDefault {
90+
fn new() -> Self {
91+
// Adding ::default() would cause clippy::default_constructed_unit_structs
92+
Self
93+
}
94+
}
95+
96+
#[derive(Default)]
97+
struct DeriveTypeDefault;
98+
impl DeriveTypeDefault {
99+
fn new() -> Self {
100+
// Adding ::default() would cause clippy::default_constructed_unit_structs
101+
return crate::DeriveTypeDefault;
102+
}
103+
}
104+
105+
// Don't warn: new() is const - user may need const fn and derived Default isn't const (yet)
106+
#[derive(Default)]
107+
struct ConstNew(i32);
108+
impl ConstNew {
109+
const fn new() -> Self {
110+
Self(42)
111+
}
112+
}
113+
114+
//
115+
// Offer suggestions
116+
//
117+
118+
#[derive(Default)]
119+
struct DeriveIntDefault {
120+
value: i32,
121+
}
122+
impl DeriveIntDefault {
123+
fn new() -> Self {
124+
Self::default()
125+
}
126+
}
127+
128+
#[derive(Default)]
129+
struct DeriveTupleDefault(i32);
130+
impl DeriveTupleDefault {
131+
fn new() -> Self {
132+
Self::default()
133+
}
134+
}
135+
136+
#[derive(Default)]
137+
struct NonZeroDeriveDefault(i32);
138+
impl NonZeroDeriveDefault {
139+
fn new() -> Self {
140+
Self::default()
141+
}
142+
}
143+
144+
#[derive(Default)]
145+
struct ExtraBlockDefault(i32);
146+
impl ExtraBlockDefault {
147+
fn new() -> Self {
148+
Self::default()
149+
}
150+
}
151+
152+
#[derive(Default)]
153+
struct ExtraBlockRetDefault(i32);
154+
impl ExtraBlockRetDefault {
155+
fn new() -> Self {
156+
Self::default()
157+
}
158+
}
159+
160+
#[derive(Default)]
161+
struct MultiStatements(i32);
162+
impl MultiStatements {
163+
fn new() -> Self {
164+
Self::default()
165+
}
166+
}
167+
168+
//
169+
// TODO: Fix in the future
170+
//
171+
#[derive(Default)]
172+
struct OptionGeneric<T>(Option<T>);
173+
impl<T> OptionGeneric<T> {
174+
fn new() -> Self {
175+
OptionGeneric(None)
176+
}
177+
}

0 commit comments

Comments
 (0)