Skip to content

Commit 529bb5f

Browse files
Correctly handle bracketed type in default_constructed_unit_struct (#14367)
There were two bugs here. Let's assume `T` is a singleton type implementing `Default` and that `f()` takes a `T`: - `f(<T>::default())` cannot be replaced by `f(<T)` as it was (incorrect spans – this is tricky because the type relative path uses a base span covering only `T`, not `<T>`) (third commit) - The argument of `f(<_>::default())` is inferred correctly, but cannot be replaced by `<_>` or `_`, as this cannot be used to infer an instance of a singleton type (first commit). The second commit offers better error messages by pointing at the whole expression. Fix #12654 changelog: [`default_constructed_unit_struct`]: do not suggest incorrect fix when using a type surrounded by brackets
2 parents e0e2a93 + 0aa0d07 commit 529bb5f

4 files changed

+83
-19
lines changed

Diff for: clippy_lints/src/default_constructed_unit_structs.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::is_ty_alias;
3+
use clippy_utils::source::SpanRangeExt as _;
34
use hir::ExprKind;
45
use hir::def::Res;
56
use rustc_errors::Applicability;
@@ -70,15 +71,26 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs {
7071
&& let var @ ty::VariantDef { ctor: Some((hir::def::CtorKind::Const, _)), .. } = def.non_enum_variant()
7172
&& !var.is_field_list_non_exhaustive()
7273
&& !expr.span.from_expansion() && !qpath.span().from_expansion()
74+
// do not suggest replacing an expression by a type name with placeholders
75+
&& !base.is_suggestable_infer_ty()
7376
{
74-
span_lint_and_sugg(
77+
let mut removals = vec![(expr.span.with_lo(qpath.qself_span().hi()), String::new())];
78+
if expr.span.with_source_text(cx, |s| s.starts_with('<')) == Some(true) {
79+
// Remove `<`, '>` has already been removed by the existing removal expression.
80+
removals.push((expr.span.with_hi(qpath.qself_span().lo()), String::new()));
81+
}
82+
span_lint_and_then(
7583
cx,
7684
DEFAULT_CONSTRUCTED_UNIT_STRUCTS,
77-
expr.span.with_lo(qpath.qself_span().hi()),
85+
expr.span,
7886
"use of `default` to create a unit struct",
79-
"remove this call to `default`",
80-
String::new(),
81-
Applicability::MachineApplicable,
87+
|diag| {
88+
diag.multipart_suggestion(
89+
"remove this call to `default`",
90+
removals,
91+
Applicability::MachineApplicable,
92+
);
93+
},
8294
);
8395
}
8496
}

Diff for: tests/ui/default_constructed_unit_structs.fixed

+14
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,17 @@ fn main() {
161161

162162
let _ = <struct_from_macro!()>::default();
163163
}
164+
165+
fn issue12654() {
166+
#[derive(Default)]
167+
struct G;
168+
169+
fn f(_g: G) {}
170+
171+
f(<_>::default());
172+
f(G);
173+
//~^ default_constructed_unit_structs
174+
175+
// No lint because `as Default` hides the singleton
176+
f(<G as Default>::default());
177+
}

Diff for: tests/ui/default_constructed_unit_structs.rs

+14
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,17 @@ fn main() {
161161

162162
let _ = <struct_from_macro!()>::default();
163163
}
164+
165+
fn issue12654() {
166+
#[derive(Default)]
167+
struct G;
168+
169+
fn f(_g: G) {}
170+
171+
f(<_>::default());
172+
f(<G>::default());
173+
//~^ default_constructed_unit_structs
174+
175+
// No lint because `as Default` hides the singleton
176+
f(<G as Default>::default());
177+
}

Diff for: tests/ui/default_constructed_unit_structs.stderr

+37-13
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,65 @@
11
error: use of `default` to create a unit struct
2-
--> tests/ui/default_constructed_unit_structs.rs:11:13
2+
--> tests/ui/default_constructed_unit_structs.rs:11:9
33
|
44
LL | Self::default()
5-
| ^^^^^^^^^^^ help: remove this call to `default`
5+
| ^^^^-----------
6+
| |
7+
| help: remove this call to `default`
68
|
79
= note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings`
810
= help: to override `-D warnings` add `#[allow(clippy::default_constructed_unit_structs)]`
911

1012
error: use of `default` to create a unit struct
11-
--> tests/ui/default_constructed_unit_structs.rs:54:31
13+
--> tests/ui/default_constructed_unit_structs.rs:54:20
1214
|
1315
LL | inner: PhantomData::default(),
14-
| ^^^^^^^^^^^ help: remove this call to `default`
16+
| ^^^^^^^^^^^-----------
17+
| |
18+
| help: remove this call to `default`
1519

1620
error: use of `default` to create a unit struct
17-
--> tests/ui/default_constructed_unit_structs.rs:128:33
21+
--> tests/ui/default_constructed_unit_structs.rs:128:13
1822
|
1923
LL | let _ = PhantomData::<usize>::default();
20-
| ^^^^^^^^^^^ help: remove this call to `default`
24+
| ^^^^^^^^^^^^^^^^^^^^-----------
25+
| |
26+
| help: remove this call to `default`
2127

2228
error: use of `default` to create a unit struct
23-
--> tests/ui/default_constructed_unit_structs.rs:130:42
29+
--> tests/ui/default_constructed_unit_structs.rs:130:31
2430
|
2531
LL | let _: PhantomData<i32> = PhantomData::default();
26-
| ^^^^^^^^^^^ help: remove this call to `default`
32+
| ^^^^^^^^^^^-----------
33+
| |
34+
| help: remove this call to `default`
2735

2836
error: use of `default` to create a unit struct
29-
--> tests/ui/default_constructed_unit_structs.rs:132:55
37+
--> tests/ui/default_constructed_unit_structs.rs:132:31
3038
|
3139
LL | let _: PhantomData<i32> = std::marker::PhantomData::default();
32-
| ^^^^^^^^^^^ help: remove this call to `default`
40+
| ^^^^^^^^^^^^^^^^^^^^^^^^-----------
41+
| |
42+
| help: remove this call to `default`
3343

3444
error: use of `default` to create a unit struct
35-
--> tests/ui/default_constructed_unit_structs.rs:134:23
45+
--> tests/ui/default_constructed_unit_structs.rs:134:13
3646
|
3747
LL | let _ = UnitStruct::default();
38-
| ^^^^^^^^^^^ help: remove this call to `default`
48+
| ^^^^^^^^^^-----------
49+
| |
50+
| help: remove this call to `default`
3951

40-
error: aborting due to 6 previous errors
52+
error: use of `default` to create a unit struct
53+
--> tests/ui/default_constructed_unit_structs.rs:172:7
54+
|
55+
LL | f(<G>::default());
56+
| ^^^^^^^^^^^^^^
57+
|
58+
help: remove this call to `default`
59+
|
60+
LL - f(<G>::default());
61+
LL + f(G);
62+
|
63+
64+
error: aborting due to 7 previous errors
4165

0 commit comments

Comments
 (0)