resolve: suggest variants with placeholders

This commit improves the diagnostic modified in #77341 to
suggest not only those variants which do not have fields, but those with
fields (by suggesting with placeholders).

Signed-off-by: David Wood <[email protected]>

Author: davidtwco

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -1330,58 +1330,32 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
 
         let suggest_only_tuple_variants =
             matches!(source, PathSource::TupleStruct(..)) || source.is_call();
-        let mut suggestable_variants = if suggest_only_tuple_variants {
+        if suggest_only_tuple_variants {
             // Suggest only tuple variants regardless of whether they have fields and do not
             // suggest path with added parenthesis.
-            variants
+            let mut suggestable_variants = variants
                 .iter()
                 .filter(|(.., kind)| *kind == CtorKind::Fn)
                 .map(|(variant, ..)| path_names_to_string(variant))
-                .collect::<Vec<_>>()
-        } else {
-            variants
-                .iter()
-                .filter(|(_, def_id, kind)| {
-                    // Suggest only variants that have no fields (these can definitely
-                    // be constructed).
-                    let has_fields =
-                        self.r.field_names.get(&def_id).map(|f| f.is_empty()).unwrap_or(false);
-                    match kind {
-                        CtorKind::Const => true,
-                        CtorKind::Fn | CtorKind::Fictive if has_fields => true,
-                        _ => false,
-                    }
-                })
-                .map(|(variant, _, kind)| (path_names_to_string(variant), kind))
-                .map(|(variant_str, kind)| {
-                    // Add constructor syntax where appropriate.
-                    match kind {
-                        CtorKind::Const => variant_str,
-                        CtorKind::Fn => format!("({}())", variant_str),
-                        CtorKind::Fictive => format!("({} {{}})", variant_str),
-                    }
-                })
-                .collect::<Vec<_>>()
-        };
+                .collect::<Vec<_>>();
 
-        let non_suggestable_variant_count = variants.len() - suggestable_variants.len();
+            let non_suggestable_variant_count = variants.len() - suggestable_variants.len();
 
-        if !suggestable_variants.is_empty() {
-            let msg = if non_suggestable_variant_count == 0 && suggestable_variants.len() == 1 {
-                "try using the enum's variant"
-            } else {
-                "try using one of the enum's variants"
-            };
+            if !suggestable_variants.is_empty() {
+                let msg = if non_suggestable_variant_count == 0 && suggestable_variants.len() == 1 {
+                    "try using the enum's variant"
+                } else {
+                    "try using one of the enum's variants"
+                };
 
-            err.span_suggestions(
-                span,
-                msg,
-                suggestable_variants.drain(..),
-                Applicability::MaybeIncorrect,
-            );
-        }
+                err.span_suggestions(
+                    span,
+                    msg,
+                    suggestable_variants.drain(..),
+                    Applicability::MaybeIncorrect,
+                );
+            }
 
-        if suggest_only_tuple_variants {
             let source_msg = if source.is_call() {
                 "to construct"
             } else if matches!(source, PathSource::TupleStruct(..)) {
@@ -1408,24 +1382,76 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
                 ));
             }
         } else {
-            let made_suggestion = non_suggestable_variant_count != variants.len();
-            if made_suggestion {
-                if non_suggestable_variant_count == 1 {
-                    err.help(
-                        "you might have meant to use the enum's other variant that has fields",
-                    );
-                } else if non_suggestable_variant_count >= 1 {
-                    err.help(
-                        "you might have meant to use one of the enum's other variants that \
-                         have fields",
-                    );
-                }
-            } else {
-                if non_suggestable_variant_count == 1 {
-                    err.help("you might have meant to use the enum's variant");
-                } else if non_suggestable_variant_count >= 1 {
-                    err.help("you might have meant to use one of the enum's variants");
+            let needs_placeholder = |def_id: DefId, kind: CtorKind| {
+                let has_no_fields =
+                    self.r.field_names.get(&def_id).map(|f| f.is_empty()).unwrap_or(false);
+                match kind {
+                    CtorKind::Const => false,
+                    CtorKind::Fn | CtorKind::Fictive if has_no_fields => false,
+                    _ => true,
                 }
+            };
+
+            let mut suggestable_variants = variants
+                .iter()
+                .filter(|(_, def_id, kind)| !needs_placeholder(*def_id, *kind))
+                .map(|(variant, _, kind)| (path_names_to_string(variant), kind))
+                .map(|(variant, kind)| match kind {
+                    CtorKind::Const => variant,
+                    CtorKind::Fn => format!("({}())", variant),
+                    CtorKind::Fictive => format!("({} {{}})", variant),
+                })
+                .collect::<Vec<_>>();
+
+            if !suggestable_variants.is_empty() {
+                let msg = if suggestable_variants.len() == 1 {
+                    "you might have meant to use the following enum variant"
+                } else {
+                    "you might have meant to use one of the following enum variants"
+                };
+
+                err.span_suggestions(
+                    span,
+                    msg,
+                    suggestable_variants.drain(..),
+                    Applicability::MaybeIncorrect,
+                );
+            }
+
+            let mut suggestable_variants_with_placeholders = variants
+                .iter()
+                .filter(|(_, def_id, kind)| needs_placeholder(*def_id, *kind))
+                .map(|(variant, _, kind)| (path_names_to_string(variant), kind))
+                .filter_map(|(variant, kind)| match kind {
+                    CtorKind::Fn => Some(format!("({}(/* fields */))", variant)),
+                    CtorKind::Fictive => Some(format!("({} {{ /* fields */ }})", variant)),
+                    _ => None,
+                })
+                .collect::<Vec<_>>();
+
+            if !suggestable_variants_with_placeholders.is_empty() {
+                let msg = match (
+                    suggestable_variants.is_empty(),
+                    suggestable_variants_with_placeholders.len(),
+                ) {
+                    (true, 1) => "the following enum variant is available",
+                    (true, _) => "the following enum variants are available",
+                    (false, 1) => "alternatively, the following enum variant is available",
+                    (false, _) => "alternatively, the following enum variants are also available",
+                };
+
+                err.span_suggestions(
+                    span,
+                    msg,
+                    suggestable_variants_with_placeholders.drain(..),
+                    Applicability::HasPlaceholders,
+                );
+            }
+        };
+
+        if def_id.is_local() {
+            if let Some(span) = self.def_span(def_id) {
+                err.span_note(span, "the enum is defined here");
             }
         }
     }

src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr

@@ -21,6 +21,11 @@ LL |     if let Example(_) = y {
    |            ^^^^^^^ help: try using one of the enum's variants: `Example::Ex`
    |
    = help: you might have meant to match against the enum's non-tuple variant
+note: the enum is defined here
+  --> $DIR/issue-43871-enum-instead-of-variant.rs:1:1
+   |
+LL | enum Example { Ex(String), NotEx }
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0423]: expected function, tuple struct or tuple variant, found enum `Void`
   --> $DIR/issue-43871-enum-instead-of-variant.rs:31:13
@@ -29,6 +34,11 @@ LL |     let y = Void();
    |             ^^^^
    |
    = help: the enum has no tuple variants to construct
+note: the enum is defined here
+  --> $DIR/issue-43871-enum-instead-of-variant.rs:3:1
+   |
+LL | enum Void {}
+   | ^^^^^^^^^^^^
 
 error[E0423]: expected function, tuple struct or tuple variant, found enum `ManyVariants`
   --> $DIR/issue-43871-enum-instead-of-variant.rs:33:13
@@ -38,6 +48,17 @@ LL |     let z = ManyVariants();
    |
    = help: the enum has no tuple variants to construct
    = help: you might have meant to construct one of the enum's non-tuple variants
+note: the enum is defined here
+  --> $DIR/issue-43871-enum-instead-of-variant.rs:5:1
+   |
+LL | / enum ManyVariants {
+LL | |     One,
+LL | |     Two,
+LL | |     Three,
+...  |
+LL | |     Ten,
+LL | | }
+   | |_^
 
 error: aborting due to 5 previous errors
 

src/test/ui/glob-resolve1.stderr

@@ -24,7 +24,17 @@ error[E0423]: expected value, found enum `B`
   --> $DIR/glob-resolve1.rs:24:5
    |
 LL |     B;
-   |     ^ help: try using the enum's variant: `B::B1`
+   |     ^
+   |
+note: the enum is defined here
+  --> $DIR/glob-resolve1.rs:12:5
+   |
+LL |     pub enum B { B1 }
+   |     ^^^^^^^^^^^^^^^^^
+help: you might have meant to use the following enum variant
+   |
+LL |     B::B1;
+   |     ^^^^^
 
 error[E0425]: cannot find value `C` in this scope
   --> $DIR/glob-resolve1.rs:25:5

src/test/ui/issues/issue-73427.stderr

@@ -4,39 +4,101 @@ error[E0423]: expected value, found enum `A`
 LL |     A.foo();
    |     ^
    |
-   = help: you might have meant to use one of the enum's other variants that have fields
-help: try using one of the enum's variants
+note: the enum is defined here
+  --> $DIR/issue-73427.rs:1:1
+   |
+LL | / enum A {
+LL | |     StructWithFields { x: () },
+LL | |     TupleWithFields(()),
+LL | |     Struct {},
+LL | |     Tuple(),
+LL | |     Unit,
+LL | | }
+   | |_^
+help: you might have meant to use one of the following enum variants
    |
 LL |     (A::Struct {}).foo();
    |     ^^^^^^^^^^^^^^
 LL |     (A::Tuple()).foo();
    |     ^^^^^^^^^^^^
 LL |     A::Unit.foo();
    |     ^^^^^^^
+help: the following enum variants are available
+   |
+LL |     (A::StructWithFields { /* fields */ }).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     (A::TupleWithFields(/* fields */)).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0423]: expected value, found enum `B`
   --> $DIR/issue-73427.rs:31:5
    |
 LL |     B.foo();
    |     ^
    |
