Find the first segment that failed to resolve for _any_ namespace

Moves this detection into `resolution_failure` to avoid doing
unnecessary work and make the control flow a little easier to work with.

3 files changed, 102 insertions, 59 deletions

diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index df8ecf26a41..d8abf411de7 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -60,8 +60,8 @@ enum ResolutionFailure<'a> {
     /// This has a partial resolution, but is not in the TypeNS and so cannot
     /// have associated items or fields.
     CannotHaveAssociatedItems(Res, Namespace),
-    /// `String` is the base name of the path (not necessarily the whole link)
-    NotInScope(Cow<'a, str>),
+    /// `name` is the base name of the path (not necessarily the whole link)
+    NotInScope { module_id: DefId, name: Cow<'a, str> },
     /// this is a primitive type without an impls (no associated methods)
     /// when will this actually happen?
     /// the `Res` is the primitive it resolved to
@@ -92,7 +92,7 @@ impl ResolutionFailure<'a> {
             | NotAVariant(res, _)
             | WrongNamespace(res, _)
             | CannotHaveAssociatedItems(res, _) => Some(*res),
-            NotInScope(_) | NoParentItem | Dummy => None,
+            NotInScope { .. } | NoParentItem | Dummy => None,
         }
     }
 
@@ -142,7 +142,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
             .expect("fold_item should ensure link is non-empty");
         let variant_name =
             // we're not sure this is a variant at all, so use the full string
-            split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.into())))?;
+            split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope{
+                module_id,
+                name: path_str.into(),
+            }))?;
         let path = split
             .next()
             .map(|f| {
@@ -153,38 +156,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                 }
                 f.to_owned()
             })
-            .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(
-                variant_name.to_string().into(),
-            )))?;
+            .ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope {
+                module_id,
+                name: variant_name.to_string().into(),
+            }))?;
         let ty_res = cx
             .enter_resolver(|resolver| {
                 resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id)
             })
             .map(|(_, res)| res)
             .unwrap_or(Res::Err);
-        // This code only gets hit if three path segments in a row don't get resolved.
-        // It's a good time to check if _any_ parent of the path gets resolved.
-        // If so, report it and say the first which failed; if not, say the first path segment didn't resolve.
         if let Res::Err = ty_res {
-            let mut current = path.as_str();
-            while let Some(parent) = current.rsplitn(2, "::").nth(1) {
-                current = parent;
-                if let Some(res) = self.check_full_res(
-                    TypeNS,
-                    &current,
-                    Some(module_id),
-                    current_item,
-                    extra_fragment,
-                ) {
-                    return Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(
-                        res,
-                        Symbol::intern(&path),
-                    )));
-                }
-            }
-            return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(
-                current.to_string().into(),
-            )));
+            return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope {
+                module_id,
+                name: path.into(),
+            }));
         }
         let ty_res = ty_res.map_id(|_| panic!("unexpected node_id"));
         match ty_res {
@@ -301,7 +287,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                     });
                 }
             }
-            Err(ResolutionFailure::NotInScope(path_str.into()))
+            Err(ResolutionFailure::NotInScope {
+                module_id: parent_id.expect("already saw `Some` when resolving as a macro"),
+                name: path_str.into(),
+            })
         })
     }
     /// Resolves a string as a path within a particular namespace. Also returns an optional
@@ -384,7 +373,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                 // So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved.
                 .ok_or_else(|| {
                     debug!("found no `::`, assumming {} was correctly not in scope", item_name);
-                    ErrorKind::Resolve(ResolutionFailure::NotInScope(item_name.to_string().into()))
+                    ErrorKind::Resolve(ResolutionFailure::NotInScope {
+                        module_id,
+                        name: item_name.to_string().into(),
+                    })
                 })?;
 
             if let Some((path, prim)) = is_primitive(&path_root, TypeNS) {
@@ -451,7 +443,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                                     }
                                 }
                             }
-                            ResolutionFailure::NotInScope(path_root.into())
+                            ResolutionFailure::NotInScope { module_id, name: path_root.into() }
                         });
                         Err(ErrorKind::Resolve(kind))
                     };
