Skip to content

Commit 294c3dd

Browse files
committed
rustdoc: add usable lint for pulldown-cmark-0.11 parsing changes
1 parent 15fbe61 commit 294c3dd

File tree

7 files changed

+278
-0
lines changed

7 files changed

+278
-0
lines changed

Cargo.lock

+12
Original file line numberDiff line numberDiff line change
@@ -3121,6 +3121,17 @@ dependencies = [
31213121
"cc",
31223122
]
31233123

3124+
[[package]]
3125+
name = "pulldown-cmark"
3126+
version = "0.9.6"
3127+
source = "registry+https://github.com/rust-lang/crates.io-index"
3128+
checksum = "57206b407293d2bcd3af849ce869d52068623f19e1b5ff8e8778e3309439682b"
3129+
dependencies = [
3130+
"bitflags 2.5.0",
3131+
"memchr",
3132+
"unicase",
3133+
]
3134+
31243135
[[package]]
31253136
name = "pulldown-cmark"
31263137
version = "0.10.3"
@@ -4890,6 +4901,7 @@ dependencies = [
48904901
"indexmap",
48914902
"itertools",
48924903
"minifier",
4904+
"pulldown-cmark 0.9.6",
48934905
"regex",
48944906
"rustdoc-json-types",
48954907
"serde",

src/librustdoc/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ base64 = "0.21.7"
1313
itertools = "0.12"
1414
indexmap = "2"
1515
minifier = "0.3.0"
16+
pulldown-cmark-old = { version = "0.9.6", package = "pulldown-cmark", default-features = false }
1617
regex = "1"
1718
rustdoc-json-types = { path = "../rustdoc-json-types" }
1819
serde_json = "1.0"

src/librustdoc/lint.rs

+9
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,14 @@ declare_rustdoc_lint! {
196196
"detects redundant explicit links in doc comments"
197197
}
198198

199+
declare_rustdoc_lint! {
200+
/// This compatibility lint checks for Markdown syntax that works in the old engine but not
201+
/// the new one.
202+
UNPORTABLE_MARKDOWN,
203+
Warn,
204+
"detects markdown that is interpreted differently in different parser"
205+
}
206+
199207
pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
200208
vec![
201209
BROKEN_INTRA_DOC_LINKS,
@@ -209,6 +217,7 @@ pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
209217
MISSING_CRATE_LEVEL_DOCS,
210218
UNESCAPED_BACKTICKS,
211219
REDUNDANT_EXPLICIT_LINKS,
220+
UNPORTABLE_MARKDOWN,
212221
]
213222
});
214223

src/librustdoc/passes/lint.rs

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ mod check_code_block_syntax;
66
mod html_tags;
77
mod redundant_explicit_links;
88
mod unescaped_backticks;
9+
mod unportable_markdown;
910

