Skip to content

Commit

Permalink
Rollup merge of #50665 - alexcrichton:fix-single-item-path-warnings, …
Browse files Browse the repository at this point in the history
…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 #50660
  • Loading branch information
alexcrichton authored May 11, 2018
2 parents 4d81fc8 + 43f6b96 commit 58481c0
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 34 deletions.
4 changes: 2 additions & 2 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ declare_lint! {
}

declare_lint! {
pub ABSOLUTE_PATH_STARTING_WITH_MODULE,
pub ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE,
Allow,
"fully qualified paths that start with a module name \
instead of `crate`, `self`, or an extern crate name"
Expand Down Expand Up @@ -328,7 +328,7 @@ impl LintPass for HardwiredLints {
TYVAR_BEHIND_RAW_POINTER,
ELIDED_LIFETIME_IN_PATH,
BARE_TRAIT_OBJECT,
ABSOLUTE_PATH_STARTING_WITH_MODULE,
ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE,
UNSTABLE_NAME_COLLISION,
)
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extern crate rustc_target;
extern crate syntax_pos;

use rustc::lint;
use rustc::lint::builtin::{BARE_TRAIT_OBJECT, ABSOLUTE_PATH_STARTING_WITH_MODULE};
use rustc::lint::builtin::{BARE_TRAIT_OBJECT, ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE};
use rustc::session;
use rustc::util;

Expand Down Expand Up @@ -282,7 +282,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
// standard library, and thus should never be removed or changed to an error.
},
FutureIncompatibleInfo {
id: LintId::of(ABSOLUTE_PATH_STARTING_WITH_MODULE),
id: LintId::of(ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE),
reference: "issue TBD",
edition: Some(Edition::Edition2018),
},
Expand Down
97 changes: 70 additions & 27 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -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(
node_id,
path,
path_span,
second_binding,
);
return PathResult::NonModule(PathResolution::with_unresolved_segments(
def, path.len() - i - 1
));
Expand All @@ -3349,33 +3361,6 @@ impl<'a> Resolver<'a> {
format!("Not a module `{}`", ident),
is_last);
}

if let Some(id) = node_id {
if i == 1 && self.session.features_untracked().crate_in_paths
&& !self.session.rust_2018() {
let prev_name = path[0].name;
if prev_name == keywords::Extern.name() ||
prev_name == keywords::CrateRoot.name() {
let mut is_crate = false;
if let NameBindingKind::Import { directive: d, .. } = binding.kind {
if let ImportDirectiveSubclass::ExternCrate(..) = d.subclass {
is_crate = true;
}
}

if !is_crate {
let diag = lint::builtin::BuiltinLintDiagnostics
::AbsPathWithModule(path_span);
self.session.buffer_lint_with_diagnostic(
lint::builtin::ABSOLUTE_PATH_STARTING_WITH_MODULE,
id, path_span,
"Absolute paths must start with `self`, `super`, \
`crate`, or an external crate name in the 2018 edition",
diag);
}
}
}
}
}
Err(Undetermined) => return PathResult::Indeterminate,
Err(Determined) => {
Expand Down Expand Up @@ -3408,9 +3393,67 @@ 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>) {
// 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 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
}

if let Some(part) = path.get(1) {
if part.name == keywords::Crate.name() {
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
}
}
}

let diag = lint::builtin::BuiltinLintDiagnostics
::AbsPathWithModule(path_span);
self.session.buffer_lint_with_diagnostic(
lint::builtin::ABSOLUTE_PATH_NOT_STARTING_WITH_CRATE,
id, path_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,
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/edition-lint-paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@ pub mod foo {
//~| WARN this was previously accepted
use super::bar::Bar2;
use crate::bar::Bar3;

use bar;
//~^ ERROR Absolute
//~| WARN this was previously accepted
use crate::{bar as something_else};

use {Bar as SomethingElse, 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() {
}
}


Expand All @@ -38,4 +54,5 @@ fn main() {
let x = bar::Bar;
let x = ::crate::bar::Bar;
let x = self::bar::Bar;
foo::test();
}
33 changes: 30 additions & 3 deletions src/test/ui/edition-lint-paths.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,34 @@ LL | #![deny(absolute_path_starting_with_module)]
= 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:24:5
--> $DIR/edition-lint-paths.rs:22: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:27: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:27: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:40:5
|
LL | use bar::Bar;
| ^^^^^^^^ help: use `crate`: `crate::bar::Bar`
Expand All @@ -22,13 +49,13 @@ LL | use bar::Bar;
= 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:35:13
--> $DIR/edition-lint-paths.rs:51: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 3 previous errors
error: aborting due to 6 previous errors

0 comments on commit 58481c0

Please sign in to comment.