Skip to content
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

Deprecated suggestion message could be better #98631

Closed
ChrisDenton opened this issue Jun 28, 2022 · 6 comments · Fixed by #98665
Closed

Deprecated suggestion message could be better #98631

ChrisDenton opened this issue Jun 28, 2022 · 6 comments · Fixed by #98665
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ChrisDenton
Copy link
Member

#![feature(deprecated_suggestion)] (#94785) produces warning messages that I think could be improved? The current suggestion message doesn't seem ideal to me:

warning: use of deprecated constant `std::sync::ONCE_INIT`: the `new` function is now preferred
 --> src/main.rs:4:32
  |
4 |     const _: Once = std::sync::ONCE_INIT;
  |                                ^^^^^^^^^ help: replace the use of the deprecated constant: `Once::new()`
  |
  = note: `#[warn(deprecated)]` on by default

Specifically this part sounds to me more like Once::new() is the deprecated constant:

help: replace the use of the deprecated constant: Once::new()

I think something like this would be better, but I don't know how easy it would be to implement a more natural sentence:

help: replace the use of the deprecated constant with Once::new()

@ChrisDenton ChrisDenton added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 28, 2022
@compiler-errors
Copy link
Member

This is because it's a suggestion, but being rendered inline. You could bump it up to a verbose suggestion so it renders more distinctly.

@ChrisDenton
Copy link
Member Author

ChrisDenton commented Jun 29, 2022

Hm, thinking more about it, could it not be simplified to:

help: try replacing with: Once::new()

As then the help message would focus on what's actionable without repeating less essential information.

@compiler-errors
Copy link
Member

compiler-errors commented Jun 29, 2022

The problem is that this is a suggestion, and when suggestions are rendered inline they always are presented like {message}: {code}

edit: whoops, i guess i missed the colon in your revised message. i guess it could be shortened like that.

@ChrisDenton
Copy link
Member Author

Yeah, sorry I edited once I started actually playing with this.

@compiler-errors
Copy link
Member

I think it's meaningful for the suggestion's message to say what it's doing -- "try replacing with" seems a bit terse.

I think I still prefer how this would render if we just bumped the suggestion to a verbose (not inclined) suggestion.

@ChrisDenton
Copy link
Member Author

Ah ok. My thought was just that it's essentially repeating warning message using slightly different language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants