-
Notifications
You must be signed in to change notification settings - Fork 13k
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
rustc: Fix crate
lint for single-item paths
#50665
rustc: Fix crate
lint for single-item paths
#50665
Conversation
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
@bors: p=1 (affects upcoming edition preview) |
let def = binding.def(); | ||
let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(def); | ||
if let Some(next_module) = binding.module() { | ||
module = Some(next_module); | ||
} else if def == Def::Err { | ||
return PathResult::NonModule(err_path_resolution()); | ||
} else if opt_ns.is_some() && (is_last || maybe_assoc) { | ||
self.lint_if_path_starts_with_module( |
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.
we should do this above as well, yes? After resolve_ident_in_lexical_scope
? I'm not sure
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.
Discussed here @Manishearth and I figured that we couldn't find other ways to get resolve to trigger those use cases, so we'll fix them up later if they come up and otherwise we'll move forward w/ this.
@bors r+ p=1 |
📌 Commit 43f6b96 has been approved by |
…arnings, r=Manishearth rustc: Fix `crate` lint for single-item paths This commit fixes recommending the `crate` prefix when migrating to 2018 for paths that look like `use foo;` or `use {bar, baz}` Closes rust-lang#50660
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 |
43f6b96
to
a7222c1
Compare
@bors: r=Manishearth |
📌 Commit a7222c1 has been approved by |
@bors: r- I've found an issue with this that needs to be tweaked |
a7222c1
to
c9585dd
Compare
Ok the bug that I found was that the previous changes would lint for imports like I've done what I hope is a thorough review of librustc_resolve for where 2015/2018 diverge and have added a lint to the appropriate locations (hopefully). Now re-r? @Manishearth |
c9585dd
to
ecb6964
Compare
@@ -653,6 +654,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { | |||
return Some((directive.span, | |||
"cannot glob-import all possible crates".to_string())); | |||
} | |||
GlobImport { .. } if self.session.features_untracked().extern_absolute_paths => { |
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 is something that was accidentally omitted from the lint entirely before, where use *
is completely forbidden in 2018 and so this starts to lint about that in 2015.
Note that there's still a "bug" in the lint for "remove extern crate
", I'll file a bug for that shortly
@bors: p=0 |
r? @oli-obk |
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.
r=me with either action taken
@@ -261,7 +261,7 @@ declare_lint! { | |||
} | |||
|
|||
declare_lint! { | |||
pub ABSOLUTE_PATH_STARTING_WITH_MODULE, | |||
pub ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE, |
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.
The renaming would need to be backported or the old name put into the deprecated lint list
This commit fixes recommending the `crate` prefix when migrating to 2018 for paths that look like `use foo;` or `use {bar, baz}` Closes rust-lang#50660
ecb6964
to
dff9ee1
Compare
@bors: r=oli-ok |
📌 Commit dff9ee1 has been approved by |
er, @bors: r=oli-obk |
💡 This pull request was already approved, no need to approve it again.
|
📌 Commit dff9ee1 has been approved by |
@bors: p=1 relevant for the edition preview, so prioritizing |
…oli-obk rustc: Fix `crate` lint for single-item paths This commit fixes recommending the `crate` prefix when migrating to 2018 for paths that look like `use foo;` or `use {bar, baz}` Closes #50660
☀️ Test successful - status-appveyor, status-travis |
This commit fixes recommending the
crate
prefix when migrating to 2018 forpaths that look like
use foo;
oruse {bar, baz}
Closes #50660