-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Strict provenance lint diagnostics improvements #96112
Strict provenance lint diagnostics improvements #96112
Conversation
Use `multipart_suggestion` and don't suggested unnecessary parenthesis.
Use `multipart_suggestion` instead of getting a snippet.
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
]; | ||
|
||
err.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect); | ||
} else { | ||
err.span_suggestion( |
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.
Is there a reason why this doesn't use multipart_suggestion
like in the branch above?
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.
Because it is not multipart? It is just a single part (single span, single suggestion String
)...
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.
It does crate some inconsistency in the diagnostics depending on whether parenthesis are necessary, so maybe it would actually be better to use a multipart suggestion here too? But it just feels weird to do a multipart suggestion with only one part...
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.
@niluxv If you want the suggestion to always have its own subwindow, you can use span_suggestion_verbose
.
Looks good to me! See the question in-line. |
@bors r+ Ah well, we can make that specific suggestion use multipart later, if needed. |
📌 Commit 1d63d6d has been approved by |
…vements, r=nagisa Strict provenance lint diagnostics improvements Use `multipart_suggestion` instead of `span_suggestion` and getting a snippet for the expression. Also don't suggest unnecessary parenthesis in `lossy_provenance_casts`. cc `@estebank` `@rustbot` label A-diagnostics
Rollup of 6 pull requests Successful merges: - rust-lang#95346 (Stablize `const_extern_fn` for "Rust" and "C") - rust-lang#95933 (htmldocck: Compare HTML tree instead of plain text html) - rust-lang#96105 (Make the debug output for `TargetSelection` less verbose) - rust-lang#96112 (Strict provenance lint diagnostics improvements) - rust-lang#96119 (update Miri) - rust-lang#96124 (to_digit tweak) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Use
multipart_suggestion
instead ofspan_suggestion
and getting a snippet for the expression. Also don't suggest unnecessary parenthesis inlossy_provenance_casts
.cc @estebank
@rustbot label A-diagnostics