1011
use super::Pass;
1112
use crate::clean::*;
@@ -31,6 +32,7 @@ impl<'a, 'tcx> DocVisitor for Linter<'a, 'tcx> {
3132
html_tags::visit_item(self.cx, item);
3233
unescaped_backticks::visit_item(self.cx, item);
3334
redundant_explicit_links::visit_item(self.cx, item);
35+
unportable_markdown::visit_item(self.cx, item);
3436

3537
self.visit_item_recur(item)
3638
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
//! Detects specific markdown syntax that's different between pulldown-cmark
2+
//! 0.9 and 0.11.
3+
//!
4+
//! This is a mitigation for old parser bugs that affected some
5+
//! real crates' docs. The old parser claimed to comply with CommonMark,
6+
//! but it did not. These warnings will eventually be removed,
7+
//! though some of them may become Clippy lints.
8+
//!
9+
//! <https://github.com/rust-lang/rust/pull/121659#issuecomment-1992752820>
10+
//!
11+
//! <https://rustc-dev-guide.rust-lang.org/bug-fix-procedure.html#add-the-lint-to-the-list-of-removed-lists>
12+
13+
use crate::clean::Item;
14+
use crate::core::DocContext;
15+
use pulldown_cmark as cmarkn;
16+
use pulldown_cmark_old as cmarko;
17+
use rustc_lint_defs::Applicability;
18+
use rustc_resolve::rustdoc::source_span_for_markdown_range;
19+
use std::collections::{BTreeMap, BTreeSet};
20+
21+
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
22+
let tcx = cx.tcx;
23+
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id) else {
24+
// If non-local, no need to check anything.
25+
return;
26+
};
27+
28+
let dox = item.doc_value();
29+
if dox.is_empty() {
30+
return;
31+
}
32+
33+
// P1: unintended strikethrough was fixed by requiring single-tildes to flank
34+
// the same way underscores do, so nothing is done here
35+
36+
// P2: block quotes without following space parsed wrong
37+
//
38+
// This is the set of starting points for block quotes with no space after
39+
// the `>`. It is populated by the new parser, and if the old parser fails to
40+
// clear it out, it'll produce a warning.
41+
let mut spaceless_block_quotes = BTreeSet::new();
42+
43+
// P3: missing footnote references
44+
//
45+
// This is populated by listening for FootnoteReference from
46+
// the new parser and old parser.
47+
let mut missing_footnote_references = BTreeMap::new();
48+
let mut found_footnote_references = BTreeSet::new();
49+
50+
// populate problem cases from new parser
51+
{
52+
pub fn main_body_opts_new() -> cmarkn::Options {
53+
cmarkn::Options::ENABLE_TABLES
54+
| cmarkn::Options::ENABLE_FOOTNOTES
55+
| cmarkn::Options::ENABLE_STRIKETHROUGH
56+
| cmarkn::Options::ENABLE_TASKLISTS
57+
| cmarkn::Options::ENABLE_SMART_PUNCTUATION
58+
}
59+
let mut parser_new = cmarkn::Parser::new_ext(&dox, main_body_opts_new()).into_offset_iter();
60+
while let Some((event, span)) = parser_new.next() {
61+
if let cmarkn::Event::Start(cmarkn::Tag::BlockQuote(_)) = event {
62+
if !dox[span.clone()].starts_with("> ") {
63+
spaceless_block_quotes.insert(span.start);
64+
}
65+
}
66+
if let cmarkn::Event::FootnoteReference(_) = event {
67+
found_footnote_references.insert(span.start + 1);
68+
}
69+
}
70+
}
71+
72+
// remove cases where they don't actually differ
73+
{
74+
pub fn main_body_opts_old() -> cmarko::Options {
75+
cmarko::Options::ENABLE_TABLES
76+
| cmarko::Options::ENABLE_FOOTNOTES
77+
| cmarko::Options::ENABLE_STRIKETHROUGH
78+
| cmarko::Options::ENABLE_TASKLISTS
79+
| cmarko::Options::ENABLE_SMART_PUNCTUATION
80+
}
81+
let mut parser_old = cmarko::Parser::new_ext(&dox, main_body_opts_old()).into_offset_iter();
82+
while let Some((event, span)) = parser_old.next() {
83+
if let cmarko::Event::Start(cmarko::Tag::BlockQuote) = event {
84+
if !dox[span.clone()].starts_with("> ") {
85+
spaceless_block_quotes.remove(&span.start);
86+
}
87+
}
88+
if let cmarko::Event::FootnoteReference(_) = event {
89+
if !found_footnote_references.contains(&(span.start + 1)) {
90+
missing_footnote_references.insert(span.start + 1, span);
91+
}
92+
}
93+
}
94+
}
95+
96+
for start in spaceless_block_quotes {
97+
let (span, precise) =
98+
source_span_for_markdown_range(tcx, &dox, &(start..start + 1), &item.attrs.doc_strings)
99+
.map(|span| (span, true))
100+
.unwrap_or_else(|| (item.attr_span(tcx), false));
101+
102+
tcx.node_span_lint(crate::lint::UNPORTABLE_MARKDOWN, hir_id, span, |lint| {
103+
lint.primary_message("unportable markdown");
104+
lint.help(format!("confusing block quote with no space after the `>` marker"));
105+
if precise {
106+
lint.span_suggestion(
107+
span.shrink_to_hi(),
108+
"if the quote is intended, add a space",
109+
" ",
110+
Applicability::MaybeIncorrect,
111+
);
112+
lint.span_suggestion(
113+
span.shrink_to_lo(),
114+
"if it should not be a quote, escape it",
115+
"\\",
116+
Applicability::MaybeIncorrect,
117+
);
118+
}
119+
});
120+
}
121+
for (_caret, span) in missing_footnote_references {
122+
let (ref_span, precise) =
123+
source_span_for_markdown_range(tcx, &dox, &span, &item.attrs.doc_strings)
124+
.map(|span| (span, true))
125+
.unwrap_or_else(|| (item.attr_span(tcx), false));
126+
127+
tcx.node_span_lint(crate::lint::UNPORTABLE_MARKDOWN, hir_id, ref_span, |lint| {
128+
lint.primary_message("unportable markdown");
129+
if precise {
130+
lint.span_suggestion(
131+
ref_span.shrink_to_lo(),
132+
"if it should not be a footnote, escape it",
133+
"\\",
134+
Applicability::MaybeIncorrect,
135+
);
136+
}
137+
if dox.as_bytes().get(span.end) == Some(&b'[') {
138+
lint.help("confusing footnote reference and link");
139+
if precise {
140+
lint.span_suggestion(
141+
ref_span.shrink_to_hi(),
142+
"if the footnote is intended, add a space",
143+
" ",
144+
Applicability::MaybeIncorrect,
145+
);
146+
} else {
147+
lint.help("there should be a space between the link and the footnote");
148+
}
149+
}
150+
});
151+
}
152+
}
+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// https://internals.rust-lang.org/t/proposal-migrate-the-syntax-of-rustdoc-markdown-footnotes-to-be-compatible-with-the-syntax-used-in-github/18929
2+
//
3+
// A series of test cases for CommonMark corner cases that pulldown-cmark 0.11 fixes.
4+
//
5+
// This version of the lint is targeted at two especially-common cases where docs got broken.
6+
// Other differences in parsing should not warn.
7+
#![allow(rustdoc::broken_intra_doc_links)]
8+
#![deny(rustdoc::unportable_markdown)]
9+
10+
/// <https://github.com/pulldown-cmark/pulldown-cmark/pull/654>
11+
///
12+
/// Test footnote [^foot].
13+
///
14+
/// [^foot]: This is nested within the footnote now, but didn't used to be.
15+
///
16+
/// This is a multi-paragraph footnote.
17+
pub struct GfmFootnotes;
18+
19+
/// <https://github.com/pulldown-cmark/pulldown-cmark/pull/773>
20+
///
21+
/// test [^foo][^bar]
22+
//~^ ERROR unportable markdown
23+
///
24+
/// [^foo]: test
25+
/// [^bar]: test2
26+
pub struct FootnoteSmashedName;
27+
28+
/// <https://github.com/pulldown-cmark/pulldown-cmark/pull/829>
29+
///
30+
/// - _t
31+
/// # test
32+
/// t_
33+
pub struct NestingCornerCase;
34+
35+
/// <https://github.com/pulldown-cmark/pulldown-cmark/pull/650>
36+
///
37+
/// *~~__emphasis strike strong__~~* ~~*__strike emphasis strong__*~~
38+
pub struct Emphasis1;
39+
40+
/// <https://github.com/pulldown-cmark/pulldown-cmark/pull/732>
41+
///
42+
/// |
43+
/// |
44+
pub struct NotEnoughTable;
45+
46+
/// <https://github.com/pulldown-cmark/pulldown-cmark/pull/675>
47+
///
48+
/// foo
49+
/// >bar
50+
//~^ ERROR unportable markdown
51+
pub struct BlockQuoteNoSpace;
52+
53+
/// Negative test.
54+
///
55+
/// foo
56+
/// > bar
57+
pub struct BlockQuoteSpace;
58+
59+
/// Negative test.
60+
///
61+
/// >bar
62+
/// baz
63+
pub struct BlockQuoteNoSpaceStart;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error: unportable markdown
2+
--> $DIR/unportable-markdown.rs:21:10
3+
|
4+
LL | /// test [^foo][^bar]
5+
| ^^^^^^
6+
|
7+
= help: confusing footnote reference and link
8+
note: the lint level is defined here
9+
--> $DIR/unportable-markdown.rs:8:9
10+
|
11+
LL | #![deny(rustdoc::unportable_markdown)]
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13+
help: if it should not be a footnote, escape it
14+
|
15+
LL | /// test \[^foo][^bar]
16+
| +
17+
help: if the footnote is intended, add a space
18+
|
19+
LL | /// test [^foo] [^bar]
20+
| +
21+
22+
error: unportable markdown
23+
--> $DIR/unportable-markdown.rs:49:5
24+
|
25+
LL | /// >bar
26+
| ^
27+
|
28+
= help: confusing block quote with no space after the `>` marker
29+
help: if the quote is intended, add a space
30+
|
31+
LL | /// > bar
32+
| +
33+
help: if it should not be a quote, escape it
34+
|
35+
LL | /// \>bar
36+
| +
37+
38+
error: aborting due to 2 previous errors
39+

0 commit comments

Comments
 (0)