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: Add a new lint for broken inline code #105848

Merged
merged 1 commit into from
Apr 29, 2023

Conversation

lukas-code
Copy link
Member

@lukas-code lukas-code commented Dec 17, 2022

This patch adds rustdoc::unescaped_backticks, a new rustdoc lint that will detect broken inline code nodes.

The lint woks by finding stray backticks and with some heuristics tries to guess where the second backtick might be missing.

Here is how it looks:

#![warn(rustdoc::unescaped_backticks)]

/// `add(a, b) is the same as `add(b, a)`.
pub fn add(a: i32, b: i32) -> i32 { a + b }
warning: unescaped backtick
 --> src/lib.rs:3:41
  |
3 | /// `add(a, b) is the same as `add(b, a)`.
  |                                         ^
  |
help: a previous inline code might be longer than expected
  |
3 | /// `add(a, b)` is the same as `add(b, a)`.
  |               +
help: if you meant to use a literal backtick, escape it
  |
3 | /// `add(a, b) is the same as `add(b, a)\`.
  |                                         +

If we can't get proper spans, for example if the doc comment comes from a macro expansion, we print the suggestion in help messages instead. Here's a real-world example:

warning: unescaped backtick
    --> /tracing-subscriber-0.3.17/src/layer/mod.rs:1400:9
     |
