From 429c09ead27f1715f07808f4b98eabfd40830520 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 6 Mar 2025 22:31:03 +0100 Subject: [PATCH 1/3] Do not suggest replacing an expression by an ambiguous type name An expression such as `<_>::default()` should not be replaced by an ambiguous type name such as `_` even if the inferred type in the original expression is a singleton. --- clippy_lints/src/default_constructed_unit_structs.rs | 2 ++ tests/ui/default_constructed_unit_structs.fixed | 9 +++++++++ tests/ui/default_constructed_unit_structs.rs | 9 +++++++++ 3 files changed, 20 insertions(+) diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index bbd5dc15542d..85a630591711 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -70,6 +70,8 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs { && let var @ ty::VariantDef { ctor: Some((hir::def::CtorKind::Const, _)), .. } = def.non_enum_variant() && !var.is_field_list_non_exhaustive() && !expr.span.from_expansion() && !qpath.span().from_expansion() + // do not suggest replacing an expression by a type name with placeholders + && !base.is_suggestable_infer_ty() { span_lint_and_sugg( cx, diff --git a/tests/ui/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed index fa4d55177823..91a6161f98af 100644 --- a/tests/ui/default_constructed_unit_structs.fixed +++ b/tests/ui/default_constructed_unit_structs.fixed @@ -161,3 +161,12 @@ fn main() { let _ = ::default(); } + +fn issue12654() { + #[derive(Default)] + struct G; + + fn f(_g: G) {} + + f(<_>::default()); +} diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs index 291cd89da0b7..571f7cde26ad 100644 --- a/tests/ui/default_constructed_unit_structs.rs +++ b/tests/ui/default_constructed_unit_structs.rs @@ -161,3 +161,12 @@ fn main() { let _ = ::default(); } + +fn issue12654() { + #[derive(Default)] + struct G; + + fn f(_g: G) {} + + f(<_>::default()); +} From 253ecb9d7dfbfe3110c7e58e5017b31058f96d80 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 6 Mar 2025 22:42:54 +0100 Subject: [PATCH 2/3] Signal the problem on the whole expression span This is more consistent with what other lints are doing: the use of `default` to create a unit struct is the whole expression. --- .../src/default_constructed_unit_structs.rs | 18 ++++++--- .../ui/default_constructed_unit_structs.fixed | 3 +- .../default_constructed_unit_structs.stderr | 38 ++++++++++++------- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index 85a630591711..7410e0a67050 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -1,5 +1,6 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_ty_alias; +use clippy_utils::sugg::DiagExt as _; use hir::ExprKind; use hir::def::Res; use rustc_errors::Applicability; @@ -73,14 +74,19 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs { // do not suggest replacing an expression by a type name with placeholders && !base.is_suggestable_infer_ty() { - span_lint_and_sugg( + span_lint_and_then( cx, DEFAULT_CONSTRUCTED_UNIT_STRUCTS, - expr.span.with_lo(qpath.qself_span().hi()), + expr.span, "use of `default` to create a unit struct", - "remove this call to `default`", - String::new(), - Applicability::MachineApplicable, + |diag| { + diag.suggest_remove_item( + cx, + expr.span.with_lo(qpath.qself_span().hi()), + "remove this call to `default`", + Applicability::MachineApplicable, + ); + }, ); } } diff --git a/tests/ui/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed index 91a6161f98af..3beddca6fb1e 100644 --- a/tests/ui/default_constructed_unit_structs.fixed +++ b/tests/ui/default_constructed_unit_structs.fixed @@ -8,8 +8,7 @@ struct UnitStruct; impl UnitStruct { fn new() -> Self { //should lint - Self - //~^ default_constructed_unit_structs + Self//~^ default_constructed_unit_structs } } diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr index 6d4e1bdc2cc8..7c0f4c0d4aca 100644 --- a/tests/ui/default_constructed_unit_structs.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -1,41 +1,53 @@ error: use of `default` to create a unit struct - --> tests/ui/default_constructed_unit_structs.rs:11:13 + --> tests/ui/default_constructed_unit_structs.rs:11:9 | -LL | Self::default() - | ^^^^^^^^^^^ help: remove this call to `default` +LL | Self::default() + | _________^^^^-^^^^^^^^^^ +LL | | + | |________- help: remove this call to `default` | = note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::default_constructed_unit_structs)]` error: use of `default` to create a unit struct - --> tests/ui/default_constructed_unit_structs.rs:54:31 + --> tests/ui/default_constructed_unit_structs.rs:54:20 | LL | inner: PhantomData::default(), - | ^^^^^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^----------- + | | + | help: remove this call to `default` error: use of `default` to create a unit struct - --> tests/ui/default_constructed_unit_structs.rs:128:33 + --> tests/ui/default_constructed_unit_structs.rs:128:13 | LL | let _ = PhantomData::::default(); - | ^^^^^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^^^^^^^^^^----------- + | | + | help: remove this call to `default` error: use of `default` to create a unit struct - --> tests/ui/default_constructed_unit_structs.rs:130:42 + --> tests/ui/default_constructed_unit_structs.rs:130:31 | LL | let _: PhantomData = PhantomData::default(); - | ^^^^^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^----------- + | | + | help: remove this call to `default` error: use of `default` to create a unit struct - --> tests/ui/default_constructed_unit_structs.rs:132:55 + --> tests/ui/default_constructed_unit_structs.rs:132:31 | LL | let _: PhantomData = std::marker::PhantomData::default(); - | ^^^^^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^^^^^^^^^^^^^^^----------- + | | + | help: remove this call to `default` error: use of `default` to create a unit struct - --> tests/ui/default_constructed_unit_structs.rs:134:23 + --> tests/ui/default_constructed_unit_structs.rs:134:13 | LL | let _ = UnitStruct::default(); - | ^^^^^^^^^^^ help: remove this call to `default` + | ^^^^^^^^^^----------- + | | + | help: remove this call to `default` error: aborting due to 6 previous errors From 0aa0d074cd1ce3f58eaffcf6019c1a236adc4dc1 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Thu, 6 Mar 2025 23:08:15 +0100 Subject: [PATCH 3/3] Remove brackets around type name when it is no longer a prefix When `::default()` is replaced by `T` when `T` is a singleton, the brackets around the type name can be removed. --- .../src/default_constructed_unit_structs.rs | 12 ++++++---- .../ui/default_constructed_unit_structs.fixed | 8 ++++++- tests/ui/default_constructed_unit_structs.rs | 5 +++++ .../default_constructed_unit_structs.stderr | 22 ++++++++++++++----- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index 7410e0a67050..f8a9037fc804 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_ty_alias; -use clippy_utils::sugg::DiagExt as _; +use clippy_utils::source::SpanRangeExt as _; use hir::ExprKind; use hir::def::Res; use rustc_errors::Applicability; @@ -74,16 +74,20 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs { // do not suggest replacing an expression by a type name with placeholders && !base.is_suggestable_infer_ty() { + let mut removals = vec![(expr.span.with_lo(qpath.qself_span().hi()), String::new())]; + if expr.span.with_source_text(cx, |s| s.starts_with('<')) == Some(true) { + // Remove `<`, '>` has already been removed by the existing removal expression. + removals.push((expr.span.with_hi(qpath.qself_span().lo()), String::new())); + } span_lint_and_then( cx, DEFAULT_CONSTRUCTED_UNIT_STRUCTS, expr.span, "use of `default` to create a unit struct", |diag| { - diag.suggest_remove_item( - cx, - expr.span.with_lo(qpath.qself_span().hi()), + diag.multipart_suggestion( "remove this call to `default`", + removals, Applicability::MachineApplicable, ); }, diff --git a/tests/ui/default_constructed_unit_structs.fixed b/tests/ui/default_constructed_unit_structs.fixed index 3beddca6fb1e..1ca9be0ceddc 100644 --- a/tests/ui/default_constructed_unit_structs.fixed +++ b/tests/ui/default_constructed_unit_structs.fixed @@ -8,7 +8,8 @@ struct UnitStruct; impl UnitStruct { fn new() -> Self { //should lint - Self//~^ default_constructed_unit_structs + Self + //~^ default_constructed_unit_structs } } @@ -168,4 +169,9 @@ fn issue12654() { fn f(_g: G) {} f(<_>::default()); + f(G); + //~^ default_constructed_unit_structs + + // No lint because `as Default` hides the singleton + f(::default()); } diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs index 571f7cde26ad..99eb8913fc3c 100644 --- a/tests/ui/default_constructed_unit_structs.rs +++ b/tests/ui/default_constructed_unit_structs.rs @@ -169,4 +169,9 @@ fn issue12654() { fn f(_g: G) {} f(<_>::default()); + f(::default()); + //~^ default_constructed_unit_structs + + // No lint because `as Default` hides the singleton + f(::default()); } diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr index 7c0f4c0d4aca..97fad792e4f7 100644 --- a/tests/ui/default_constructed_unit_structs.stderr +++ b/tests/ui/default_constructed_unit_structs.stderr @@ -1,10 +1,10 @@ error: use of `default` to create a unit struct --> tests/ui/default_constructed_unit_structs.rs:11:9 | -LL | Self::default() - | _________^^^^-^^^^^^^^^^ -LL | | - | |________- help: remove this call to `default` +LL | Self::default() + | ^^^^----------- + | | + | help: remove this call to `default` | = note: `-D clippy::default-constructed-unit-structs` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::default_constructed_unit_structs)]` @@ -49,5 +49,17 @@ LL | let _ = UnitStruct::default(); | | | help: remove this call to `default` -error: aborting due to 6 previous errors +error: use of `default` to create a unit struct + --> tests/ui/default_constructed_unit_structs.rs:172:7 + | +LL | f(::default()); + | ^^^^^^^^^^^^^^ + | +help: remove this call to `default` + | +LL - f(::default()); +LL + f(G); + | + +error: aborting due to 7 previous errors