about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJoshua Nelson <jyn514@gmail.com>2020-08-21 16:37:31 -0400
committerJoshua Nelson <jyn514@gmail.com>2020-09-05 13:48:19 -0400
commit418f6089e93d7f784643d55e6782055c9bb479d5 (patch)
treef0dde355e2fba31cf78ac9c10a0f8d726d28d567
parent6875220e1abea26c67885833c27365854cd7f73c (diff)
downloadrust-418f6089e93d7f784643d55e6782055c9bb479d5.tar.gz
rust-418f6089e93d7f784643d55e6782055c9bb479d5.zip
Give a better error message when linking to a macro with the wrong disambiguator
Before:

```
warning: unresolved link to `m`
 --> m.rs:1:6
  |
1 | /// [value@m]
  |      ^^^^^^^
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default
  = note: no item named `m` is in scope
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
```

After:

```
warning: unresolved link to `m`
 --> m.rs:1:6
  |
1 | /// [value@m]
  |      ^^^^^^^ help: to link to the macro, use its disambiguator: `m!`
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default
  = note: this link resolves to the macro `m`, which is not in the value namespace
```
-rw-r--r--src/librustdoc/passes/collect_intra_doc_links.rs119
-rw-r--r--src/test/rustdoc-ui/intra-link-errors.rs21
-rw-r--r--src/test/rustdoc-ui/intra-link-errors.stderr52
3 files changed, 142 insertions, 50 deletions
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 1c709cead70..7bd4b8ca854 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -74,8 +74,6 @@ enum ResolutionFailure<'a> {
     NoAssocItem(Res, Symbol),
     /// should not ever happen
     NoParentItem,
-    /// the root of this path resolved, but it was not an enum.
-    NotAnEnum(Res),
     /// this could be an enum variant, but the last path fragment wasn't resolved.
     /// the `String` is the variant that didn't exist
     NotAVariant(Res, Symbol),
@@ -91,7 +89,6 @@ impl ResolutionFailure<'a> {
             NoPrimitiveAssocItem { res, .. }
             | NoAssocItem(res, _)
             | NoPrimitiveImpl(res, _)
-            | NotAnEnum(res)
             | NotAVariant(res, _)
             | WrongNamespace(res, _)
             | CannotHaveAssociatedItems(res, _) => Some(*res),
@@ -133,6 +130,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         path_str: &'path str,
         current_item: &Option<String>,
         module_id: DefId,
+        extra_fragment: &Option<String>,
     ) -> Result<(Res, Option<String>), ErrorKind<'path>> {
         let cx = self.cx;
 
@@ -202,7 +200,28 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                     _ => unreachable!(),
                 }
             }
-            _ => Err(ErrorKind::Resolve(ResolutionFailure::NotAnEnum(ty_res))),
+            // `variant_field` looks at 3 different path segments in a row.
+            // But `NoAssocItem` assumes there are only 2. Check to see if there's
+            // an intermediate segment that resolves.
+            _ => {
+                let intermediate_path = format!("{}::{}", path, variant_name);
+                // NOTE: we have to be careful here, because we're already in `resolve`.
+                // We know this doesn't recurse forever because we use a shorter path each time.
+                // NOTE: this uses `TypeNS` because nothing else has a valid path segment after
+                let kind = if let Some(intermediate) = self.check_full_res(
+                    TypeNS,
+                    &intermediate_path,
+                    Some(module_id),
+                    current_item,
+                    extra_fragment,
+                ) {
+                    ResolutionFailure::NoAssocItem(intermediate, variant_field_name)
+                } else {
+                    // Even with the shorter path, it didn't resolve, so say that.
+                    ResolutionFailure::NoAssocItem(ty_res, variant_name)
+                };
+                Err(ErrorKind::Resolve(kind))
+            }
         }
     }
 
@@ -376,7 +395,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
             let ty_res = match ty_res {
                 Err(()) | Ok(Res::Err) => {
                     return if ns == Namespace::ValueNS {
-                        self.variant_field(path_str, current_item, module_id)
+                        self.variant_field(path_str, current_item, module_id, extra_fragment)
                     } else {
                         // See if it only broke because of the namespace.
                         let kind = cx.enter_resolver(|resolver| {
@@ -533,7 +552,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
             };
             res.unwrap_or_else(|| {
                 if ns == Namespace::ValueNS {
-                    self.variant_field(path_str, current_item, module_id)
+                    self.variant_field(path_str, current_item, module_id, extra_fragment)
                 } else {
                     Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(ty_res, item_name)))
                 }
@@ -543,6 +562,41 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
             Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem))
         }
     }
