Skip to content

Highlight when errors have automatically applicable suggestions #59103

Open

Description

@davidtwco

With tool_only_span_diagnostics having landed, some suggestions that can be automatically applied aren't shown to the user as labels with help: ... messages, so users who look for that to know they can automatically apply the fix won't know about those suggestions.

We should add some indicator that tells the user that an error can be automatically fixed.

See this conversation.

Activity

  1. added
    C-enhancementCategory: An issue proposing an enhancement or a PR with one.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    A-suggestion-diagnosticsArea: Suggestions generated by the compiler applied by `cargo fix`
    on Mar 11, 2019
  2. estebank commented on Mar 11, 2019

    @estebank
    Contributor

    I believe some iteration of the following would be appropriate:

    error[E0252]*: the name `X` is defined multiple times
      --> $DIR/double-type-import.rs:3:9
       |
    LL |     pub use self::bar::X;
       |             ------------ previous import of the type `X` here
    LL |     use self::bar::X;
       |         ^^^^^^^^^^^^ `X` reimported here
       |
       = note: `X` must be defined only once in the type namespace of this module
    
    error: aborting due to previous error
    
    For more information about this error, try `rustc --explain E0252`.
    * `rustfix` applicable suggestion is available for this error
    

    With proper coloring, this should be easy to follow.

    Does rustfix apply MaybeIncorrect suggestions? If not, this should be only included for MachineApplicable suggestions.

    CC @killercup

  3. killercup commented on Mar 11, 2019

    @killercup
    Contributor
  4. estebank commented on Mar 11, 2019

    @estebank
    Contributor

    In that case, we should only provide the asterisk (or whatever we land on) for MachineApplicable suggestions.

    The position of the asterisk and the last line text is bikeshedable to death:

    error[E0252]*: the name `X` is defined multiple times
    *error[E0252]: the name `X` is defined multiple times
    error*[E0252]: the name `X` is defined multiple times
    error[E0252]: the name `X` is defined multiple times*
    error[E0252]:* the name `X` is defined multiple times
    
  5. davidtwco commented on Mar 11, 2019

    @davidtwco
    MemberAuthor

    I'll kick off the bikeshed by saying that I prefer the fourth option.

  6. killercup commented on Mar 11, 2019

    @killercup
    Contributor

    +1 this comment if you think 1st option is best option

  7. estebank commented on Mar 15, 2019

    @estebank
    Contributor

    I personally feel that all the presented options are suboptimal, but I don't care enough to fight too much. I think overall that either 1 or 4 lead the pack. Vote on the individual comments below for your preference.

  8. estebank commented on Mar 15, 2019

    @estebank
    Contributor
    error[E0252]*: the name `X` is defined multiple times
    
  9. estebank commented on Mar 15, 2019

    @estebank
    Contributor
    *error[E0252]: the name `X` is defined multiple times
    
  10. estebank commented on Mar 15, 2019

    @estebank
    Contributor
    error*[E0252]: the name `X` is defined multiple times
    
  11. estebank commented on Mar 15, 2019

    @estebank
    Contributor
    error[E0252]: the name `X` is defined multiple times*
    
  12. estebank commented on Mar 15, 2019

    @estebank
    Contributor
    error[E0252]:* the name `X` is defined multiple times
    
  13. estebank commented on Mar 15, 2019

    @estebank
    Contributor
    error[E0252*]: the name `X` is defined multiple times
    
  14. estebank commented on Mar 15, 2019

    @estebank
    Contributor

    bikeshed-signal to @rust-lang/wg-diagnostics

  15. oli-obk commented on Mar 15, 2019

    @oli-obk
    Contributor

    Won't that asterisk (or any other symbol) just be very confusing if you don't already know what to look for? I'd prefer a nonsigil solution, so something like

    autofixable error[E0252]: the name `X` is defined multiple times
    
  16. oli-obk commented on Mar 15, 2019

    @oli-obk
    Contributor

    or

    error[E0252] (autofixable): the name `X` is defined multiple times
    
  17. oli-obk commented on Mar 15, 2019

    @oli-obk
    Contributor

    or

    error[E0252]: the name `X` is defined multiple times (automatically fixable)
    
  18. davidtwco commented on Mar 15, 2019

    @davidtwco
    MemberAuthor

    Perhaps if --teach is provided (or something similar to that) then we could add a (* means that the error is automatically fixable) somewhere (or similar). Otherwise I'd be concerned that it will just be a lot of noise/clutter for those who do know what it means.

  19. tesuji commented on Mar 15, 2019

    @tesuji
    Contributor

    s/autofixable/rustfix/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-suggestion-diagnosticsArea: Suggestions generated by the compiler applied by `cargo fix`C-enhancementCategory: An issue proposing an enhancement or a PR with one.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.WG-diagnosticsWorking group: Diagnostics

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions