Auto merge of #50969 - nikomatsakis:issue-50673-broken-migration-lint, r=alexcrichton

fix suggestions with nested paths

Fixes #50673

cc @Manishearth @petrochenkov
r? @alexcrichton
This commit is contained in:
bors 2018-05-22 18:55:54 +00:00
commit d034ae53c4
11 changed files with 236 additions and 43 deletions

View File

@ -100,6 +100,8 @@ fn insert_field_names(&mut self, def_id: DefId, field_names: Vec<Name>) {
}
fn build_reduced_graph_for_use_tree(&mut self,
root_use_tree: &ast::UseTree,
root_id: NodeId,
use_tree: &ast::UseTree,
id: NodeId,
vis: ty::Visibility,
@ -182,7 +184,14 @@ fn build_reduced_graph_for_use_tree(&mut self,
type_ns_only,
};
self.add_import_directive(
module_path, subclass, use_tree.span, id, vis, expansion,
module_path,
subclass,
use_tree.span,
id,
root_use_tree.span,
root_id,
vis,
expansion,
);
}
ast::UseTreeKind::Glob => {
@ -191,7 +200,14 @@ fn build_reduced_graph_for_use_tree(&mut self,
max_vis: Cell::new(ty::Visibility::Invisible),
};
self.add_import_directive(
module_path, subclass, use_tree.span, id, vis, expansion,
module_path,
subclass,
use_tree.span,
id,
root_use_tree.span,
root_id,
vis,
expansion,
);
}
ast::UseTreeKind::Nested(ref items) => {
@ -226,7 +242,7 @@ fn build_reduced_graph_for_use_tree(&mut self,
for &(ref tree, id) in items {
self.build_reduced_graph_for_use_tree(
tree, id, vis, &prefix, true, item, expansion
root_use_tree, root_id, tree, id, vis, &prefix, true, item, expansion
);
}
}
@ -249,7 +265,7 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) {
};
self.build_reduced_graph_for_use_tree(
use_tree, item.id, vis, &prefix, false, item, expansion,
use_tree, item.id, use_tree, item.id, vis, &prefix, false, item, expansion,
);
}
@ -266,10 +282,12 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, expansion: Mark) {
let binding =
(module, ty::Visibility::Public, sp, expansion).to_name_binding(self.arenas);
let directive = self.arenas.alloc_import_directive(ImportDirective {
root_id: item.id,
id: item.id,
parent,
imported_module: Cell::new(Some(module)),
subclass: ImportDirectiveSubclass::ExternCrate(orig_name),
root_span: item.span,
span: item.span,
module_path: Vec::new(),
vis: Cell::new(vis),
@ -640,10 +658,12 @@ fn process_legacy_macro_imports(&mut self, item: &Item, module: Module<'a>, expa
let (graph_root, arenas) = (self.graph_root, self.arenas);
let macro_use_directive = |span| arenas.alloc_import_directive(ImportDirective {
root_id: item.id,
id: item.id,
parent: graph_root,
imported_module: Cell::new(Some(module)),
subclass: ImportDirectiveSubclass::MacroUse,
root_span: span,
span,
module_path: Vec::new(),
vis: Cell::new(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))),

View File

@ -12,6 +12,7 @@
html_favicon_url = "https://doc.rust-lang.org/favicon.ico",
html_root_url = "https://doc.rust-lang.org/nightly/")]
#![feature(crate_visibility_modifier)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
@ -1634,11 +1635,17 @@ fn resolve_hir_path_cb<F>(&mut self, path: &mut hir::Path, is_value: bool, error
.map(|seg| Ident::new(seg.name, span))
.collect();
// FIXME (Manishearth): Intra doc links won't get warned of epoch changes
match self.resolve_path(&path, Some(namespace), true, span, None) {
match self.resolve_path(&path, Some(namespace), true, span, CrateLint::No) {
PathResult::Module(module) => *def = module.def().unwrap(),
PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 =>
*def = path_res.base_def(),
PathResult::NonModule(..) => match self.resolve_path(&path, None, true, span, None) {
PathResult::NonModule(..) => match self.resolve_path(
&path,
None,
true,
span,
CrateLint::No,
) {
PathResult::Failed(span, msg, _) => {
error_callback(self, span, ResolutionError::FailedToResolve(&msg));
}
@ -2352,8 +2359,13 @@ fn with_optional_trait_ref<T, F>(&mut self, opt_trait_ref: Option<&TraitRef>, f:
if def != Def::Err {
new_id = Some(def.def_id());
let span = trait_ref.path.span;
if let PathResult::Module(module) = self.resolve_path(&path, None, false, span,
Some(trait_ref.ref_id)) {
if let PathResult::Module(module) = self.resolve_path(
&path,
None,
false,
span,
CrateLint::SimplePath(trait_ref.ref_id),
) {
new_val = Some((module, trait_ref.clone()));
}
}
@ -2813,7 +2825,7 @@ fn smart_resolve_path_fragment(&mut self,
} else {
let mod_path = &path[..path.len() - 1];
let mod_prefix = match this.resolve_path(mod_path, Some(TypeNS),
false, span, None) {
false, span, CrateLint::No) {
PathResult::Module(module) => module.def(),
_ => None,
}.map_or(format!(""), |def| format!("{} ", def.kind_name()));
@ -3143,7 +3155,13 @@ fn resolve_qpath(&mut self,
));
}
let result = match self.resolve_path(&path, Some(ns), true, span, Some(id)) {
let result = match self.resolve_path(
&path,
Some(ns),
true,
span,
CrateLint::SimplePath(id),
) {
PathResult::NonModule(path_res) => path_res,
PathResult::Module(module) if !module.is_normal() => {
PathResolution::new(module.def().unwrap())
@ -3180,7 +3198,13 @@ fn resolve_qpath(&mut self,
path[0].name != keywords::CrateRoot.name() &&
path[0].name != keywords::DollarCrate.name() {
let unqualified_result = {
match self.resolve_path(&[*path.last().unwrap()], Some(ns), false, span, None) {
match self.resolve_path(
&[*path.last().unwrap()],
Some(ns),
false,
span,
CrateLint::No,
) {
PathResult::NonModule(path_res) => path_res.base_def(),
PathResult::Module(module) => module.def().unwrap(),
_ => return Some(result),
@ -3195,14 +3219,14 @@ fn resolve_qpath(&mut self,
Some(result)
}
fn resolve_path(&mut self,
path: &[Ident],
opt_ns: Option<Namespace>, // `None` indicates a module path
record_used: bool,
path_span: Span,
node_id: Option<NodeId>) // None indicates that we don't care about linting
// `::module` paths
-> PathResult<'a> {
fn resolve_path(
&mut self,
path: &[Ident],
opt_ns: Option<Namespace>, // `None` indicates a module path
record_used: bool,
path_span: Span,
crate_lint: CrateLint,
) -> PathResult<'a> {
let mut module = None;
let mut allow_super = true;
let mut second_binding = None;
@ -3321,7 +3345,7 @@ fn resolve_path(&mut self,
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,
crate_lint,
path,
path_span,
second_binding,
@ -3366,19 +3390,22 @@ fn resolve_path(&mut self,
}
}
self.lint_if_path_starts_with_module(node_id, path, path_span, second_binding);
self.lint_if_path_starts_with_module(crate_lint, 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,
fn lint_if_path_starts_with_module(
&self,
crate_lint: CrateLint,
path: &[Ident],
path_span: Span,
second_binding: Option<&NameBinding>,
) {
let (diag_id, diag_span) = match crate_lint {
CrateLint::No => return,
CrateLint::SimplePath(id) => (id, path_span),
CrateLint::UsePath { root_id, root_span } => (root_id, root_span),
};
let first_name = match path.get(0) {
@ -3414,7 +3441,7 @@ fn lint_if_path_starts_with_module(&self,
}
}
self.lint_path_starts_with_module(id, path_span);
self.lint_path_starts_with_module(diag_id, diag_span);
}
fn lint_path_starts_with_module(&self, id: NodeId, span: Span) {
@ -3650,7 +3677,7 @@ fn lookup_typo_candidate<FilterFn>(&mut self,
// Search in module.
let mod_path = &path[..path.len() - 1];
if let PathResult::Module(module) = self.resolve_path(mod_path, Some(TypeNS),
false, span, None) {
false, span, CrateLint::No) {
add_module_candidates(module, &mut names);
}
}
@ -4427,4 +4454,19 @@ pub enum MakeGlobMap {
No,
}
enum CrateLint {
/// Do not issue the lint
No,
/// This lint applies to some random path like `impl ::foo::Bar`
/// or whatever. In this case, we can take the span of that path.
SimplePath(NodeId),
/// This lint comes from a `use` statement. In this case, what we
/// care about really is the *root* `use` statement; e.g., if we
/// have nested things like `use a::{b, c}`, we care about the
/// `use a` part.
UsePath { root_id: NodeId, root_span: Span },
}
__build_diagnostic_array! { librustc_resolve, DIAGNOSTICS }

View File

@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use {AmbiguityError, Resolver, ResolutionError, resolve_error};
use {AmbiguityError, CrateLint, Resolver, ResolutionError, resolve_error};
use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult};
use Namespace::{self, MacroNS};
use build_reduced_graph::BuildReducedGraphVisitor;
@ -436,7 +436,7 @@ pub fn resolve_macro_to_def_inner(&mut self, scope: Mark, path: &ast::Path,
return Err(Determinacy::Determined);
}
let def = match self.resolve_path(&path, Some(MacroNS), false, span, None) {
let def = match self.resolve_path(&path, Some(MacroNS), false, span, CrateLint::No) {
PathResult::NonModule(path_res) => match path_res.base_def() {
Def::Err => Err(Determinacy::Determined),
def @ _ => {
@ -613,7 +613,7 @@ pub fn resolve_legacy_scope(&mut self,
pub fn finalize_current_module_macro_resolutions(&mut self) {
let module = self.current_module;
for &(ref path, span) in module.macro_resolutions.borrow().iter() {
match self.resolve_path(&path, Some(MacroNS), true, span, None) {
match self.resolve_path(&path, Some(MacroNS), true, span, CrateLint::No) {
PathResult::NonModule(_) => {},
PathResult::Failed(span, msg, _) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));

View File

@ -10,7 +10,7 @@
use self::ImportDirectiveSubclass::*;
use {AmbiguityError, Module, PerNS};
use {AmbiguityError, CrateLint, Module, PerNS};
use Namespace::{self, TypeNS, MacroNS};
use {NameBinding, NameBindingKind, ToNameBinding, PathResult, PrivacyError};
use Resolver;
@ -55,12 +55,36 @@ pub enum ImportDirectiveSubclass<'a> {
/// One import directive.
#[derive(Debug,Clone)]
pub struct ImportDirective<'a> {
/// The id of the `extern crate`, `UseTree` etc that imported this `ImportDirective`.
///
/// In the case where the `ImportDirective` was expanded from a "nested" use tree,
/// this id is the id of the leaf tree. For example:
///
/// ```ignore (pacify the mercilous tidy)
/// use foo::bar::{a, b}
/// ```
///
/// If this is the import directive for `foo::bar::a`, we would have the id of the `UseTree`
/// for `a` in this field.
pub id: NodeId,
/// The `id` of the "root" use-kind -- this is always the same as
/// `id` except in the case of "nested" use trees, in which case
/// it will be the `id` of the root use tree. e.g., in the example
/// from `id`, this would be the id of the `use foo::bar`
/// `UseTree` node.
pub root_id: NodeId,
/// Span of this use tree.
pub span: Span,
/// Span of the *root* use tree (see `root_id`).
pub root_span: Span,
pub parent: Module<'a>,
pub module_path: Vec<Ident>,
pub imported_module: Cell<Option<Module<'a>>>, // the resolution of `module_path`
pub subclass: ImportDirectiveSubclass<'a>,
pub span: Span,
pub vis: Cell<ty::Visibility>,
pub expansion: Mark,
pub used: Cell<bool>,
@ -70,6 +94,10 @@ impl<'a> ImportDirective<'a> {
pub fn is_glob(&self) -> bool {
match self.subclass { ImportDirectiveSubclass::GlobImport { .. } => true, _ => false }
}
crate fn crate_lint(&self) -> CrateLint {
CrateLint::UsePath { root_id: self.root_id, root_span: self.root_span }
}
}
#[derive(Clone, Default, Debug)]
@ -295,6 +323,8 @@ pub fn add_import_directive(&mut self,
subclass: ImportDirectiveSubclass<'a>,
span: Span,
id: NodeId,
root_span: Span,
root_id: NodeId,
vis: ty::Visibility,
expansion: Mark) {
let current_module = self.current_module;
@ -305,6 +335,8 @@ pub fn add_import_directive(&mut self,
subclass,
span,
id,
root_span,
root_id,
vis: Cell::new(vis),
expansion,
used: Cell::new(false),
@ -569,7 +601,7 @@ fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> bool {
// while resolving its module path.
directive.vis.set(ty::Visibility::Invisible);
let result = self.resolve_path(&directive.module_path[..], None, false,
directive.span, Some(directive.id));
directive.span, directive.crate_lint());
directive.vis.set(vis);
match result {
@ -702,7 +734,13 @@ fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<(Spa
}
}
let module_result = self.resolve_path(&module_path, None, true, span, Some(directive.id));
let module_result = self.resolve_path(
&module_path,
None,
true,
span,
directive.crate_lint(),
);
let module = match module_result {
PathResult::Module(module) => module,
PathResult::Failed(span, msg, false) => {
@ -717,7 +755,7 @@ fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<(Spa
!(self_path.len() > 1 && is_special(self_path[1])) {
self_path[0].name = keywords::SelfValue.name();
self_result = Some(self.resolve_path(&self_path, None, false,
span, None));
span, CrateLint::No));
}
return if let Some(PathResult::Module(..)) = self_result {
Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..]))))

View File

@ -0,0 +1,28 @@
// 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.
// run-rustfix
#![feature(rust_2018_preview)]
#![deny(absolute_path_not_starting_with_crate)]
use crate::foo::{a, b};
//~^ ERROR absolute paths must start with
//~| this was previously accepted
mod foo {
crate fn a() {}
crate fn b() {}
}
fn main() {
a();
b();
}

View File

@ -0,0 +1,28 @@
// 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.
// run-rustfix
#![feature(rust_2018_preview)]
#![deny(absolute_path_not_starting_with_crate)]
use foo::{a, b};
//~^ ERROR absolute paths must start with
//~| this was previously accepted
mod foo {
crate fn a() {}
crate fn b() {}
}
fn main() {
a();
b();
}

View File

@ -0,0 +1,16 @@
error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/edition-lint-nested-paths.rs:16:5
|
LL | use foo::{a, b};
| ^^^^^^^^^^^ help: use `crate`: `crate::foo::{a, b}`
|
note: lint level defined here
--> $DIR/edition-lint-nested-paths.rs:14: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: aborting due to previous error

View File

@ -40,6 +40,8 @@ pub mod foo {
pub fn test() {
}
pub trait SomeTrait { }
}
use crate::bar::Bar;
@ -59,6 +61,10 @@ mod baz {
//~| WARN this was previously accepted
}
impl crate::foo::SomeTrait for u32 { }
//~^ ERROR absolute
//~| WARN this was previously accepted
fn main() {
let x = crate::bar::Bar;
//~^ ERROR absolute

View File

@ -40,6 +40,8 @@ pub mod foo {
pub fn test() {
}
pub trait SomeTrait { }
}
use bar::Bar;
@ -59,6 +61,10 @@ mod baz {
//~| WARN this was previously accepted
}
impl ::foo::SomeTrait for u32 { }
//~^ ERROR absolute
//~| WARN this was previously accepted
fn main() {
let x = ::bar::Bar;
//~^ ERROR absolute

View File

@ -40,7 +40,7 @@ LL | use {Bar as SomethingElse, main};
= 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
--> $DIR/edition-lint-paths.rs:47:5
|
LL | use bar::Bar;
| ^^^^^^^^ help: use `crate`: `crate::bar::Bar`
@ -49,7 +49,7 @@ 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:57:9
--> $DIR/edition-lint-paths.rs:59:9
|
LL | use *;
| ^ help: use `crate`: `crate::*`
@ -58,7 +58,16 @@ LL | use *;
= 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
--> $DIR/edition-lint-paths.rs:64:6
|
LL | impl ::foo::SomeTrait for u32 { }
| ^^^^^^^^^^^^^^^^ help: use `crate`: `crate::foo::SomeTrait`
|
= 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:69:13
|
LL | let x = ::bar::Bar;
| ^^^^^^^^^^ help: use `crate`: `crate::bar::Bar`
@ -66,5 +75,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 7 previous errors
error: aborting due to 8 previous errors