-
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
Add lint to deny diagnostics composed of static strings #108760
Conversation
Cool! I like this a lot. Do you plan on landing this as a lint enabled against the compiler, or are you just using it locally? Couldn't tell what your next steps were in "but opening this as a draft in case anyone wants to develop on it.". |
This comment has been minimized.
This comment has been minimized.
I don't think tihs is something that should end up landing since it's reasonably hacky and also hopefully should be a one-time run to apply all the fixes it can. let mut err = struct_span_err(..);
err.help(..);
err.note(..);
err.emit(); |
I guess so -- it would be nice if we could enforce some lint to make sure people don't add new usages of |
Maybe this could be landed )with improved messages) but with all the code that does fixing and file modification cfg'd out? Then the detection part of the lint stays working and the rest is readily available if the lint is enhanced |
Happy to review a version of this lint with all of that logic removed, but would also be open to considering it cfg'd out if it's pretty easily separable from the other logic. Not a huge fan of cfg'ing it out, just because it's like committing commented out code :) but definitely could change my mind. |
☔ The latest upstream changes (presumably #108682) made this pull request unmergeable. Please resolve the merge conflicts. |
@rustbot author |
@clubby789 any thoughts on this? |
ea582c4
to
bf0a59b
Compare
This comment has been minimized.
This comment has been minimized.
bf0a59b
to
09521df
Compare
This comment has been minimized.
This comment has been minimized.
09521df
to
16d2954
Compare
47f4188
to
93bedb9
Compare
This comment has been minimized.
This comment has been minimized.
93bedb9
to
86138c2
Compare
In a future PR I'll probably relax the static string check, since any diagnostic created in a chain of diagnostic functions in a single statement should be easily translatable to derive diagnostics, without requiring subdiagnostics or much refactoring of the logic. |
☔ The latest upstream changes (presumably #106934) made this pull request unmergeable. Please resolve the merge conflicts. |
86138c2
to
81f09e8
Compare
81f09e8
to
8f8ec29
Compare
(oops, I left this as ghost) |
☔ The latest upstream changes (presumably #110325) made this pull request unmergeable. Please resolve the merge conflicts. |
8f8ec29
to
0138513
Compare
Thanks @clubby789! Using a lint to find these was a really good idea 👍 @bors r+ rollup |
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#108760 (Add lint to deny diagnostics composed of static strings) - rust-lang#109444 (Change tidy error message for TODOs) - rust-lang#110419 (Spelling library) - rust-lang#110550 (Suggest deref on comparison binop RHS even if type is not Copy) - rust-lang#110641 (Add new rustdoc book chapter to describe in-doc settings) - rust-lang#110798 (pass `unused_extern_crates` in `librustdoc::doctest::make_test`) - rust-lang#110819 (simplify TrustedLen impls) - rust-lang#110825 (diagnostics: add test case for already-solved issue) - rust-lang#110835 (Make some region folders a little stricter.) - rust-lang#110847 (rustdoc-json: Time serialization.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…ial, r=compiler-errors Migrate trivially translatable `rustc_parse` diagnostics cc rust-lang#100717 Migrate diagnostics in `rustc_parse` which are emitted in a single statement. I worked on this by expanding the lint introduced in rust-lang#108760, although that isn't included here as there is much more work to be done to satisfy it
…ial, r=compiler-errors Migrate trivially translatable `rustc_parse` diagnostics cc rust-lang#100717 Migrate diagnostics in `rustc_parse` which are emitted in a single statement. I worked on this by expanding the lint introduced in rust-lang#108760, although that isn't included here as there is much more work to be done to satisfy it
r? ghost
I'm hoping to have a lint that semi-automatically converts simple diagnostics such as
struct_span_err(span, "msg").help("msg").span_note(span2, "msg").emit()
to typed session diagnostics. It's quite hacky and not entirely working because of problems withx fix
but should hopefully help reduce some of the work.I'm going to start trying to apply what I can from this, but opening this as a draft in case anyone wants to develop on it.
cc #100717