Skip to content

Commit

Permalink
Auto merge of #50983 - nikomatsakis:issue-50970-migration-lint-fq-pat…
Browse files Browse the repository at this point in the history
…h, r=alexcrichton

Issue 50970 migration lint fq path

Similar issue, but for fully qualified paths.

Fixes #50970

r? @alexcrichton
  • Loading branch information
bors committed May 23, 2018
2 parents e249596 + 688324e commit 27649b7
Show file tree
Hide file tree
Showing 11 changed files with 379 additions and 30 deletions.
150 changes: 130 additions & 20 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
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 @@ -2247,10 +2258,16 @@ impl<'a> Resolver<'a> {

if items.len() == 0 {
// Resolve prefix of an import with empty braces (issue #28388).
self.smart_resolve_path(id, None, &path, PathSource::ImportPrefix);
self.smart_resolve_path_with_crate_lint(
id,
None,
&path,
PathSource::ImportPrefix,
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 @@ -2354,7 +2371,8 @@ impl<'a> Resolver<'a> {
None,
&path,
trait_ref.path.span,
PathSource::Trait(AliasPossibility::No)
PathSource::Trait(AliasPossibility::No),
CrateLint::SimplePath(trait_ref.ref_id),
).base_def();
if def != Def::Err {
new_id = Some(def.def_id());
Expand Down Expand Up @@ -2787,18 +2805,38 @@ impl<'a> Resolver<'a> {
path: &Path,
source: PathSource)
-> PathResolution {
self.smart_resolve_path_with_crate_lint(id, qself, path, source, CrateLint::SimplePath(id))
}

/// A variant of `smart_resolve_path` where you also specify extra
/// information about where the path came from; this extra info is
/// sometimes needed for the lint that recommends rewriting
/// absoluate paths to `crate`, so that it knows how to frame the
/// suggestion. If you are just resolving a path like `foo::bar`
/// that appears...somewhere, though, then you just want
/// `CrateLint::SimplePath`, which is what `smart_resolve_path`
/// already provides.
fn smart_resolve_path_with_crate_lint(
&mut self,
id: NodeId,
qself: Option<&QSelf>,
path: &Path,
source: PathSource,
crate_lint: CrateLint
) -> PathResolution {
let segments = &path.segments.iter()
.map(|seg| seg.ident)
.collect::<Vec<_>>();
self.smart_resolve_path_fragment(id, qself, segments, path.span, source)
self.smart_resolve_path_fragment(id, qself, segments, path.span, source, crate_lint)
}

fn smart_resolve_path_fragment(&mut self,
id: NodeId,
qself: Option<&QSelf>,
path: &[Ident],
span: Span,
source: PathSource)
source: PathSource,
crate_lint: CrateLint)
-> PathResolution {
let ident_span = path.last().map_or(span, |ident| ident.span);
let ns = source.namespace();
Expand Down Expand Up @@ -2999,9 +3037,16 @@ impl<'a> Resolver<'a> {
err_path_resolution()
};

let resolution = match self.resolve_qpath_anywhere(id, qself, path, ns, span,
source.defer_to_typeck(),
source.global_by_default()) {
let resolution = match self.resolve_qpath_anywhere(
id,
qself,
path,
ns,
span,
source.defer_to_typeck(),
source.global_by_default(),
crate_lint,
) {
Some(resolution) if resolution.unresolved_segments() == 0 => {
if is_expected(resolution.base_def()) || resolution.base_def() == Def::Err {
resolution
Expand Down Expand Up @@ -3102,14 +3147,15 @@ impl<'a> Resolver<'a> {
primary_ns: Namespace,
span: Span,
defer_to_typeck: bool,
global_by_default: bool)
global_by_default: bool,
crate_lint: CrateLint)
-> Option<PathResolution> {
let mut fin_res = None;
// FIXME: can't resolve paths in macro namespace yet, macros are
// processed by the little special hack below.
for (i, ns) in [primary_ns, TypeNS, ValueNS, /*MacroNS*/].iter().cloned().enumerate() {
if i == 0 || ns != primary_ns {
match self.resolve_qpath(id, qself, path, ns, span, global_by_default) {
match self.resolve_qpath(id, qself, path, ns, span, global_by_default, crate_lint) {
// If defer_to_typeck, then resolution > no resolution,
// otherwise full resolution > partial resolution > no resolution.
Some(res) if res.unresolved_segments() == 0 || defer_to_typeck =>
Expand Down Expand Up @@ -3137,19 +3183,60 @@ impl<'a> Resolver<'a> {
path: &[Ident],
ns: Namespace,
span: Span,
global_by_default: bool)
global_by_default: bool,
crate_lint: CrateLint)
-> Option<PathResolution> {
debug!(
"resolve_qpath(id={:?}, qself={:?}, path={:?}, \
ns={:?}, span={:?}, global_by_default={:?})",
id,
qself,
path,
ns,
span,
global_by_default,
);

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));
let res = self.smart_resolve_path_fragment(
id,
None,
&path[..qself.position + 1],
span,
PathSource::TraitItem(ns),
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 @@ -3160,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 @@ -3231,6 +3318,16 @@ impl<'a> Resolver<'a> {
let mut allow_super = true;
let mut second_binding = None;

debug!(
"resolve_path(path={:?}, opt_ns={:?}, record_used={:?}, \
path_span={:?}, crate_lint={:?})",
path,
opt_ns,
record_used,
path_span,
crate_lint,
);

for (i, &ident) in path.iter().enumerate() {
debug!("resolve_path ident {} {:?}", i, ident);
let is_last = i == path.len() - 1;
Expand Down Expand Up @@ -3406,6 +3503,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 @@ -4093,8 +4191,14 @@ impl<'a> Resolver<'a> {
let segments = path.make_root().iter().chain(path.segments.iter())
.map(|seg| seg.ident)
.collect::<Vec<_>>();
let def = self.smart_resolve_path_fragment(id, None, &segments, path.span,
PathSource::Visibility).base_def();
let def = self.smart_resolve_path_fragment(
id,
None,
&segments,
path.span,
PathSource::Visibility,
CrateLint::SimplePath(id),
).base_def();
if def == Def::Err {
ty::Visibility::Public
} else {
Expand Down Expand Up @@ -4454,6 +4558,7 @@ pub enum MakeGlobMap {
No,
}

#[derive(Copy, Clone, Debug)]
enum CrateLint {
/// Do not issue the lint
No,
Expand All @@ -4467,6 +4572,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 }
5 changes: 5 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,11 @@ pub enum ExprKind {
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct QSelf {
pub ty: P<Ty>,

/// The span of `a::b::Trait` in a path like `<Vec<T> as
/// a::b::Trait>::AssociatedItem`; in the case where `position ==
/// 0`, this is an empty span.
pub path_span: Span,
pub position: usize
}

Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ext/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {

(ast::QSelf {
ty: self_type,
path_span: path.span,
position: path.segments.len() - 1
}, path)
}
Expand Down
12 changes: 9 additions & 3 deletions src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,10 @@ pub fn noop_fold_ty<T: Folder>(t: P<Ty>, fld: &mut T) -> P<Ty> {
TyKind::Tup(tys) => TyKind::Tup(tys.move_map(|ty| fld.fold_ty(ty))),
TyKind::Paren(ty) => TyKind::Paren(fld.fold_ty(ty)),
TyKind::Path(qself, path) => {
let qself = qself.map(|QSelf { ty, position }| {
let qself = qself.map(|QSelf { ty, path_span, position }| {
QSelf {
ty: fld.fold_ty(ty),
path_span: fld.new_span(path_span),
position,
}
});
Expand Down Expand Up @@ -1131,7 +1132,11 @@ pub fn noop_fold_pat<T: Folder>(p: P<Pat>, folder: &mut T) -> P<Pat> {
}
PatKind::Path(opt_qself, pth) => {
let opt_qself = opt_qself.map(|qself| {
QSelf { ty: folder.fold_ty(qself.ty), position: qself.position }
QSelf {
ty: folder.fold_ty(qself.ty),
path_span: folder.new_span(qself.path_span),
position: qself.position,
}
});
PatKind::Path(opt_qself, folder.fold_path(pth))
}
Expand Down Expand Up @@ -1292,9 +1297,10 @@ pub fn noop_fold_expr<T: Folder>(Expr {id, node, span, attrs}: Expr, folder: &mu
lim)
}
ExprKind::Path(qself, path) => {
let qself = qself.map(|QSelf { ty, position }| {
let qself = qself.map(|QSelf { ty, path_span, position }| {
QSelf {
ty: folder.fold_ty(ty),
path_span: folder.new_span(path_span),
position,
}
});
Expand Down
28 changes: 21 additions & 7 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1715,8 +1715,11 @@ impl<'a> Parser<'a> {
self.parse_path_segments(&mut segments, T::PATH_STYLE, true)?;

let span = ty.span.to(self.prev_span);
let recovered =
base.to_recovered(Some(QSelf { ty, position: 0 }), ast::Path { segments, span });
let path_span = span.to(span); // use an empty path since `position` == 0
let recovered = base.to_recovered(
Some(QSelf { ty, path_span, position: 0 }),
ast::Path { segments, span },
);

self.diagnostic()
.struct_span_err(span, "missing angle brackets in associated item path")
Expand Down Expand Up @@ -1905,21 +1908,32 @@ impl<'a> Parser<'a> {
/// `qualified_path = <type [as trait_ref]>::path`
///
/// # Examples
/// `<T>::default`
/// `<T as U>::a`
/// `<T as U>::F::a<S>` (without disambiguator)
/// `<T as U>::F::a::<S>` (with disambiguator)
fn parse_qpath(&mut self, style: PathStyle) -> PResult<'a, (QSelf, ast::Path)> {
let lo = self.prev_span;
let ty = self.parse_ty()?;
let mut path = if self.eat_keyword(keywords::As) {
self.parse_path(PathStyle::Type)?

// `path` will contain the prefix of the path up to the `>`,
// if any (e.g., `U` in the `<T as U>::*` examples
// above). `path_span` has the span of that path, or an empty
// span in the case of something like `<T>::Bar`.
let (mut path, path_span);
if self.eat_keyword(keywords::As) {
let path_lo = self.span;
path = self.parse_path(PathStyle::Type)?;
path_span = path_lo.to(self.prev_span);
} else {
ast::Path { segments: Vec::new(), span: syntax_pos::DUMMY_SP }
};
path = ast::Path { segments: Vec::new(), span: syntax_pos::DUMMY_SP };
path_span = self.span.to(self.span);
}

self.expect(&token::Gt)?;
self.expect(&token::ModSep)?;

let qself = QSelf { ty, position: path.segments.len() };
let qself = QSelf { ty, path_span, position: path.segments.len() };
self.parse_path_segments(&mut path.segments, style, true)?;

Ok((qself, ast::Path { segments: path.segments, span: lo.to(self.prev_span) }))
Expand Down
Loading

0 comments on commit 27649b7

Please sign in to comment.