From adf31e95e44ff0b7fbc798fe2b8cc5a42a3abf4b Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 12 Oct 2020 16:43:49 +0100 Subject: [PATCH 1/2] resolve: suggest variants with placeholders This commit improves the diagnostic modified in rust-lang/rust#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 --- .../rustc_resolve/src/late/diagnostics.rs | 146 +++++++++++------- ...issue-43871-enum-instead-of-variant.stderr | 21 +++ src/test/ui/glob-resolve1.stderr | 12 +- src/test/ui/issues/issue-73427.stderr | 98 +++++++++++- src/test/ui/resolve/privacy-enum-ctor.stderr | 112 ++++++++++++-- 5 files changed, 311 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index bee05e7738280..8ef99f36a04af 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/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::>() - } 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::>() - }; + .collect::>(); - 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::>(); + + 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::>(); + + 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"); } } } diff --git a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr index e1325b789d2e9..62a7649e2adf7 100644 --- a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr +++ b/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 diff --git a/src/test/ui/glob-resolve1.stderr b/src/test/ui/glob-resolve1.stderr index 3c818f3ae48ea..cd128c1ea0b48 100644 --- a/src/test/ui/glob-resolve1.stderr +++ b/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 diff --git a/src/test/ui/issues/issue-73427.stderr b/src/test/ui/issues/issue-73427.stderr index 88d19943f0226..4da5305e6870f 100644 --- a/src/test/ui/issues/issue-73427.stderr +++ b/src/test/ui/issues/issue-73427.stderr @@ -4,8 +4,18 @@ 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(); | ^^^^^^^^^^^^^^ @@ -13,6 +23,12 @@ 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 @@ -20,23 +36,69 @@ error[E0423]: expected value, found enum `B` 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 { } diff --git a/src/test/ui/resolve/privacy-enum-ctor.stderr b/src/test/ui/resolve/privacy-enum-ctor.stderr index 77429f800f1a3..807dadf417bf5 100644 --- a/src/test/ui/resolve/privacy-enum-ctor.stderr +++ b/src/test/ui/resolve/privacy-enum-ctor.stderr @@ -2,17 +2,57 @@ error[E0423]: expected value, found enum `n::Z` --> $DIR/privacy-enum-ctor.rs:23:9 | LL | n::Z; - | ^^^^ help: try using one of the enum's variants: `m::Z::Unit` + | ^^^^ | - = help: you might have meant to use one of the enum's other variants that have fields +note: the enum is defined here + --> $DIR/privacy-enum-ctor.rs:11:9 + | +LL | / pub(in m) enum Z { +LL | | Fn(u8), +LL | | Struct { +LL | | s: u8, +LL | | }, +LL | | Unit, +LL | | } + | |_________^ +help: you might have meant to use the following enum variant + | +LL | m::Z::Unit; + | ^^^^^^^^^^ +help: the following enum variants are available + | +LL | (m::Z::Fn(/* fields */)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | (m::Z::Struct { /* fields */ }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0423]: expected value, found enum `Z` --> $DIR/privacy-enum-ctor.rs:25:9 | LL | Z; - | ^ help: try using one of the enum's variants: `m::Z::Unit` + | ^ + | +note: the enum is defined here + --> $DIR/privacy-enum-ctor.rs:11:9 | - = help: you might have meant to use one of the enum's other variants that have fields +LL | / pub(in m) enum Z { +LL | | Fn(u8), +LL | | Struct { +LL | | s: u8, +LL | | }, +LL | | Unit, +LL | | } + | |_________^ +help: you might have meant to use the following enum variant + | +LL | m::Z::Unit; + | ^^^^^^^^^^ +help: the following enum variants are available + | +LL | (m::Z::Fn(/* fields */)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | (m::Z::Struct { /* fields */ }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0423]: expected value, found struct variant `Z::Struct` --> $DIR/privacy-enum-ctor.rs:29:20 @@ -34,11 +74,27 @@ LL | fn f() { LL | let _: E = m::E; | ^^^^ | - = 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/privacy-enum-ctor.rs:2:5 + | +LL | / pub enum E { +LL | | Fn(u8), +LL | | Struct { +LL | | s: u8, +LL | | }, +LL | | Unit, +LL | | } + | |_____^ +help: you might have meant to use the following enum variant | LL | let _: E = E::Unit; | ^^^^^^^ +help: the following enum variants are available + | +LL | let _: E = (E::Fn(/* fields */)); + | ^^^^^^^^^^^^^^^^^^^^^ +LL | let _: E = (E::Struct { /* fields */ }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: a function with a similar name exists | LL | let _: E = m::f; @@ -67,11 +123,27 @@ error[E0423]: expected value, found enum `E` LL | let _: E = E; | ^ | - = 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/privacy-enum-ctor.rs:2:5 + | +LL | / pub enum E { +LL | | Fn(u8), +LL | | Struct { +LL | | s: u8, +LL | | }, +LL | | Unit, +LL | | } + | |_____^ +help: you might have meant to use the following enum variant | LL | let _: E = E::Unit; | ^^^^^^^ +help: the following enum variants are available + | +LL | let _: E = (E::Fn(/* fields */)); + | ^^^^^^^^^^^^^^^^^^^^^ +LL | let _: E = (E::Struct { /* fields */ }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider importing one of these items instead | LL | use std::f32::consts::E; @@ -112,9 +184,29 @@ error[E0423]: expected value, found enum `m::n::Z` --> $DIR/privacy-enum-ctor.rs:57:16 | LL | let _: Z = m::n::Z; - | ^^^^^^^ help: try using one of the enum's variants: `m::Z::Unit` + | ^^^^^^^ + | +note: the enum is defined here + --> $DIR/privacy-enum-ctor.rs:11:9 + | +LL | / pub(in m) enum Z { +LL | | Fn(u8), +LL | | Struct { +LL | | s: u8, +LL | | }, +LL | | Unit, +LL | | } + | |_________^ +help: you might have meant to use the following enum variant + | +LL | let _: Z = m::Z::Unit; + | ^^^^^^^^^^ +help: the following enum variants are available | - = help: you might have meant to use one of the enum's other variants that have fields +LL | let _: Z = (m::Z::Fn(/* fields */)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Z = (m::Z::Struct { /* fields */ }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0412]: cannot find type `Z` in this scope --> $DIR/privacy-enum-ctor.rs:61:12 From f897162f3efc841461adce9510bb627cea9bac45 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 12 Oct 2020 16:52:51 +0100 Subject: [PATCH 2/2] resolve: improve "try using tuple struct" message This commit improves the tuple struct case added in rust-lang/rust#77341 so that the context is mentioned in more of the message. Signed-off-by: David Wood --- .../rustc_resolve/src/late/diagnostics.rs | 22 +++++++++---------- ...issue-43871-enum-instead-of-variant.stderr | 6 ++--- src/test/ui/issues/issue-73427.stderr | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 8ef99f36a04af..c24b383f3b811 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1341,29 +1341,29 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { let non_suggestable_variant_count = variants.len() - suggestable_variants.len(); + let source_msg = if source.is_call() { + "to construct" + } else if matches!(source, PathSource::TupleStruct(..)) { + "to match against" + } else { + unreachable!() + }; + if !suggestable_variants.is_empty() { let msg = if non_suggestable_variant_count == 0 && suggestable_variants.len() == 1 { - "try using the enum's variant" + format!("try {} the enum's variant", source_msg) } else { - "try using one of the enum's variants" + format!("try {} one of the enum's variants", source_msg) }; err.span_suggestions( span, - msg, + &msg, suggestable_variants.drain(..), Applicability::MaybeIncorrect, ); } - let source_msg = if source.is_call() { - "to construct" - } else if matches!(source, PathSource::TupleStruct(..)) { - "to match against" - } else { - unreachable!() - }; - // If the enum has no tuple variants.. if non_suggestable_variant_count == variants.len() { err.help(&format!("the enum has no tuple variants {}", source_msg)); diff --git a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr index 62a7649e2adf7..bca493e67d543 100644 --- a/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr +++ b/src/test/ui/did_you_mean/issue-43871-enum-instead-of-variant.stderr @@ -2,7 +2,7 @@ error[E0423]: expected function, tuple struct or tuple variant, found enum `Opti --> $DIR/issue-43871-enum-instead-of-variant.rs:19:13 | LL | let x = Option(1); - | ^^^^^^ help: try using one of the enum's variants: `std::option::Option::Some` + | ^^^^^^ help: try to construct one of the enum's variants: `std::option::Option::Some` | = help: you might have meant to construct the enum's non-tuple variant @@ -10,7 +10,7 @@ error[E0532]: expected tuple struct or tuple variant, found enum `Option` --> $DIR/issue-43871-enum-instead-of-variant.rs:21:12 | LL | if let Option(_) = x { - | ^^^^^^ help: try using one of the enum's variants: `std::option::Option::Some` + | ^^^^^^ help: try to match against one of the enum's variants: `std::option::Option::Some` | = help: you might have meant to match against the enum's non-tuple variant @@ -18,7 +18,7 @@ error[E0532]: expected tuple struct or tuple variant, found enum `Example` --> $DIR/issue-43871-enum-instead-of-variant.rs:27:12 | LL | if let Example(_) = y { - | ^^^^^^^ help: try using one of the enum's variants: `Example::Ex` + | ^^^^^^^ help: try to match against 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 diff --git a/src/test/ui/issues/issue-73427.stderr b/src/test/ui/issues/issue-73427.stderr index 4da5305e6870f..4b5f65b346174 100644 --- a/src/test/ui/issues/issue-73427.stderr +++ b/src/test/ui/issues/issue-73427.stderr @@ -118,7 +118,7 @@ LL | | Tuple(), LL | | Unit, LL | | } | |_^ -help: try using one of the enum's variants +help: try to construct one of the enum's variants | LL | let x = A::TupleWithFields(3); | ^^^^^^^^^^^^^^^^^^ @@ -143,7 +143,7 @@ LL | | Tuple(), LL | | Unit, LL | | } | |_^ -help: try using one of the enum's variants +help: try to match against one of the enum's variants | LL | if let A::TupleWithFields(3) = x { } | ^^^^^^^^^^^^^^^^^^