+
+    // used for reporting better errors
+    fn check_full_res(
+        &self,
+        ns: Namespace,
+        path_str: &str,
+        base_node: Option<DefId>,
+        current_item: &Option<String>,
+        extra_fragment: &Option<String>,
+    ) -> Option<Res> {
+        let check_full_res_inner = |this: &Self, result: Result<Res, ErrorKind<'_>>| {
+            let res = match result {
+                Ok(res) => Some(res),
+                Err(ErrorKind::Resolve(kind)) => kind.full_res(),
+                Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => {
+                    Some(res)
+                }
+                Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None,
+            };
+            this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res)
+        };
+        // cannot be used for macro namespace
+        let check_full_res = |this: &Self, ns| {
+            let result = this.resolve(path_str, ns, current_item, base_node, extra_fragment);
+            check_full_res_inner(this, result.map(|(res, _)| res))
+        };
+        let check_full_res_macro = |this: &Self| {
+            let result = this.macro_resolve(path_str, base_node);
+            check_full_res_inner(this, result.map_err(ErrorKind::Resolve))
+        };
+        match ns {
+            Namespace::MacroNS => check_full_res_macro(self),
+            Namespace::TypeNS | Namespace::ValueNS => check_full_res(self, ns),
+        }
+    }
 }
 
 fn resolve_associated_trait_item(
@@ -652,7 +706,7 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx
             let trait_ref = cx.tcx.impl_trait_ref(impl_).expect("this is not an inherent impl");
             // Check if these are the same type.
             let impl_type = trait_ref.self_ty();
-            debug!(
+            trace!(
                 "comparing type {} with kind {:?} against type {:?}",
                 impl_type,
                 impl_type.kind(),
@@ -875,27 +929,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                     }
                 }
 
-                // used for reporting better errors
-                let check_full_res = |this: &mut Self, ns| {
-                    let res =
-                        match this.resolve(path_str, ns, &current_item, base_node, &extra_fragment)
-                        {
-                            Ok(res) => {
-                                debug!(
-                                    "check_full_res: saw res for {} in {:?} ns: {:?}",
-                                    path_str, ns, res.0
-                                );
-                                Some(res.0)
-                            }
-                            Err(ErrorKind::Resolve(kind)) => kind.full_res(),
-                            Err(ErrorKind::AnchorFailure(
-                                AnchorFailure::RustdocAnchorConflict(res),
-                            )) => Some(res),
-                            Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None,
-                        };
-                    this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res)
-                };
-
                 match disambiguator.map(Disambiguator::ns) {
                     Some(ns @ (ValueNS | TypeNS)) => {
                         match self.resolve(path_str, ns, &current_item, base_node, &extra_fragment)
@@ -903,12 +936,19 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                             Ok(res) => res,
                             Err(ErrorKind::Resolve(mut kind)) => {
                                 // We only looked in one namespace. Try to give a better error if possible.
-                                // TODO: handle MacroNS too
                                 if kind.full_res().is_none() {
                                     let other_ns = if ns == ValueNS { TypeNS } else { ValueNS };
-                                    if let Some(res) = check_full_res(self, other_ns) {
-                                        // recall that this stores the _expected_ namespace
-                                        kind = ResolutionFailure::WrongNamespace(res, ns);
+                                    for &ns in &[other_ns, MacroNS] {
+                                        if let Some(res) = self.check_full_res(
+                                            ns,
+                                            path_str,
+                                            base_node,
+                                            &current_item,
+                                            &extra_fragment,
+                                        ) {
+                                            kind = ResolutionFailure::WrongNamespace(res, ns);
+                                            break;
+                                        }
                                     }
                                 }
                                 resolution_failure(
@@ -1033,7 +1073,13 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
                             Err(mut kind) => {
                                 // `macro_resolve` only looks in the macro namespace. Try to give a better error if possible.
                                 for &ns in &[TypeNS, ValueNS] {
-                                    if let Some(res) = check_full_res(self, ns) {
+                                    if let Some(res) = self.check_full_res(
+                                        ns,
+                                        path_str,
+                                        base_node,
+                                        &current_item,
+                                        &extra_fragment,
+                                    ) {
                                         kind = ResolutionFailure::WrongNamespace(res, MacroNS);
                                         break;
                                     }
@@ -1525,13 +1571,6 @@ fn resolution_failure(
                     ResolutionFailure::CannotHaveAssociatedItems(res, _) => {
                         assoc_item_not_allowed(res, diag)
                     }
-                    // TODO: is there ever a case where this happens?
-                    ResolutionFailure::NotAnEnum(res) => {
-                        let note =
-                            format!("this link resolves to {}, which is not an enum", item(res));
-                        diag.note(&note);
-                        diag.note("if this were an enum, it might have a variant which resolved");
-                    }
                     ResolutionFailure::NotAVariant(res, variant) => {
                         let note = format!(
                             "this link partially resolves to {}, but there is no variant named {}",
diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs
index ecf2b05e281..33c8d1b3c49 100644
--- a/src/test/rustdoc-ui/intra-link-errors.rs
+++ b/src/test/rustdoc-ui/intra-link-errors.rs
@@ -10,6 +10,18 @@
 //~| NOTE no item named `path::to` is in scope
 //~| HELP to escape
 
+/// [std::io::not::here]
+//~^ ERROR unresolved link
+//~| NOTE the module `io` has no inner item
+
+/// [std::io::Error::x]
+//~^ ERROR unresolved link
+//~| NOTE the struct `Error` has no field
+
+/// [std::io::ErrorKind::x]
+//~^ ERROR unresolved link
+//~| NOTE the enum `ErrorKind` has no variant
+
 /// [f::A]
 //~^ ERROR unresolved link
 //~| NOTE `f` is a function, not a module
@@ -60,3 +72,12 @@ impl S {
 pub trait T {
     fn g() {}
 }
+
+/// [m()]
+//~^ ERROR unresolved link
+//~| HELP to link to the macro
+//~| NOTE not in the value namespace
+#[macro_export]
+macro_rules! m {
+    () => {};
+}
diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr
index ba091ff854c..0b9149cd8ea 100644
--- a/src/test/rustdoc-ui/intra-link-errors.stderr
+++ b/src/test/rustdoc-ui/intra-link-errors.stderr
@@ -12,16 +12,40 @@ LL | #![deny(broken_intra_doc_links)]
    = note: no item named `path::to` is in scope
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
 
-error: unresolved link to `f::A`
+error: unresolved link to `std::io::not::here`
   --> $DIR/intra-link-errors.rs:13: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:17:6
+   |
+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:21:6
+   |
+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:25:6
+   |
 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:17:6
+  --> $DIR/intra-link-errors.rs:29:6
    |
 LL | /// [S::A]
    |      ^^^^
@@ -29,7 +53,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:21:6
+  --> $DIR/intra-link-errors.rs:33:6
    |
 LL | /// [S::fmt]
    |      ^^^^^^
@@ -37,7 +61,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:25:6
+  --> $DIR/intra-link-errors.rs:37:6
    |
 LL | /// [E::D]
    |      ^^^^
@@ -45,7 +69,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:29:6
+  --> $DIR/intra-link-errors.rs:41:6
    |
 LL | /// [u8::not_found]
    |      ^^^^^^^^^^^^^
@@ -53,7 +77,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:33:6
+  --> $DIR/intra-link-errors.rs:45:6
    |
 LL | /// [S!]
    |      ^^ help: to link to the struct, use its disambiguator: `struct@S`
@@ -61,7 +85,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:51:6
+  --> $DIR/intra-link-errors.rs:63:6
    |
 LL | /// [type@T::g]
    |      ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()`
@@ -69,7 +93,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:56:6
+  --> $DIR/intra-link-errors.rs:68:6
    |
 LL | /// [T::h!]
    |      ^^^^^
@@ -78,12 +102,20 @@ LL | /// [T::h!]
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
 
 error: unresolved link to `S::h`
-  --> $DIR/intra-link-errors.rs:43:6
+  --> $DIR/intra-link-errors.rs:55:6
    |
 LL | /// [type@S::h]
    |      ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()`
    |
    = note: this link resolves to the associated function `h`, which is not in the type namespace
 
-error: aborting due to 10 previous errors
+error: unresolved link to `m`
+  --> $DIR/intra-link-errors.rs:76:6
+   |
+LL | /// [m()]
+   |      ^^^ help: to link to the macro, use its disambiguator: `m!`
+   |
+   = note: this link resolves to the macro `m`, which is not in the value namespace
+
+error: aborting due to 14 previous errors