Skip to content

Commit

Permalink
resolve: suggest variants with placeholders
Browse files Browse the repository at this point in the history
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 <david@davidtw.co>
  • Loading branch information
davidtwco committed Oct 15, 2020
1 parent 7f58716 commit adf31e9
Show file tree
Hide file tree
Showing 5 changed files with 311 additions and 78 deletions.
146 changes: 86 additions & 60 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Expand Up @@ -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(..)) {
Expand All @@ -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");
}
}
}
Expand Down
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
12 changes: 11 additions & 1 deletion src/test/ui/glob-resolve1.stderr
Expand Up @@ -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
Expand Down
98 changes: 91 additions & 7 deletions src/test/ui/issues/issue-73427.stderr
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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 { }
Expand Down

0 comments on commit adf31e9

Please sign in to comment.