Skip to content

Commit ad89ade

Browse files
authored
Unrolled build for #148169
Rollup merge of #148169 - fmease:rustdoc-bad-intra-bad-preprocess, r=lolbinarycat Fix bad intra-doc-link preprocessing How did #147981 happen? 1. We don't parse intra-doc links as Rust paths or qpaths. Instead they follow a very lenient bespoke grammar. We completely ignore Markdown links if they contain characters that don't match `/[a-zA-Z0-9_:<>, !*&;]/` (we don't even emit lint *broken-intra-doc-links* for these). 2. PR [#132748](#132748) made rustdoc intepret more Markdown links as potential intra-doc links. Namely, if the link is surrounded by backticks (and some other conditions apply) then it doesn't matter if the (partially processed) link contains bad characters as defined above (cc `ignore_urllike && should_ignore_link(path_str)`). 3. However, rustdoc's `preprocess_link` must be kept in sync with a simplified counterpart in rustc. More specifically, whenever rustdoc's preprocessor returns a successful result then rustc's must yield the same result. Otherwise, rustc doesn't resolve the necessary links for rustdoc. 4. This uncovered a "dormant bug" / "mistake" in rustc's `preprocess_link`. Namely, when presented with a link like `struct@Type@suffix`, it didn't cut off the disambiguator if present (here: `struct@`). Instead it `rsplit('``@')``` which is incorrect if the "path" contains ```@``` itself (yielding `suffix` instead of `Type@suffix` here). Prior to PR [#132748](#132748), a link like ``[`struct@Type@suffix`]`` was not considered a potential intra-doc link / worth querying rustc for. Now it is due to the backticks. 5. Finally, since rustc didn't record a resolution for `Type@suffix` (it only recorded `suffix` (to be `Res::Err`)), we triggered an assertion we have in place to catch cases like this. Fixes #147981. I didn't and still don't have the time to investigate if #132748 led to more rustc/rustdoc mismatches (after all, the PR made rustdoc's `preprocess_link` return `Some(Ok(_))` in more cases). I've at least added another much needed "warning banner" & made the existing one more flashy. While this fixes a stable-to-beta regression, I don't think it's worth beta backporting, esp. since it's only P-medium and since the final 1.91 release steps commence today / the next days, so it would only be stressful to get it in on time. However, feel free to nominate. <sub>(I've written such a verbose PR description since I tend to reread my old PR descriptions in the far future to fully freshen my memories when I have to work again in this area)</sub> r? ``@lolbinarycat``
2 parents f40a70d + 55d90c0 commit ad89ade

File tree

5 files changed

+64
-13
lines changed

5 files changed

+64
-13
lines changed

compiler/rustc_resolve/src/rustdoc.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,13 +393,15 @@ pub fn has_primitive_or_keyword_or_attribute_docs(attrs: &[impl AttributeExt]) -
393393
}
394394

395395
/// Simplified version of the corresponding function in rustdoc.
396-
/// If the rustdoc version returns a successful result, this function must return the same result.
397-
/// Otherwise this function may return anything.
398396
fn preprocess_link(link: &str) -> Box<str> {
397+
// IMPORTANT: To be kept in sync with the corresponding function in rustdoc.
398+
// Namely, whenever the rustdoc function returns a successful result for a given input,
399+
// this function *MUST* return a link that's equal to `PreprocessingInfo.path_str`!
400+
399401
let link = link.replace('`', "");
400402
let link = link.split('#').next().unwrap();
401403
let link = link.trim();
402-
let link = link.rsplit('@').next().unwrap();
404+
let link = link.split_once('@').map_or(link, |(_, rhs)| rhs);
403405
let link = link.trim_suffix("()");
404406
let link = link.trim_suffix("{}");
405407
let link = link.trim_suffix("[]");

src/librustdoc/passes/collect_intra_doc_links.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -901,14 +901,18 @@ pub(crate) struct PreprocessedMarkdownLink(
901901

902902
/// Returns:
903903
/// - `None` if the link should be ignored.
904-
/// - `Some(Err)` if the link should emit an error
905-
/// - `Some(Ok)` if the link is valid
904+
/// - `Some(Err(_))` if the link should emit an error
905+
/// - `Some(Ok(_))` if the link is valid
906906
///
907907
/// `link_buffer` is needed for lifetime reasons; it will always be overwritten and the contents ignored.
908908
fn preprocess_link(
909909
ori_link: &MarkdownLink,
910910
dox: &str,
911911
) -> Option<Result<PreprocessingInfo, PreprocessingError>> {
912+
// IMPORTANT: To be kept in sync with the corresponding function in `rustc_resolve::rustdoc`.
913+
// Namely, whenever this function returns a successful result for a given input,
914+
// the rustc counterpart *MUST* return a link that's equal to `PreprocessingInfo.path_str`!
915+
912916
// certain link kinds cannot have their path be urls,
913917
// so they should not be ignored, no matter how much they look like urls.
914918
// e.g. [https://example.com/] is not a link to example.com.

tests/rustdoc-ui/intra-doc/empty-associated-items.rs

Lines changed: 0 additions & 8 deletions
This file was deleted.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// This test ensures that (syntactically) malformed paths will not crash rustdoc.
2+
#![deny(rustdoc::broken_intra_doc_links)]
3+
4+
// This is a regression test for <https://github.com/rust-lang/rust/issues/140026>.
5+
//! [`Type::`]
6+
//~^ ERROR
7+
8+
// This is a regression test for <https://github.com/rust-lang/rust/issues/147981>.
9+
//! [`struct@Type@field`]
10+
//~^ ERROR
11+
12+
//! [Type&content]
13+
//~^ ERROR
14+
//! [`Type::field%extra`]
15+
//~^ ERROR
16+
17+
pub struct Type { pub field: () }
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
error: unresolved link to `Type::`
2+
--> $DIR/malformed-paths.rs:5:7
3+
|
4+
LL | //! [`Type::`]
5+
| ^^^^^^ the struct `Type` has no field or associated item named ``
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/malformed-paths.rs:2:9
9+
|
10+
LL | #![deny(rustdoc::broken_intra_doc_links)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: unresolved link to `Type@field`
14+
--> $DIR/malformed-paths.rs:9:7
15+
|
16+
LL | //! [`struct@Type@field`]
17+
| ^^^^^^^^^^^^^^^^^ no item named `Type@field` in scope
18+
|
19+
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
20+
21+
error: unresolved link to `Type&content`
22+
--> $DIR/malformed-paths.rs:12:6
23+
|
24+
LL | //! [Type&content]
25+
| ^^^^^^^^^^^^ no item named `Type&content` in scope
26+
|
27+
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
28+
29+
error: unresolved link to `Type::field%extra`
30+
--> $DIR/malformed-paths.rs:14:7
31+
|
32+
LL | //! [`Type::field%extra`]
33+
| ^^^^^^^^^^^^^^^^^ the struct `Type` has no field or associated item named `field%extra`
34+
35+
error: aborting due to 4 previous errors
36+

0 commit comments

Comments
 (0)