-
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 for items deprecated in future #56203
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
This looks good! Just a couple of minor comments and then it'll be good to go!
r? @varkor |
@@ -0,0 +1,16 @@ | |||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or |
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.
@varkor, should I rename this file to deprecation_in_future-lint.rs or something similar?
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
You probably have to add |
That explains it. Thank you! |
Sorry for taking a while to get back on this. If you could get rid of the unwrap by adding the |
☔ The latest upstream changes (presumably #56502) made this pull request unmergeable. Please resolve the merge conflicts. |
@@ -614,8 +639,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { | |||
if let Some(&Stability{rustc_depr: Some(attr::RustcDeprecation { reason, since }), ..}) | |||
= stability { | |||
if let Some(id) = id { | |||
let path = self.item_path_str(def_id); | |||
let message = format!("use of deprecated item '{}'", path); | |||
if deprecation_in_effect(&since.as_str()) { |
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.
Sorry, I didn't notice before, but we need to make the same changes here. This warning is for deprecated items inside the standard library, etc. It should be affected by DEPRECATED_IN_FUTURE
as well.
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.
No problem. Done in 5c79eab88e6e71134175a7dce9795c8f8f3f4949
If this is acceptable I'll squash the commits 😃
Thanks! Just one final comment (which is particularly important for addressing #55892). |
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.
Great! r=me after the extra blank line is removed (it can be amend
ed on to the previous commit).
Add lint for items deprecated in future Resolves rust-lang#55892
Add lint for items deprecated in future Resolves rust-lang#55892
Add lint for items deprecated in future Resolves rust-lang#55892
Add lint for items deprecated in future Resolves rust-lang#55892
Add lint for items deprecated in future Resolves rust-lang#55892
Add lint for items deprecated in future Resolves rust-lang#55892
Add lint for items deprecated in future Resolves rust-lang#55892
@bors rollup- |
Add lint for items deprecated in future Resolves #55892
☀️ Test successful - status-appveyor, status-travis |
Resolves #55892