1400 | /         /// Returns an optional hint of the highest [verbosity level][level] that
1401 | |         /// this `Filter` will enable.
1402 | |         ///
1403 | |         /// If this method returns a [`LevelFilter`], it will be used as a hint to
...    |
1427 | |         /// [`Interest`]: tracing_core::subscriber::Interest
1428 | |         /// [rebuild]: tracing_core::callsite::rebuild_interest_cache
     | |_____________________________________________________________________^
     |
     = help: a previous inline code might be longer than expected
              change: Therefore, if the `Filter will change the value returned by this
             to this: Therefore, if the `Filter` will change the value returned by this
     = help: if you meant to use a literal backtick, escape it
              change: [`rebuild_interest_cache`][rebuild] is called after the value of the max
             to this: [`rebuild_interest_cache\`][rebuild] is called after the value of the max

You can find more examples here.

A limitation of the current implementation is, that it cannot suggest removing misplaced backticks, for example here.

The lint is allowed by default and nightly-only for now, but without a feature gate. This is similar to how rustdoc::invalid_html_tags and rustdoc::bare_urls were handled.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 17, 2022
@lukas-code lukas-code force-pushed the backticks branch 2 times, most recently from 76bb565 to 3de5177 Compare December 17, 2022 23:31
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Dec 17, 2022
@lukas-code lukas-code force-pushed the backticks branch 3 times, most recently from 45bed3b to a9133ca Compare December 18, 2022 01:37
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Dec 29, 2022

☔ The latest upstream changes (presumably #105741) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jan 14, 2023

☔ The latest upstream changes (presumably #106833) made this pull request unmergeable. Please resolve the merge conflicts.

@lukas-code
Copy link
Member Author

@lcnr It looks like there is something missing at the end of this doc comment, but I can't figure out what. Can you help me out here?

/// If the self type of a goal is a projection, computing the relevant candidates is difficult.
///
/// To deal with this, we first try to normalize the self type and add the candidates for the normalized
/// self type to the list of candidates in case that succeeds. Note that we can't just eagerly return in
/// this case as projections as self types add `
fn assemble_candidates_after_normalizing_self_ty(&mut self, goal: Goal<'tcx, G>) {

@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor

lcnr commented Jan 16, 2023

    /// self type to the list of candidates in case that succeeds. Note that we can't just eagerly return in
    /// this case as projections as self types can add additional candidates.

probably wanted to write

@bors
Copy link
Contributor

bors commented Apr 28, 2023

⌛ Testing commit f5e16d5 with merge d795284384f6445b1ed810e4f039f74d88523acc...

@bors
Copy link
Contributor

bors commented Apr 28, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 28, 2023
@ehuss
Copy link
Contributor

ehuss commented Apr 28, 2023

@bors retry

python setuptools issue

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 28, 2023
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Apr 29, 2023

☔ The latest upstream changes (presumably #110643) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 29, 2023
@GuillaumeGomez
Copy link
Member

@bors r=GuillaumeGomez,jyn514,notriddle rollup

@bors
Copy link
Contributor

bors commented Apr 29, 2023

📌 Commit 4f15a77 has been approved by GuillaumeGomez,jyn514,notriddle

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 29, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 29, 2023
…omez,jyn514,notriddle

rustdoc: Add a new lint for broken inline code

This patch adds `rustdoc::unescaped_backticks`, a new rustdoc lint that will detect broken inline code nodes.

The lint woks by finding stray backticks and with some heuristics tries to guess where the second backtick might be missing.

Here is how it looks:
```rust
#![warn(rustdoc::unescaped_backticks)]

/// `add(a, b) is the same as `add(b, a)`.
pub fn add(a: i32, b: i32) -> i32 { a + b }
```
```text
warning: unescaped backtick
 --> src/lib.rs:3:41
  |
3 | /// `add(a, b) is the same as `add(b, a)`.
  |                                         ^
  |
help: a previous inline code might be longer than expected
  |
3 | /// `add(a, b)` is the same as `add(b, a)`.
  |               +
help: if you meant to use a literal backtick, escape it
  |
3 | /// `add(a, b) is the same as `add(b, a)\`.
  |                                         +
```

If we can't get proper spans, for example if the doc comment comes from a macro expansion, we print the suggestion in help messages instead. Here's a [real-world example](https://docs.rs/tracing-subscriber/0.3.17/tracing_subscriber/layer/trait.Filter.html#method.max_level_hint):

```text
warning: unescaped backtick
    --> /tracing-subscriber-0.3.17/src/layer/mod.rs:1400:9
     |
1400 | /         /// Returns an optional hint of the highest [verbosity level][level] that
1401 | |         /// this `Filter` will enable.
1402 | |         ///
1403 | |         /// If this method returns a [`LevelFilter`], it will be used as a hint to
...    |
1427 | |         /// [`Interest`]: tracing_core::subscriber::Interest
1428 | |         /// [rebuild]: tracing_core::callsite::rebuild_interest_cache
     | |_____________________________________________________________________^
     |
     = help: a previous inline code might be longer than expected
              change: Therefore, if the `Filter will change the value returned by this
             to this: Therefore, if the `Filter` will change the value returned by this
     = help: if you meant to use a literal backtick, escape it
              change: [`rebuild_interest_cache`][rebuild] is called after the value of the max
             to this: [`rebuild_interest_cache\`][rebuild] is called after the value of the max
```

You can find more examples [here](https://gist.github.com/lukas-code/7678ddf5c608aee97b3a669de80d3465).

A limitation of the current implementation is, that it cannot suggest removing misplaced backticks, for example [here](https://docs.rs/tikv-jemalloc-sys/0.5.3+5.3.0-patched/tikv_jemalloc_sys/fn.mallctl.html).

The lint is allowed by default ~~and nightly-only~~ for now, ~~but without a feature gate. This is similar to how `rustdoc::invalid_html_tags` and `rustdoc::bare_urls` were handled.~~
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 29, 2023
…omez,jyn514,notriddle

rustdoc: Add a new lint for broken inline code

This patch adds `rustdoc::unescaped_backticks`, a new rustdoc lint that will detect broken inline code nodes.

The lint woks by finding stray backticks and with some heuristics tries to guess where the second backtick might be missing.

Here is how it looks:
```rust
#![warn(rustdoc::unescaped_backticks)]

/// `add(a, b) is the same as `add(b, a)`.
pub fn add(a: i32, b: i32) -> i32 { a + b }
```
```text
warning: unescaped backtick
 --> src/lib.rs:3:41
  |
3 | /// `add(a, b) is the same as `add(b, a)`.
  |                                         ^
  |
help: a previous inline code might be longer than expected
  |
3 | /// `add(a, b)` is the same as `add(b, a)`.
  |               +
help: if you meant to use a literal backtick, escape it
  |
3 | /// `add(a, b) is the same as `add(b, a)\`.
  |                                         +
```

If we can't get proper spans, for example if the doc comment comes from a macro expansion, we print the suggestion in help messages instead. Here's a [real-world example](https://docs.rs/tracing-subscriber/0.3.17/tracing_subscriber/layer/trait.Filter.html#method.max_level_hint):

```text
warning: unescaped backtick
    --> /tracing-subscriber-0.3.17/src/layer/mod.rs:1400:9
     |
1400 | /         /// Returns an optional hint of the highest [verbosity level][level] that
1401 | |         /// this `Filter` will enable.
1402 | |         ///
1403 | |         /// If this method returns a [`LevelFilter`], it will be used as a hint to
...    |
1427 | |         /// [`Interest`]: tracing_core::subscriber::Interest
1428 | |         /// [rebuild]: tracing_core::callsite::rebuild_interest_cache
     | |_____________________________________________________________________^
     |
     = help: a previous inline code might be longer than expected
              change: Therefore, if the `Filter will change the value returned by this
             to this: Therefore, if the `Filter` will change the value returned by this
     = help: if you meant to use a literal backtick, escape it
              change: [`rebuild_interest_cache`][rebuild] is called after the value of the max
             to this: [`rebuild_interest_cache\`][rebuild] is called after the value of the max
```

You can find more examples [here](https://gist.github.com/lukas-code/7678ddf5c608aee97b3a669de80d3465).

A limitation of the current implementation is, that it cannot suggest removing misplaced backticks, for example [here](https://docs.rs/tikv-jemalloc-sys/0.5.3+5.3.0-patched/tikv_jemalloc_sys/fn.mallctl.html).

The lint is allowed by default ~~and nightly-only~~ for now, ~~but without a feature gate. This is similar to how `rustdoc::invalid_html_tags` and `rustdoc::bare_urls` were handled.~~
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 29, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#105848 (rustdoc: Add a new lint for broken inline code)
 - rust-lang#110644 (Update tests for libtest `--format json`)
 - rust-lang#110950 (Deny the `unsafe_op_in_unsafe_fn` lint in `rustc_arena`.)
 - rust-lang#110951 (Add support for LibreSSL 3.7.x)
 - rust-lang#110964 (rustdoc: fix weird margins between Deref impl items)
 - rust-lang#110979 (windows: kill rust-analyzer-proc-macro-srv before deleting stage0 directory)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 825bc60 into rust-lang:master Apr 29, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 29, 2023
@lukas-code lukas-code deleted the backticks branch April 29, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants