Skip to content

Add new unescaped_pipe_in_table_cell rustdoc lint#159583

Open
GuillaumeGomez wants to merge 3 commits into
rust-lang:mainfrom
GuillaumeGomez:unescaped_pipe_in_table_cell
Open

Add new unescaped_pipe_in_table_cell rustdoc lint#159583
GuillaumeGomez wants to merge 3 commits into
rust-lang:mainfrom
GuillaumeGomez:unescaped_pipe_in_table_cell

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

Fixes #159186.

r? @Urgau

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 19, 2026
@rust-log-analyzer

Copy link
Copy Markdown
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
checking C++ file formatting
spellchecking files
building external tool typos from package typos-cli@1.38.1
finished building tool typos
error: `discared` should be `discarded`, `discard`
  ╭▸ src/librustdoc/passes/lint/table_pipe_escape.rs:1:62
  │
1 │ //! Detects table rows where some content seems to have been discared because there are too many
  ╰╴                                                             ━━━━━━━━
rerun with `--bless` to fix typos: `./x.py test tidy --extra-checks=spellcheck --bless`
tidy [extra_checks:spellcheck]: checks with external tool 'typos' failed
tidy [extra_checks:spellcheck]: FAIL
yarn install v1.22.22
warning package.json: No license field
warning ../../package.json: License should be a valid SPDX license expression
warning No license field
[1/4] Resolving packages...
---
Running eslint on rustdoc JS files
info: ES-Check: checking 7 files...
info: ✓ ES-Check passed! All files are ES10 compatible.
typechecking javascript files
tidy: The following check failed: extra_checks:spellcheck
Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools-bin/rust-tidy --root-path=/checkout --cargo-path=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo --output-dir=/checkout/obj/build --concurrency=4 --npm-path=/node/bin/yarn --ci=true --extra-checks=py,cpp,js,spellcheck` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:1630:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:1646:29

Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `test src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`
Currently active steps:
test::Tidy {  } at src/bootstrap/src/core/build_steps/test.rs:1568
Build completed unsuccessfully in 0:02:49
  local time: Sun Jul 19 22:55:53 UTC 2026

@@ -0,0 +1,16 @@
#![deny(rustdoc::unescaped_pipe_in_table_cell)]

//! | col1 |

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to test edge-cases, what about adding another test case with two tables on the same doc comment.

Would also be interesting to have a test case with multiple wrong rows for the same table.

View changes since the review

Comment on lines +55 to +56
if c == '\\' {
iter.next();

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a test case for this escaping as well.

View changes since the review

#[help("to escape `|` characters in tables, add a `\\` before them like `\\|`")]
struct UnescapedPipeInTableCell {
#[primary_span]
#[suggestion("any content after this column divider is discarded", code = "")]

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a label instead of an empty suggestion?

Suggested change
#[suggestion("any content after this column divider is discarded", code = "")]
#[label("any content after this column divider is discarded")]

View changes since the review

if let Event::Start(Tag::Table(_)) = event
&& let Some((Event::Start(Tag::TableHead), _)) = p.next()
{
let mut expected_count = 0;

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut expected_count = 0;
let mut expected_cells = 0;

View changes since the review

// was left out. We now check the number of unescaped `|`.
let row = &dox[range.clone()];
let mut iter = row.chars();
let mut count = 0;

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut count = 0;
let mut divider_count = 0;

View changes since the review

Comment on lines +46 to +47
if let Some(prev_range) = &prev_range
&& prev_range.end + 1 != range.end

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this is checking. Is it checking the line width? or is it checking the position of the last TableCell with the last TableRow?

Either way, a comment would greatly help.

View changes since the review

Comment on lines +28 to +29
if let Event::Start(Tag::Table(_)) = event
&& let Some((Event::Start(Tag::TableHead), _)) = p.next()

@Urgau Urgau Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too bad that we have to do this checking manually, I don't suppose could pulldown_cmark to do this checking for us?

View changes since the review

@Urgau Urgau 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

Markdown tables can silently discard content containing |

4 participants