mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #129975 - notriddle:notriddle/lint-skip, r=GuillaumeGomez

Browse Source 
rustdoc: unify the short-circuit on all lints

This is a bit of an experiment to see if it improves perf.
This commit is contained in:
bors 2024-09-11 01:17:54 +00:00
commit a9fb00bfa4
7 changed files with 189 additions and 207 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/librustdoc/passes/lint.rs
View File

@ -27,12 +27,33 @@ pub(crate) fn run_lints(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
impl<'a, 'tcx> DocVisitor for Linter<'a, 'tcx> {
fn visit_item(&mut self, item: &Item) {
bare_urls::visit_item(self.cx, item);
check_code_block_syntax::visit_item(self.cx, item);
html_tags::visit_item(self.cx, item);
unescaped_backticks::visit_item(self.cx, item);
redundant_explicit_links::visit_item(self.cx, item);
unportable_markdown::visit_item(self.cx, item);
let Some(hir_id) = DocContext::as_local_hir_id(self.cx.tcx, item.item_id) else {
// If non-local, no need to check anything.
return;
};
let dox = item.doc_value();
if !dox.is_empty() {
let may_have_link = dox.contains(&[':', '['][..]);
let may_have_block_comment_or_html = dox.contains(&['<', '>']);
// ~~~rust
// // This is a real, supported commonmark syntax for block code
// ~~~
let may_have_code = dox.contains(&['~', '`', '\t'][..]) || dox.contains(" ");
if may_have_link {
bare_urls::visit_item(self.cx, item, hir_id, &dox);
redundant_explicit_links::visit_item(self.cx, item, hir_id);
}
if may_have_code {
check_code_block_syntax::visit_item(self.cx, item, &dox);
unescaped_backticks::visit_item(self.cx, item, hir_id, &dox);
}
if may_have_block_comment_or_html {
html_tags::visit_item(self.cx, item, hir_id, &dox);
unportable_markdown::visit_item(self.cx, item, hir_id, &dox);
} else if may_have_link {
unportable_markdown::visit_item(self.cx, item, hir_id, &dox);
}
}
self.visit_item_recur(item)
}

68
src/librustdoc/passes/lint/bare_urls.rs
View File

@ -8,6 +8,7 @@
use pulldown_cmark::{Event, Parser, Tag};
use regex::Regex;
use rustc_errors::Applicability;
use rustc_hir::HirId;
use rustc_resolve::rustdoc::source_span_for_markdown_range;
use tracing::trace;
@ -15,50 +16,43 @@
use crate::core::DocContext;
use crate::html::markdown::main_body_opts;
pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) {
let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) else {
// If non-local, no need to check anything.
return;
pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
.unwrap_or_else(|| item.attr_span(cx.tcx));
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
lint.primary_message(msg)
.note("bare URLs are not automatically turned into clickable links")
.multipart_suggestion(
"use an automatic link instead",
vec![
(sp.shrink_to_lo(), "<".to_string()),
(sp.shrink_to_hi(), ">".to_string()),
],
Applicability::MachineApplicable,
);
});
};
let dox = item.doc_value();
if !dox.is_empty() {
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs.doc_strings)
.unwrap_or_else(|| item.attr_span(cx.tcx));
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
lint.primary_message(msg)
.note("bare URLs are not automatically turned into clickable links")
.multipart_suggestion(
"use an automatic link instead",
vec![
(sp.shrink_to_lo(), "<".to_string()),
(sp.shrink_to_hi(), ">".to_string()),
],
Applicability::MachineApplicable,
);
});
};
let mut p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter();
let mut p = Parser::new_ext(&dox, main_body_opts()).into_offset_iter();
while let Some((event, range)) = p.next() {
match event {
Event::Text(s) => find_raw_urls(cx, &s, range, &report_diag),
// We don't want to check the text inside code blocks or links.
Event::Start(tag @ (Tag::CodeBlock(_) | Tag::Link { .. })) => {
while let Some((event, _)) = p.next() {
match event {
Event::End(end)
if mem::discriminant(&end) == mem::discriminant(&tag.to_end()) =>
{
break;
}
_ => {}
while let Some((event, range)) = p.next() {
match event {
Event::Text(s) => find_raw_urls(cx, &s, range, &report_diag),
// We don't want to check the text inside code blocks or links.
Event::Start(tag @ (Tag::CodeBlock(_) | Tag::Link { .. })) => {
while let Some((event, _)) = p.next() {
match event {
Event::End(end)
if mem::discriminant(&end) == mem::discriminant(&tag.to_end()) =>
{
break;
}
_ => {}
}
}
_ => {}
}
_ => {}
}
}
}

6
src/librustdoc/passes/lint/check_code_block_syntax.rs
View File

@ -15,10 +15,8 @@
use crate::core::DocContext;
use crate::html::markdown::{self, RustCodeBlock};
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item) {
if let Some(def_id) = item.item_id.as_local_def_id()
&& let Some(dox) = &item.opt_doc_value()
{
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item, dox: &str) {
if let Some(def_id) = item.item_id.as_local_def_id() {
let sp = item.attr_span(cx.tcx);
let extra = crate::html::markdown::ExtraInfo::new(cx.tcx, def_id, sp);
for code_block in markdown::rust_code_blocks(dox, &extra) {

258
src/librustdoc/passes/lint/html_tags.rs
View File

@ -5,159 +5,149 @@
use std::str::CharIndices;
use pulldown_cmark::{BrokenLink, Event, LinkType, Parser, Tag, TagEnd};
use rustc_hir::HirId;
use rustc_resolve::rustdoc::source_span_for_markdown_range;
use crate::clean::*;
use crate::core::DocContext;
use crate::html::markdown::main_body_opts;
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
let tcx = cx.tcx;
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id)
// If non-local, no need to check anything.
else {
return;
};
let dox = item.doc_value();
if !dox.is_empty() {
let report_diag = |msg: String, range: &Range<usize>, is_open_tag: bool| {
let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs.doc_strings)
let report_diag = |msg: String, range: &Range<usize>, is_open_tag: bool| {
let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs.doc_strings) {
Some(sp) => sp,
None => item.attr_span(tcx),
};
tcx.node_span_lint(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| {
use rustc_lint_defs::Applicability;
lint.primary_message(msg);
// If a tag looks like `<this>`, it might actually be a generic.
// We don't try to detect stuff `<like, this>` because that's not valid HTML,
// and we don't try to detect stuff `<like this>` because that's not valid Rust.
let mut generics_end = range.end;
if let Some(Some(mut generics_start)) = (is_open_tag
&& dox[..generics_end].ends_with('>'))
.then(|| extract_path_backwards(&dox, range.start))
{
Some(sp) => sp,
None => item.attr_span(tcx),
};
tcx.node_span_lint(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| {
use rustc_lint_defs::Applicability;
lint.primary_message(msg);
// If a tag looks like `<this>`, it might actually be a generic.
// We don't try to detect stuff `<like, this>` because that's not valid HTML,
// and we don't try to detect stuff `<like this>` because that's not valid Rust.
let mut generics_end = range.end;
if let Some(Some(mut generics_start)) = (is_open_tag
&& dox[..generics_end].ends_with('>'))
.then(|| extract_path_backwards(&dox, range.start))
while generics_start != 0
&& generics_end < dox.len()
&& dox.as_bytes()[generics_start - 1] == b'<'
&& dox.as_bytes()[generics_end] == b'>'
{
while generics_start != 0
&& generics_end < dox.len()
&& dox.as_bytes()[generics_start - 1] == b'<'
&& dox.as_bytes()[generics_end] == b'>'
{
generics_end += 1;
generics_start -= 1;
if let Some(new_start) = extract_path_backwards(&dox, generics_start) {
generics_start = new_start;
}
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
generics_end = new_end;
}
generics_end += 1;
generics_start -= 1;
if let Some(new_start) = extract_path_backwards(&dox, generics_start) {
generics_start = new_start;
}
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
generics_end = new_end;
}
let generics_sp = match source_span_for_markdown_range(
tcx,
&dox,
&(generics_start..generics_end),
&item.attrs.doc_strings,
) {
Some(sp) => sp,
None => item.attr_span(tcx),
};
// Sometimes, we only extract part of a path. For example, consider this:
//
// <[u32] as IntoIter<u32>>::Item
// ^^^^^ unclosed HTML tag `u32`
//
// We don't have any code for parsing fully-qualified trait paths.
// In theory, we could add it, but doing it correctly would require
// parsing the entire path grammar, which is problematic because of
// overlap between the path grammar and Markdown.
//
// The example above shows that ambiguity. Is `[u32]` intended to be an
// intra-doc link to the u32 primitive, or is it intended to be a slice?
//
// If the below conditional were removed, we would suggest this, which is
// not what the user probably wants.
//
// <[u32] as `IntoIter<u32>`>::Item
//
// We know that the user actually wants to wrap the whole thing in a code
// block, but the only reason we know that is because `u32` does not, in
// fact, implement IntoIter. If the example looks like this:
//
// <[Vec<i32>] as IntoIter<i32>::Item
//
// The ideal fix would be significantly different.
if (generics_start > 0 && dox.as_bytes()[generics_start - 1] == b'<')
|| (generics_end < dox.len() && dox.as_bytes()[generics_end] == b'>')
{
return;
}
// multipart form is chosen here because ``Vec<i32>`` would be confusing.
lint.multipart_suggestion(
"try marking as source code",
vec![
(generics_sp.shrink_to_lo(), String::from("`")),
(generics_sp.shrink_to_hi(), String::from("`")),
],
Applicability::MaybeIncorrect,
);
}
});
};
let mut tags = Vec::new();
let mut is_in_comment = None;
let mut in_code_block = false;
let link_names = item.link_names(&cx.cache);
let mut replacer = |broken_link: BrokenLink<'_>| {
if let Some(link) =
link_names.iter().find(|link| *link.original_text == *broken_link.reference)
{
Some((link.href.as_str().into(), link.new_text.to_string().into()))
} else if matches!(
&broken_link.link_type,
LinkType::Reference | LinkType::ReferenceUnknown
) {
// If the link is shaped [like][this], suppress any broken HTML in the [this] part.
// The `broken_intra_doc_links` will report typos in there anyway.
Some((
broken_link.reference.to_string().into(),
broken_link.reference.to_string().into(),
))
} else {
None
}
};
let p = Parser::new_with_broken_link_callback(&dox, main_body_opts(), Some(&mut replacer))
.into_offset_iter();
for (event, range) in p {
match event {
Event::Start(Tag::CodeBlock(_)) => in_code_block = true,
Event::Html(text) | Event::InlineHtml(text) if !in_code_block => {
extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
generics_end = new_end;
}
Event::End(TagEnd::CodeBlock) => in_code_block = false,
_ => {}
let generics_sp = match source_span_for_markdown_range(
tcx,
&dox,
&(generics_start..generics_end),
&item.attrs.doc_strings,
) {
Some(sp) => sp,
None => item.attr_span(tcx),
};
// Sometimes, we only extract part of a path. For example, consider this:
//
// <[u32] as IntoIter<u32>>::Item
// ^^^^^ unclosed HTML tag `u32`
//
// We don't have any code for parsing fully-qualified trait paths.
// In theory, we could add it, but doing it correctly would require
// parsing the entire path grammar, which is problematic because of
// overlap between the path grammar and Markdown.
//
// The example above shows that ambiguity. Is `[u32]` intended to be an
// intra-doc link to the u32 primitive, or is it intended to be a slice?
//
// If the below conditional were removed, we would suggest this, which is
// not what the user probably wants.
//
// <[u32] as `IntoIter<u32>`>::Item
//
// We know that the user actually wants to wrap the whole thing in a code
// block, but the only reason we know that is because `u32` does not, in
// fact, implement IntoIter. If the example looks like this:
//
// <[Vec<i32>] as IntoIter<i32>::Item
//
// The ideal fix would be significantly different.
if (generics_start > 0 && dox.as_bytes()[generics_start - 1] == b'<')
|| (generics_end < dox.len() && dox.as_bytes()[generics_end] == b'>')
{
return;
}
// multipart form is chosen here because ``Vec<i32>`` would be confusing.
lint.multipart_suggestion(
"try marking as source code",
vec![
(generics_sp.shrink_to_lo(), String::from("`")),
(generics_sp.shrink_to_hi(), String::from("`")),
],
Applicability::MaybeIncorrect,
);
}
}
});
};
for (tag, range) in tags.iter().filter(|(t, _)| {
let t = t.to_lowercase();
!ALLOWED_UNCLOSED.contains(&t.as_str())
}) {
report_diag(format!("unclosed HTML tag `{tag}`"), range, true);
}
let mut tags = Vec::new();
let mut is_in_comment = None;
let mut in_code_block = false;
if let Some(range) = is_in_comment {
report_diag("Unclosed HTML comment".to_string(), &range, false);
let link_names = item.link_names(&cx.cache);
let mut replacer = |broken_link: BrokenLink<'_>| {
if let Some(link) =
link_names.iter().find(|link| *link.original_text == *broken_link.reference)
{
Some((link.href.as_str().into(), link.new_text.to_string().into()))
} else if matches!(&broken_link.link_type, LinkType::Reference | LinkType::ReferenceUnknown)
{
// If the link is shaped [like][this], suppress any broken HTML in the [this] part.
// The `broken_intra_doc_links` will report typos in there anyway.
Some((
broken_link.reference.to_string().into(),
broken_link.reference.to_string().into(),
))
} else {
None
}
};
let p = Parser::new_with_broken_link_callback(&dox, main_body_opts(), Some(&mut replacer))
.into_offset_iter();
for (event, range) in p {
match event {
Event::Start(Tag::CodeBlock(_)) => in_code_block = true,
Event::Html(text) | Event::InlineHtml(text) if !in_code_block => {
extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
}
Event::End(TagEnd::CodeBlock) => in_code_block = false,
_ => {}
}
}
for (tag, range) in tags.iter().filter(|(t, _)| {
let t = t.to_lowercase();
!ALLOWED_UNCLOSED.contains(&t.as_str())
}) {
report_diag(format!("unclosed HTML tag `{tag}`"), range, true);
}
if let Some(range) = is_in_comment {
report_diag("Unclosed HTML comment".to_string(), &range, false);
}
}

7
src/librustdoc/passes/lint/redundant_explicit_links.rs
View File

@ -24,12 +24,7 @@ struct LinkData {
display_link: String,
}
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) else {
// If non-local, no need to check anything.
return;
};
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId) {
let hunks = prepare_to_doc_link_resolution(&item.attrs.doc_strings);
for (item_id, doc) in hunks {
if let Some(item_id) = item_id.or(item.def_id())

12
src/librustdoc/passes/lint/unescaped_backticks.rs
View File

@ -4,6 +4,7 @@
use pulldown_cmark::{BrokenLink, Event, Parser};
use rustc_errors::Diag;
use rustc_hir::HirId;
use rustc_lint_defs::Applicability;
use rustc_resolve::rustdoc::source_span_for_markdown_range;
@ -11,17 +12,8 @@
use crate::core::DocContext;
use crate::html::markdown::main_body_opts;
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
let tcx = cx.tcx;
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id) else {
// If non-local, no need to check anything.
return;
};
let dox = item.doc_value();
if dox.is_empty() {
return;
}
let link_names = item.link_names(&cx.cache);
let mut replacer = |broken_link: BrokenLink<'_>| {

12
src/librustdoc/passes/lint/unportable_markdown.rs
View File

@ -12,6 +12,7 @@
use std::collections::{BTreeMap, BTreeSet};
use rustc_hir::HirId;
use rustc_lint_defs::Applicability;
use rustc_resolve::rustdoc::source_span_for_markdown_range;
use {pulldown_cmark as cmarkn, pulldown_cmark_old as cmarko};
@ -19,17 +20,8 @@
use crate::clean::Item;
use crate::core::DocContext;
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
let tcx = cx.tcx;
let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id) else {
// If non-local, no need to check anything.
return;
};
let dox = item.doc_value();
if dox.is_empty() {
return;
}
// P1: unintended strikethrough was fixed by requiring single-tildes to flank
// the same way underscores do, so nothing is done here