Doc alias checks: ensure only items appearing in search index can use it#74098
Doc alias checks: ensure only items appearing in search index can use it#74098bors merged 7 commits intorust-lang:masterfrom
Conversation
|
The search index also doesn't include associated types and consts in trait impls so we'll need to error on those too. I'm not actually sure why they're not included though. |
|
I think it's in case of conflicts between two implementations. I'll add them as well, good catch! |
9e93555 to
1cc5b45
Compare
|
@ollie27 Updated! |
|
cc @rust-lang/rustdoc |
|
Ideally this check would be done inside rustc with the other checks. Unfortunately I think #73566 meant that the checks moved in #74148 no longer run in rustdoc. That should have bee caught by CI but the test was moved rather than copied. Assuming it's possible to get the |
|
Good point! I'll move those checks then. |
It should be possible to add it back as long as it doesn't need typeck. See https://github.com/rust-lang/rust/blob/master/src/librustdoc/core.rs#L460 for an example. |
|
So the discussion is in one place: @GuillaumeGomez tried this and found that checking for unused attributes is in the same pass as checking for unused code. So it currently requires typeck: It will have to be split into two passes for this to work: one for unused attrs, and the other for unreachable expressions. |
|
rust/src/librustc_interface/passes.rs Line 847 in 2186722 so I tested adding: for &module in tcx.hir().krate().modules.keys() {
let local_def_id = tcx.hir().local_def_id(module);
tcx.ensure().check_mod_attrs(local_def_id);
};just after Lines 460 to 462 in 2186722 |
17b2e1d to
c0bb98d
Compare
|
Updated! I added the equivalent tests both in |
|
ping @rust-lang/rustdoc |
|
Do you want review from others? Seems like jyn and Ollie are reviewing this PR |
|
You can provide one as well if you want. But it was more like a reminder for the actual reviewers. :) |
|
Nah, I don't have many opinions on doc(alias) and I trust the others |
|
☔ The latest upstream changes (presumably #74932) made this pull request unmergeable. Please resolve the merge conflicts. |
c0bb98d to
2234543
Compare
|
Rebased. |
|
ping @ollie27 |
2234543 to
3e5eb20
Compare
|
@ollie27 The impl const items are now allowed to have a |
3e5eb20 to
fc6fb3f
Compare
|
Forgot that we tested in both |
|
ping @ollie27 |
ollie27
left a comment
There was a problem hiding this comment.
Sorry for the delay.
As I've said associated consts in trait impls also don't appear in the search index so should also produce errors.
| @@ -5,7 +5,7 @@ const QUERY = [ | |||
| 'StructFieldItem', | |||
| 'StructMethodItem', | |||
| 'ImplTraitItem', | |||
There was a problem hiding this comment.
ImplTraitItem should be removed from here as well now it's been removed from doc-alias.rs.
There was a problem hiding this comment.
That's why the search for it is returning nothing below. The goal was to check that nothing was returned.
There was a problem hiding this comment.
ImplTraitItem doesn't appear anywhere in the source file anymore so of course it's not going to produce any search results. This isn't a particularly useful test.
There was a problem hiding this comment.
But that's the point of the test: it's supposed to not return anything, so we ensure that it doesn't.
|
I guess we're good to go then! \o/ @bors: r=ollie27 |
|
📌 Commit fc6fb3f has been approved by |
|
☀️ Test successful - checks-actions, checks-azure |
Following the discussion in #73721, I added checks to ensure that only items appearing in the search are allowed to have doc alias.
r? @ollie27