Skip to content

Commit 706a961

Browse files
committed
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 b90c80a commit 706a961

12 files changed

+708
-111
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5607,6 +5607,7 @@ Released 2018-09-13
56075607
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
56085608
[`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs
56095609
[`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty
5610+
[`default_mismatches_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_mismatches_new
56105611
[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback
56115612
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
56125613
[`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
@@ -576,6 +576,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
576576
crate::needless_update::NEEDLESS_UPDATE_INFO,
577577
crate::neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD_INFO,
578578
crate::neg_multiply::NEG_MULTIPLY_INFO,
579+
crate::new_without_default::DEFAULT_MISMATCHES_NEW_INFO,
579580
crate::new_without_default::NEW_WITHOUT_DEFAULT_INFO,
580581
crate::no_effect::NO_EFFECT_INFO,
581582
crate::no_effect::NO_EFFECT_UNDERSCORE_BINDING_INFO,

clippy_lints/src/default_constructed_unit_structs.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,30 +47,23 @@ declare_clippy_lint! {
4747
}
4848
declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]);
4949

50-
fn is_alias(ty: hir::Ty<'_>) -> bool {
51-
if let hir::TyKind::Path(ref qpath) = ty.kind {
52-
is_ty_alias(qpath)
53-
} else {
54-
false
55-
}
56-
}
57-
5850
impl LateLintPass<'_> for DefaultConstructedUnitStructs {
5951
fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
6052
if let ExprKind::Call(fn_expr, &[]) = expr.kind
6153
// make sure we have a call to `Default::default`
6254
&& let ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(base, _)) = fn_expr.kind
6355
// make sure this isn't a type alias:
6456
// `<Foo as Bar>::Assoc` cannot be used as a constructor
65-
&& !is_alias(*base)
57+
&& !matches!(base.kind, hir::TyKind::Path(ref qpath) if is_ty_alias(qpath))
6658
&& let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id)
6759
&& cx.tcx.is_diagnostic_item(sym::default_fn, def_id)
6860
// make sure we have a struct with no fields (unit struct)
6961
&& let ty::Adt(def, ..) = cx.typeck_results().expr_ty(expr).kind()
7062
&& def.is_struct()
7163
&& let var @ ty::VariantDef { ctor: Some((hir::def::CtorKind::Const, _)), .. } = def.non_enum_variant()
7264
&& !var.is_field_list_non_exhaustive()
73-
&& !expr.span.from_expansion() && !qpath.span().from_expansion()
65+
&& !expr.span.from_expansion()
66+
&& !qpath.span().from_expansion()
7467
// do not suggest replacing an expression by a type name with placeholders
7568
&& !base.is_suggestable_infer_ty()
7669
{

clippy_lints/src/new_without_default.rs

Lines changed: 267 additions & 88 deletions
Large diffs are not rendered by default.

tests/ui/default_constructed_unit_structs.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(unused)]
1+
#![allow(unused, clippy::default_mismatches_new)]
22
#![warn(clippy::default_constructed_unit_structs)]
33
use std::marker::PhantomData;
44

tests/ui/default_constructed_unit_structs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(unused)]
1+
#![allow(unused, clippy::default_mismatches_new)]
22
#![warn(clippy::default_constructed_unit_structs)]
33
use std::marker::PhantomData;
44

tests/ui/default_mismatches_new.fixed

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

0 commit comments

Comments
 (0)