From 56ace0aac21c46e8340e0912e822724c5d15922e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 19 Apr 2018 17:27:31 -0700 Subject: [PATCH 1/5] Pass down NodeId to resolve_path --- src/librustc_resolve/lib.rs | 20 ++++++++++++-------- src/librustc_resolve/macros.rs | 4 ++-- src/librustc_resolve/resolve_imports.rs | 8 +++++--- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 671856c4e549e..1abf336ed73dc 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1654,11 +1654,12 @@ impl<'a> Resolver<'a> { let path: Vec = 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)); } @@ -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())); } } @@ -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())); @@ -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()) @@ -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, Some(id)) { PathResult::NonModule(path_res) => path_res.base_def(), PathResult::Module(module) => module.def().unwrap(), _ => return Some(result), @@ -3205,7 +3208,8 @@ impl<'a> Resolver<'a> { path: &[Ident], opt_ns: Option, // `None` indicates a module path record_used: bool, - path_span: Span) + path_span: Span, + _node_id: Option) -> PathResult<'a> { let mut module = None; let mut allow_super = true; @@ -3571,7 +3575,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); } } diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 0388465b485cb..922ffe7714791 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -426,7 +426,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 @ _ => { @@ -604,7 +604,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)); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 37c62a7b0b45b..d6230cc4a67ae 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -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 { @@ -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) => { @@ -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, Some(directive.id))); } return if let Some(PathResult::Module(..)) = self_result { Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..])))) From 37d3bea3ec3d47dbf1f7352c32fab75ce408798c Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 19 Apr 2018 16:45:33 -0700 Subject: [PATCH 2/5] Add ABSOLUTE_PATH_STARTING_WITH_MODULE epoch lint for path breakage --- src/librustc/lint/builtin.rs | 8 +++++++ src/librustc_lint/lib.rs | 7 +++++- src/librustc_resolve/lib.rs | 29 +++++++++++++++++++++++-- src/librustc_resolve/resolve_imports.rs | 2 +- 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 97cfcf0f60795..30ae830ee464d 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -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, @@ -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, ) } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 16e8600f2d89b..038d47dd5eae5 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -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; @@ -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 diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1abf336ed73dc..c0bb6921b6934 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3189,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, Some(id)) { + 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), @@ -3209,7 +3209,8 @@ impl<'a> Resolver<'a> { opt_ns: Option, // `None` indicates a module path record_used: bool, path_span: Span, - _node_id: Option) + node_id: Option) // None indicates that we don't care about linting + // `::module` paths -> PathResult<'a> { let mut module = None; let mut allow_super = true; @@ -3328,6 +3329,30 @@ 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 { + self.session.buffer_lint( + lint::builtin::ABSOLUTE_PATH_STARTING_WITH_MODULE, + id, path_span, + "Fully-qualified paths must start with `self`, `super`, + `crate`, or an external crate name in the 2018 edition"); + } + } + } + } } Err(Undetermined) => return PathResult::Indeterminate, Err(Determined) => { diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index d6230cc4a67ae..e2a7f5668d251 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -679,7 +679,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { !(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, Some(directive.id))); + span, None)); } return if let Some(PathResult::Module(..)) = self_result { Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..])))) From f56c61face0e6ca8eaef8d872413e984c717657b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 19 Apr 2018 19:51:53 -0700 Subject: [PATCH 3/5] Add suggestion to lint --- src/librustc/lint/builtin.rs | 20 +++++++++++++++++++- src/librustc_resolve/lib.rs | 9 ++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 30ae830ee464d..109edffcde38a 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -332,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 { @@ -347,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::") + }; + db.span_suggestion(span, "use `crate`", sugg); + } } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index c0bb6921b6934..a41cd62ea7a8b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3344,11 +3344,14 @@ impl<'a> Resolver<'a> { } if !is_crate { - self.session.buffer_lint( + let diag = lint::builtin::BuiltinLintDiagnostics + ::AbsPathWithModule(path_span); + self.session.buffer_lint_with_diagnostic( lint::builtin::ABSOLUTE_PATH_STARTING_WITH_MODULE, id, path_span, - "Fully-qualified paths must start with `self`, `super`, - `crate`, or an external crate name in the 2018 edition"); + "Absolute paths must start with `self`, `super`, \ + `crate`, or an external crate name in the 2018 edition", + diag); } } } From 11c62de242620d6ed738feff8ee3b2269b913fb7 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 20 Apr 2018 15:04:07 -0700 Subject: [PATCH 4/5] mention that extern absolute paths should gate on rust 2018 --- src/librustc_resolve/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index a41cd62ea7a8b..7a87a72afc294 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3258,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); From 2a5ce10dfb20e794d324cb1621e5ac892b7e421a Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 20 Apr 2018 16:30:14 -0700 Subject: [PATCH 5/5] Add test --- src/test/ui/edition-lint-paths.rs | 41 +++++++++++++++++++++++++++ src/test/ui/edition-lint-paths.stderr | 34 ++++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 src/test/ui/edition-lint-paths.rs create mode 100644 src/test/ui/edition-lint-paths.stderr diff --git a/src/test/ui/edition-lint-paths.rs b/src/test/ui/edition-lint-paths.rs new file mode 100644 index 0000000000000..0b49e72ccd94c --- /dev/null +++ b/src/test/ui/edition-lint-paths.rs @@ -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 or the MIT license +// , 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; +} diff --git a/src/test/ui/edition-lint-paths.stderr b/src/test/ui/edition-lint-paths.stderr new file mode 100644 index 0000000000000..509527e03743c --- /dev/null +++ b/src/test/ui/edition-lint-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-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 +