Skip to content

Commit

Permalink
rustc: Fix crate lint for single-item paths
Browse files Browse the repository at this point in the history
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 committed May 15, 2018
1 parent 3e955a0 commit dff9ee1
Show file tree
Hide file tree
Showing 8 changed files with 287 additions and 47 deletions.
4 changes: 2 additions & 2 deletions src/librustc/lint/builtin.rs
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
6 changes: 4 additions & 2 deletions src/librustc_lint/lib.rs
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 Expand Up @@ -321,4 +321,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`");
}
107 changes: 80 additions & 27 deletions src/librustc_resolve/lib.rs
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,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,
Expand Down
25 changes: 25 additions & 0 deletions src/librustc_resolve/resolve_imports.rs
Expand Up @@ -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.
Expand All @@ -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() {
Expand All @@ -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
};

Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/auxiliary/edition-lint-paths.rs
@@ -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() {}
77 changes: 77 additions & 0 deletions src/test/ui/edition-lint-paths.fixed
@@ -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();
}
}

0 comments on commit dff9ee1

Please sign in to comment.