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

4 files changed, 83 insertions, 19 deletions

diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs
index bbd5dc15542..f8a9037fc80 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::source::SpanRangeExt as _;
 use hir::ExprKind;
 use hir::def::Res;
 use rustc_errors::Applicability;
@@ -70,15 +71,26 @@ 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(
+            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.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.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 fa4d5517782..1ca9be0cedd 100644
--- a/tests/ui/default_constructed_unit_structs.fixed
+++ b/tests/ui/default_constructed_unit_structs.fixed
@@ -161,3 +161,17 @@ fn main() {
 
     let _ = <struct_from_macro!()>::default();
 }
+
+fn issue12654() {
+    #[derive(Default)]
+    struct G;
+
+    fn f(_g: G) {}
+
+    f(<_>::default());
+    f(G);
+    //~^ default_constructed_unit_structs
+
+    // No lint because `as Default` hides the singleton
+    f(<G as Default>::default());
+}
diff --git a/tests/ui/default_constructed_unit_structs.rs b/tests/ui/default_constructed_unit_structs.rs
index 291cd89da0b..99eb8913fc3 100644
--- a/tests/ui/default_constructed_unit_structs.rs
+++ b/tests/ui/default_constructed_unit_structs.rs
@@ -161,3 +161,17 @@ fn main() {
 
     let _ = <struct_from_macro!()>::default();
 }
+
+fn issue12654() {
+    #[derive(Default)]
+    struct G;
+
+    fn f(_g: G) {}
+
+    f(<_>::default());
+    f(<G>::default());
+    //~^ default_constructed_unit_structs
+
+    // No lint because `as Default` hides the singleton
+    f(<G as Default>::default());
+}
diff --git a/tests/ui/default_constructed_unit_structs.stderr b/tests/ui/default_constructed_unit_structs.stderr
index 6d4e1bdc2cc..97fad792e4f 100644
--- a/tests/ui/default_constructed_unit_structs.stderr
+++ b/tests/ui/default_constructed_unit_structs.stderr
@@ -1,41 +1,65 @@
 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`
+   |         ^^^^-----------
+   |             |
+   |             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::<usize>::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<i32> = 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<i32> = 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
+error: use of `default` to create a unit struct
+  --> tests/ui/default_constructed_unit_structs.rs:172:7
+   |
+LL |     f(<G>::default());
+   |       ^^^^^^^^^^^^^^
+   |
+help: remove this call to `default`
+   |
+LL -     f(<G>::default());
+LL +     f(G);
+   |
+
+error: aborting due to 7 previous errors