]> git.lizzy.rs Git - rust.git/commit - src/tools/miri
Auto merge of #75756 - jyn514:diagnostic-suggestions, r=estebank
authorbors <bors@rust-lang.org>
Sat, 12 Sep 2020 05:52:14 +0000 (05:52 +0000)
committerbors <bors@rust-lang.org>
Sat, 12 Sep 2020 05:52:14 +0000 (05:52 +0000)
commit0f5c76951327b912c8e92e83235430ebd9b349d9
tree2b93234a580d70cf038fc7b0d3bd247f0feaf0a3
parent8b6838b6e1637b430b0c77294d938effd8e36eb8
parent5ea3eaf237793c978cad9d48f7850619c7f49d69
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 https://github.com/rust-lang/rust/issues/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`