Skip to content

Commit b36567a

Browse files
committed
Implement default_mismatches_new lint
### What it does 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. ### Why is this bad? 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. ### Example ```rust #[derive(Default)] 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 #[derive(Default)] 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 8178530 commit b36567a

12 files changed

+710
-113
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5549,6 +5549,7 @@ Released 2018-09-13
55495549
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
55505550
[`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs
55515551
[`default_instead_of_iter_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_instead_of_iter_empty
5552+
[`default_mismatches_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_mismatches_new
55525553
[`default_numeric_fallback`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_numeric_fallback
55535554
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
55545555
[`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: &[&crate::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_without_default::DEFAULT_MISMATCHES_NEW_INFO,
575576
crate::new_without_default::NEW_WITHOUT_DEFAULT_INFO,
576577
crate::no_effect::NO_EFFECT_INFO,
577578
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
@@ -46,30 +46,23 @@ declare_clippy_lint! {
4646
}
4747
declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]);
4848

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

clippy_lints/src/new_without_default.rs

Lines changed: 269 additions & 90 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)