resolve: Unify reporting of ambiguity errors for macro paths

Author: petrochenkov

Diff for: src/librustc_resolve/lib.rs

@@ -4431,6 +4431,42 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         vis.is_accessible_from(module.normal_ancestor_id, self)
     }
 
+    fn report_ambiguity_error(
+        &self, name: Name, span: Span, _lexical: bool,
+        def1: Def, is_import1: bool, is_glob1: bool, from_expansion1: bool, span1: Span,
+        def2: Def, is_import2: bool, _is_glob2: bool, _from_expansion2: bool, span2: Span,
+    ) {
+        let participle = |is_import: bool| if is_import { "imported" } else { "defined" };
+        let msg1 = format!("`{}` could refer to the name {} here", name, participle(is_import1));
+        let msg2 =
+            format!("`{}` could also refer to the name {} here", name, participle(is_import2));
+        let note = if from_expansion1 {
+            Some(if let Def::Macro(..) = def1 {
+                format!("macro-expanded {} do not shadow",
+                        if is_import1 { "macro imports" } else { "macros" })
+            } else {
+                format!("macro-expanded {} do not shadow when used in a macro invocation path",
+                        if is_import1 { "imports" } else { "items" })
+            })
+        } else if is_glob1 {
+            Some(format!("consider adding an explicit import of `{}` to disambiguate", name))
+        } else {
+            None
+        };
+
+        let mut err = struct_span_err!(self.session, span, E0659, "`{}` is ambiguous", name);
+        err.span_note(span1, &msg1);
+        match def2 {
+            Def::Macro(..) if span2.is_dummy() =>
+                err.note(&format!("`{}` is also a builtin macro", name)),
+            _ => err.span_note(span2, &msg2),
+        };
+        if let Some(note) = note {
+            err.note(&note);
+        }
+        err.emit();
+    }
+
     fn report_errors(&mut self, krate: &Crate) {
         self.report_shadowing_errors();
         self.report_with_use_injections(krate);
@@ -4446,30 +4482,15 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
         }
 
         for &AmbiguityError { span, name, b1, b2, lexical } 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 refer to the name {} here", name, participle(b1));
-            let msg2 = format!("`{}` could also refer to the name {} here", name, participle(b2));
-            let note = if b1.expansion == Mark::root() || !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" })
-            };
-
-            let mut err = struct_span_err!(self.session, span, E0659, "`{}` is ambiguous", name);
-            err.span_note(b1.span, &msg1);
-            match b2.def() {
-                Def::Macro(..) if b2.span.is_dummy() =>
-                    err.note(&format!("`{}` is also a builtin macro", name)),
-                _ => err.span_note(b2.span, &msg2),
-            };
-            err.note(&note).emit();
+            if reported_spans.insert(span) {
+                self.report_ambiguity_error(
+                    name, span, lexical,
+                    b1.def(), b1.is_import(), b1.is_glob_import(),
+                    b1.expansion != Mark::root(), b1.span,
+                    b2.def(), b2.is_import(), b2.is_glob_import(),
+                    b2.expansion != Mark::root(), b2.span,
+                );
+            }
         }
 
         for &PrivacyError(span, name, binding) in &self.privacy_errors {

Diff for: src/librustc_resolve/macros.rs

@@ -871,16 +871,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     self.suggest_macro_name(&ident.as_str(), kind, &mut err, span);
                     err.emit();
                 },
-                (Some((legacy_binding, _)), Ok((binding, FromPrelude(false)))) |
-                (Some((legacy_binding, FromExpansion(true))), Ok((binding, FromPrelude(true)))) => {
+                (Some((legacy_binding, FromExpansion(from_expansion))),
+                 Ok((binding, FromPrelude(false)))) |
+                (Some((legacy_binding, FromExpansion(from_expansion @ true))),
+                 Ok((binding, FromPrelude(true)))) => {
                     if legacy_binding.def() != binding.def_ignoring_ambiguity() {
-                        let msg1 = format!("`{}` could refer to the macro defined here", ident);
-                        let msg2 =
-                            format!("`{}` could also refer to the macro imported here", ident);
-                        self.session.struct_span_err(span, &format!("`{}` is ambiguous", ident))
-                            .span_note(legacy_binding.span, &msg1)
-                            .span_note(binding.span, &msg2)
-                            .emit();
+                        self.report_ambiguity_error(
+                            ident.name, span, true,
+                            legacy_binding.def(), false, false,
+                            from_expansion, legacy_binding.span,
+                            binding.def(), binding.is_import(), binding.is_glob_import(),
+                            binding.expansion != Mark::root(), binding.span,
+                        );
                     }
                 },
                 // OK, non-macro-expanded legacy wins over macro prelude even if defs are different

Diff for: src/test/ui/imports/issue-53269.stderr

@@ -4,23 +4,24 @@ error[E0432]: unresolved import `nonexistent_module`
 LL |     use nonexistent_module::mac; //~ ERROR unresolved import `nonexistent_module`
    |         ^^^^^^^^^^^^^^^^^^ Maybe a missing `extern crate nonexistent_module;`?
 
-error: `mac` is ambiguous
+error[E0659]: `mac` is ambiguous
   --> $DIR/issue-53269.rs:18:5
    |
 LL |     mac!(); //~ ERROR `mac` is ambiguous
    |     ^^^
    |
-note: `mac` could refer to the macro defined here
+note: `mac` could refer to the name defined here
   --> $DIR/issue-53269.rs:13:1
    |
 LL | macro_rules! mac { () => () }
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: `mac` could also refer to the macro imported here
+note: `mac` could also refer to the name imported here
   --> $DIR/issue-53269.rs:16:9
    |
 LL |     use nonexistent_module::mac; //~ ERROR unresolved import `nonexistent_module`
    |         ^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 2 previous errors
 
-For more information about this error, try `rustc --explain E0432`.
+Some errors occurred: E0432, E0659.
+For more information about an error, try `rustc --explain E0432`.

Diff for: src/test/ui/imports/macros.stderr

@@ -1,15 +1,15 @@
-error: `m` is ambiguous
+error[E0659]: `m` is ambiguous
   --> $DIR/macros.rs:48:5
    |
 LL |     m!(); //~ ERROR ambiguous
    |     ^
    |
-note: `m` could refer to the macro defined here
+note: `m` could refer to the name defined here
   --> $DIR/macros.rs:46:5
    |
 LL |     macro_rules! m { () => {} }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: `m` could also refer to the macro imported here
+note: `m` could also refer to the name imported here
   --> $DIR/macros.rs:47:9
    |
 LL |     use two_macros::m;

Diff for: src/test/ui/imports/shadow_builtin_macros.stderr

@@ -1,18 +1,19 @@
-error: `panic` is ambiguous
+error[E0659]: `panic` is ambiguous
   --> $DIR/shadow_builtin_macros.rs:43:5
    |
 LL |     panic!(); //~ ERROR `panic` is ambiguous
    |     ^^^^^
    |
-note: `panic` could refer to the macro defined here
+note: `panic` could refer to the name defined here
   --> $DIR/shadow_builtin_macros.rs:40:9
    |
 LL |         macro_rules! panic { () => {} }
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 LL |     } }
 LL |     m!();
    |     ----- in this macro invocation
-note: `panic` could also refer to the macro imported here
+   = note: `panic` is also a builtin macro
+   = note: macro-expanded macros do not shadow
 
 error[E0659]: `panic` is ambiguous
   --> $DIR/shadow_builtin_macros.rs:25:14