Skip to content

Tweak suggestions when using incorrect type of enum literal #127891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 19, 2024
Merged

Tweak suggestions when using incorrect type of enum literal #127891

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2259db6
Add svg test for incorrect literal type suggestion
estebank Jul 10, 2024
ec7a188
More accurate suggestions when writing wrong style of enum variant li…
estebank Jul 11, 2024
33bd4bd
Tweak "field not found" suggestion when giving struct literal for tup…
estebank Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions compiler/rustc_hir_typeck/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2218,8 +2218,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);

let variant_ident_span = self.tcx.def_ident_span(variant.def_id).unwrap();
match variant.ctor_kind() {
Some(CtorKind::Fn) => match ty.kind() {
match variant.ctor {
Some((CtorKind::Fn, def_id)) => match ty.kind() {
ty::Adt(adt, ..) if adt.is_enum() => {
err.span_label(
variant_ident_span,
Expand All @@ -2230,28 +2230,44 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
),
);
err.span_label(field.ident.span, "field does not exist");
let fn_sig = self.tcx.fn_sig(def_id).instantiate_identity();
let inputs = fn_sig.inputs().skip_binder();
let fields = format!(
"({})",
inputs.iter().map(|i| format!("/* {i} */")).collect::<Vec<_>>().join(", ")
);
let (replace_span, sugg) = match expr.kind {
hir::ExprKind::Struct(qpath, ..) => {
(qpath.span().shrink_to_hi().with_hi(expr.span.hi()), fields)
}
_ => {
(expr.span, format!("{ty}::{variant}{fields}", variant = variant.name))
}
};
err.span_suggestion_verbose(
expr.span,
replace_span,
format!(
"`{adt}::{variant}` is a tuple {kind_name}, use the appropriate syntax",
adt = ty,
variant = variant.name,
),
format!(
"{adt}::{variant}(/* fields */)",
adt = ty,
variant = variant.name,
),
sugg,
Applicability::HasPlaceholders,
);
}
_ => {
err.span_label(variant_ident_span, format!("`{ty}` defined here"));
err.span_label(field.ident.span, "field does not exist");
let fn_sig = self.tcx.fn_sig(def_id).instantiate_identity();
let inputs = fn_sig.inputs().skip_binder();
let fields = format!(
"({})",
inputs.iter().map(|i| format!("/* {i} */")).collect::<Vec<_>>().join(", ")
);
err.span_suggestion_verbose(
expr.span,
format!("`{ty}` is a tuple {kind_name}, use the appropriate syntax",),
format!("{ty}(/* fields */)"),
format!("{ty}{fields}"),
Applicability::HasPlaceholders,
);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/issues/issue-4736.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ LL | let z = NonCopyable{ p: () };
|
help: `NonCopyable` is a tuple struct, use the appropriate syntax
|
LL | let z = NonCopyable(/* fields */);
| ~~~~~~~~~~~~~~~~~~~~~~~~~
LL | let z = NonCopyable(/* () */);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one's a little confusing because () is the name of the type and its one value. But it's probably not worth specializing it to removes the comment markers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a "Give us a default value for this type" logic. I do think that this and others are edge cases that we can ignore for now.

| ~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 1 previous error

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/issues/issue-80607.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ LL | Enum::V1 { x }
|
help: `Enum::V1` is a tuple variant, use the appropriate syntax
|
LL | Enum::V1(/* fields */)
| ~~~~~~~~~~~~~~~~~~~~~~
LL | Enum::V1(/* i32 */)
| ~~~~~~~~~~~

error: aborting due to 1 previous error

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/numeric/numeric-fields.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ LL | let s = S{0b1: 10, 0: 11};
|
help: `S` is a tuple struct, use the appropriate syntax
|
LL | let s = S(/* fields */);
| ~~~~~~~~~~~~~~~
LL | let s = S(/* u8 */, /* u16 */);
| ~~~~~~~~~~~~~~~~~~~~~~

error[E0026]: struct `S` does not have a field named `0x1`
--> $DIR/numeric-fields.rs:7:17
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/suggestions/incorrect-variant-literal.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 16 additions & 16 deletions tests/ui/suggestions/nested-non-tuple-tuple-struct.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ LL | let _x = (S { x: 1.0, y: 2.0 }, S { x: 3.0, y: 4.0 });
|
help: `S` is a tuple struct, use the appropriate syntax
|
LL | let _x = (S(/* fields */), S { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~
LL | let _x = (S(/* f32 */, /* f32 */), S { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~~~~~~~~~
Comment on lines 10 to +13
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nnethercote this is a case where the "reuse the literal values that were already there" gets harder: should it have been S(3.0, 4.0) or S(4.0, 3.0)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing these help suggestions aren't required to be 100% correct, right? Though of course we want them to be as close to 100% as reasonable.

So I would answer S(3.0, 4.0), because that matches the order the float literals appear in the code. Sure, it's possible that somebody wrote the literal with the fields in a different order than in the type declaration, but I would expect that is less likely than matching the type declaration order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we ever handle this case, I'd only hazard a guess on moving existing code when there is no ambiguity, and leave the comment placeholder for cases like this one so that the user has to make a determination manually (with IDEs a user could accidentally accept a suggestion without noticing the subtlety of what happened).


error[E0560]: struct `S` has no field named `y`
--> $DIR/nested-non-tuple-tuple-struct.rs:8:27
Expand All @@ -23,8 +23,8 @@ LL | let _x = (S { x: 1.0, y: 2.0 }, S { x: 3.0, y: 4.0 });
|
help: `S` is a tuple struct, use the appropriate syntax
|
LL | let _x = (S(/* fields */), S { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~
LL | let _x = (S(/* f32 */, /* f32 */), S { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~~~~~~~~~

error[E0560]: struct `S` has no field named `x`
--> $DIR/nested-non-tuple-tuple-struct.rs:8:41
Expand All @@ -37,8 +37,8 @@ LL | let _x = (S { x: 1.0, y: 2.0 }, S { x: 3.0, y: 4.0 });
|
help: `S` is a tuple struct, use the appropriate syntax
|
LL | let _x = (S { x: 1.0, y: 2.0 }, S(/* fields */));
| ~~~~~~~~~~~~~~~
LL | let _x = (S { x: 1.0, y: 2.0 }, S(/* f32 */, /* f32 */));
| ~~~~~~~~~~~~~~~~~~~~~~~

error[E0560]: struct `S` has no field named `y`
--> $DIR/nested-non-tuple-tuple-struct.rs:8:49
Expand All @@ -51,8 +51,8 @@ LL | let _x = (S { x: 1.0, y: 2.0 }, S { x: 3.0, y: 4.0 });
|
help: `S` is a tuple struct, use the appropriate syntax
|
LL | let _x = (S { x: 1.0, y: 2.0 }, S(/* fields */));
| ~~~~~~~~~~~~~~~
LL | let _x = (S { x: 1.0, y: 2.0 }, S(/* f32 */, /* f32 */));
| ~~~~~~~~~~~~~~~~~~~~~~~

error[E0559]: variant `E::V` has no field named `x`
--> $DIR/nested-non-tuple-tuple-struct.rs:13:22
Expand All @@ -65,8 +65,8 @@ LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V { x: 3.0, y: 4.0 });
|
help: `E::V` is a tuple variant, use the appropriate syntax
|
LL | let _y = (E::V(/* fields */), E::V { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~~~~
LL | let _y = (E::V(/* f32 */, /* f32 */), E::V { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~~~~~~~~

error[E0559]: variant `E::V` has no field named `y`
--> $DIR/nested-non-tuple-tuple-struct.rs:13:30
Expand All @@ -79,8 +79,8 @@ LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V { x: 3.0, y: 4.0 });
|
help: `E::V` is a tuple variant, use the appropriate syntax
|
LL | let _y = (E::V(/* fields */), E::V { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~~~~
LL | let _y = (E::V(/* f32 */, /* f32 */), E::V { x: 3.0, y: 4.0 });
| ~~~~~~~~~~~~~~~~~~~~~~

error[E0559]: variant `E::V` has no field named `x`
--> $DIR/nested-non-tuple-tuple-struct.rs:13:47
Expand All @@ -93,8 +93,8 @@ LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V { x: 3.0, y: 4.0 });
|
help: `E::V` is a tuple variant, use the appropriate syntax
|
LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V(/* fields */));
| ~~~~~~~~~~~~~~~~~~
LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V(/* f32 */, /* f32 */));
| ~~~~~~~~~~~~~~~~~~~~~~

error[E0559]: variant `E::V` has no field named `y`
--> $DIR/nested-non-tuple-tuple-struct.rs:13:55
Expand All @@ -107,8 +107,8 @@ LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V { x: 3.0, y: 4.0 });
|
help: `E::V` is a tuple variant, use the appropriate syntax
|
LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V(/* fields */));
| ~~~~~~~~~~~~~~~~~~
LL | let _y = (E::V { x: 1.0, y: 2.0 }, E::V(/* f32 */, /* f32 */));
| ~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 8 previous errors

Expand Down
Loading