rustc: Fix crate lint for single-item paths
#50665
+287
−47
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -3232,6 +3232,7 @@ impl<'a> Resolver<'a> { | ||
| -> PathResult<'a> { | ||
| let mut module = None; | ||
| let mut allow_super = true; | ||
| let mut second_binding = None; | ||
|
|
||
| for (i, &ident) in path.iter().enumerate() { | ||
| debug!("resolve_path ident {} {:?}", i, ident); | ||
| @@ -3321,7 +3322,9 @@ impl<'a> Resolver<'a> { | ||
| .map(MacroBinding::binding) | ||
| } else { | ||
| match self.resolve_ident_in_lexical_scope(ident, ns, record_used, path_span) { | ||
| // we found a locally-imported or available item/module | ||
| Some(LexicalScopeBinding::Item(binding)) => Ok(binding), | ||
| // we found a local variable or type param | ||
| Some(LexicalScopeBinding::Def(def)) | ||
| if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => { | ||
| return PathResult::NonModule(PathResolution::with_unresolved_segments( | ||
| @@ -3334,13 +3337,22 @@ impl<'a> Resolver<'a> { | ||
|
|
||
| match binding { | ||
| Ok(binding) => { | ||
| if i == 1 { | ||
| second_binding = Some(binding); | ||
| } | ||
| 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. we should do this above as well, yes? After There was a problem hiding this comment. 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. |
||
| node_id, | ||
| path, | ||
| path_span, | ||
| second_binding, | ||
| ); | ||
| return PathResult::NonModule(PathResolution::with_unresolved_segments( | ||
| def, path.len() - i - 1 | ||
| )); | ||
| @@ -3349,33 +3361,6 @@ impl<'a> Resolver<'a> { | ||
| format!("Not a module `{}`", ident), | ||
| is_last); | ||
| } | ||
| } | ||
| Err(Undetermined) => return PathResult::Indeterminate, | ||
| Err(Determined) => { | ||
| @@ -3408,9 +3393,77 @@ impl<'a> Resolver<'a> { | ||
| } | ||
| } | ||
|
|
||
| self.lint_if_path_starts_with_module(node_id, path, path_span, second_binding); | ||
|
|
||
| PathResult::Module(module.unwrap_or(self.graph_root)) | ||
| } | ||
|
|
||
| fn lint_if_path_starts_with_module(&self, | ||
| id: Option<NodeId>, | ||
| path: &[Ident], | ||
| path_span: Span, | ||
| second_binding: Option<&NameBinding>) { | ||
| let id = match id { | ||
| Some(id) => id, | ||
| None => return, | ||
| }; | ||
|
|
||
| let first_name = match path.get(0) { | ||
| Some(ident) => ident.name, | ||
| None => return, | ||
| }; | ||
|
|
||
| // We're only interested in `use` paths which should start with | ||
| // `{{root}}` or `extern` currently. | ||
| if first_name != keywords::Extern.name() && first_name != keywords::CrateRoot.name() { | ||
| return | ||
| } | ||
|
|
||
| match path.get(1) { | ||
| // If this import looks like `crate::...` it's already good | ||
| Some(name) if name.name == keywords::Crate.name() => return, | ||
| // Otherwise go below to see if it's an extern crate | ||
| Some(_) => {} | ||
| // If the path has length one (and it's `CrateRoot` most likely) | ||
| // then we don't know whether we're gonna be importing a crate or an | ||
| // item in our crate. Defer this lint to elsewhere | ||
| None => return, | ||
| } | ||
|
|
||
| // If the first element of our path was actually resolved to an | ||
| // `ExternCrate` (also used for `crate::...`) then no need to issue a | ||
| // warning, this looks all good! | ||
| if let Some(binding) = second_binding { | ||
| if let NameBindingKind::Import { directive: d, .. } = binding.kind { | ||
| if let ImportDirectiveSubclass::ExternCrate(..) = d.subclass { | ||
| return | ||
| } | ||
| } | ||
| } | ||
|
|
||
| self.lint_path_starts_with_module(id, path_span); | ||
| } | ||
|
|
||
| fn lint_path_starts_with_module(&self, id: NodeId, span: Span) { | ||
| // In the 2018 edition this lint is a hard error, so nothing to do | ||
| if self.session.rust_2018() { | ||
| return | ||
| } | ||
| // In the 2015 edition there's no use in emitting lints unless the | ||
| // crate's already enabled the feature that we're going to suggest | ||
| if !self.session.features_untracked().crate_in_paths { | ||
| return | ||
| } | ||
| let diag = lint::builtin::BuiltinLintDiagnostics | ||
| ::AbsPathWithModule(span); | ||
| self.session.buffer_lint_with_diagnostic( | ||
| lint::builtin::ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE, | ||
| id, span, | ||
| "absolute paths must start with `self`, `super`, \ | ||
| `crate`, or an external crate name in the 2018 edition", | ||
| diag); | ||
| } | ||
|
|
||
| // Resolve a local definition, potentially adjusting for closures. | ||
| fn adjust_local_def(&mut self, | ||
| ns: Namespace, | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // Copyright 2018 The Rust Project Developers. See the COPYRIGHT | ||
| // file at the top-level directory of this distribution and at | ||
| // http://rust-lang.org/COPYRIGHT. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
| // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
| // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
| // option. This file may not be copied, modified, or distributed | ||
| // except according to those terms. | ||
|
|
||
| pub fn foo() {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // Copyright 2018 The Rust Project Developers. See the COPYRIGHT | ||
| // file at the top-level directory of this distribution and at | ||
| // http://rust-lang.org/COPYRIGHT. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
| // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
| // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
| // option. This file may not be copied, modified, or distributed | ||
| // except according to those terms. | ||
|
|
||
| // aux-build:edition-lint-paths.rs | ||
| // run-rustfix | ||
|
|
||
| #![feature(rust_2018_preview)] | ||
| #![deny(absolute_path_not_starting_with_crate)] | ||
| #![allow(unused)] | ||
|
|
||
| extern crate edition_lint_paths; | ||
|
|
||
| pub mod foo { | ||
| use edition_lint_paths; | ||
| use crate::bar::Bar; | ||
| //~^ ERROR absolute | ||
| //~| WARN this was previously accepted | ||
| use super::bar::Bar2; | ||
| use crate::bar::Bar3; | ||
|
|
||
| use crate::bar; | ||
| //~^ ERROR absolute | ||
| //~| WARN this was previously accepted | ||
| use crate::{bar as something_else}; | ||
|
|
||
| use {crate::Bar as SomethingElse, crate::main}; | ||
| //~^ ERROR absolute | ||
| //~| WARN this was previously accepted | ||
| //~| ERROR absolute | ||
| //~| WARN this was previously accepted | ||
|
|
||
| use crate::{Bar as SomethingElse2, main as another_main}; | ||
|
|
||
| pub fn test() { | ||
| } | ||
| } | ||
|
|
||
| use crate::bar::Bar; | ||
| //~^ ERROR absolute | ||
| //~| WARN this was previously accepted | ||
|
|
||
| pub mod bar { | ||
| use edition_lint_paths as foo; | ||
| pub struct Bar; | ||
| pub type Bar2 = Bar; | ||
| pub type Bar3 = Bar; | ||
| } | ||
|
|
||
| mod baz { | ||
| use crate::*; | ||
| //~^ ERROR absolute | ||
| //~| WARN this was previously accepted | ||
| } | ||
|
|
||
| fn main() { | ||
| let x = crate::bar::Bar; | ||
| //~^ ERROR absolute | ||
| //~| WARN this was previously accepted | ||
| let x = bar::Bar; | ||
| let x = ::crate::bar::Bar; | ||
| let x = self::bar::Bar; | ||
| foo::test(); | ||
|
|
||
| { | ||
| use edition_lint_paths as bar; | ||
| edition_lint_paths::foo(); | ||
| bar::foo(); | ||
| ::edition_lint_paths::foo(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,34 +1,70 @@ | ||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:22:9 | ||
| | | ||
| LL | use ::bar::Bar; | ||
| | ^^^^^^^^^^ help: use `crate`: `crate::bar::Bar` | ||
| | | ||
| note: lint level defined here | ||
| --> $DIR/edition-lint-paths.rs:15:9 | ||
| | | ||
| LL | #![deny(absolute_path_not_starting_with_crate)] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:28:9 | ||
| | | ||
| LL | use bar; | ||
| | ^^^ help: use `crate`: `crate::bar` | ||
| | | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:33:10 | ||
| | | ||
| LL | use {Bar as SomethingElse, main}; | ||
| | ^^^^^^^^^^^^^^^^^^^^ help: use `crate`: `crate::Bar as SomethingElse` | ||
| | | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:33:32 | ||
| | | ||
| LL | use {Bar as SomethingElse, main}; | ||
| | ^^^^ help: use `crate`: `crate::main` | ||
| | | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:45:5 | ||
| | | ||
| LL | use bar::Bar; | ||
| | ^^^^^^^^ help: use `crate`: `crate::bar::Bar` | ||
| | | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:57:9 | ||
| | | ||
| LL | use *; | ||
| | ^ help: use `crate`: `crate::*` | ||
| | | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition | ||
| --> $DIR/edition-lint-paths.rs:63:13 | ||
| | | ||
| LL | let x = ::bar::Bar; | ||
| | ^^^^^^^^^^ help: use `crate`: `crate::bar::Bar` | ||
| | | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition! | ||
| = note: for more information, see issue TBD | ||
|
|
||
| error: aborting due to 7 previous errors | ||
|
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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