resolve: improve "try using the enum's variant" #77341
Conversation
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 <[email protected]>
rust-highfive
added
the
S-waiting-on-review
|
I'm slightly concerned that by not showing suggestions for the variants with fields we might mislead people into thinking those don't exist. I also notice that we only check that the expression is tuple variant-like only, we do not check that the number of fields match the suggested variants. In my mind we would provide suggestions for the cases that don't match (like a struct variant with multiple fields) with placeholders. I like the new logic overall otherwise and appreciate moving it to a new method. |
| 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"); | ||
| } |
There was a problem hiding this comment.
I would prefer this to somehow suggest the non-tuple variants with placeholders. It would also mean getting the full span, if I'm reading correctly.
I tried to alleviate this with the help notes that the PR adds.
I was tempted to do this, but the PR was already changing a lot and I wasn't sure how complicated it would end up ( |
|
Ok, I'm r+ but let's wait until the new beta is cut-off to merge so that we have time to do the follow up work with a relaxed timeline so that we land the whole thing on the same stable release. |
|
Beta cutoff was last Friday -- you can check src/version for the current version number generally speaking and if that's nightly+1 according to https://forge.rust-lang.org/ then we're in the new cycle. |
|
Neat, for some reason I conflated the stable and beta release dates (which of course make no sense to be tied together). @bors r+ |
|
📌 Commit 9ef68f5 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions, checks-azure |
This commit improves the diagnostic modified in rust-lang#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]>
This commit improves the tuple struct case added in rust-lang#77341 so that the context is mentioned in more of the message. Signed-off-by: David Wood <[email protected]>
…nstructable-variants, r=estebank resolve: further improvements to "try using the enum's variant" diagnostic Follow-up on rust-lang#77341 (comment). This PR improves the diagnostic modified in rust-lang#77341 to suggest not only those variants which do not have fields, but those with fields (by suggesting with placeholders). In addition, the wording of the tuple-variant-only case is improved slightly. I've not made further changes to the tuple-variant-only case (e.g. to only suggest variants with the correct number of fields) because I don't think I have enough information to do so reliably (e.g. in the case where there is an attempt to construct a tuple variant, I have no information on how many fields were provided; and in the case of pattern matching, I only have a slice of spans and would need to check for things like `..` in those spans, which doesn't seem worth it). r? @estebank
…nstructable-variants, r=estebank resolve: further improvements to "try using the enum's variant" diagnostic Follow-up on rust-lang#77341 (comment). This PR improves the diagnostic modified in rust-lang#77341 to suggest not only those variants which do not have fields, but those with fields (by suggesting with placeholders). In addition, the wording of the tuple-variant-only case is improved slightly. I've not made further changes to the tuple-variant-only case (e.g. to only suggest variants with the correct number of fields) because I don't think I have enough information to do so reliably (e.g. in the case where there is an attempt to construct a tuple variant, I have no information on how many fields were provided; and in the case of pattern matching, I only have a slice of spans and would need to check for things like `..` in those spans, which doesn't seem worth it). r? @estebank
…nstructable-variants, r=estebank resolve: further improvements to "try using the enum's variant" diagnostic Follow-up on rust-lang#77341 (comment). This PR improves the diagnostic modified in rust-lang#77341 to suggest not only those variants which do not have fields, but those with fields (by suggesting with placeholders). In addition, the wording of the tuple-variant-only case is improved slightly. I've not made further changes to the tuple-variant-only case (e.g. to only suggest variants with the correct number of fields) because I don't think I have enough information to do so reliably (e.g. in the case where there is an attempt to construct a tuple variant, I have no information on how many fields were provided; and in the case of pattern matching, I only have a slice of spans and would need to check for things like `..` in those spans, which doesn't seem worth it). r? @estebank
Fixes #73427.
This PR improves the "try using the enum's variant" suggestion:
All of the diagnostic logic introduced by this PR is separated from the normal code path for a successful compilation.
r? @estebank