Skip to content

Commit 432fe0c

Browse files
borsbors
committed
Auto merge of #50100 - Manishearth:edition-path-lint, r=nikomatsakis
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
2 parents 0c5740f + 2a5ce10 commit 432fe0c

File tree

7 files changed

+157
-15
lines changed

7 files changed

+157
-15
lines changed

‎src/librustc/lint/builtin.rs

+27-1
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,13 @@ declare_lint! {
254254
"suggest using `dyn Trait` for trait objects"
255255
}
256256

257+
declare_lint! {
258+
pub ABSOLUTE_PATH_STARTING_WITH_MODULE,
259+
Allow,
260+
"fully qualified paths that start with a module name \
261+
instead of `crate`, `self`, or an extern crate name"
262+
}
263+
257264
declare_lint! {
258265
pub ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
259266
Warn,
@@ -314,6 +321,7 @@ impl LintPass for HardwiredLints {
314321
TYVAR_BEHIND_RAW_POINTER,
315322
ELIDED_LIFETIME_IN_PATH,
316323
BARE_TRAIT_OBJECT,
324+
ABSOLUTE_PATH_STARTING_WITH_MODULE,
317325
UNSTABLE_NAME_COLLISION,
318326
)
319327
}
@@ -324,7 +332,8 @@ impl LintPass for HardwiredLints {
324332
#[derive(PartialEq, RustcEncodable, RustcDecodable, Debug)]
325333
pub enum BuiltinLintDiagnostics {
326334
Normal,
327-
BareTraitObject(Span, /* is_global */ bool)
335+
BareTraitObject(Span, /* is_global */ bool),
336+
AbsPathWithModule(Span),
328337
}
329338

330339
impl BuiltinLintDiagnostics {
@@ -339,6 +348,23 @@ impl BuiltinLintDiagnostics {
339348
};
340349
db.span_suggestion(span, "use `dyn`", sugg);
341350
}
351+
BuiltinLintDiagnostics::AbsPathWithModule(span) => {
352+
let sugg = match sess.codemap().span_to_snippet(span) {
353+
Ok(ref s) => {
354+
// FIXME(Manishearth) ideally the emitting code
355+
// can tell us whether or not this is global
356+
let opt_colon = if s.trim_left().starts_with("::") {
357+
""
358+
} else {
359+
"::"
360+
};
361+
362+
format!("crate{}{}", opt_colon, s)
363+
}
364+
Err(_) => format!("crate::<path>")
365+
};
366+
db.span_suggestion(span, "use `crate`", sugg);
367+
}
342368
}
343369
}
344370
}

‎src/librustc_lint/lib.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ extern crate rustc_mir;
4040
extern crate syntax_pos;
4141

4242
use rustc::lint;
43-
use rustc::lint::builtin::BARE_TRAIT_OBJECT;
43+
use rustc::lint::builtin::{BARE_TRAIT_OBJECT, ABSOLUTE_PATH_STARTING_WITH_MODULE};
4444
use rustc::session;
4545
use rustc::util;
4646

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

283288
// Register renamed and removed lints

‎src/librustc_resolve/lib.rs