-   = help: you might have meant to use one of the enum's variants
+note: the enum is defined here
+  --> $DIR/issue-73427.rs:9:1
+   |
+LL | / enum B {
+LL | |     StructWithFields { x: () },
+LL | |     TupleWithFields(()),
+LL | | }
+   | |_^
+help: the following enum variants are available
+   |
+LL |     (B::StructWithFields { /* fields */ }).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     (B::TupleWithFields(/* fields */)).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0423]: expected value, found enum `C`
   --> $DIR/issue-73427.rs:33:5
    |
 LL |     C.foo();
-   |     ^ help: try using one of the enum's variants: `C::Unit`
+   |     ^
+   |
+note: the enum is defined here
+  --> $DIR/issue-73427.rs:14:1
    |
-   = help: you might have meant to use one of the enum's other variants that have fields
+LL | / enum C {
+LL | |     StructWithFields { x: () },
+LL | |     TupleWithFields(()),
+LL | |     Unit,
+LL | | }
+   | |_^
+help: you might have meant to use the following enum variant
+   |
+LL |     C::Unit.foo();
+   |     ^^^^^^^
+help: the following enum variants are available
+   |
+LL |     (C::StructWithFields { /* fields */ }).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     (C::TupleWithFields(/* fields */)).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0423]: expected value, found enum `D`
   --> $DIR/issue-73427.rs:35:5
    |
 LL |     D.foo();