@@ -996,7 +988,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                                     }
                                 }
                                 resolution_failure(
-                                    cx,
+                                    self,
                                     &item,
                                     path_str,
                                     disambiguator,
@@ -1076,7 +1068,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                         if len == 0 {
                             drop(candidates_iter);
                             resolution_failure(
-                                cx,
+                                self,
                                 &item,
                                 path_str,
                                 disambiguator,
@@ -1096,8 +1088,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                         } else {
                             drop(candidates_iter);
                             if is_derive_trait_collision(&candidates) {
-                                candidates.macro_ns =
-                                    Err(ResolutionFailure::NotInScope(path_str.into()));
+                                candidates.macro_ns = Err(ResolutionFailure::Dummy);
                             }
                             // If we're reporting an ambiguity, don't mention the namespaces that failed
                             let candidates =
@@ -1131,7 +1122,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                                     }
                                 }
                                 resolution_failure(
-                                    cx,
+                                    self,
                                     &item,
                                     path_str,
                                     disambiguator,
@@ -1507,7 +1498,7 @@ fn report_diagnostic(
 }
 
 fn resolution_failure(
-    cx: &DocContext<'_>,
+    collector: &LinkCollector<'_, '_>,
     item: &Item,
     path_str: &str,
     disambiguator: Option<Disambiguator>,
@@ -1516,7 +1507,7 @@ fn resolution_failure(
     kinds: SmallVec<[ResolutionFailure<'_>; 3]>,
 ) {
     report_diagnostic(
-        cx,
+        collector.cx,
         &format!("unresolved link to `{}`", path_str),
         item,
         dox,
@@ -1524,11 +1515,15 @@ fn resolution_failure(
         |diag, sp| {
             let in_scope = kinds.iter().any(|kind| kind.res().is_some());
             let item = |res: Res| {
-                format!("the {} `{}`", res.descr(), cx.tcx.item_name(res.def_id()).to_string())
+                format!(
+                    "the {} `{}`",
+                    res.descr(),
+                    collector.cx.tcx.item_name(res.def_id()).to_string()
+                )
             };
             let assoc_item_not_allowed = |res: Res, diag: &mut DiagnosticBuilder<'_>| {
                 let def_id = res.def_id();
-                let name = cx.tcx.item_name(def_id);
+                let name = collector.cx.tcx.item_name(def_id);
                 let note = format!(
                     "`{}` is {} {}, not a module or type, and cannot have associated items",
                     name,
@@ -1539,18 +1534,42 @@ fn resolution_failure(
             };
             // ignore duplicates
             let mut variants_seen = SmallVec::<[_; 3]>::new();
-            for failure in kinds {
+            for mut failure in kinds {
+                // Check if _any_ parent of the path gets resolved.
+                // If so, report it and say the first which failed; if not, say the first path segment didn't resolve.
+                if let ResolutionFailure::NotInScope { module_id, name } = &mut failure {
+                    let mut current = name.as_ref();
+                    loop {
+                        current = match current.rsplitn(2, "::").nth(1) {
+                            Some(p) => p,
+                            None => {
+                                *name = current.to_owned().into();
+                                break;
+                            }
+                        };
+                        if let Some(res) = collector.check_full_res(
+                            TypeNS,
+                            &current,
+                            Some(*module_id),
+                            &None,
+                            &None,
+                        ) {
+                            failure = ResolutionFailure::NoAssocItem(res, Symbol::intern(current));
+                            break;
+                        }
+                    }
+                }
                 let variant = std::mem::discriminant(&failure);
                 if variants_seen.contains(&variant) {
                     continue;
                 }
                 variants_seen.push(variant);
                 match failure {
-                    ResolutionFailure::NotInScope(base) => {
+                    ResolutionFailure::NotInScope { name, .. } => {
                         if in_scope {
                             continue;
                         }
-                        diag.note(&format!("no item named `{}` is in scope", base));
+                        diag.note(&format!("no item named `{}` is in scope", name));
                         // If the link has `::` in the path, assume it's meant to be an intra-doc link
                         if !path_str.contains("::") {
                             // Otherwise, the `[]` might be unrelated.
@@ -1608,7 +1627,7 @@ fn resolution_failure(
                                 x,
                             ),
                         };
-                        let name = cx.tcx.item_name(def_id);
+                        let name = collector.cx.tcx.item_name(def_id);
                         let path_description = if let Some(disambiguator) = disambiguator {
                             disambiguator.descr()
                         } else {
diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs
index ae89f418984..477ad6464f8 100644
--- a/src/test/rustdoc-ui/intra-link-errors.rs
+++ b/src/test/rustdoc-ui/intra-link-errors.rs
@@ -8,6 +8,14 @@
 //~^ ERROR unresolved link
 //~| NOTE no item named `path` is in scope
 
+/// [path::to::nonexistent::macro!]
+//~^ ERROR unresolved link
+//~| NOTE no item named `path` is in scope
+
+/// [type@path::to::nonexistent::type]
+//~^ ERROR unresolved link
+//~| NOTE no item named `path` is in scope
+
 /// [std::io::not::here]
 //~^ ERROR unresolved link
 //~| NOTE the module `io` has no inner item
diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr
index 73e16bef0aa..3b1a09f913e 100644
--- a/src/test/rustdoc-ui/intra-link-errors.stderr
+++ b/src/test/rustdoc-ui/intra-link-errors.stderr
@@ -11,16 +11,32 @@ LL | #![deny(broken_intra_doc_links)]
    |         ^^^^^^^^^^^^^^^^^^^^^^
    = note: no item named `path` is in scope
 
-error: unresolved link to `std::io::not::here`
+error: unresolved link to `path::to::nonexistent::macro`
   --> $DIR/intra-link-errors.rs:11:6
    |
+LL | /// [path::to::nonexistent::macro!]
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: no item named `path` is in scope
+
+error: unresolved link to `path::to::nonexistent::type`
+  --> $DIR/intra-link-errors.rs:15:6
+   |
+LL | /// [type@path::to::nonexistent::type]
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: no item named `path` is in scope
+
+error: unresolved link to `std::io::not::here`
+  --> $DIR/intra-link-errors.rs:19:6
+   |
 LL | /// [std::io::not::here]
    |      ^^^^^^^^^^^^^^^^^^
    |
    = note: the module `io` has no inner item named `not`
 
 error: unresolved link to `std::io::Error::x`
-  --> $DIR/intra-link-errors.rs:15:6
+  --> $DIR/intra-link-errors.rs:23:6
    |
 LL | /// [std::io::Error::x]
    |      ^^^^^^^^^^^^^^^^^
@@ -28,7 +44,7 @@ LL | /// [std::io::Error::x]
    = note: the struct `Error` has no field or associated item named `x`
 
 error: unresolved link to `std::io::ErrorKind::x`
-  --> $DIR/intra-link-errors.rs:19:6
+  --> $DIR/intra-link-errors.rs:27:6
    |
 LL | /// [std::io::ErrorKind::x]
    |      ^^^^^^^^^^^^^^^^^^^^^
@@ -36,7 +52,7 @@ LL | /// [std::io::ErrorKind::x]
    = note: the enum `ErrorKind` has no variant or associated item named `x`
 
 error: unresolved link to `f::A`
-  --> $DIR/intra-link-errors.rs:23:6
+  --> $DIR/intra-link-errors.rs:31:6
    |
 LL | /// [f::A]
    |      ^^^^
@@ -44,7 +60,7 @@ LL | /// [f::A]
    = note: `f` is a function, not a module or type, and cannot have associated items
 
 error: unresolved link to `S::A`
-  --> $DIR/intra-link-errors.rs:27:6
+  --> $DIR/intra-link-errors.rs:35:6
    |
 LL | /// [S::A]
    |      ^^^^
@@ -52,7 +68,7 @@ LL | /// [S::A]
    = note: the struct `S` has no field or associated item named `A`
 
 error: unresolved link to `S::fmt`
-  --> $DIR/intra-link-errors.rs:31:6
+  --> $DIR/intra-link-errors.rs:39:6
    |
 LL | /// [S::fmt]
    |      ^^^^^^
@@ -60,7 +76,7 @@ LL | /// [S::fmt]
    = note: the struct `S` has no field or associated item named `fmt`
 
 error: unresolved link to `E::D`
-  --> $DIR/intra-link-errors.rs:35:6
+  --> $DIR/intra-link-errors.rs:43:6
    |
 LL | /// [E::D]
    |      ^^^^
@@ -68,7 +84,7 @@ LL | /// [E::D]
    = note: the enum `E` has no variant or associated item named `D`
 
 error: unresolved link to `u8::not_found`
-  --> $DIR/intra-link-errors.rs:39:6
+  --> $DIR/intra-link-errors.rs:47:6
    |
 LL | /// [u8::not_found]
    |      ^^^^^^^^^^^^^
@@ -76,7 +92,7 @@ LL | /// [u8::not_found]
    = note: the builtin type `u8` does not have an associated item named `not_found`
 
 error: unresolved link to `S`
-  --> $DIR/intra-link-errors.rs:43:6
+  --> $DIR/intra-link-errors.rs:51:6
    |
 LL | /// [S!]
    |      ^^ help: to link to the struct, prefix with `struct@`: `struct@S`
@@ -84,7 +100,7 @@ LL | /// [S!]
    = note: this link resolves to the struct `S`, which is not in the macro namespace
 
 error: unresolved link to `T::g`
-  --> $DIR/intra-link-errors.rs:61:6
+  --> $DIR/intra-link-errors.rs:69:6
    |
 LL | /// [type@T::g]
    |      ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()`
@@ -92,7 +108,7 @@ LL | /// [type@T::g]
    = note: this link resolves to the associated function `g`, which is not in the type namespace
 
 error: unresolved link to `T::h`
-  --> $DIR/intra-link-errors.rs:66:6
+  --> $DIR/intra-link-errors.rs:74:6
    |
 LL | /// [T::h!]
    |      ^^^^^
@@ -100,7 +116,7 @@ LL | /// [T::h!]
    = note: the trait `T` has no macro named `h`
 
 error: unresolved link to `S::h`
-  --> $DIR/intra-link-errors.rs:53:6
+  --> $DIR/intra-link-errors.rs:61:6
    |
 LL | /// [type@S::h]
    |      ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()`
@@ -108,12 +124,12 @@ LL | /// [type@S::h]
    = note: this link resolves to the associated function `h`, which is not in the type namespace
 
 error: unresolved link to `m`
-  --> $DIR/intra-link-errors.rs:73:6
+  --> $DIR/intra-link-errors.rs:81:6
    |
 LL | /// [m()]
    |      ^^^ help: to link to the macro, add an exclamation mark: `m!`
    |
    = note: this link resolves to the macro `m`, which is not in the value namespace
 
-error: aborting due to 14 previous errors
+error: aborting due to 16 previous errors