Skip to content

Commit

Permalink
handle fully qualified paths properly when linting
Browse files Browse the repository at this point in the history
fixes #50970
  • Loading branch information
nikomatsakis committed May 23, 2018
1 parent be2bb80 commit 42d0b36
Show file tree
Hide file tree
Showing 7 changed files with 261 additions and 8 deletions.
56 changes: 48 additions & 8 deletions src/librustc_resolve/lib.rs
Expand Up @@ -2222,7 +2222,7 @@ impl<'a> Resolver<'a> {
segments: use_tree.prefix.make_root().into_iter().collect(),
span: use_tree.span,
};
self.resolve_use_tree(item.id, use_tree, &path);
self.resolve_use_tree(item.id, use_tree.span, item.id, use_tree, &path);
}

ItemKind::ExternCrate(_) | ItemKind::MacroDef(..) | ItemKind::GlobalAsm(_) => {
Expand All @@ -2233,7 +2233,18 @@ impl<'a> Resolver<'a> {
}
}

fn resolve_use_tree(&mut self, id: NodeId, use_tree: &ast::UseTree, prefix: &Path) {
/// For the most part, use trees are desugared into `ImportDirective` instances
/// when building the reduced graph (see `build_reduced_graph_for_use_tree`). But
/// there is one special case we handle here: an empty nested import like
/// `a::{b::{}}`, which desugares into...no import directives.
fn resolve_use_tree(
&mut self,
root_id: NodeId,
root_span: Span,
id: NodeId,
use_tree: &ast::UseTree,
prefix: &Path,
) {
match use_tree.kind {
ast::UseTreeKind::Nested(ref items) => {
let path = Path {
Expand All @@ -2252,11 +2263,11 @@ impl<'a> Resolver<'a> {
None,
&path,
PathSource::ImportPrefix,
CrateLint::SimplePath(id), // TODO seems wrong
CrateLint::UsePath { root_id, root_span },
);
} else {
for &(ref tree, nested_id) in items {
self.resolve_use_tree(nested_id, tree, &path);
self.resolve_use_tree(root_id, root_span, nested_id, tree, &path);
}
}
}
Expand Down Expand Up @@ -3188,21 +3199,44 @@ impl<'a> Resolver<'a> {

if let Some(qself) = qself {
if qself.position == 0 {
// FIXME: Create some fake resolution that can't possibly be a type.
// This is a case like `<T>::B`, where there is no
// trait to resolve. In that case, we leave the `B`
// segment to be resolved by type-check.
return Some(PathResolution::with_unresolved_segments(
Def::Mod(DefId::local(CRATE_DEF_INDEX)), path.len()
));
}
// Make sure `A::B` in `<T as A>::B::C` is a trait item.

// Make sure `A::B` in `<T as A::B>::C` is a trait item.
//
// Currently, `path` names the full item (`A::B::C`, in
// our example). so we extract the prefix of that that is
// the trait (the slice upto and including
// `qself.position`). And then we recursively resolve that,
// but with `qself` set to `None`.
//
// However, setting `qself` to none (but not changing the
// span) loses the information about where this path
// *actually* appears, so for the purposes of the crate
// lint we pass along information that this is the trait
// name from a fully qualified path, and this also
// contains the full span (the `CrateLint::QPathTrait`).
let ns = if qself.position + 1 == path.len() { ns } else { TypeNS };
let res = self.smart_resolve_path_fragment(
id,
None,
&path[..qself.position + 1],
span,
PathSource::TraitItem(ns),
crate_lint, // TODO wrong
CrateLint::QPathTrait {
qpath_id: id,
qpath_span: qself.path_span,
},
);

// The remaining segments (the `C` in our example) will
// have to be resolved by type-check, since that requires doing
// trait resolution.
return Some(PathResolution::with_unresolved_segments(
res.base_def(), res.unresolved_segments() + path.len() - qself.position - 1
));
Expand All @@ -3213,7 +3247,7 @@ impl<'a> Resolver<'a> {
Some(ns),
true,
span,
CrateLint::SimplePath(id),
crate_lint,
) {
PathResult::NonModule(path_res) => path_res,
PathResult::Module(module) if !module.is_normal() => {
Expand Down Expand Up @@ -3468,6 +3502,7 @@ impl<'a> Resolver<'a> {
CrateLint::No => return,
CrateLint::SimplePath(id) => (id, path_span),
CrateLint::UsePath { root_id, root_span } => (root_id, root_span),
CrateLint::QPathTrait { qpath_id, qpath_span } => (qpath_id, qpath_span),
};

let first_name = match path.get(0) {
Expand Down Expand Up @@ -4536,6 +4571,11 @@ enum CrateLint {
/// have nested things like `use a::{b, c}`, we care about the
/// `use a` part.
UsePath { root_id: NodeId, root_span: Span },

/// This is the "trait item" from a fully qualified path. For example,
/// we might be resolving `X::Y::Z` from a path like `<T as X::Y>::Z`.
/// The `path_span` is the span of the to the trait itself (`X::Y`).
QPathTrait { qpath_id: NodeId, qpath_span: Span },
}

__build_diagnostic_array! { librustc_resolve, DIAGNOSTICS }
37 changes: 37 additions & 0 deletions src/test/ui/rust-2018/edition-lint-fully-qualified-paths.fixed
@@ -0,0 +1,37 @@
// 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)]

mod foo {
crate trait Foo {
type Bar;
}

crate struct Baz { }

impl Foo for Baz {
type Bar = ();
}
}


fn main() {
let _: <foo::Baz as crate::foo::Foo>::Bar = ();
//~^ ERROR absolute paths must start with
//~| this was previously accepted

let _: <crate::foo::Baz as foo::Foo>::Bar = ();
//~^ ERROR absolute paths must start with
//~| this was previously accepted
}
37 changes: 37 additions & 0 deletions src/test/ui/rust-2018/edition-lint-fully-qualified-paths.rs
@@ -0,0 +1,37 @@
// 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)]

mod foo {
crate trait Foo {
type Bar;
}

crate struct Baz { }

impl Foo for Baz {
type Bar = ();
}
}


fn main() {
let _: <foo::Baz as ::foo::Foo>::Bar = ();
//~^ ERROR absolute paths must start with
//~| this was previously accepted

let _: <::foo::Baz as foo::Foo>::Bar = ();
//~^ ERROR absolute paths must start with
//~| this was previously accepted
}
25 changes: 25 additions & 0 deletions src/test/ui/rust-2018/edition-lint-fully-qualified-paths.stderr
@@ -0,0 +1,25 @@
error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/edition-lint-fully-qualified-paths.rs:30:25
|
LL | let _: <foo::Baz as ::foo::Foo>::Bar = ();
| ^^^^^^^^^^ help: use `crate`: `crate::foo::Foo`
|
note: lint level defined here
--> $DIR/edition-lint-fully-qualified-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: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/edition-lint-fully-qualified-paths.rs:34:13
|
LL | let _: <::foo::Baz as foo::Foo>::Bar = ();
| ^^^^^^^^^^ help: use `crate`: `crate::foo::Baz`
|
= 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 2 previous errors

40 changes: 40 additions & 0 deletions src/test/ui/rust-2018/edition-lint-nested-empty-paths.fixed
@@ -0,0 +1,40 @@
// 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)]
#![allow(unused_imports)]
#![allow(dead_code)]

crate mod foo {
crate mod bar {
crate mod baz { }
crate mod baz1 { }

crate struct XX;
}
}

use crate::foo::{bar::{baz::{}}};
//~^ ERROR absolute paths must start with
//~| WARN this was previously accepted

use crate::foo::{bar::{XX, baz::{}}};
//~^ ERROR absolute paths must start with
//~| WARN this was previously accepted

use crate::foo::{bar::{baz::{}, baz1::{}}};
//~^ ERROR absolute paths must start with
//~| WARN this was previously accepted

fn main() {
}
40 changes: 40 additions & 0 deletions src/test/ui/rust-2018/edition-lint-nested-empty-paths.rs
@@ -0,0 +1,40 @@
// 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)]
#![allow(unused_imports)]
#![allow(dead_code)]

crate mod foo {
crate mod bar {
crate mod baz { }
crate mod baz1 { }

crate struct XX;
}
}

use foo::{bar::{baz::{}}};
//~^ ERROR absolute paths must start with
//~| WARN this was previously accepted

use foo::{bar::{XX, baz::{}}};
//~^ ERROR absolute paths must start with
//~| WARN this was previously accepted

use foo::{bar::{baz::{}, baz1::{}}};
//~^ ERROR absolute paths must start with
//~| WARN this was previously accepted

fn main() {
}
34 changes: 34 additions & 0 deletions src/test/ui/rust-2018/edition-lint-nested-empty-paths.stderr
@@ -0,0 +1,34 @@
error: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/edition-lint-nested-empty-paths.rs:27:5
|
LL | use foo::{bar::{baz::{}}};
| ^^^^^^^^^^^^^^^^^^^^^ help: use `crate`: `crate::foo::{bar::{baz::{}}}`
|
note: lint level defined here
--> $DIR/edition-lint-nested-empty-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: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
--> $DIR/edition-lint-nested-empty-paths.rs:31:5
|
LL | use foo::{bar::{XX, baz::{}}};
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `crate`: `crate::foo::{bar::{XX, baz::{}}}`
|
= 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-nested-empty-paths.rs:35:5
|
LL | use foo::{bar::{baz::{}, baz1::{}}};
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `crate`: `crate::foo::{bar::{baz::{}, baz1::{}}}`
|
= 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

0 comments on commit 42d0b36

Please sign in to comment.