From 9ef68f53e1f882afb63f824a10ff33ccd2c4764b Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 29 Sep 2020 16:52:43 +0100 Subject: [PATCH] resolve: improve "try using the enum's variant" This commit improves the "try using the enum's variant" suggestion: - Variants in suggestions would not result in more errors (e.g. use of a struct variant is only suggested if the suggestion can trivially construct that variant). Therefore, suggestions are only emitted for variants that have no fields (since the suggestion can't know what value fields would have). - Suggestions include the syntax for constructing the variant. If a struct or tuple variant is suggested, then it is constructed in the suggestion - unless in pattern-matching or when arguments are already provided. - A help message is added which mentions the variants which are no longer suggested. Signed-off-by: David Wood --- .../rustc_resolve/src/late/diagnostics.rs | 145 +++++++++++++++--- ...issue-43871-enum-instead-of-variant.stderr | 42 ++--- src/test/ui/issues/issue-73427.rs | 44 ++++++ src/test/ui/issues/issue-73427.stderr | 72 +++++++++ src/test/ui/resolve/privacy-enum-ctor.stderr | 43 +----- 5 files changed, 258 insertions(+), 88 deletions(-) create mode 100644 src/test/ui/issues/issue-73427.rs create mode 100644 src/test/ui/issues/issue-73427.stderr diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index ced272e474d11..bf42fae748091 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -12,7 +12,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def::Namespace::{self, *}; -use rustc_hir::def::{self, CtorKind, DefKind}; +use rustc_hir::def::{self, CtorKind, CtorOf, DefKind}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::PrimTy; use rustc_session::config::nightly_options; @@ -725,24 +725,8 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { // We already suggested changing `:` into `::` during parsing. return false; } - if let Some(variants) = self.collect_enum_variants(def_id) { - if !variants.is_empty() { - let msg = if variants.len() == 1 { - "try using the enum's variant" - } else { - "try using one of the enum's variants" - }; - err.span_suggestions( - span, - msg, - variants.iter().map(path_names_to_string), - Applicability::MaybeIncorrect, - ); - } - } else { - err.note("you might have meant to use one of the enum's variants"); - } + self.suggest_using_enum_variant(err, source, def_id, span); } (Res::Def(DefKind::Struct, def_id), _) if ns == ValueNS => { if let Some((ctor_def, ctor_vis, fields)) = @@ -1125,20 +1109,139 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { result } - fn collect_enum_variants(&mut self, def_id: DefId) -> Option> { + fn collect_enum_ctors(&mut self, def_id: DefId) -> Option> { self.find_module(def_id).map(|(enum_module, enum_import_suggestion)| { let mut variants = Vec::new(); enum_module.for_each_child(self.r, |_, ident, _, name_binding| { - if let Res::Def(DefKind::Variant, _) = name_binding.res() { + if let Res::Def(DefKind::Ctor(CtorOf::Variant, kind), def_id) = name_binding.res() { let mut segms = enum_import_suggestion.path.segments.clone(); segms.push(ast::PathSegment::from_ident(ident)); - variants.push(Path { span: name_binding.span, segments: segms, tokens: None }); + let path = Path { span: name_binding.span, segments: segms, tokens: None }; + variants.push((path, def_id, kind)); } }); variants }) } + /// Adds a suggestion for using an enum's variant when an enum is used instead. + fn suggest_using_enum_variant( + &mut self, + err: &mut DiagnosticBuilder<'a>, + source: PathSource<'_>, + def_id: DefId, + span: Span, + ) { + let variants = match self.collect_enum_ctors(def_id) { + Some(variants) => variants, + None => { + err.note("you might have meant to use one of the enum's variants"); + return; + } + }; + + let suggest_only_tuple_variants = + matches!(source, PathSource::TupleStruct(..)) || source.is_call(); + let mut suggestable_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 + .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::>() + }; + + 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" + }; + + 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(..)) { + "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)); + } + + // If there are also non-tuple variants.. + if non_suggestable_variant_count == 1 { + err.help(&format!( + "you might have meant {} the enum's non-tuple variant", + source_msg + )); + } else if non_suggestable_variant_count >= 1 { + err.help(&format!( + "you might have meant {} one of the enum's non-tuple variants", + source_msg + )); + } + } 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"); + } + } + } + } + crate fn report_missing_type_error( &self, path: &[Segment], 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 2140fd3a5a037..e1325b789d2e9 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,46 +2,33 @@ 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 using one of the enum's variants - | -LL | let x = std::option::Option::None(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -LL | let x = std::option::Option::Some(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: you might have meant to construct the enum's non-tuple variant 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 + | ^^^^^^ help: try using one of the enum's variants: `std::option::Option::Some` | -LL | if let std::option::Option::None(_) = x { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ -LL | if let std::option::Option::Some(_) = x { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: you might have meant to match against the enum's non-tuple variant 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 + | ^^^^^^^ help: try using one of the enum's variants: `Example::Ex` | -LL | if let Example::Ex(_) = y { - | ^^^^^^^^^^^ -LL | if let Example::NotEx(_) = y { - | ^^^^^^^^^^^^^^ + = help: you might have meant to match against the enum's non-tuple variant error[E0423]: expected function, tuple struct or tuple variant, found enum `Void` --> $DIR/issue-43871-enum-instead-of-variant.rs:31:13 | LL | let y = Void(); | ^^^^ + | + = help: the enum has no tuple variants to construct error[E0423]: expected function, tuple struct or tuple variant, found enum `ManyVariants` --> $DIR/issue-43871-enum-instead-of-variant.rs:33:13 @@ -49,17 +36,8 @@ error[E0423]: expected function, tuple struct or tuple variant, found enum `Many LL | let z = ManyVariants(); | ^^^^^^^^^^^^ | -help: try using one of the enum's variants - | -LL | let z = ManyVariants::One(); - | ^^^^^^^^^^^^^^^^^ -LL | let z = ManyVariants::Two(); - | ^^^^^^^^^^^^^^^^^ -LL | let z = ManyVariants::Three(); - | ^^^^^^^^^^^^^^^^^^^ -LL | let z = ManyVariants::Four(); - | ^^^^^^^^^^^^^^^^^^ - and 6 other candidates + = help: the enum has no tuple variants to construct + = help: you might have meant to construct one of the enum's non-tuple variants error: aborting due to 5 previous errors diff --git a/src/test/ui/issues/issue-73427.rs b/src/test/ui/issues/issue-73427.rs new file mode 100644 index 0000000000000..3c62782a89799 --- /dev/null +++ b/src/test/ui/issues/issue-73427.rs @@ -0,0 +1,44 @@ +enum A { + StructWithFields { x: () }, + TupleWithFields(()), + Struct {}, + Tuple(), + Unit, +} + +enum B { + StructWithFields { x: () }, + TupleWithFields(()), +} + +enum C { + StructWithFields { x: () }, + TupleWithFields(()), + Unit, +} + +enum D { + TupleWithFields(()), + Unit, +} + +fn main() { + // Only variants without fields are suggested (and others mentioned in a note) where an enum + // is used rather than a variant. + + A.foo(); + //~^ ERROR expected value, found enum `A` + B.foo(); + //~^ ERROR expected value, found enum `B` + C.foo(); + //~^ ERROR expected value, found enum `C` + D.foo(); + //~^ ERROR expected value, found enum `D` + + // Only tuple variants are suggested in calls or tuple struct pattern matching. + + let x = A(3); + //~^ ERROR expected function, tuple struct or tuple variant, found enum `A` + if let A(3) = x { } + //~^ ERROR expected tuple struct or tuple variant, found enum `A` +} diff --git a/src/test/ui/issues/issue-73427.stderr b/src/test/ui/issues/issue-73427.stderr new file mode 100644 index 0000000000000..88d19943f0226 --- /dev/null +++ b/src/test/ui/issues/issue-73427.stderr @@ -0,0 +1,72 @@ +error[E0423]: expected value, found enum `A` + --> $DIR/issue-73427.rs:29:5 + | +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 + | +LL | (A::Struct {}).foo(); + | ^^^^^^^^^^^^^^ +LL | (A::Tuple()).foo(); + | ^^^^^^^^^^^^ +LL | A::Unit.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 + +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` + | + = help: you might have meant to use one of the enum's other variants that have fields + +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` + | + = help: you might have meant to use the enum's other variant that has fields + +error[E0423]: expected function, tuple struct or tuple variant, found enum `A` + --> $DIR/issue-73427.rs:40:13 + | +LL | let x = A(3); + | ^ + | + = help: you might have meant to construct one of the enum's non-tuple variants +help: try using one of the enum's variants + | +LL | let x = A::TupleWithFields(3); + | ^^^^^^^^^^^^^^^^^^ +LL | let x = A::Tuple(3); + | ^^^^^^^^ + +error[E0532]: expected tuple struct or tuple variant, found enum `A` + --> $DIR/issue-73427.rs:42:12 + | +LL | if let A(3) = x { } + | ^ + | + = help: you might have meant to match against one of the enum's non-tuple variants +help: try using one of the enum's variants + | +LL | if let A::TupleWithFields(3) = x { } + | ^^^^^^^^^^^^^^^^^^ +LL | if let A::Tuple(3) = x { } + | ^^^^^^^^ + +error: aborting due to 6 previous errors + +Some errors have detailed explanations: E0423, E0532. +For more information about an error, try `rustc --explain E0423`. diff --git a/src/test/ui/resolve/privacy-enum-ctor.stderr b/src/test/ui/resolve/privacy-enum-ctor.stderr index 32eff15119648..77429f800f1a3 100644 --- a/src/test/ui/resolve/privacy-enum-ctor.stderr +++ b/src/test/ui/resolve/privacy-enum-ctor.stderr @@ -2,31 +2,17 @@ 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: try using one of the enum's variants - | -LL | m::Z::Fn; - | ^^^^^^^^ -LL | m::Z::Struct; - | ^^^^^^^^^^^^ -LL | m::Z::Unit; - | ^^^^^^^^^^ + = help: you might have meant to use one of the enum's other variants that have 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` | -help: try using one of the enum's variants - | -LL | m::Z::Fn; - | ^^^^^^^^ -LL | m::Z::Struct; - | ^^^^^^^^^^^^ -LL | m::Z::Unit; - | ^^^^^^^^^^ + = help: you might have meant to use one of the enum's other variants that have fields error[E0423]: expected value, found struct variant `Z::Struct` --> $DIR/privacy-enum-ctor.rs:29:20 @@ -48,12 +34,9 @@ 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 | -LL | let _: E = E::Fn; - | ^^^^^ -LL | let _: E = E::Struct; - | ^^^^^^^^^ LL | let _: E = E::Unit; | ^^^^^^^ help: a function with a similar name exists @@ -84,12 +67,9 @@ 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 | -LL | let _: E = E::Fn; - | ^^^^^ -LL | let _: E = E::Struct; - | ^^^^^^^^^ LL | let _: E = E::Unit; | ^^^^^^^ help: consider importing one of these items instead @@ -132,16 +112,9 @@ 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` | -help: try using one of the enum's variants - | -LL | let _: Z = m::Z::Fn; - | ^^^^^^^^ -LL | let _: Z = m::Z::Struct; - | ^^^^^^^^^^^^ -LL | let _: Z = m::Z::Unit; - | ^^^^^^^^^^ + = help: you might have meant to use one of the enum's other variants that have fields error[E0412]: cannot find type `Z` in this scope --> $DIR/privacy-enum-ctor.rs:61:12