Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a9fb00b

Browse files
borsbors
committedSep 11, 2024
Auto merge of #129975 - notriddle:notriddle/lint-skip, r=GuillaumeGomez
rustdoc: unify the short-circuit on all lints This is a bit of an experiment to see if it improves perf.
2 parents 6f7229c + d4b246b commit a9fb00b

File tree

7 files changed

+184
-202
lines changed

7 files changed

+184
-202
lines changed
 

‎src/librustdoc/passes/lint.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,33 @@ pub(crate) fn run_lints(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
2727

2828
impl<'a, 'tcx> DocVisitor for Linter<'a, 'tcx> {
2929
fn visit_item(&mut self, item: &Item) {
30-
bare_urls::visit_item(self.cx, item);
31-
check_code_block_syntax::visit_item(self.cx, item);
32-
html_tags::visit_item(self.cx, item);
33-
unescaped_backticks::visit_item(self.cx, item);
34-
redundant_explicit_links::visit_item(self.cx, item);
35-
unportable_markdown::visit_item(self.cx, item);
30+
let Some(hir_id) = DocContext::as_local_hir_id(self.cx.tcx, item.item_id) else {
31+
// If non-local, no need to check anything.
32+
return;
33+
};
34+
let dox = item.doc_value();
35+
if !dox.is_empty() {
36+
let may_have_link = dox.contains(&[':', '['][..]);
37+
let may_have_block_comment_or_html = dox.contains(&['<', '>']);
38+
// ~~~rust
39+
// // This is a real, supported commonmark syntax for block code
40+
// ~~~
41+
let may_have_code = dox.contains(&['~', '`', '\t'][..]) || dox.contains(" ");
42+
if may_have_link {
43+
bare_urls::visit_item(self.cx, item, hir_id, &dox);
44+
redundant_explicit_links::visit_item(self.cx, item, hir_id);
45+
}
46+
if may_have_code {
47+
check_code_block_syntax::visit_item(self.cx, item, &dox);
48+
unescaped_backticks::visit_item(self.cx, item, hir_id, &dox);
49+
}
50+
if may_have_block_comment_or_html {
51+
html_tags::visit_item(self.cx, item, hir_id, &dox);
52+
unportable_markdown::visit_item(self.cx, item, hir_id, &dox);
53+
} else if may_have_link {
54+
unportable_markdown::visit_item(self.cx, item, hir_id, &dox);
55+
}
56+
}
3657

3758
self.visit_item_recur(item)
3859
}

‎src/librustdoc/passes/lint/bare_urls.rs

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,57 +8,51 @@ use std::sync::LazyLock;
88
use pulldown_cmark::{Event, Parser, Tag};
99
use regex::Regex;
1010
use rustc_errors::Applicability;
11+
use rustc_hir::HirId;
1112
use rustc_resolve::rustdoc::source_span_for_markdown_range;
1213
use tracing::trace;
1314

1415
use crate::clean::*;
1516
use crate::core::DocContext;
1617
use crate::html::markdown::main_body_opts;
1718

18-
pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) {
19-
let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) else {
20-
// If non-local, no need to check anything.
21-
return;
19+
pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
20+
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
21+
let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
22+
.unwrap_or_else(|| item.attr_span(cx.tcx));
23+
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
24+
lint.primary_message(msg)
25+
.note("bare URLs are not automatically turned into clickable links")
26+
.multipart_suggestion(
27+
"use an automatic link instead",
28+
vec![
29+
(sp.shrink_to_lo(), "<".to_string()),
30+
(sp.shrink_to_hi(), ">".to_string()),
31+
],
32+
Applicability::MachineApplicable,
33+
);
34+
});
2235
};
23-
let dox = item.doc_value();
24-
if !dox.is_empty() {
25-
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
26-
let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
27-
.unwrap_or_else(|| item.attr_span(cx.tcx));
28-
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
29-
lint.primary_message(msg)
30-
.note("bare URLs are not automatically turned into clickable links")
31-
.multipart_suggestion(
32-
"use an automatic link instead",
33-
vec![
34-
(sp.shrink_to_lo(), "<".to_string()),
35-
(sp.shrink_to_hi(), ">".to_string()),
36-
],
37-
Applicability::MachineApplicable,
38-
);
39-
});
40-
};
4136

