Skip to content

Commit

Permalink
Auto merge of #50100 - Manishearth:edition-path-lint, r=nikomatsakis
Browse files Browse the repository at this point in the history
Edition breakage lint for absolute paths starting with modules

We plan to enable `extern_absolute_paths` in the 2018 edition. To allow for that, folks must transition their paths in a previous edition to the new one. This makes paths which import module contents via `use module::` or `::module::` obsolete, and we must edition-lint these.

https://internals.rust-lang.org/t/the-great-module-adventure-continues/6678/205?u=manishearth is the current plan for paths.

r? @nikomatsakis

Fixes #48722
  • Loading branch information
bors committed Apr 25, 2018
2 parents 0c5740f + 2a5ce10 commit 432fe0c
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 15 deletions.
28 changes: 27 additions & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,13 @@ declare_lint! {
"suggest using `dyn Trait` for trait objects"
}

declare_lint! {
pub ABSOLUTE_PATH_STARTING_WITH_MODULE,
Allow,
"fully qualified paths that start with a module name \
instead of `crate`, `self`, or an extern crate name"
}

declare_lint! {
pub ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
Warn,
Expand Down Expand Up @@ -314,6 +321,7 @@ impl LintPass for HardwiredLints {
TYVAR_BEHIND_RAW_POINTER,
ELIDED_LIFETIME_IN_PATH,
BARE_TRAIT_OBJECT,
ABSOLUTE_PATH_STARTING_WITH_MODULE,
UNSTABLE_NAME_COLLISION,
)
}
Expand All @@ -324,7 +332,8 @@ impl LintPass for HardwiredLints {
#[derive(PartialEq, RustcEncodable, RustcDecodable, Debug)]
pub enum BuiltinLintDiagnostics {
Normal,
BareTraitObject(Span, /* is_global */ bool)
BareTraitObject(Span, /* is_global */ bool),
AbsPathWithModule(Span),
}

impl BuiltinLintDiagnostics {
Expand All @@ -339,6 +348,23 @@ impl BuiltinLintDiagnostics {
};
db.span_suggestion(span, "use `dyn`", sugg);
}
BuiltinLintDiagnostics::AbsPathWithModule(span) => {
let sugg = match sess.codemap().span_to_snippet(span) {
Ok(ref s) => {
// FIXME(Manishearth) ideally the emitting code
// can tell us whether or not this is global
let opt_colon = if s.trim_left().starts_with("::") {
""
} else {
"::"
};

format!("crate{}{}", opt_colon, s)
}
Err(_) => format!("crate::<path>")
};
db.span_suggestion(span, "use `crate`", sugg);
}
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ extern crate rustc_mir;
extern crate syntax_pos;

use rustc::lint;
use rustc::lint::builtin::BARE_TRAIT_OBJECT;
use rustc::lint::builtin::{BARE_TRAIT_OBJECT, ABSOLUTE_PATH_STARTING_WITH_MODULE};
use rustc::session;
use rustc::util;

Expand Down Expand Up @@ -278,6 +278,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
// Note: this item represents future incompatibility of all unstable functions in the
// standard library, and thus should never be removed or changed to an error.
},
FutureIncompatibleInfo {
id: LintId::of(ABSOLUTE_PATH_STARTING_WITH_MODULE),
reference: "issue TBD",
edition: Some(Edition::Edition2018),
},
]);

