From e2d69f2eb115b0b6a433977ae7c5a73c249a4f98 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 21 Aug 2020 13:35:50 -0400 Subject: [PATCH] Make errors more concise and helpful Before: ``` = note: this link partially resolves to the struct `S` = note: no `fmt` in `S` ``` After: ``` = note: the struct `S` has no field or associated item named `fmt` ``` --- .../passes/collect_intra_doc_links.rs | 95 ++++++++++++------- .../rustdoc-ui/assoc-item-not-in-scope.stderr | 4 +- .../rustdoc-ui/intra-doc-alias-ice.stderr | 3 +- src/test/rustdoc-ui/intra-link-errors.rs | 10 +- src/test/rustdoc-ui/intra-link-errors.stderr | 28 +++--- .../rustdoc-ui/intra-links-warning.stderr | 3 +- src/test/rustdoc-ui/lint-group.stderr | 3 +- 7 files changed, 81 insertions(+), 65 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 0d07f8f0328..c69850f9d97 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1406,11 +1406,18 @@ fn resolution_failure( let in_scope = kinds.iter().any(|kind| kind.res().is_some()); let mut reported_not_in_scope = false; let item = |res: Res| { - if let Some(id) = res.opt_def_id() { - (format!("the {} `{}`", res.descr(), cx.tcx.item_name(id).to_string()), ",") - } else { - (format!("{} {}", res.article(), res.descr()), "") - } + format!("the {} `{}`", res.descr(), 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 note = format!( + "`{}` is {} {}, not a module or type, and cannot have associated items", + name, + res.article(), + res.descr() + ); + diag.note(¬e); }; for failure in kinds { match failure { @@ -1425,11 +1432,9 @@ fn resolution_failure( } ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace(res, expected_ns) => { - let (item, comma) = item(res); let note = format!( - "this link resolves to {}{} which is not in the {} namespace", - item, - comma, + "this link resolves to {}, which is not in the {} namespace", + item(res), expected_ns.descr() ); diag.note(¬e); @@ -1450,10 +1455,9 @@ fn resolution_failure( panic!("all intra doc links should have a parent item") } ResolutionFailure::NoPrimitiveImpl(res, _) => { - let (item, comma) = item(res); let note = format!( - "this link partially resolves to {}{} which does not have any associated items", - item, comma, + "this link partially resolves to {}, which does not have any associated items", + item(res), ); diag.note(¬e); } @@ -1465,41 +1469,62 @@ fn resolution_failure( diag.note(¬e); } ResolutionFailure::NoAssocItem(res, assoc_item) => { - let (item, _) = item(res); - diag.note(&format!("this link partially resolves to {}", item)); - // FIXME: when are items neither a primitive nor a Def? - if let Res::Def(_, def_id) = res { - let name = cx.tcx.item_name(def_id); - let note = format!("no `{}` in `{}`", assoc_item, name,); - diag.note(¬e); - } + use DefKind::*; + + let (kind, def_id) = match res { + Res::Def(kind, def_id) => (kind, def_id), + _ => unreachable!( + "primitives are covered above and other `Res` variants aren't possible at module scope" + ), + }; + let name = cx.tcx.item_name(def_id); + let path_description = match kind { + Mod | ForeignMod => "inner item", + Struct => "field or associated item", + Enum | Union => "variant or associated item", + Variant + | Field + | Closure + | Generator + | AssocTy + | AssocConst + | AssocFn + | Fn + | Macro(_) + | Const + | ConstParam + | ExternCrate + | Use + | LifetimeParam + | Ctor(_, _) + | AnonConst => return assoc_item_not_allowed(res, diag), + Trait | TyAlias | ForeignTy | OpaqueTy | TraitAlias | TyParam + | Static => "associated item", + Impl | GlobalAsm => unreachable!("not a path"), + }; + let note = format!( + "the {} `{}` has no {} named `{}`", + res.descr(), + name, + path_description, + assoc_item + ); + diag.note(¬e); } ResolutionFailure::CannotHaveAssociatedItems(res, _) => { - let (item, _) = item(res); - diag.note(&format!("this link partially resolves to {}", item)); - if let Res::Def(kind, def_id) = res { - let name = cx.tcx.item_name(def_id); - let note = format!( - "`{}` is {} {}, not a module or type, and cannot have associated items", - name, - kind.article(), - kind.descr(def_id) - ); - diag.note(¬e); - } + assoc_item_not_allowed(res, diag) } // TODO: is there ever a case where this happens? ResolutionFailure::NotAnEnum(res) => { - let (item, comma) = item(res); let note = - format!("this link resolves to {}{} which is not an enum", item, comma); + format!("this link resolves to {}, which is not an enum", item(res)); diag.note(¬e); 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 {}", - item(res).0, + item(res), variant ); diag.note(¬e); diff --git a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr index 8827c9351a6..8acfcca2139 100644 --- a/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr +++ b/src/test/rustdoc-ui/assoc-item-not-in-scope.stderr @@ -2,14 +2,14 @@ error: unresolved link to `S::fmt` --> $DIR/assoc-item-not-in-scope.rs:4:14 | LL | /// Link to [`S::fmt`] - | ^^^^^^^^ unresolved link + | ^^^^^^^^ | note: the lint level is defined here --> $DIR/assoc-item-not-in-scope.rs:1:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` + = note: the struct `S` has no field or associated item named `fmt` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr index 58abe4406a8..5379a918837 100644 --- a/src/test/rustdoc-ui/intra-doc-alias-ice.stderr +++ b/src/test/rustdoc-ui/intra-doc-alias-ice.stderr @@ -9,8 +9,7 @@ note: the lint level is defined here | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ - = note: this link partially resolves to the type alias `TypeAlias` - = note: no `hoge` in `TypeAlias` + = note: the type alias `TypeAlias` has no associated item named `hoge` error: aborting due to previous error diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 5bfbdcc495f..ecf2b05e281 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -12,23 +12,19 @@ /// [f::A] //~^ ERROR unresolved link -//~| NOTE this link partially resolves //~| NOTE `f` is a function, not a module /// [S::A] //~^ ERROR unresolved link -//~| NOTE this link partially resolves -//~| NOTE no `A` in `S` +//~| NOTE struct `S` has no field or associated item /// [S::fmt] //~^ ERROR unresolved link -//~| NOTE this link partially resolves -//~| NOTE no `fmt` in `S` +//~| NOTE struct `S` has no field or associated item /// [E::D] //~^ ERROR unresolved link -//~| NOTE this link partially resolves -//~| NOTE no `D` in `E` +//~| NOTE enum `E` has no variant or associated item /// [u8::not_found] //~^ ERROR unresolved link diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 0244d2bfde8..ba091ff854c 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -13,43 +13,39 @@ LL | #![deny(broken_intra_doc_links)] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:14:6 + --> $DIR/intra-link-errors.rs:13:6 | LL | /// [f::A] | ^^^^ | - = note: this link partially resolves to the function `f` = 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:19:6 + --> $DIR/intra-link-errors.rs:17:6 | LL | /// [S::A] | ^^^^ | - = note: this link partially resolves to the struct `S` - = note: no `A` in `S` + = note: the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` - --> $DIR/intra-link-errors.rs:24:6 + --> $DIR/intra-link-errors.rs:21:6 | LL | /// [S::fmt] | ^^^^^^ | - = note: this link partially resolves to the struct `S` - = note: no `fmt` in `S` + = note: the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` - --> $DIR/intra-link-errors.rs:29:6 + --> $DIR/intra-link-errors.rs:25:6 | LL | /// [E::D] | ^^^^ | - = note: this link partially resolves to the enum `E` - = note: no `D` in `E` + = note: the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` - --> $DIR/intra-link-errors.rs:34:6 + --> $DIR/intra-link-errors.rs:29:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ @@ -57,7 +53,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:38:6 + --> $DIR/intra-link-errors.rs:33:6 | LL | /// [S!] | ^^ help: to link to the struct, use its disambiguator: `struct@S` @@ -65,7 +61,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:56:6 + --> $DIR/intra-link-errors.rs:51:6 | LL | /// [type@T::g] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()` @@ -73,7 +69,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:61:6 + --> $DIR/intra-link-errors.rs:56:6 | LL | /// [T::h!] | ^^^^^ @@ -82,7 +78,7 @@ LL | /// [T::h!] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: unresolved link to `S::h` - --> $DIR/intra-link-errors.rs:48:6 + --> $DIR/intra-link-errors.rs:43:6 | LL | /// [type@S::h] | ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()` diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index b51c8e89600..988a65c7a59 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -5,8 +5,7 @@ LL | //! Test with [Foo::baz], [Bar::foo], ... | ^^^^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default - = note: this link partially resolves to the struct `Foo` - = note: no `baz` in `Foo` + = note: the struct `Foo` has no field or associated item named `baz` warning: unresolved link to `Bar::foo` --> $DIR/intra-links-warning.rs:3:35 diff --git a/src/test/rustdoc-ui/lint-group.stderr b/src/test/rustdoc-ui/lint-group.stderr index 04296d2e44a..10fcebb27fe 100644 --- a/src/test/rustdoc-ui/lint-group.stderr +++ b/src/test/rustdoc-ui/lint-group.stderr @@ -32,7 +32,7 @@ error: unresolved link to `error` --> $DIR/lint-group.rs:9:29 | LL | /// what up, let's make an [error] - | ^^^^^ unresolved link + | ^^^^^ | note: the lint level is defined here --> $DIR/lint-group.rs:7:9 @@ -40,6 +40,7 @@ note: the lint level is defined here LL | #![deny(rustdoc)] | ^^^^^^^ = note: `#[deny(broken_intra_doc_links)]` implied by `#[deny(rustdoc)]` + = note: no item named `error` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` error: aborting due to 3 previous errors