Auto merge of #32284 - jseyfried:name_conflict_diagnostics, r=eddyb

Resolve: improve diagnostics for duplicate definitions and imports

This PR improves and regularizes the diagnostics for duplicate definitions and imports.

After this PR, the second of two duplicate definitions/imports will have the following form:
> a(n) [value|type|module|trait|extern crate] named \`*name*\` has already been [defined|imported] in this [module|block|trait|enum]

with a note referencing this first of the two duplicate definitions/imports:
> previous [definition|import] of \`*name*\` here

The error indices remain unchanged.

r? @eddyb
This commit is contained in:
bors 2016-03-16 16:18:12 -07:00
commit 6e0f2f2f05
25 changed files with 134 additions and 174 deletions

View file

@ -105,36 +105,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
/// otherwise, reports an error.
fn define<T: ToNameBinding<'b>>(&self, parent: Module<'b>, name: Name, ns: Namespace, def: T) {
let binding = def.to_name_binding();
let old_binding = match parent.try_define_child(name, ns, binding.clone()) {
Ok(()) => return,
Err(old_binding) => old_binding,
};
let span = binding.span.unwrap_or(DUMMY_SP);
if !old_binding.is_extern_crate() && !binding.is_extern_crate() {
// Record an error here by looking up the namespace that had the duplicate
let ns_str = match ns { TypeNS => "type or module", ValueNS => "value" };
let resolution_error = ResolutionError::DuplicateDefinition(ns_str, name);
let mut err = resolve_struct_error(self, span, resolution_error);
if let Some(sp) = old_binding.span {
let note = format!("first definition of {} `{}` here", ns_str, name);
err.span_note(sp, &note);
}
err.emit();
} else if old_binding.is_extern_crate() && binding.is_extern_crate() {
span_err!(self.session,
span,
E0259,
"an external crate named `{}` has already been imported into this module",
name);
} else {
span_err!(self.session,
span,
E0260,
"the name `{}` conflicts with an external crate \
that has been imported into this module",
name);
if let Err(old_binding) = parent.try_define_child(name, ns, binding.clone()) {
self.report_conflict(parent, name, ns, old_binding, &binding);
}
}

View file

@ -183,8 +183,6 @@ pub enum ResolutionError<'a> {
UndeclaredLabel(&'a str),
/// error E0427: cannot use `ref` binding mode with ...
CannotUseRefBindingModeWith(&'a str),
/// error E0428: duplicate definition
DuplicateDefinition(&'a str, Name),
/// error E0429: `self` imports are only allowed within a { } list
SelfImportsOnlyAllowedWithin,
/// error E0430: `self` import can only appear once in the list
@ -490,14 +488,6 @@ fn resolve_struct_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>,
"cannot use `ref` binding mode with {}",
descr)
}
ResolutionError::DuplicateDefinition(namespace, name) => {
struct_span_err!(resolver.session,
span,
E0428,
"duplicate definition of {} `{}`",
namespace,
name)
}
ResolutionError::SelfImportsOnlyAllowedWithin => {
struct_span_err!(resolver.session,
span,
@ -3530,8 +3520,62 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
}
}
fn report_conflict(&self,
parent: Module,
name: Name,
ns: Namespace,
binding: &NameBinding,
old_binding: &NameBinding) {
// Error on the second of two conflicting names
if old_binding.span.unwrap().lo > binding.span.unwrap().lo {
return self.report_conflict(parent, name, ns, old_binding, binding);
}
let container = match parent.def {
Some(Def::Mod(_)) => "module",
Some(Def::Trait(_)) => "trait",
None => "block",
_ => "enum",
};
let (participle, noun) = match old_binding.is_import() || old_binding.is_extern_crate() {
true => ("imported", "import"),
false => ("defined", "definition"),
};
let span = binding.span.unwrap();
let msg = {
let kind = match (ns, old_binding.module()) {
(ValueNS, _) => "a value",
(TypeNS, Some(module)) if module.extern_crate_id.is_some() => "an extern crate",
(TypeNS, Some(module)) if module.is_normal() => "a module",
(TypeNS, Some(module)) if module.is_trait() => "a trait",
(TypeNS, _) => "a type",
};
format!("{} named `{}` has already been {} in this {}",
kind, name, participle, container)
};
let mut err = match (old_binding.is_extern_crate(), binding.is_extern_crate()) {
(true, true) => struct_span_err!(self.session, span, E0259, "{}", msg),
(true, _) | (_, true) if binding.is_import() || old_binding.is_import() =>
struct_span_err!(self.session, span, E0254, "{}", msg),
(true, _) | (_, true) => struct_span_err!(self.session, span, E0260, "{}", msg),
_ => match (old_binding.is_import(), binding.is_import()) {
(false, false) => struct_span_err!(self.session, span, E0428, "{}", msg),
(true, true) => struct_span_err!(self.session, span, E0252, "{}", msg),
_ => struct_span_err!(self.session, span, E0255, "{}", msg),
},
};
let span = old_binding.span.unwrap();
if span != codemap::DUMMY_SP {
err.span_note(span, &format!("previous {} of `{}` here", noun, name));
}
err.emit();
}
}
fn names_to_string(names: &[Name]) -> String {
let mut first = true;

View file

@ -513,7 +513,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
let imported_binding = directive.import(binding, privacy_error);
let conflict = module_.try_define_child(target, ns, imported_binding);
if let Err(old_binding) = conflict {
self.report_conflict(target, ns, &directive.import(binding, None), old_binding);
let binding = &directive.import(binding, None);
self.resolver.report_conflict(module_, target, ns, binding, old_binding);
}
}
@ -650,67 +651,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
return Success(());
}
fn report_conflict(&mut self,
name: Name,
ns: Namespace,
binding: &NameBinding,
old_binding: &NameBinding) {
// Error on the second of two conflicting imports
if old_binding.is_import() && binding.is_import() &&
old_binding.span.unwrap().lo > binding.span.unwrap().lo {
self.report_conflict(name, ns, old_binding, binding);
return;
}
if old_binding.is_extern_crate() {
let msg = format!("import `{0}` conflicts with imported crate \
in this module (maybe you meant `use {0}::*`?)",
name);
span_err!(self.resolver.session, binding.span.unwrap(), E0254, "{}", &msg);
} else if old_binding.is_import() {
let ns_word = match (ns, old_binding.module()) {
(ValueNS, _) => "value",
(TypeNS, Some(module)) if module.is_normal() => "module",
(TypeNS, Some(module)) if module.is_trait() => "trait",
(TypeNS, _) => "type",
};
let mut err = struct_span_err!(self.resolver.session,
binding.span.unwrap(),
E0252,
"a {} named `{}` has already been imported \
in this module",
ns_word,
name);
err.span_note(old_binding.span.unwrap(),
&format!("previous import of `{}` here", name));
err.emit();
} else if ns == ValueNS { // Check for item conflicts in the value namespace
let mut err = struct_span_err!(self.resolver.session,
binding.span.unwrap(),
E0255,
"import `{}` conflicts with value in this module",
name);
err.span_note(old_binding.span.unwrap(), "conflicting value here");
err.emit();
} else { // Check for item conflicts in the type namespace
let (what, note) = match old_binding.module() {
Some(ref module) if module.is_normal() =>
("existing submodule", "note conflicting module here"),
Some(ref module) if module.is_trait() =>
("trait in this module", "note conflicting trait here"),
_ => ("type in this module", "note conflicting type here"),
};
let mut err = struct_span_err!(self.resolver.session,
binding.span.unwrap(),
E0256,
"import `{}` conflicts with {}",
name,
what);
err.span_note(old_binding.span.unwrap(), note);
err.emit();
}
}
// Miscellaneous post-processing, including recording reexports, recording shadowed traits,
// reporting conflicts, reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
fn finalize_resolutions(&mut self, module: Module<'b>, report_unresolved_imports: bool) {
@ -720,7 +660,10 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
let mut reexports = Vec::new();
for (&(name, ns), resolution) in module.resolutions.borrow().iter() {
resolution.report_conflicts(|b1, b2| self.report_conflict(name, ns, b1, b2));
resolution.report_conflicts(|b1, b2| {
self.resolver.report_conflict(module, name, ns, b1, b2)
});
let binding = match resolution.binding {
Some(binding) => binding,
None => continue,

View file

@ -14,7 +14,7 @@ fn main() {
{
struct Bar;
use foo::Bar;
//~^ ERROR import `Bar` conflicts with type in this module
//~^^ ERROR import `Bar` conflicts with value in this module
//~^ ERROR a type named `Bar` has already been defined in this block
//~^^ ERROR a value named `Bar` has already been defined in this block
}
}

View file

@ -8,8 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
mod foo { pub mod foo { } }
mod foo { pub mod foo { } } //~ NOTE previous definition of `foo` here
use foo::foo; //~ ERROR import `foo` conflicts with existing submodule
use foo::foo; //~ ERROR a module named `foo` has already been defined in this module
fn main() {}

View file

@ -8,11 +8,11 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
enum Foo {
enum Foo { //~ NOTE previous definition
X
}
mod Foo { //~ ERROR duplicate definition of type or module `Foo`
mod Foo { //~ ERROR a type named `Foo` has already been defined
pub static X: isize = 42;
fn f() { f() } // Check that this does not result in a resolution error
}

View file

@ -8,14 +8,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use self::A; //~ ERROR import `A` conflicts with existing submodule
use self::B; //~ ERROR import `B` conflicts with existing submodule
mod A {}
pub mod B {}
use self::A; //~ NOTE previous import of `A` here
use self::B; //~ NOTE previous import of `B` here
mod A {} //~ ERROR a module named `A` has already been imported in this module
pub mod B {} //~ ERROR a module named `B` has already been imported in this module
mod C {
use C::D; //~ ERROR import `D` conflicts with existing submodule
mod D {}
use C::D; //~ NOTE previous import of `D` here
mod D {} //~ ERROR a module named `D` has already been imported in this module
}
fn main() {}

View file

@ -12,54 +12,54 @@
#[allow(non_snake_case)]
mod Foo { }
//~^ NOTE first definition of type or module `Foo`
//~^ NOTE previous definition of `Foo` here
#[allow(dead_code)]
struct Foo;
//~^ ERROR duplicate definition of type or module `Foo`
//~^ ERROR a module named `Foo` has already been defined in this module
#[allow(non_snake_case)]
mod Bar { }
//~^ NOTE first definition of type or module `Bar`
//~^ NOTE previous definition of `Bar` here
#[allow(dead_code)]
struct Bar(i32);
//~^ ERROR duplicate definition of type or module `Bar`
//~^ ERROR a module named `Bar` has already been defined
#[allow(dead_code)]
struct Baz(i32);
//~^ NOTE first definition of type or module
//~^ NOTE previous definition
#[allow(non_snake_case)]
mod Baz { }
//~^ ERROR duplicate definition of type or module `Baz`
//~^ ERROR a type named `Baz` has already been defined
#[allow(dead_code)]
struct Qux { x: bool }
//~^ NOTE first definition of type or module
//~^ NOTE previous definition
#[allow(non_snake_case)]
mod Qux { }
//~^ ERROR duplicate definition of type or module `Qux`
//~^ ERROR a type named `Qux` has already been defined
#[allow(dead_code)]
struct Quux;
//~^ NOTE first definition of type or module
//~^ NOTE previous definition
#[allow(non_snake_case)]
mod Quux { }
//~^ ERROR duplicate definition of type or module `Quux`
//~^ ERROR a type named `Quux` has already been defined
#[allow(dead_code)]
enum Corge { A, B }
//~^ NOTE previous definition
#[allow(non_snake_case)]
mod Corge { }
//~^ ERROR duplicate definition of type or module `Corge`
//~^ ERROR a type named `Corge` has already been defined
fn main() { }

View file

@ -8,16 +8,16 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use std::ops::Add; //~ ERROR import `Add` conflicts with type in this module
use std::ops::Sub; //~ ERROR import `Sub` conflicts with type in this module
use std::ops::Mul; //~ ERROR import `Mul` conflicts with type in this module
use std::ops::Div; //~ ERROR import `Div` conflicts with existing submodule
use std::ops::Rem; //~ ERROR import `Rem` conflicts with trait in this module
use std::ops::Add; //~ NOTE previous import
use std::ops::Sub; //~ NOTE previous import
use std::ops::Mul; //~ NOTE previous import
use std::ops::Div; //~ NOTE previous import
use std::ops::Rem; //~ NOTE previous import
type Add = bool;
struct Sub { x: f32 }
enum Mul { A, B }
mod Div { }
trait Rem { }
type Add = bool; //~ ERROR a trait named `Add` has already been imported in this module
struct Sub { x: f32 } //~ ERROR a trait named `Sub` has already been imported in this module
enum Mul { A, B } //~ ERROR a trait named `Mul` has already been imported in this module
mod Div { } //~ ERROR a trait named `Div` has already been imported in this module
trait Rem { } //~ ERROR a trait named `Rem` has already been imported in this module
fn main() {}

View file

@ -13,10 +13,10 @@
extern {
fn foo();
pub //~ ERROR duplicate definition
pub //~ ERROR a value named `foo` has already been defined
fn foo();
pub //~ ERROR duplicate definition
pub //~ ERROR a value named `foo` has already been defined
static mut foo: u32;
}

View file

@ -10,6 +10,6 @@
enum a { b, c }
enum a { d, e } //~ ERROR duplicate definition of type or module `a`
enum a { d, e } //~ ERROR a type named `a` has already been defined in this module
fn main() {}

View file

@ -10,6 +10,6 @@
pub mod a {}
pub mod a {} //~ ERROR duplicate definition of type or module `a`
pub mod a {} //~ ERROR a module named `a` has already been defined in this module
fn main() {}

View file

@ -12,7 +12,7 @@ fn a(x: String) -> String {
format!("First function with {}", x)
}
fn a(x: String, y: String) -> String { //~ ERROR duplicate definition of value `a`
fn a(x: String, y: String) -> String { //~ ERROR a value named `a` has already been defined
format!("Second function with {} and {}", x, y)
}

View file

@ -12,17 +12,17 @@ struct T;
mod t1 {
type Foo = ::T;
mod Foo {} //~ ERROR: duplicate definition of type or module `Foo`
mod Foo {} //~ ERROR: `Foo` has already been defined
}
mod t2 {
type Foo = ::T;
struct Foo; //~ ERROR: duplicate definition of type or module `Foo`
struct Foo; //~ ERROR: `Foo` has already been defined
}
mod t3 {
type Foo = ::T;
enum Foo {} //~ ERROR: duplicate definition of type or module `Foo`
enum Foo {} //~ ERROR: `Foo` has already been defined
}
mod t4 {
@ -32,7 +32,7 @@ mod t4 {
mod t5 {
type Bar<T> = T;
mod Bar {} //~ ERROR: duplicate definition of type or module `Bar`
mod Bar {} //~ ERROR: `Bar` has already been defined
}
mod t6 {

View file

@ -9,6 +9,6 @@
// except according to those terms.
static X: isize = 0;
struct X; //~ ERROR error: duplicate definition of value `X`
struct X; //~ ERROR `X` has already been defined
fn main() {}

View file

@ -12,8 +12,8 @@
mod foo {
use baz::bar;
//~^ ERROR import `bar` conflicts with existing submodule
mod bar {}
//~^ ERROR a module named `bar` has already been imported
}
mod baz { pub mod bar {} }

View file

@ -10,7 +10,7 @@
#![no_std]
extern crate core; //~ ERROR: an external crate named `core` has already
extern crate core; //~ ERROR: an extern crate named `core` has already
extern crate std;
fn main() {}

View file

@ -9,6 +9,6 @@
// except according to those terms.
extern crate std;
//~^ ERROR an external crate named `std` has already been imported
//~^ ERROR an extern crate named `std` has already been imported
fn main(){}

View file

@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use std::slice as std; //~ ERROR import `std` conflicts with imported crate
use std::slice as std; //~ ERROR an extern crate named `std` has already been imported
fn main() {
}

View file

@ -9,7 +9,7 @@
// except according to those terms.
fn std() {}
mod std {} //~ ERROR the name `std` conflicts with an external crate
mod std {} //~ ERROR an extern crate named `std` has already been imported
fn main() {
}

View file

@ -9,9 +9,10 @@
// except according to those terms.
use std::mem::transmute;
//~^ ERROR import `transmute` conflicts with value in this module
//~^ NOTE previous import of `transmute` here
fn transmute() {}
//~^ ERROR a value named `transmute` has already been imported in this module
fn main() {
}

View file

@ -9,9 +9,9 @@
// except according to those terms.
use std::slice::Iter;
//~^ ERROR import `Iter` conflicts with type in this module
struct Iter;
//~^ ERROR a type named `Iter` has already been imported in this module
fn main() {
}

View file

@ -9,8 +9,8 @@
// except according to those terms.
trait Foo {
fn orange(&self);
fn orange(&self); //~ ERROR error: duplicate definition of value `orange`
fn orange(&self); //~ NOTE previous definition of `orange` here
fn orange(&self); //~ ERROR a value named `orange` has already been defined in this trait
}
fn main() {}

View file

@ -10,6 +10,6 @@
extern crate core;
use core;
//~^ ERROR import `core` conflicts with imported crate in this module
//~^ ERROR an extern crate named `core` has already been imported in this module
fn main() {}

View file

@ -10,22 +10,6 @@
// aux-build:variant-namespacing.rs
extern crate variant_namespacing;
pub use variant_namespacing::XE::*;
//~^ ERROR import `XStruct` conflicts with type in this module
//~| ERROR import `XStruct` conflicts with value in this module
//~| ERROR import `XTuple` conflicts with type in this module
//~| ERROR import `XTuple` conflicts with value in this module
//~| ERROR import `XUnit` conflicts with type in this module
//~| ERROR import `XUnit` conflicts with value in this module
pub use E::*;
//~^ ERROR import `Struct` conflicts with type in this module
//~| ERROR import `Struct` conflicts with value in this module
//~| ERROR import `Tuple` conflicts with type in this module
//~| ERROR import `Tuple` conflicts with value in this module
//~| ERROR import `Unit` conflicts with type in this module
//~| ERROR import `Unit` conflicts with value in this module
enum E {
Struct { a: u8 },
Tuple(u8),
@ -46,4 +30,20 @@ const XStruct: u8 = 0;
const XTuple: u8 = 0;
const XUnit: u8 = 0;
extern crate variant_namespacing;
pub use variant_namespacing::XE::*;
//~^ ERROR `XStruct` has already been defined
//~| ERROR `XStruct` has already been defined
//~| ERROR `XTuple` has already been defined
//~| ERROR `XTuple` has already been defined
//~| ERROR `XUnit` has already been defined
//~| ERROR `XUnit` has already been defined
pub use E::*;
//~^ ERROR `Struct` has already been defined
//~| ERROR `Struct` has already been defined
//~| ERROR `Tuple` has already been defined
//~| ERROR `Tuple` has already been defined
//~| ERROR `Unit` has already been defined
//~| ERROR `Unit` has already been defined
fn main() {}