diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 02c1ece163497..667c2590fa996 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -211,6 +211,12 @@ declare_lint! { not named `mod.rs`" } +declare_lint! { + pub LEGACY_IMPORTS, + Warn, + "detects names that resolve to ambiguous glob imports with RFC 1560" +} + declare_lint! { pub DEPRECATED, Warn, @@ -257,6 +263,7 @@ impl LintPass for HardwiredLints { PATTERNS_IN_FNS_WITHOUT_BODY, EXTRA_REQUIREMENT_IN_IMPL, LEGACY_DIRECTORY_OWNERSHIP, + LEGACY_IMPORTS, DEPRECATED ) } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 6c9a3e99a0458..a53d43b2a257a 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -234,6 +234,10 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { id: LintId::of(LEGACY_DIRECTORY_OWNERSHIP), reference: "issue #37872 ", }, + FutureIncompatibleInfo { + id: LintId::of(LEGACY_IMPORTS), + reference: "issue #38260 ", + }, ]); // Register renamed and removed lints diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index f7aaf2475f65e..fa2897924933d 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -65,7 +65,7 @@ use syntax::ast::{Item, ItemKind, ImplItem, ImplItemKind}; use syntax::ast::{Local, Mutability, Pat, PatKind, Path}; use syntax::ast::{PathSegment, PathParameters, QSelf, TraitItemKind, TraitRef, Ty, TyKind}; -use syntax_pos::{Span, DUMMY_SP}; +use syntax_pos::{Span, DUMMY_SP, MultiSpan}; use errors::DiagnosticBuilder; use std::cell::{Cell, RefCell}; @@ -897,6 +897,7 @@ enum NameBindingKind<'a> { Ambiguity { b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>, + legacy: bool, } } @@ -908,6 +909,7 @@ struct AmbiguityError<'a> { lexical: bool, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>, + legacy: bool, } impl<'a> NameBinding<'a> { @@ -915,6 +917,7 @@ impl<'a> NameBinding<'a> { match self.kind { NameBindingKind::Module(module) => Some(module), NameBindingKind::Import { binding, .. } => binding.module(), + NameBindingKind::Ambiguity { legacy: true, b1, .. } => b1.module(), _ => None, } } @@ -924,6 +927,7 @@ impl<'a> NameBinding<'a> { NameBindingKind::Def(def) => def, NameBindingKind::Module(module) => module.def().unwrap(), NameBindingKind::Import { binding, .. } => binding.def(), + NameBindingKind::Ambiguity { legacy: true, b1, .. } => b1.def(), NameBindingKind::Ambiguity { .. } => Def::Err, } } @@ -1350,11 +1354,14 @@ impl<'a> Resolver<'a> { self.record_use(name, ns, binding, span) } NameBindingKind::Import { .. } => false, - NameBindingKind::Ambiguity { b1, b2 } => { + NameBindingKind::Ambiguity { b1, b2, legacy } => { self.ambiguity_errors.push(AmbiguityError { - span: span, name: name, lexical: false, b1: b1, b2: b2, + span: span, name: name, lexical: false, b1: b1, b2: b2, legacy: legacy, }); - true + if legacy { + self.record_use(name, ns, b1, span); + } + !legacy } _ => false } @@ -3064,26 +3071,39 @@ impl<'a> Resolver<'a> { self.report_shadowing_errors(); let mut reported_spans = FxHashSet(); - for &AmbiguityError { span, name, b1, b2, lexical } in &self.ambiguity_errors { + for &AmbiguityError { span, name, b1, b2, lexical, legacy } in &self.ambiguity_errors { if !reported_spans.insert(span) { continue } let participle = |binding: &NameBinding| { if binding.is_import() { "imported" } else { "defined" } }; let msg1 = format!("`{}` could resolve to the name {} here", name, participle(b1)); let msg2 = format!("`{}` could also resolve to the name {} here", name, participle(b2)); - self.session.struct_span_err(span, &format!("`{}` is ambiguous", name)) - .span_note(b1.span, &msg1) - .span_note(b2.span, &msg2) - .note(&if !lexical && b1.is_glob_import() { - format!("consider adding an explicit import of `{}` to disambiguate", name) - } else if let Def::Macro(..) = b1.def() { - format!("macro-expanded {} do not shadow", - if b1.is_import() { "macro imports" } else { "macros" }) - } else { - format!("macro-expanded {} do not shadow when used in a macro invocation path", - if b1.is_import() { "imports" } else { "items" }) - }) - .emit(); + let note = if !lexical && b1.is_glob_import() { + format!("consider adding an explicit import of `{}` to disambiguate", name) + } else if let Def::Macro(..) = b1.def() { + format!("macro-expanded {} do not shadow", + if b1.is_import() { "macro imports" } else { "macros" }) + } else { + format!("macro-expanded {} do not shadow when used in a macro invocation path", + if b1.is_import() { "imports" } else { "items" }) + }; + if legacy { + let id = match b2.kind { + NameBindingKind::Import { directive, .. } => directive.id, + _ => unreachable!(), + }; + let mut span = MultiSpan::from_span(span); + span.push_span_label(b1.span, msg1); + span.push_span_label(b2.span, msg2); + let msg = format!("`{}` is ambiguous", name); + self.session.add_lint(lint::builtin::LEGACY_IMPORTS, id, span, msg); + } else { + self.session.struct_span_err(span, &format!("`{}` is ambiguous", name)) + .span_note(b1.span, &msg1) + .span_note(b2.span, &msg2) + .note(¬e) + .emit(); + } } for &PrivacyError(span, name, binding) in &self.privacy_errors { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 6c02967672d84..613829bab8be3 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -288,6 +288,7 @@ impl<'a> Resolver<'a> { Some(shadower) if shadower.def() != binding.def() => { self.ambiguity_errors.push(AmbiguityError { span: span, name: name, b1: shadower, b2: binding, lexical: true, + legacy: false, }); return Ok(shadower); } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index b634d57a842f6..4a620edbe823e 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -161,6 +161,7 @@ impl<'a> Resolver<'a> { binding.def() != shadowed_glob.def() { self.ambiguity_errors.push(AmbiguityError { span: span, name: name, lexical: false, b1: binding, b2: shadowed_glob, + legacy: false, }); } } @@ -338,9 +339,9 @@ impl<'a> Resolver<'a> { } pub fn ambiguity(&mut self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>) - -> &'a NameBinding<'a> { + -> &'a NameBinding<'a> { self.arenas.alloc_name_binding(NameBinding { - kind: NameBindingKind::Ambiguity { b1: b1, b2: b2 }, + kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: false }, vis: if b1.vis.is_at_least(b2.vis, self) { b1.vis } else { b2.vis }, span: b1.span, expansion: Mark::root(), @@ -728,7 +729,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } for (&(name, ns), resolution) in module.resolutions.borrow().iter() { - let resolution = resolution.borrow(); + let resolution = &mut *resolution.borrow_mut(); let binding = match resolution.binding { Some(binding) => binding, None => continue, @@ -745,14 +746,34 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } - if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind { - if ns == TypeNS && orig_binding.is_variant() && - !orig_binding.vis.is_at_least(binding.vis, self) { - let msg = format!("variant `{}` is private, and cannot be reexported \ - (error E0364), consider declaring its enum as `pub`", - name); - self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg); + match binding.kind { + NameBindingKind::Import { binding: orig_binding, directive, .. } => { + if ns == TypeNS && orig_binding.is_variant() && + !orig_binding.vis.is_at_least(binding.vis, self) { + let msg = format!("variant `{}` is private, and cannot be reexported \ + (error E0364), consider declaring its enum as `pub`", + name); + self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg); + } + } + NameBindingKind::Ambiguity { b1, b2, .. } + if b1.is_glob_import() && b2.is_glob_import() => { + let (orig_b1, orig_b2) = match (&b1.kind, &b2.kind) { + (&NameBindingKind::Import { binding: b1, .. }, + &NameBindingKind::Import { binding: b2, .. }) => (b1, b2), + _ => continue, + }; + let (b1, b2) = match (orig_b1.vis, orig_b2.vis) { + (ty::Visibility::Public, ty::Visibility::Public) => continue, + (ty::Visibility::Public, _) => (b1, b2), + (_, ty::Visibility::Public) => (b2, b1), + _ => continue, + }; + resolution.binding = Some(self.arenas.alloc_name_binding(NameBinding { + kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: true }, ..*b1 + })); } + _ => {} } } diff --git a/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs b/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs new file mode 100644 index 0000000000000..bed10c87ae185 --- /dev/null +++ b/src/test/compile-fail/imports/rfc-1560-warning-cycle.rs @@ -0,0 +1,30 @@ +// Copyright 2016 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(rustc_attrs)] +#![allow(unused)] + +pub struct Foo; + +mod bar { + struct Foo; + + mod baz { + use *; //~ NOTE `Foo` could resolve to the name imported here + use bar::*; //~ NOTE `Foo` could also resolve to the name imported here + fn f(_: Foo) {} + //~^ WARN `Foo` is ambiguous + //~| WARN hard error in a future release + //~| NOTE see issue #38260 + } +} + +#[rustc_error] +fn main() {} //~ ERROR compilation successful