Auto merge of #50665 - alexcrichton:fix-single-item-path-warnings, r=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
This commit is contained in:
bors 2018-05-17 05:00:14 +00:00
commit ed9a29a976
8 changed files with 287 additions and 47 deletions

View file

@ -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"
@ -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,
)
}

View file

@ -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;
@ -283,7 +283,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),
},
@ -322,4 +322,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
"converted into hard error, see https://github.com/rust-lang/rust/issues/48950");
store.register_removed("resolve_trait_on_defaulted_unit",
"converted into hard error, see https://github.com/rust-lang/rust/issues/48950");
store.register_removed("absolute_path_starting_with_module",
"renamed to `absolute_path_not_starting_with_crate`");
}

View file

@ -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,6 +3337,9 @@ 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() {
@ -3341,6 +3347,12 @@ impl<'a> Resolver<'a> {
} 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
));
@ -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) => {
@ -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,

View file

@ -640,6 +640,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<(Span, String)> {
self.current_module = directive.parent;
let ImportDirective { ref module_path, span, .. } = *directive;
let mut warn_if_binding_comes_from_local_crate = false;
// FIXME: Last path segment is treated specially in import resolution, so extern crate
// mode for absolute paths needs some special support for single-segment imports.
@ -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 => {
self.lint_path_starts_with_module(directive.id, span);
}
SingleImport { source, target, .. } => {
let crate_root = if source.name == keywords::Crate.name() &&
module_path[0].name != keywords::Extern.name() {
@ -676,6 +680,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
self.populate_module_if_necessary(crate_root);
Some(crate_root)
} else {
warn_if_binding_comes_from_local_crate = true;
None
};
@ -870,6 +875,26 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
}
if warn_if_binding_comes_from_local_crate {
let mut warned = false;
self.per_ns(|this, ns| {
let binding = match result[ns].get().ok() {
Some(b) => b,
None => return
};
if let NameBindingKind::Import { directive: d, .. } = binding.kind {
if let ImportDirectiveSubclass::ExternCrate(..) = d.subclass {
return
}
}
if warned {
return
}
warned = true;
this.lint_path_starts_with_module(directive.id, span);
});
}
// Record what this import resolves to for later uses in documentation,
// this may resolve to either a value or a type, but for documentation
// purposes it's good enough to just favor one over the other.

View file

@ -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() {}

View file

@ -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();
}
}

View file

@ -8,34 +8,70 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(crate_in_paths)]
#![deny(absolute_path_starting_with_module)]
// 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 ::bar::Bar;
//~^ ERROR Absolute
//~^ ERROR absolute
//~| 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() {
}
}
use bar::Bar;
//~^ ERROR Absolute
//~^ 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 *;
//~^ ERROR absolute
//~| WARN this was previously accepted
}
fn main() {
let x = ::bar::Bar;
//~^ ERROR Absolute
//~^ 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();
}
}

View file

@ -1,19 +1,46 @@
error: Absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/edition-lint-paths.rs:16:9
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:12:9
--> $DIR/edition-lint-paths.rs:15:9
|
LL | #![deny(absolute_path_starting_with_module)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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:24:5
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`
@ -21,8 +48,17 @@ LL | use 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:35:13
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`
@ -30,5 +66,5 @@ LL | let x = ::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 7 previous errors