resolve: further improvements to "try using the enum's variant" diagnostic #77855
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
bugadani
reviewed
Oct 12, 2020
davidtwco
force-pushed
the
pr-77341-follow-up-non-constructable-variants
branch
from
d5f9e40
to
16e16aa
Compare
estebank
reviewed
Oct 14, 2020
Comment on lines
71
to
146
| help: try using one of the enum's variants | ||
| help: try using the enum's variant which have fields | ||
| | | ||
| LL | let _: E = E::Unit; | ||
| | ^^^^^^^ | ||
| help: try using one of the enum's variants which have fields | ||
| | | ||
| LL | let _: E = (E::Fn(/* fields */)); | ||
| | ^^^^^^^^^^^^^^^^^^^^^ | ||
| LL | let _: E = (E::Struct { /* fields */ }); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
There was a problem hiding this comment.
What about
help: you might have meant to use the following enum variant(s)
|
help: alternatively, the following enum variants are also available
|
There was a problem hiding this comment.
I've updated the PR to use this wording.
Comment on lines
1385
to
1393
| 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, | ||
| } | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
I think it would be useful to either include the field names in the suggestion (instead of /*fields*/) with some placeholder per field, or to have a span pointing at the def, otherwise you are forcing people to consult the docs/code to fix the issue.
There was a problem hiding this comment.
I've added a note that points at the definition of the enum, this seemed more straightforward than adjusting the suggestions while still having the desired effect.
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]>
davidtwco
force-pushed
the
pr-77341-follow-up-non-constructable-variants
branch
from
16e16aa
to
f897162
Compare
|
@bors r+ |
|
📌 Commit f897162 has been approved by |
bors
added
S-waiting-on-bors
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Oct 16, 2020
…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
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Oct 16, 2020
…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
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 16, 2020
Rollup of 10 pull requests Successful merges: - rust-lang#75209 (Suggest imports of unresolved macros) - rust-lang#77547 (stabilize union with 'ManuallyDrop' fields and 'impl Drop for Union') - rust-lang#77827 (Don't link to nightly primitives on stable channel) - rust-lang#77855 (resolve: further improvements to "try using the enum's variant" diagnostic) - rust-lang#77900 (Use fdatasync for File::sync_data on more OSes) - rust-lang#77925 (Suggest minimal subset features in `incomplete_features` lint) - rust-lang#77971 (Deny broken intra-doc links in linkchecker) - rust-lang#77991 (Bump backtrace-rs) - rust-lang#77992 (instrument-coverage: try our best to not ICE) - rust-lang#78013 (Fix sidebar scroll on mobile devices) Failed merges: r? `@ghost`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up on #77341 (comment).
This PR 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). 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