Avoid documenting top-level private imports
PR #88447 aimed to make rustdoc's `--document-private-items` mode only document imports that are visible outside the importing module. Unfortunately, I inadvertently set things up so that imports at the crate top-level are always documented, regardless of their visibility. This behavior was unintended and is [not desirable](https://github.com/rust-lang/rust/issues/90865#issuecomment-971172649).
This PR treats top-level imports as never being visible outside their parent module. In practice, the only way a top-level import can be visible externally is if it's fully public, and there's a seperate check for that.
It's worth calling attention to the fact that this change means that `pub(crate)` imports will be visible in lower level modules, but not at the top-level. This is because, at the top level of the crate, `pub(crate)` means the same thing as `pub(self)`.
It turned out that there were existing tests checking for the only behavior, which I didn't notice at the time of my previous PR. I have updated them to check for the new behavior and substantially extended them to handle differences between the top-level module and lower level modules. I may have gone overboard, so please tell me if there's anything I should cut.
r? `@jyn514`
Fixes#90865.
Set color for <a> in a more straightforward way.
Previously, we set the default color for <a> tags to black, and then had an override with a bunch of not() clauses to set anchors in
docblocks to blue.
Instead, we should set the default color for <a> to blue (or equivalent in other themes), and override it for places like the sidebar or search results, where we don't want them to be styled as links.
Demo at https://rustdoc.crud.net/jsha/theme-anchor/std/string/struct.String.html. This should result in no visible changes.
r? `@GuillaumeGomez`
Previously, we set the default color for <a> tags to black, and then
had an override with a bunch of not() clauses to set anchors in
docblocks to blue.
Instead, we should set the default color for <a> to blue (or equivalent
in other themes), and override it for places like the sidebar or search
results, where we don't want them to be styled as links.
Make scrollbar in the sidebar always visible for visual consistency
Fixes#90943.
I had to add a background in `dark` and `ayu` themes, otherwise it was looking strange (like an invisible margin). So it looks like this:
Sadly, I wasn't able to add a GUI test to ensure that the scrollbar was always displayed because it seems not possible in puppeteer for whatever reason... I used this method: on small pages (like `lib2/sub_mod/index.html`), comparing `.navbar`'s `clientWidth` with `offsetWidth` (the first doesn't include the sidebar in the computed amount). When checking in the browser, it works fine but in puppeteer it almost never works...
In case anyone want to try to solve the bug, here is the puppeteer code:
<details>
More information about this: I tried another approach which was to get the element in `evaluate` directly (by calling it from `page.evaluate(() => { .. });` directly instead of `parseAssertElemProp.evaluate(e => {...});`.
```js
const puppeteer = require('puppeteer');
(async () => {
const browser = await puppeteer.launch();
const page = await browser.newPage();
await page.goto("file:///path/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/lib2/sub_mod/index.html");
await page.waitFor(".sidebar");
let parseAssertElemProp = await page.$(".sidebar");
if (parseAssertElemProp === null) { throw '".sidebar" not found'; }
await parseAssertElemProp.evaluate(e => {
const parseAssertElemPropDict = {"clientWidth": "192", "offsetWidth":"200"};
for (const [parseAssertElemPropKey, parseAssertElemPropValue] of Object.entries(parseAssertElemPropDict)) {
if (e[parseAssertElemPropKey] === undefined || String(e[parseAssertElemPropKey]) != parseAssertElemPropValue) {
throw 'expected `' + parseAssertElemPropValue + '` for property `' + parseAssertElemPropKey + '` for selector `.sidebar`, found `' + e[parseAssertElemPropKey] + '`';
}
}
}).catch(e => console.error(e));
await browser.close();
})();
```
</details>
r? ``@jsha``
rustdoc doctest: detect `fn main` after an unexpected semicolon
Fixes#91014
The basic problem with this is that rustdoc, when hunting for `fn main`, will stop parsing after it reaches a fatal error. This unexpected semicolon was a fatal error, so in `src/test/rustdoc-ui/failed-doctest-extra-semicolon-on-item.rs`, it would wrap the doctest in an implied main function, turning it into this:
fn main() {
struct S {};
fn main() {
assert_eq!(0, 1);
}
}
This, as it turns out, is totally valid, and it executes no assertions, so *it passes,* even though the user wanted it to execute the assertion.
The Rust parser already has the ability to recover from these unexpected semicolons, but to do so, it needs to use the `parse_mod` function, so this PR changes it to do that.
The basic problem with this is that rustdoc, when hunting for `fn main`, will stop
parsing after it reaches a fatal error. This unexpected semicolon was a fatal error,
so in `src/test/rustdoc-ui/failed-doctest-extra-semicolon-on-item.rs`, it would wrap
the doctest in an implied main function, turning it into this:
fn main() {
struct S {};
fn main() {
assert_eq!(0, 1);
}
}
This, as it turns out, is totally valid, and it executes no assertions, so *it passes,*
even though the user wanted it to execute the assertion.
The Rust parser already has the ability to recover from these unexpected semicolons,
but to do so, it needs to use the `parse_mod` function, so this commit changes it to do that.
rustdoc: Replace where-bounded Clean impl with simple function
This is the first step in removing the Clean impls for tuples. Either way, this
significantly simplifies the code since it reduces the amount of "trait magic".
(To clarify, I'm referring to impls like `impl Clean for (A, B)`, not Clean impls
that work on tuples in the user's program.)
cc ``@jyn514``
This function parameter attribute was introduced in https://github.com/rust-lang/rust/pull/44866 as an intermediate step in implementing `impl Trait`, it's not necessary or used anywhere by itself.
Remove unneeded FIXMEs comments in search index generation
Original comment:
> Instead of recreating a new `vec` for each arguments, we re-use the same. The impact on performance should be minor but worth a try.
After testing it, we reached the conclusion that the code readability drop wasn't worth the almost unnoticeable performance improvement.
r? `@camelid`
Add more comments to explain the code to generate the search index
Fixes#90766.
I tried to put comments when the code wasn't easy to understand at first sight and added more documentation on the recursive function. Please tell me if I misused the terminology or if comments can be improved or added into other places.
r? `@notriddle`
Remove potential useless data for search index
I uncovered this case when working on https://github.com/rust-lang/rust/pull/90726 to debug https://github.com/rust-lang/rust/pull/90385.
Explanations: if we have a full generic, we check if it has generics then we do the following:
* If it has only one generic, we remove one nested level in order to not keep the "parent" generic (since it has empty name, it's useless after all).
* Otherwise we add it alongside its generics.
However, I didn't handle the case where a generic had no generics. Meaning that we were adding items with empty names in the search index. So basically useless data in the search index.
r? `@camelid`
Otherwise, rustdoc panics with messages like this:
thread 'rustc' panicked at 'assertion failed: cx.impl_trait_bounds.is_empty()',
src/librustdoc/clean/utils.rs:462:5
This ordering requirement is unrelated to the `clean_fn_decl_with_args`
refactoring, but the requirement was uncovered as part of that change.
I'm not sure if *all* of these places have the requirement, but I added
comments to them just in case.
