Skip to content

Commit 0f5c769

Browse files
committed
Auto merge of #75756 - jyn514:diagnostic-suggestions, r=estebank
Improve suggestions for broken intra-doc links ~~Depends on #74489 and should not be merged before that PR.~~ Merged 🎉 ~~Depends on #75916 and should not be merged before.~~ Merged Fixes #75305. This does a lot of different things 😆. - Add `PerNS::into_iter()` so I didn't have to keep rewriting hacks around it. Also add `PerNS::iter()` for consistency. Let me know if this should be `impl IntoIterator` instead. - Make `ResolutionFailure` an enum instead of a unit variant. This was most of the changes: everywhere that said `ErrorKind::ResolutionFailure` now has to say _why_ the link failed to resolve. - Store the resolution in case of an anchor failure. Previously this was implemented as variants on `AnchorFailure` which was prone to typos and had inconsistent output compared to the rest of the diagnostics. - Turn some `Err`ors into unwrap() or panic()s, because they're rustdoc bugs and not user error. These have comments as to why they're bugs (in particular this would have caught #76073 as a bug a while ago). - If an item is not in scope at all, say the first segment in the path that failed to resolve - If an item exists but not in the current namespaces, say that and suggests linking to that namespace. - If there is a partial resolution for an item (part of the segments resolved, but not all of them), say the partial resolution and why the following segment didn't resolve. - Add the `DefId` of associated items to `kind_side_channel` so it can be used for diagnostics (tl;dr of the hack: the rest of rustdoc expects the id of the item, but for diagnostics we need the associated item). - No longer suggests escaping the brackets for every link that failed to resolve; this was pretty obnoxious. Now it only suggests `\[ \]` if no segment resolved and there is no `::` in the link. - Add `Suggestion`, which says _what_ to prefix the link with, not just 'prefix with the item kind'. Places where this is currently buggy: <details><summary>All outdated</summary> ~~1. When the link has the wrong namespace:~~ Now fixed. <details> ```rust /// [type@S::h] impl S { pub fn h() {} } /// [type@T::g] pub trait T { fn g() {} } ``` ``` error: unresolved link to `T::g` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:53:6 | 53 | /// [type@T::g] | ^^^^^^^^^ | = note: this link partially resolves to the trait `T`, = note: `T` has no field, variant, or associated item named `g` error: unresolved link to `S::h` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:48:6 | 48 | /// [type@S::h] | ^^^^^^^^^ | = note: this link partially resolves to the struct `S`, = note: `S` has no field, variant, or associated item named `h` ``` Instead it should suggest changing the disambiguator, the way it currently does for macros: ``` error: unresolved link to `S` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:38:6 | 38 | /// [S!] | ^^ help: to link to the unit struct, use its disambiguator: `value@S` | = note: this link resolves to the unit struct `S`, which is not in the macro namespace ``` </details> 2. ~~Associated items for values. It says that the value isn't in scope; instead it should say that values can't have associated items.~~ Fixed. <details> ``` error: unresolved link to `f::A` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:14:6 | 14 | /// [f::A] | ^^^^ | = note: no item named `f` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` ``` This is _mostly_ fixed, it now says ```rust warning: unresolved link to `f::A` --> /home/joshua/test-rustdoc/f.rs:1:6 | 1 | /// [f::A] | ^^^^ | = note: this link partially resolves to the function `f` = note: `f` is a function, not a module ``` 'function, not a module' seems awfully terse when what I actually mean is '`::` isn't allowed here', though. </details> It looks a lot nicer now, it says ``` error: unresolved link to `f::A` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:13:6 | 13 | /// [f::A] | ^^^^ | = note: `f` is a function, not a module or type, and cannot have associated items ``` 3. ~~I'm also not very happy with the second note for this error:~~ <details> ``` error: unresolved link to `S::A` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:19:6 | 19 | /// [S::A] | ^^^^ | = note: this link partially resolves to the struct `S`, = note: `S` has no field, variant, or associated item named `A` ``` but I'm not sure how better to word it. I ended up going with 'no `A` in `S`' to match `rustc_resolve` but that seems terse as well. </details> This now says ``` error: unresolved link to `S::A` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:17:6 | 17 | /// [S::A] | ^^^^ | = note: the struct `S` has no field or associated item named `A` ``` which I think looks pretty good :) 4. This is minor, but it would be nice to say that `path` wasn't found instead of the full thing: ``` error: unresolved link to `path::to::nonexistent::module` --> /home/joshua/rustc/src/test/rustdoc-ui/intra-link-errors.rs:8:6 | 8 | /// [path::to::nonexistent::module] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` It will now look at most 3 paths up (so it reports `path::to` as not in scope), but it doesn't work with arbitrarily many paths. </details> ~~I recommend only reviewing the last few commits - the first 7 are all from #74489.~~ Rebased so that only the relevant commits are shown. Let me know if I should squash the history some more. r? `@estebank`
2 parents 8b6838b + 5ea3eaf commit 0f5c769

18 files changed

+877
-282
lines changed

compiler/rustc_hir/src/def.rs

+9
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use rustc_ast::NodeId;
66
use rustc_macros::HashStable_Generic;
77
use rustc_span::hygiene::MacroKind;
88

9+
use std::array::IntoIter;
910
use std::fmt::Debug;
1011

1112
/// Encodes if a `DefKind::Ctor` is the constructor of an enum variant or a struct.
@@ -291,6 +292,14 @@ impl<T> PerNS<T> {
291292
pub fn map<U, F: FnMut(T) -> U>(self, mut f: F) -> PerNS<U> {
292293
PerNS { value_ns: f(self.value_ns), type_ns: f(self.type_ns), macro_ns: f(self.macro_ns) }
293294
}
295+
296+
pub fn into_iter(self) -> IntoIter<T, 3> {
297+
IntoIter::new([self.value_ns, self.type_ns, self.macro_ns])
298+
}
299+
300+
pub fn iter(&self) -> IntoIter<&T, 3> {
301+
IntoIter::new([&self.value_ns, &self.type_ns, &self.macro_ns])
302+
}
294303
}
295304

296305
impl<T> ::std::ops::Index<Namespace> for PerNS<T> {

compiler/rustc_hir/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//!
33
//! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/hir.html
44
5+
#![feature(array_value_iter)]
56
#![feature(crate_visibility_modifier)]
67
#![feature(const_fn)] // For the unsizing cast on `&[]`
78
#![feature(const_panic)]

0 commit comments

Comments
 (0)