42-
let mut p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter();
37+
let mut p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter();
4338

44-
while let Some((event, range)) = p.next() {
45-
match event {
46-
Event::Text(s) => find_raw_urls(cx, &s, range, &report_diag),
47-
// We don't want to check the text inside code blocks or links.
48-
Event::Start(tag @ (Tag::CodeBlock(_) | Tag::Link { .. })) => {
49-
while let Some((event, _)) = p.next() {
50-
match event {
51-
Event::End(end)
52-
if mem::discriminant(&end) == mem::discriminant(&tag.to_end()) =>
53-
{
54-
break;
55-
}
56-
_ => {}
39+
while let Some((event, range)) = p.next() {
40+
match event {
41+
Event::Text(s) => find_raw_urls(cx, &s, range, &report_diag),
42+
// We don't want to check the text inside code blocks or links.
43+
Event::Start(tag @ (Tag::CodeBlock(_) | Tag::Link { .. })) => {
44+
while let Some((event, _)) = p.next() {
45+
match event {
46+
Event::End(end)
47+
if mem::discriminant(&end) == mem::discriminant(&tag.to_end()) =>
48+
{
49+
break;
5750
}
51+
_ => {}
5852
}
5953
}
60-
_ => {}
6154
}
55+
_ => {}
6256
}
6357
}
6458
}

‎src/librustdoc/passes/lint/check_code_block_syntax.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,8 @@ use crate::clean;
1515
use crate::core::DocContext;
1616
use crate::html::markdown::{self, RustCodeBlock};
1717

18-
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item) {
19-
if let Some(def_id) = item.item_id.as_local_def_id()
20-
&& let Some(dox) = &item.opt_doc_value()
21-
{
18+
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item, dox: &str) {
19+
if let Some(def_id) = item.item_id.as_local_def_id() {
2220
let sp = item.attr_span(cx.tcx);
2321
let extra = crate::html::markdown::ExtraInfo::new(cx.tcx, def_id, sp);
2422
for code_block in markdown::rust_code_blocks(dox, &extra) {

‎src/librustdoc/passes/lint/html_tags.rs

Lines changed: 119 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -5,159 +5,149 @@ use std::ops::Range;
55
use std::str::CharIndices;
66

77
use pulldown_cmark::{BrokenLink, Event, LinkType, Parser, Tag, TagEnd};
8+
use rustc_hir::HirId;
89
use rustc_resolve::rustdoc::source_span_for_markdown_range;
910

1011
use crate::clean::*;
1112
use crate::core::DocContext;
1213
use crate::html::markdown::main_body_opts;
1314

14-
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
15+
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
1516
let tcx = cx.tcx;
16-
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id)
17-
// If non-local, no need to check anything.
18-
else {
19-
return;
20-
};
21-
let dox = item.doc_value();
22-
if !dox.is_empty() {
23-
let report_diag = |msg: String, range: &Range<usize>, is_open_tag: bool| {
24-
let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs.doc_strings)
25-
{
26-
Some(sp) => sp,
27-
None => item.attr_span(tcx),
28-
};
29-
tcx.node_span_lint(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| {
30-
use rustc_lint_defs::Applicability;
17+
let report_diag = |msg: String, range: &Range<usize>, is_open_tag: bool| {
18+
let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs.doc_strings) {
19+
Some(sp) => sp,
20+
None => item.attr_span(tcx),
21+
};
22+
tcx.node_span_lint(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| {
23+
use rustc_lint_defs::Applicability;
3124

32-
lint.primary_message(msg);
25+
lint.primary_message(msg);
3326

34-
// If a tag looks like `<this>`, it might actually be a generic.
35-
// We don't try to detect stuff `<like, this>` because that's not valid HTML,
36-
// and we don't try to detect stuff `<like this>` because that's not valid Rust.
37-
let mut generics_end = range.end;
38-
if let Some(Some(mut generics_start)) = (is_open_tag
39-
&& dox[..generics_end].ends_with('>'))
40-
.then(|| extract_path_backwards(&dox, range.start))
27+
// If a tag looks like `<this>`, it might actually be a generic.
28+
// We don't try to detect stuff `<like, this>` because that's not valid HTML,
29+
// and we don't try to detect stuff `<like this>` because that's not valid Rust.
30+
let mut generics_end = range.end;
31+
if let Some(Some(mut generics_start)) = (is_open_tag
32+
&& dox[..generics_end].ends_with('>'))
33+
.then(|| extract_path_backwards(&dox, range.start))
34+
{
35+
while generics_start != 0
36+
&& generics_end < dox.len()
37+
&& dox.as_bytes()[generics_start - 1] == b'<'
38+
&& dox.as_bytes()[generics_end] == b'>'
4139
{
42-
while generics_start != 0
43-
&& generics_end < dox.len()
44-
&& dox.as_bytes()[generics_start - 1] == b'<'
45-
&& dox.as_bytes()[generics_end] == b'>'
46-
{
47-
generics_end += 1;
48-
generics_start -= 1;
49-
if let Some(new_start) = extract_path_backwards(&dox, generics_start) {
50-
generics_start = new_start;
51-
}
52-
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
53-
generics_end = new_end;
54-
}
40+
generics_end += 1;
41+
generics_start -= 1;
42+
if let Some(new_start) = extract_path_backwards(&dox, generics_start) {
43+
generics_start = new_start;
5544
}
5645
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
5746
generics_end = new_end;
5847
}
59-
let generics_sp = match source_span_for_markdown_range(
60-
tcx,
61-
&dox,
62-
&(generics_start..generics_end),
63-
&item.attrs.doc_strings,
64-
) {
65-
Some(sp) => sp,
66-
None => item.attr_span(tcx),
67-
};
68-
// Sometimes, we only extract part of a path. For example, consider this:
69-
//
70-
// <[u32] as IntoIter<u32>>::Item
71-
// ^^^^^ unclosed HTML tag `u32`
72-
//
73-
// We don't have any code for parsing fully-qualified trait paths.
74-
// In theory, we could add it, but doing it correctly would require
75-
// parsing the entire path grammar, which is problematic because of
76-
// overlap between the path grammar and Markdown.
77-
//
78-
// The example above shows that ambiguity. Is `[u32]` intended to be an
79-
// intra-doc link to the u32 primitive, or is it intended to be a slice?
80-
//
81-
// If the below conditional were removed, we would suggest this, which is
82-
// not what the user probably wants.
83-
//
84-
// <[u32] as `IntoIter<u32>`>::Item
85-
//
86-
// We know that the user actually wants to wrap the whole thing in a code
87-
// block, but the only reason we know that is because `u32` does not, in
88-
// fact, implement IntoIter. If the example looks like this:
89-
//
90-
// <[Vec<i32>] as IntoIter<i32>::Item
91-
//
92-
// The ideal fix would be significantly different.
93-
if (generics_start > 0 && dox.as_bytes()[generics_start - 1] == b'<')
94-
|| (generics_end < dox.len() && dox.as_bytes()[generics_end] == b'>')
95-
{
96-
return;
97-
}
98-
// multipart form is chosen here because ``Vec<i32>`` would be confusing.
99-
lint.multipart_suggestion(
100-
"try marking as source code",
101-
vec![
102-
(generics_sp.shrink_to_lo(), String::from("`")),
103-
(generics_sp.shrink_to_hi(), String::from("`")),
104-
],
105-
Applicability::MaybeIncorrect,
106-
);
10748
}
108-
});
109-
};
49+
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
50+
generics_end = new_end;
51+
}
52+
let generics_sp = match source_span_for_markdown_range(
53+
tcx,
54+
&dox,
55+
&(generics_start..generics_end),
56+
&item.attrs.doc_strings,
57+
) {
58+
Some(sp) => sp,
59+
None => item.attr_span(tcx),
60+
};
61+
// Sometimes, we only extract part of a path. For example, consider this:
62+
//
63+
// <[u32] as IntoIter<u32>>::Item
64+
// ^^^^^ unclosed HTML tag `u32`
65+
//
66+
// We don't have any code for parsing fully-qualified trait paths.
67+
// In theory, we could add it, but doing it correctly would require
68+
// parsing the entire path grammar, which is problematic because of
69+
// overlap between the path grammar and Markdown.
70+
//
71+
// The example above shows that ambiguity. Is `[u32]` intended to be an
72+
// intra-doc link to the u32 primitive, or is it intended to be a slice?
73+
//
74+
// If the below conditional were removed, we would suggest this, which is
75+
// not what the user probably wants.
76+
//
77+
// <[u32] as `IntoIter<u32>`>::Item
78+
//
79+
// We know that the user actually wants to wrap the whole thing in a code
80+
// block, but the only reason we know that is because `u32` does not, in
81+
// fact, implement IntoIter. If the example looks like this:
82+
//
83+
// <[Vec<i32>] as IntoIter<i32>::Item
84+
//
85+
// The ideal fix would be significantly different.
86+
if (generics_start > 0 && dox.as_bytes()[generics_start - 1] == b'<')
87+
|| (generics_end < dox.len() && dox.as_bytes()[generics_end] == b'>')
88+
{
89+
return;
90+
}
91+
// multipart form is chosen here because ``Vec<i32>`` would be confusing.
92+
lint.multipart_suggestion(
93+
"try marking as source code",
94+
vec![
95+
(generics_sp.shrink_to_lo(), String::from("`")),
96+
(generics_sp.shrink_to_hi(), String::from("`")),
97+
],
98+
Applicability::MaybeIncorrect,
99+
);
100+
}
101+
});
102+
};
110103

111-
let mut tags = Vec::new();
112-
let mut is_in_comment = None;
113-
let mut in_code_block = false;
104+
let mut tags = Vec::new();
105+
let mut is_in_comment = None;
106+
let mut in_code_block = false;
114107

115-
let link_names = item.link_names(&cx.cache);
108+
let link_names = item.link_names(&cx.cache);
116109

117-
let mut replacer = |broken_link: BrokenLink<'_>| {
118-
if let Some(link) =
119-
link_names.iter().find(|link| *link.original_text == *broken_link.reference)
120-
{
121-
Some((link.href.as_str().into(), link.new_text.to_string().into()))
122-
} else if matches!(
123-
&broken_link.link_type,
124-
LinkType::Reference | LinkType::ReferenceUnknown
125-
) {
126-
// If the link is shaped [like][this], suppress any broken HTML in the [this] part.
127-
// The `broken_intra_doc_links` will report typos in there anyway.
128-
Some((
129-
broken_link.reference.to_string().into(),
130-
broken_link.reference.to_string().into(),
131-
))
132-
} else {
133-
None
134-
}
135-
};
110+
let mut replacer = |broken_link: BrokenLink<'_>| {
111+
if let Some(link) =
112+
link_names.iter().find(|link| *link.original_text == *broken_link.reference)
113+
{
114+
Some((link.href.as_str().into(), link.new_text.to_string().into()))
115+
} else if matches!(&broken_link.link_type, LinkType::Reference | LinkType::ReferenceUnknown)
116+
{
117+
// If the link is shaped [like][this], suppress any broken HTML in the [this] part.
118+
// The `broken_intra_doc_links` will report typos in there anyway.
119+
Some((
120+
broken_link.reference.to_string().into(),
121+
broken_link.reference.to_string().into(),
122+
))
123+
} else {
124+
None
125+
}
126+
};
136127

137-
let p = Parser::new_with_broken_link_callback(&dox, main_body_opts(), Some(&mut replacer))
138-
.into_offset_iter();
128+
let p = Parser::new_with_broken_link_callback(&dox, main_body_opts(), Some(&mut replacer))
129+
.into_offset_iter();
139130

140-
for (event, range) in p {
141-
match event {
142-
Event::Start(Tag::CodeBlock(_)) => in_code_block = true,
143-
Event::Html(text) | Event::InlineHtml(text) if !in_code_block => {
144-
extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
145-
}
146-
Event::End(TagEnd::CodeBlock) => in_code_block = false,
147-
_ => {}
131+
for (event, range) in p {
132+
match event {
133+
Event::Start(Tag::CodeBlock(_)) => in_code_block = true,
134+
Event::Html(text) | Event::InlineHtml(text) if !in_code_block => {
135+
extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
148136
}
137+
Event::End(TagEnd::CodeBlock) => in_code_block = false,
138+
_ => {}
149139
}
140+
}
150141

151-
for (tag, range) in tags.iter().filter(|(t, _)| {
152-
let t = t.to_lowercase();
153-
!ALLOWED_UNCLOSED.contains(&t.as_str())
154-
}) {
155-
report_diag(format!("unclosed HTML tag `{tag}`"), range, true);
156-
}
142+
for (tag, range) in tags.iter().filter(|(t, _)| {
143+
let t = t.to_lowercase();
144+
!ALLOWED_UNCLOSED.contains(&t.as_str())
145+
}) {
146+
report_diag(format!("unclosed HTML tag `{tag}`"), range, true);
147+
}
157148

158-
if let Some(range) = is_in_comment {
159-
report_diag("Unclosed HTML comment".to_string(), &range, false);
160-
}
149+
if let Some(range) = is_in_comment {
150+
report_diag("Unclosed HTML comment".to_string(), &range, false);
161151
}
162152
}
163153

‎src/librustdoc/passes/lint/redundant_explicit_links.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,7 @@ struct LinkData {
2424
display_link: String,
2525
}
2626

27-
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
28-
let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) else {
29-
// If non-local, no need to check anything.
30-
return;
31-
};
32-
27+
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId) {
3328
let hunks = prepare_to_doc_link_resolution(&item.attrs.doc_strings);
3429
for (item_id, doc) in hunks {
3530
if let Some(item_id) = item_id.or(item.def_id())

‎src/librustdoc/passes/lint/unescaped_backticks.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,16 @@ use std::ops::Range;
44

55
use pulldown_cmark::{BrokenLink, Event, Parser};
66
use rustc_errors::Diag;
7+
use rustc_hir::HirId;
78
use rustc_lint_defs::Applicability;
89
use rustc_resolve::rustdoc::source_span_for_markdown_range;
910

1011
use crate::clean::Item;
1112
use crate::core::DocContext;
1213
use crate::html::markdown::main_body_opts;
1314

14-
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
15+
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
1516
let tcx = cx.tcx;
16-
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id) else {
17-
// If non-local, no need to check anything.
18-
return;
19-
};
20-
21-
let dox = item.doc_value();
22-
if dox.is_empty() {
23-
return;
24-
}
2517

2618
let link_names = item.link_names(&cx.cache);
2719
let mut replacer = |broken_link: BrokenLink<'_>| {

‎src/librustdoc/passes/lint/unportable_markdown.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,16 @@
1212
1313
use std::collections::{BTreeMap, BTreeSet};
1414

15+
use rustc_hir::HirId;
1516
use rustc_lint_defs::Applicability;
1617
use rustc_resolve::rustdoc::source_span_for_markdown_range;
1718
use {pulldown_cmark as cmarkn, pulldown_cmark_old as cmarko};
1819

1920
use crate::clean::Item;
2021
use crate::core::DocContext;
2122

22-
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
23+
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
2324
let tcx = cx.tcx;
24-
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id) else {
25-
// If non-local, no need to check anything.
26-
return;
27-
};
28-
29-
let dox = item.doc_value();
30-
if dox.is_empty() {
31-
return;
32-
}
3325

3426
// P1: unintended strikethrough was fixed by requiring single-tildes to flank
3527
// the same way underscores do, so nothing is done here

0 commit comments

Comments
 (0)
Please sign in to comment.