-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
Suggest the correct pattern syntax on usage of unit variant pattern for a struct variant #129520
Suggest the correct pattern syntax on usage of unit variant pattern for a struct variant #129520
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
compiler/rustc_hir_typeck/src/lib.rs
Outdated
} else { | ||
format!(" {{ {} }}", fields.join(", ")) | ||
}, | ||
Applicability::MachineApplicable, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two suggestions cannot both be MachineApplicable
. I would downgrade this down to HasPlaceholders
if there are variants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or honestly, it may be worth just suggesting field: _, field: _
unconditionally (or ..
if there are.. e.g. > 4 fields), and adjusting the wording to note that they're being ignored. Two suggestions seems a bit excessive...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, use span_suggestions
which supports MUTUALLY EXCLUSIVE suggestions. You'll need to make a single wording for both of them, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I fixed the diagnostic to just suggest a placeholder.
compiler/rustc_hir_typeck/src/lib.rs
Outdated
err.span_suggestion_verbose( | ||
qpath.span().shrink_to_hi().to(span.shrink_to_hi()), | ||
"the struct variant's fields are being ignored", | ||
" { /* field: _, field: _ */ }", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, I meant taking the fields themselves and emitting for each field name name: _
. Like,
struct Foo { a: i32, b: i32 }
would result in Foo { a: _, b: _ }
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I'll fix it. sorry for my misunderstanding 😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, when there are no fields, I provide a diagnostic use the struct variant pattern syntax
instead of the struct variant's field{s} {are} being ignored
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls adjust the suggestion sorry I wasn't clear 😅
@rustbot author |
052eb70
to
ada9ba3
Compare
@rustbot review |
ada9ba3
to
f71979b
Compare
oh, I forget to remove a comment out from the suggestion. @rustbot author |
…ttern for a struct variant
f71979b
to
2ddcbca
Compare
Thanks for the review! I fixed and rebased @rustbot review |
@bors r+ rollup |
…attern-syntax, r=compiler-errors Suggest the correct pattern syntax on usage of unit variant pattern for a struct variant Closes rust-lang#126243 I add a suggestion on usage of unit variant pattern for a struct variant.
…kingjubilee Rollup of 14 pull requests Successful merges: - rust-lang#129260 (Don't suggest adding return type for closures with default return type) - rust-lang#129520 (Suggest the correct pattern syntax on usage of unit variant pattern for a struct variant) - rust-lang#129696 (update stdarch) - rust-lang#129759 (Stabilize `const_refs_to_static`) - rust-lang#129835 (enable const-float-classify test, and test_next_up/down on 32bit x86) - rust-lang#129866 (Clarify documentation labelling and definitions for std::collections) - rust-lang#130052 (Don't leave debug locations for constants sitting on the builder indefinitely) - rust-lang#130077 (Fix linking error when compiling for 32-bit watchOS) - rust-lang#130123 (Report the `note` when specified in `diagnostic::on_unimplemented`) - rust-lang#130156 (Add test for S_OBJNAME & update test for LF_BUILDINFO cl and cmd) - rust-lang#130206 (Map `WSAEDQUOT` to `ErrorKind::FilesystemQuotaExceeded`) - rust-lang#130207 (Map `ERROR_CANT_RESOLVE_FILENAME` to `ErrorKind::FilesystemLoop`) - rust-lang#130219 (Fix false positive with `missing_docs` and `#[test]`) - rust-lang#130221 (Make SearchPath::new public) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#129260 (Don't suggest adding return type for closures with default return type) - rust-lang#129520 (Suggest the correct pattern syntax on usage of unit variant pattern for a struct variant) - rust-lang#129866 (Clarify documentation labelling and definitions for std::collections) - rust-lang#130123 (Report the `note` when specified in `diagnostic::on_unimplemented`) - rust-lang#130161 (refactor merge base logic and fix `x fmt`) - rust-lang#130206 (Map `WSAEDQUOT` to `ErrorKind::FilesystemQuotaExceeded`) - rust-lang#130207 (Map `ERROR_CANT_RESOLVE_FILENAME` to `ErrorKind::FilesystemLoop`) - rust-lang#130219 (Fix false positive with `missing_docs` and `#[test]`) - rust-lang#130221 (Make SearchPath::new public) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#129520 - tunawasabi:suggest-adding-struct-pattern-syntax, r=compiler-errors Suggest the correct pattern syntax on usage of unit variant pattern for a struct variant Closes rust-lang#126243 I add a suggestion on usage of unit variant pattern for a struct variant.
Closes #126243
I add a suggestion on usage of unit variant pattern for a struct variant.