+42-8
Original file line numberDiff line numberDiff line change
@@ -1654,11 +1654,12 @@ impl<'a> Resolver<'a> {
16541654
let path: Vec<Ident> = segments.iter()
16551655
.map(|seg| Ident::new(seg.name, span))
16561656
.collect();
1657-
match self.resolve_path(&path, Some(namespace), true, span) {
1657+
// FIXME (Manishearth): Intra doc links won't get warned of epoch changes
1658+
match self.resolve_path(&path, Some(namespace), true, span, None) {
16581659
PathResult::Module(module) => *def = module.def().unwrap(),
16591660
PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 =>
16601661
*def = path_res.base_def(),
1661-
PathResult::NonModule(..) => match self.resolve_path(&path, None, true, span) {
1662+
PathResult::NonModule(..) => match self.resolve_path(&path, None, true, span, None) {
16621663
PathResult::Failed(span, msg, _) => {
16631664
error_callback(self, span, ResolutionError::FailedToResolve(&msg));
16641665
}
@@ -2360,7 +2361,8 @@ impl<'a> Resolver<'a> {
23602361
if def != Def::Err {
23612362
new_id = Some(def.def_id());
23622363
let span = trait_ref.path.span;
2363-
if let PathResult::Module(module) = self.resolve_path(&path, None, false, span) {
2364+
if let PathResult::Module(module) = self.resolve_path(&path, None, false, span,
2365+
Some(trait_ref.ref_id)) {
23642366
new_val = Some((module, trait_ref.clone()));
23652367
}
23662368
}
@@ -2819,7 +2821,8 @@ impl<'a> Resolver<'a> {
28192821
(format!(""), format!("the crate root"))
28202822
} else {
28212823
let mod_path = &path[..path.len() - 1];
2822-
let mod_prefix = match this.resolve_path(mod_path, Some(TypeNS), false, span) {
2824+
let mod_prefix = match this.resolve_path(mod_path, Some(TypeNS),
2825+
false, span, None) {
28232826
PathResult::Module(module) => module.def(),
28242827
_ => None,
28252828
}.map_or(format!(""), |def| format!("{} ", def.kind_name()));
@@ -3149,7 +3152,7 @@ impl<'a> Resolver<'a> {
31493152
));
31503153
}
31513154

3152-
let result = match self.resolve_path(&path, Some(ns), true, span) {
3155+
let result = match self.resolve_path(&path, Some(ns), true, span, Some(id)) {
31533156
PathResult::NonModule(path_res) => path_res,
31543157
PathResult::Module(module) if !module.is_normal() => {
31553158
PathResolution::new(module.def().unwrap())
@@ -3186,7 +3189,7 @@ impl<'a> Resolver<'a> {
31863189
path[0].name != keywords::CrateRoot.name() &&
31873190
path[0].name != keywords::DollarCrate.name() {
31883191
let unqualified_result = {
3189-
match self.resolve_path(&[*path.last().unwrap()], Some(ns), false, span) {
3192+
match self.resolve_path(&[*path.last().unwrap()], Some(ns), false, span, None) {
31903193
PathResult::NonModule(path_res) => path_res.base_def(),
31913194
PathResult::Module(module) => module.def().unwrap(),
31923195
_ => return Some(result),
@@ -3205,7 +3208,9 @@ impl<'a> Resolver<'a> {
32053208
path: &[Ident],
32063209
opt_ns: Option<Namespace>, // `None` indicates a module path
32073210
record_used: bool,
3208-
path_span: Span)
3211+
path_span: Span,
3212+
node_id: Option<NodeId>) // None indicates that we don't care about linting
3213+
// `::module` paths
32093214
-> PathResult<'a> {
32103215
let mut module = None;
32113216
let mut allow_super = true;
@@ -3253,6 +3258,8 @@ impl<'a> Resolver<'a> {
32533258
let prev_name = path[0].name;
32543259
if prev_name == keywords::Extern.name() ||
32553260
prev_name == keywords::CrateRoot.name() &&
3261+
// Note: When this feature stabilizes, this should
3262+
// be gated on sess.rust_2018()
32563263
self.session.features_untracked().extern_absolute_paths {
32573264
// `::extern_crate::a::b`
32583265
let crate_id = self.crate_loader.process_path_extern(name, ident.span);
@@ -3324,6 +3331,33 @@ impl<'a> Resolver<'a> {
33243331
format!("Not a module `{}`", ident),
33253332
is_last);
33263333
}
3334+
3335+
if let Some(id) = node_id {
3336+
if i == 1 && self.session.features_untracked().crate_in_paths
3337+
&& !self.session.rust_2018() {
3338+
let prev_name = path[0].name;
3339+
if prev_name == keywords::Extern.name() ||
3340+
prev_name == keywords::CrateRoot.name() {
3341+
let mut is_crate = false;
3342+
if let NameBindingKind::Import { directive: d, .. } = binding.kind {
3343+
if let ImportDirectiveSubclass::ExternCrate(..) = d.subclass {
3344+
is_crate = true;
3345+
}
3346+
}
3347+
3348+
if !is_crate {
3349+
let diag = lint::builtin::BuiltinLintDiagnostics
3350+
::AbsPathWithModule(path_span);
3351+
self.session.buffer_lint_with_diagnostic(
3352+
lint::builtin::ABSOLUTE_PATH_STARTING_WITH_MODULE,
3353+
id, path_span,
3354+
"Absolute paths must start with `self`, `super`, \
3355+
`crate`, or an external crate name in the 2018 edition",
3356+
diag);
3357+
}
3358+
}
3359+
}
3360+
}
33273361
}
33283362
Err(Undetermined) => return PathResult::Indeterminate,
33293363
Err(Determined) => {
@@ -3571,7 +3605,7 @@ impl<'a> Resolver<'a> {
35713605
// Search in module.
35723606
let mod_path = &path[..path.len() - 1];
35733607
if let PathResult::Module(module) = self.resolve_path(mod_path, Some(TypeNS),
3574-
false, span) {
3608+
false, span, None) {
35753609
add_module_candidates(module, &mut names);
35763610
}
35773611
}

‎src/librustc_resolve/macros.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ impl<'a> Resolver<'a> {
438438
return Err(Determinacy::Determined);
439439
}
440440

441-
let def = match self.resolve_path(&path, Some(MacroNS), false, span) {
441+
let def = match self.resolve_path(&path, Some(MacroNS), false, span, None) {
442442
PathResult::NonModule(path_res) => match path_res.base_def() {
443443
Def::Err => Err(Determinacy::Determined),
444444
def @ _ => {
@@ -616,7 +616,7 @@ impl<'a> Resolver<'a> {
616616
pub fn finalize_current_module_macro_resolutions(&mut self) {
617617
let module = self.current_module;
618618
for &(ref path, span) in module.macro_resolutions.borrow().iter() {
619-
match self.resolve_path(&path, Some(MacroNS), true, span) {
619+
match self.resolve_path(&path, Some(MacroNS), true, span, None) {
620620
PathResult::NonModule(_) => {},
621621
PathResult::Failed(span, msg, _) => {
622622
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));

‎src/librustc_resolve/resolve_imports.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
535535
// For better failure detection, pretend that the import will not define any names
536536
// while resolving its module path.
537537
directive.vis.set(ty::Visibility::Invisible);
538-
let result = self.resolve_path(&directive.module_path[..], None, false, directive.span);
538+
let result = self.resolve_path(&directive.module_path[..], None, false,
539+
directive.span, Some(directive.id));
539540
directive.vis.set(vis);
540541

541542
match result {
@@ -663,7 +664,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
663664
}
664665
}
665666

666-
let module_result = self.resolve_path(&module_path, None, true, span);
667+
let module_result = self.resolve_path(&module_path, None, true, span, Some(directive.id));
667668
let module = match module_result {
668669
PathResult::Module(module) => module,
669670
PathResult::Failed(span, msg, false) => {
@@ -677,7 +678,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
677678
if !self_path.is_empty() && !is_special(self_path[0]) &&
678679
!(self_path.len() > 1 && is_special(self_path[1])) {
679680
self_path[0].name = keywords::SelfValue.name();
680-
self_result = Some(self.resolve_path(&self_path, None, false, span));
681+
self_result = Some(self.resolve_path(&self_path, None, false,
682+
span, None));
681683
}
682684
return if let Some(PathResult::Module(..)) = self_result {
683685
Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..]))))

‎src/test/ui/edition-lint-paths.rs

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(crate_in_paths)]
12+
#![deny(absolute_path_starting_with_module)]
13+
#![allow(unused)]
14+
15+
pub mod foo {
16+
use ::bar::Bar;
17+
//~^ ERROR Absolute
18+
//~| WARN this was previously accepted
19+
use super::bar::Bar2;
20+
use crate::bar::Bar3;
21+
}
22+
23+
24+
use bar::Bar;
25+
//~^ ERROR Absolute
26+
//~| WARN this was previously accepted
27+
28+
pub mod bar {
29+
pub struct Bar;
30+
pub type Bar2 = Bar;
31+
pub type Bar3 = Bar;
32+
}
33+
34+
fn main() {
35+
let x = ::bar::Bar;
36+
//~^ ERROR Absolute
37+
//~| WARN this was previously accepted
38+
let x = bar::Bar;
39+
let x = ::crate::bar::Bar;
40+
let x = self::bar::Bar;
41+
}

‎src/test/ui/edition-lint-paths.stderr

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
error: Absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
2+
--> $DIR/edition-lint-paths.rs:16:9
3+
|
4+
LL | use ::bar::Bar;
5+
| ^^^^^^^^^^ help: use `crate`: `crate::bar::Bar`
6+
|
7+
note: lint level defined here
8+
--> $DIR/edition-lint-paths.rs:12:9
9+
|
10+
LL | #![deny(absolute_path_starting_with_module)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
13+
= note: for more information, see issue TBD
14+
15+
error: Absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
16+
--> $DIR/edition-lint-paths.rs:24:5
17+
|
18+
LL | use bar::Bar;
19+
| ^^^^^^^^ help: use `crate`: `crate::bar::Bar`
20+
|
21+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
22+
= note: for more information, see issue TBD
23+
24+
error: Absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
25+
--> $DIR/edition-lint-paths.rs:35:13
26+
|
27+
LL | let x = ::bar::Bar;
28+
| ^^^^^^^^^^ help: use `crate`: `crate::bar::Bar`
29+
|
30+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
31+
= note: for more information, see issue TBD
32+
33+
error: aborting due to 3 previous errors
34+

0 commit comments

Comments
 (0)