-   |     ^ help: try using one of the enum's variants: `D::Unit`
+   |     ^
+   |
+note: the enum is defined here
+  --> $DIR/issue-73427.rs:20:1
+   |
+LL | / enum D {
+LL | |     TupleWithFields(()),
+LL | |     Unit,
+LL | | }
+   | |_^
+help: you might have meant to use the following enum variant
    |
-   = help: you might have meant to use the enum's other variant that has fields
+LL |     D::Unit.foo();
+   |     ^^^^^^^
+help: the following enum variant is available
+   |
+LL |     (D::TupleWithFields(/* fields */)).foo();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0423]: expected function, tuple struct or tuple variant, found enum `A`
   --> $DIR/issue-73427.rs:40:13
@@ -45,6 +107,17 @@ LL |     let x = A(3);
    |             ^
    |
    = help: you might have meant to construct one of the enum's non-tuple variants
+note: the enum is defined here
+  --> $DIR/issue-73427.rs:1:1
+   |
+LL | / enum A {
+LL | |     StructWithFields { x: () },
+LL | |     TupleWithFields(()),
+LL | |     Struct {},
+LL | |     Tuple(),
+LL | |     Unit,
+LL | | }
+   | |_^
 help: try using one of the enum's variants
    |
 LL |     let x = A::TupleWithFields(3);
@@ -59,6 +132,17 @@ LL |     if let A(3) = x { }
    |            ^
    |
    = help: you might have meant to match against one of the enum's non-tuple variants
+note: the enum is defined here
+  --> $DIR/issue-73427.rs:1:1
+   |
+LL | / enum A {
+LL | |     StructWithFields { x: () },
+LL | |     TupleWithFields(()),
+LL | |     Struct {},
+LL | |     Tuple(),
+LL | |     Unit,
+LL | | }
+   | |_^
 help: try using one of the enum's variants
    |
 LL |     if let A::TupleWithFields(3) = x { }