Conversation
rustbot
added
the
S-waiting-on-community-reviews
|
Thanks for the pull request. A reviewer will take a look after it receives 2 community reviews. In the meantime, we would highly appreciate if you could try to review any of PRs waiting on community reviews. |
rustbot
added
needs-fcp
|
Lintcheck changes for bdf5913
This comment will be updated if you push new changes |
1803dbf to
b65800d
Compare
There was a problem hiding this comment.
community review: looking at the lintcheck reports a bunch of hits that seem like false positives, e.g. the very first one is from
This is a convenience function around the [`Adler32`] type
with the suggestion to instead link with
This is a convenience function around the [`Adler32`](struct@crate::Adler32) type.
but there wasn't any hard-coded html in the original?
It looks like the cause is the link being defined later with
/// [`Adler32`]: struct.Adler32.html
so that is where the suggestion should be emitted
136e377 to
21554f2
Compare
This comment has been minimized.
This comment has been minimized.
Encourages documentation authors to write links using paths, instead of writing rustdoc URLs by hand. This lint detects the two common cases. It avoids warning on paths that point at crates that aren't dependencies, because those can't be written as paths. It doesn't detect broken links, because rustdoc itself should detect them.
21554f2 to
7f321e1
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
ced25f0 to
ed57545
Compare
|
@DanielEScherzer @poliorcetics I implemented both of your suggestions. Could you please mark your reviews as "approved"? |
|
Already did :) |
| help: consider linking by path instead | ||
| | | ||
| LL - //! [refdef]: index.html | ||
| LL + //! [refdef]: mod@crate |
There was a problem hiding this comment.
it seems odd that the ^^^ aren't under where the actual link is defined in cases like this where the definition is separate
| help: consider linking by path instead | ||
| | | ||
| LL - /// Link to [libstd vec](https://doc.rust-lang.org/nightly/std/index.html) | ||
| LL + /// Link to [libstd vec](mod@crate::std) |
There was a problem hiding this comment.
what about links that are intentionally pointed at nightly (or beta) because the target doesn't reach stable for another 12 weeks?
especially since we are claiming that this lint is machine applicable... - changing nightly and beta links seems like something that is potentially incorrect
There was a problem hiding this comment.
I think we should produce a lint on nightly links, because, for example, if you're on https://docs.rs/regex/latest/regex/struct.Regex.html#method.as_str and you click the str link in that function signature, you wind up on the nightly docs. This means naive users could easily go there, and copy-paste one of those URLs into their doc comment, without deliberately seeking out nightly docs.
Maybe we shouldn't make these suggestions MachineApplicable?
| @@ -0,0 +1,388 @@ | |||
| error: manual intra-doc link | |||
There was a problem hiding this comment.
can you please also include test cases for
- linking to a specific version of a crate on docs.rs, rather than latest (both where that version is the dependency version, and that version is different from the dependency version)?
There was a problem hiding this comment.
linking to a specific version of a crate on docs.rs
I've added a test case for this.
both where that version is the dependency version, and that version is different from the dependency version
The lint doesn't make that distinction, because it doesn't know what the version number is. AFAICT, Clippy can't get that information at all. In any case, the test case doesn't supply it.
rustbot
removed
the
S-waiting-on-community-reviews
|
r? @llogiq rustbot has assigned @llogiq for the project review. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Co-authored-by: Daniel Scherzer <daniel.e.scherzer@gmail.com>
`tests/compile-test.rs` and `clippy_dev/src/serve.rs` seem to build the website bu parsing markdown without resolving intra-doc links. So, these lint docs need to write full URLs.
This branch has not been deployed
5 participants
Encourages documentation authors to write links using paths, instead of writing rustdoc URLs by hand.
This lint detects the two common cases: local URLs and docs.rs. It avoids warning on paths that point at crates that aren't dependencies, because those can't be written as paths. It doesn't detect broken links, because rustdoc itself should detect them.
I tested all the generated intra-doc links using rustdoc in https://gist.github.com/notriddle/1db64b4aa206fce9284871ec1b4116ee
changelog: [
manual_intra_doc_links]: lint hardcoded.htmllinks in docsFixes #1912
Fixes rust-lang/rust#75805