Auto merge of #42076 - alex-ozdemir:master, r=nrc

Clearer Error Message for Duplicate Definition

Clearer use of the error message and span labels to communicate duplication definitions/imports.

fixes #42061

Author: bors

src/librustc_resolve/lib.rs

@@ -3450,11 +3450,11 @@ impl<'a> Resolver<'a> {
                        parent: Module,
                        ident: Ident,
                        ns: Namespace,
-                       binding: &NameBinding,
+                       new_binding: &NameBinding,
                        old_binding: &NameBinding) {
         // Error on the second of two conflicting names
-        if old_binding.span.lo > binding.span.lo {
-            return self.report_conflict(parent, ident, ns, old_binding, binding);
+        if old_binding.span.lo > new_binding.span.lo {
+            return self.report_conflict(parent, ident, ns, old_binding, new_binding);
         }
 
         let container = match parent.kind {
@@ -3464,49 +3464,65 @@ impl<'a> Resolver<'a> {
             _ => "enum",
         };
 
-        let (participle, noun) = match old_binding.is_import() {
-            true => ("imported", "import"),
-            false => ("defined", "definition"),
+        let old_noun = match old_binding.is_import() {
+            true => "import",
+            false => "definition",
         };
 
-        let (name, span) = (ident.name, binding.span);
+        let new_participle = match new_binding.is_import() {
+            true => "imported",
+            false => "defined",
+        };
+
+        let (name, span) = (ident.name, new_binding.span);
 
         if let Some(s) = self.name_already_seen.get(&name) {
             if s == &span {
                 return;
             }
         }
 
-        let msg = {
-            let kind = match (ns, old_binding.module()) {
-                (ValueNS, _) => "a value",
-                (MacroNS, _) => "a macro",
-                (TypeNS, _) if old_binding.is_extern_crate() => "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 old_kind = match (ns, old_binding.module()) {
+            (ValueNS, _) => "value",
+            (MacroNS, _) => "macro",
+            (TypeNS, _) if old_binding.is_extern_crate() => "extern crate",
+            (TypeNS, Some(module)) if module.is_normal() => "module",
+            (TypeNS, Some(module)) if module.is_trait() => "trait",
+            (TypeNS, _) => "type",
+        };
+
+        let namespace = match ns {
+            ValueNS => "value",
+            MacroNS => "macro",
+            TypeNS => "type",
         };
 
-        let mut err = match (old_binding.is_extern_crate(), binding.is_extern_crate()) {
+        let msg = format!("the name `{}` is defined multiple times", name);
+
+        let mut err = match (old_binding.is_extern_crate(), new_binding.is_extern_crate()) {
             (true, true) => struct_span_err!(self.session, span, E0259, "{}", msg),
-            (true, _) | (_, true) => match binding.is_import() && old_binding.is_import() {
+            (true, _) | (_, true) => match new_binding.is_import() && old_binding.is_import() {
                 true => struct_span_err!(self.session, span, E0254, "{}", msg),
                 false => struct_span_err!(self.session, span, E0260, "{}", msg),
             },
-            _ => match (old_binding.is_import(), binding.is_import()) {
+            _ => match (old_binding.is_import(), new_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),
             },
         };
 
-        err.span_label(span, format!("`{}` already {}", name, participle));
+        err.note(&format!("`{}` must be defined only once in the {} namespace of this {}",
+                          name,
+                          namespace,
+                          container));
+
+        err.span_label(span, format!("`{}` re{} here", name, new_participle));
         if old_binding.span != syntax_pos::DUMMY_SP {
-            err.span_label(old_binding.span, format!("previous {} of `{}` here", noun, name));
+            err.span_label(old_binding.span, format!("previous {} of the {} `{}` here",
+                                                      old_noun, old_kind, name));
         }
+
         err.emit();
         self.name_already_seen.insert(name, span);
     }

src/test/compile-fail-fulldeps/proc-macro/shadow.rs

@@ -13,6 +13,6 @@
 #[macro_use]
 extern crate derive_a;
 #[macro_use]
-extern crate derive_a; //~ ERROR `derive_a` has already been imported
+extern crate derive_a; //~ ERROR the name `derive_a` is defined multiple times
 
 fn main() {}

src/test/compile-fail/E0254.rs

@@ -11,7 +11,7 @@
 #![feature(alloc)]
 
 extern crate alloc;
-//~^ NOTE previous import of `alloc` here
+//~^ NOTE previous import of the extern crate `alloc` here
 
 mod foo {
     pub trait alloc {
@@ -21,6 +21,7 @@ mod foo {
 
 use foo::alloc;
 //~^ ERROR E0254
-//~| NOTE already imported
+//~| NOTE `alloc` reimported here
+//~| NOTE `alloc` must be defined only once in the type namespace of this module
 
 fn main() {}

src/test/compile-fail/E0259.rs

@@ -11,10 +11,11 @@
 #![feature(alloc, libc)]
 
 extern crate alloc;
-//~^ NOTE previous import of `alloc` here
+//~^ NOTE previous import of the extern crate `alloc` here
 
 extern crate libc as alloc;
 //~^ ERROR E0259
-//~| NOTE `alloc` already imported
+//~| NOTE `alloc` reimported here
+//~| NOTE `alloc` must be defined only once in the type namespace of this module
 
 fn main() {}

src/test/compile-fail/E0260.rs

@@ -11,11 +11,12 @@
 #![feature(alloc)]
 
 extern crate alloc;
-//~^ NOTE previous import of `alloc` here
+//~^ NOTE previous import of the extern crate `alloc` here
 
 mod alloc {
-//~^ ERROR `alloc` has already been imported in this module [E0260]
-//~| NOTE `alloc` already imported
+//~^ ERROR the name `alloc` is defined multiple times [E0260]
+//~| NOTE `alloc` redefined here
+//~| NOTE `alloc` must be defined only once in the type namespace of this module
     pub trait MyTrait {
         fn do_something();
     }

src/test/compile-fail/E0428.rs

@@ -8,9 +8,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-struct Bar; //~ previous definition of `Bar` here
+struct Bar; //~ previous definition of the type `Bar` here
 struct Bar; //~ ERROR E0428
-            //~| NOTE already defined
+            //~| NOTE `Bar` redefined here
+            //~| NOTE `Bar` must be defined only once in the type namespace of this module
 
 fn main () {
 }

src/test/compile-fail/blind-item-block-item-shadow.rs

@@ -14,6 +14,6 @@ fn main() {
     {
         struct Bar;
         use foo::Bar;
-        //~^ ERROR a type named `Bar` has already been defined in this block
+        //~^ ERROR the name `Bar` is defined multiple times
     }
 }

src/test/compile-fail/blind-item-item-shadow.rs

@@ -8,10 +8,11 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-mod foo { pub mod foo {  } } //~ NOTE previous definition of `foo` here
+mod foo { pub mod foo {  } } //~ NOTE previous definition of the module `foo` here
 
 use foo::foo;
-//~^ ERROR a module named `foo` has already been defined in this module
-//~| `foo` already defined
+//~^ ERROR the name `foo` is defined multiple times
+//~| `foo` reimported here
+//~| NOTE `foo` must be defined only once in the type namespace of this module
 
 fn main() {}

src/test/compile-fail/double-import.rs

@@ -19,8 +19,9 @@ mod sub2 {
     pub fn foo() {} // implementation 2
 }
 
-use sub1::foo; //~ NOTE previous import of `foo` here
-use sub2::foo; //~ ERROR a value named `foo` has already been imported in this module [E0252]
-               //~| NOTE already imported
+use sub1::foo; //~ NOTE previous import of the value `foo` here
+use sub2::foo; //~ ERROR the name `foo` is defined multiple times
+               //~| NOTE `foo` reimported here
+               //~| NOTE `foo` must be defined only once in the value namespace of this module
 
 fn main() {}

src/test/compile-fail/double-type-import.rs

@@ -11,7 +11,7 @@
 mod foo {
     pub use self::bar::X;
     use self::bar::X;
-    //~^ ERROR a type named `X` has already been imported in this module
+    //~^ ERROR the name `X` is defined multiple times
 
     mod bar {
         pub struct X;