// Register renamed and removed lints
Expand Down
50 changes: 42 additions & 8 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1654,11 +1654,12 @@ impl<'a> Resolver<'a> {
let path: Vec<Ident> = segments.iter()
.map(|seg| Ident::new(seg.name, span))
.collect();
match self.resolve_path(&path, Some(namespace), true, span) {
// FIXME (Manishearth): Intra doc links won't get warned of epoch changes
match self.resolve_path(&path, Some(namespace), true, span, None) {
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) {
PathResult::NonModule(..) => match self.resolve_path(&path, None, true, span, None) {
PathResult::Failed(span, msg, _) => {
error_callback(self, span, ResolutionError::FailedToResolve(&msg));
}
Expand Down Expand Up @@ -2360,7 +2361,8 @@ impl<'a> Resolver<'a> {
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) {
if let PathResult::Module(module) = self.resolve_path(&path, None, false, span,
Some(trait_ref.ref_id)) {
new_val = Some((module, trait_ref.clone()));
}
}
Expand Down Expand Up @@ -2819,7 +2821,8 @@ impl<'a> Resolver<'a> {
(format!(""), format!("the crate root"))
} else {
let mod_path = &path[..path.len() - 1];
let mod_prefix = match this.resolve_path(mod_path, Some(TypeNS), false, span) {
let mod_prefix = match this.resolve_path(mod_path, Some(TypeNS),
false, span, None) {
PathResult::Module(module) => module.def(),
_ => None,
}.map_or(format!(""), |def| format!("{} ", def.kind_name()));
Expand Down Expand Up @@ -3149,7 +3152,7 @@ impl<'a> Resolver<'a> {
));
}

let result = match self.resolve_path(&path, Some(ns), true, span) {
let result = match self.resolve_path(&path, Some(ns), true, span, Some(id)) {
PathResult::NonModule(path_res) => path_res,
PathResult::Module(module) if !module.is_normal() => {
PathResolution::new(module.def().unwrap())
Expand Down Expand Up @@ -3186,7 +3189,7 @@ impl<'a> Resolver<'a> {
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) {
match self.resolve_path(&[*path.last().unwrap()], Some(ns), false, span, None) {
PathResult::NonModule(path_res) => path_res.base_def(),
PathResult::Module(module) => module.def().unwrap(),
_ => return Some(result),
Expand All @@ -3205,7 +3208,9 @@ impl<'a> Resolver<'a> {
path: &[Ident],
opt_ns: Option<Namespace>, // `None` indicates a module path
record_used: bool,
path_span: Span)
path_span: Span,
node_id: Option<NodeId>) // None indicates that we don't care about linting
// `::module` paths
-> PathResult<'a> {
let mut module = None;
let mut allow_super = true;
Expand Down Expand Up @@ -3253,6 +3258,8 @@ impl<'a> Resolver<'a> {
let prev_name = path[0].name;
if prev_name == keywords::Extern.name() ||
prev_name == keywords::CrateRoot.name() &&
// Note: When this feature stabilizes, this should
// be gated on sess.rust_2018()
self.session.features_untracked().extern_absolute_paths {
// `::extern_crate::a::b`
let crate_id = self.crate_loader.process_path_extern(name, ident.span);
Expand Down Expand Up @@ -3324,6 +3331,33 @@ 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 @@ -3571,7 +3605,7 @@ impl<'a> Resolver<'a> {
// Search in module.
let mod_path = &path[..path.len() - 1];
if let PathResult::Module(module) = self.resolve_path(mod_path, Some(TypeNS),
false, span) {
false, span, None) {
add_module_candidates(module, &mut names);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ impl<'a> Resolver<'a> {
return Err(Determinacy::Determined);
}

let def = match self.resolve_path(&path, Some(MacroNS), false, span) {
let def = match self.resolve_path(&path, Some(MacroNS), false, span, None) {
PathResult::NonModule(path_res) => match path_res.base_def() {
Def::Err => Err(Determinacy::Determined),
def @ _ => {
Expand Down Expand Up @@ -616,7 +616,7 @@ impl<'a> Resolver<'a> {
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) {
match self.resolve_path(&path, Some(MacroNS), true, span, None) {
PathResult::NonModule(_) => {},
PathResult::Failed(span, msg, _) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
// For better failure detection, pretend that the import will not define any names
// while resolving its module path.
directive.vis.set(ty::Visibility::Invisible);
let result = self.resolve_path(&directive.module_path[..], None, false, directive.span);
let result = self.resolve_path(&directive.module_path[..], None, false,
directive.span, Some(directive.id));
directive.vis.set(vis);

match result {
Expand Down Expand Up @@ -663,7 +664,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
}

let module_result = self.resolve_path(&module_path, None, true, span);
let module_result = self.resolve_path(&module_path, None, true, span, Some(directive.id));
let module = match module_result {
PathResult::Module(module) => module,
PathResult::Failed(span, msg, false) => {
Expand All @@ -677,7 +678,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
if !self_path.is_empty() && !is_special(self_path[0]) &&
!(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));
self_result = Some(self.resolve_path(&self_path, None, false,
span, None));
}
return if let Some(PathResult::Module(..)) = self_result {
Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..]))))
Expand Down
41 changes: 41 additions & 0 deletions src/test/ui/edition-lint-paths.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// 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.

#![feature(crate_in_paths)]
#![deny(absolute_path_starting_with_module)]
#![allow(unused)]

pub mod foo {
use ::bar::Bar;
//~^ ERROR Absolute
//~| WARN this was previously accepted
use super::bar::Bar2;
use crate::bar::Bar3;
}


use bar::Bar;
//~^ ERROR Absolute
//~| WARN this was previously accepted

pub mod bar {
pub struct Bar;
pub type Bar2 = Bar;
pub type Bar3 = Bar;
}

fn main() {
let x = ::bar::Bar;
//~^ ERROR Absolute
//~| WARN this was previously accepted
let x = bar::Bar;
let x = ::crate::bar::Bar;
let x = self::bar::Bar;
}
34 changes: 34 additions & 0 deletions src/test/ui/edition-lint-paths.stderr
Original file line number Diff line number Diff line change
@@ -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-paths.rs:16:9
|
LL | use ::bar::Bar;
| ^^^^^^^^^^ help: use `crate`: `crate::bar::Bar`
|
note: lint level defined here
--> $DIR/edition-lint-paths.rs:12:9
|
LL | #![deny(absolute_path_starting_with_module)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= 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
|
LL | use bar::Bar;
| ^^^^^^^^ help: use `crate`: `crate::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
|
LL | let x = ::bar::Bar;
| ^^^^^^^^^^ help: use `crate`: `crate::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

0 comments on commit 432fe0c

Please sign in to comment.