- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
Add lint to deny diagnostics composed of static strings #108760
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
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 fixbut 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