Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustdoc does not warn about broken links if they contain . or [] #54191

Open
whatisaphone opened this issue Sep 13, 2018 · 11 comments · May be fixed by #132748
Open

rustdoc does not warn about broken links if they contain . or [] #54191

whatisaphone opened this issue Sep 13, 2018 · 11 comments · May be fixed by #132748
Assignees
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@whatisaphone
Copy link
Contributor

I got a PR yesterday that included something like this:

/// The tour de force of my career is [`foo.bar()`]
pub mod foo {
    pub fn bar() {}
}

In the current nightly rustdoc, foo.bar() neither resolves nor warns:

Our CI with --deny warnings did not catch this mistake (although I did 😄). I think rustdoc should trigger intra-doc-link-resolution-failure for this case.

@Havvy Havvy added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Sep 13, 2018
@GuillaumeGomez
Copy link
Member

The resolution failure is expected (should have been foo::bar) however, no warning is strange. I think it considers it as a path (as in a url) because of the ..

@QuietMisdreavus QuietMisdreavus added the A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name label Nov 6, 2018
@QuietMisdreavus
Copy link
Member

This is caused by this section of the intra-doc link code:

if path_str.contains(|ch: char| !(ch.is_alphanumeric() ||
ch == ':' || ch == '_')) {
continue;
}

I think the idea here is that because of the dot, the link could be to a file, so it doesn't try to check it. (There's a similar skip earlier in the loop for if the link contains /.)

@jyn514 jyn514 changed the title rustdoc: intra-doc-link-resolution-failure: Missed case rustdoc does not warn about broken links if they contain a '.' Aug 19, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 2, 2021
Update intra-doc link documentation to match the implementation

r? `@Manishearth`
cc `@camelid` `@m-ou-se`

Relevant PRs:
- rust-lang#74489
- rust-lang#80181
- rust-lang#76078
- rust-lang#77519
- rust-lang#73101

Relevant issues:
- rust-lang#78800
- rust-lang#77200
- rust-lang#77199 / rust-lang#54191

I haven't documented things that I consider 'just bugs', like rust-lang#77732, but I have documented features that aren't implemented, like rust-lang#78800.
@jyn514
Copy link
Member

jyn514 commented Nov 29, 2021

@dylni mentioned in #77199 that this also applies to things like [&[String]], because of the brackets; it would be nice to warn about those. I know @Manishearth has been concerned in the past about warning too much in case it has false positives - maybe we could make this a new opt-in lint? as a bikeshed warn(rustdoc::strange_intra_doc_links) seems ok

@jyn514 jyn514 changed the title rustdoc does not warn about broken links if they contain a '.' rustdoc does not warn about broken links if they contain a . or [] Nov 29, 2021
@jyn514 jyn514 changed the title rustdoc does not warn about broken links if they contain a . or [] rustdoc does not warn about broken links if they contain . or [] Nov 29, 2021
@Manishearth
Copy link
Member

I suspect a thing we can do is unconditionally hit the intra doc resolution path if it has backticks in it, at least. I'd rather not introduce another lint; we should work on improving the diagnostic instead.

@Nemo157
Copy link
Member

Nemo157 commented Nov 29, 2021

Can we check whether it resolved as a markdown link too?

[`foo.bar()`] is creating a link to an undefined reference `foo.bar()`, so should be warned about as a broken link whether it was intended to be an intra-doc link or not.

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2021

@Nemo157 none of these links will ever resolve. . and [] are not paths and rustdoc doesn't special case them:

let prim = match path_str {
"isize" => Isize,
"i8" => I8,
"i16" => I16,
"i32" => I32,
"i64" => I64,
"i128" => I128,
"usize" => Usize,
"u8" => U8,
"u16" => U16,
"u32" => U32,
"u64" => U64,
"u128" => U128,
"f32" => F32,
"f64" => F64,
"char" => Char,
"bool" | "true" | "false" => Bool,
"str" | "&str" => Str,
// See #80181 for why these don't have symbols associated.
"slice" => Slice,
"array" => Array,
"tuple" => Tuple,
"unit" => Unit,
"pointer" | "*const" | "*mut" => RawPointer,
"reference" | "&" | "&mut" => Reference,
"fn" => Fn,
"never" | "!" => Never,
_ => return None,
};

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2021

Oh, you mean as a URL. Hmm, maybe? I have to look back over the pulldown API.

@Nemo157
Copy link
Member

Nemo157 commented Nov 29, 2021

The [foo] form does not link to a URL, it references a URL defined elsewhere inside the markdown document using the link label foo. If that label is not defined, I think technically it is not a link, it's just the text [foo]; but this is a syntax niche that intra-doc-links has already inserted itself into so I think it's fine for us to warn on all reference links that are missing a link reference definition (including the implicit intra-doc-links definitions).

@Manishearth
Copy link
Member

@Nemo157 you misunderstand; we only hit this codepath for broken refs and for actual URLs. If it resolved to a markdown ref the ref title will never be seen here, though the URL passed to the ref title might (if it's sufficiently non-URLlike)

@Manishearth
Copy link
Member

Oh, though I guess we could unconditionally warn on unresolving broken refs (but use the intra doc diagnostic on broken urls)

@lolbinarycat
Copy link
Contributor

another case where these get confusing:

/// [whatever][1]
///
/// some other text
/// [1]: https://example.com/whatever

the [1]: does not actually get turned into a link reference, since there is no paragraph break before it.

@lolbinarycat lolbinarycat self-assigned this Nov 7, 2024
lolbinarycat added a commit to lolbinarycat/rust that referenced this issue Nov 7, 2024
rustdoc will not try to do intra-doc linking if the "path"
of a link looks too much like a "real url".

however, only inline links ([text](url)) can actually contain
a url, other types of links (reference links, shortcut links)
contain a *reference* which is later resolved to an actual url.

the "path" in this case cannot be a url, and therefore it should
not be skipped due to looking like a url.

fixes rust-lang#54191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants