bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Consolidate rustdoc's lint passes into a single pass

Browse source 
This should improve performance and simplify the code.
This commit is contained in:
Noah Lev 2022-11-22 19:42:49 -08:00
parent ff8c8dfbe6
commit 9eb85777fc
8 changed files with 441 additions and 505 deletions
Download patch file Download diff file Expand all files Collapse all files

110
src/librustdoc/passes/bare_urls.rs
View file

@ -1,110 +0,0 @@
//! Detects links that are not linkified, e.g., in Markdown such as `Go to https://example.com/.`
//! Suggests wrapping the link with angle brackets: `Go to <https://example.com/>.` to linkify it.
use super::Pass;
use crate::clean::*;
use crate::core::DocContext;
use crate::html::markdown::main_body_opts;
use crate::visit::DocVisitor;
use core::ops::Range;
use pulldown_cmark::{Event, Parser, Tag};
use regex::Regex;
use rustc_errors::Applicability;
use std::mem;
use std::sync::LazyLock;
pub(crate) const CHECK_BARE_URLS: Pass = Pass {
name: "check-bare-urls",
run: check_bare_urls,
description: "detects URLs that are not hyperlinks",
};
static URL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(concat!(
r"https?://", // url scheme
r"([-a-zA-Z0-9@:%._\+~#=]{2,256}\.)+", // one or more subdomains
r"[a-zA-Z]{2,63}", // root domain
r"\b([-a-zA-Z0-9@:%_\+.~#?&/=]*)" // optional query or url fragments
))
.expect("failed to build regex")
});
struct BareUrlsLinter<'a, 'tcx> {
cx: &'a mut DocContext<'tcx>,
}
impl<'a, 'tcx> BareUrlsLinter<'a, 'tcx> {
fn find_raw_urls(
&self,
text: &str,
range: Range<usize>,
f: &impl Fn(&DocContext<'_>, &str, &str, Range<usize>),
) {
trace!("looking for raw urls in {}", text);
// For now, we only check "full" URLs (meaning, starting with "http://" or "https://").
for match_ in URL_REGEX.find_iter(text) {
let url = match_.as_str();
let url_range = match_.range();
f(
self.cx,
"this URL is not a hyperlink",
url,
Range { start: range.start + url_range.start, end: range.start + url_range.end },
);
}
}
}
pub(crate) fn check_bare_urls(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
BareUrlsLinter { cx }.visit_crate(&krate);
krate
}
impl<'a, 'tcx> DocVisitor for BareUrlsLinter<'a, 'tcx> {
fn visit_item(&mut self, item: &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.attrs.collapsed_doc_value().unwrap_or_default();
if !dox.is_empty() {
let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range<usize>| {
let sp = super::source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)
.unwrap_or_else(|| item.attr_span(cx.tcx));
cx.tcx.struct_span_lint_hir(crate::lint::BARE_URLS, hir_id, sp, msg, |lint| {
lint.note("bare URLs are not automatically turned into clickable links")
.span_suggestion(
sp,
"use an automatic link instead",
format!("<{}>", url),
Applicability::MachineApplicable,
)
});
};
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) => self.find_raw_urls(&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) =>
{
break;
}
_ => {}
}
}
}
_ => {}
}
}
}
self.visit_item_recur(item)
}
}

209
src/librustdoc/passes/check_code_block_syntax.rs
View file

@ -1,209 +0,0 @@
//! Validates syntax inside Rust code blocks (\`\`\`rust).
use rustc_data_structures::sync::{Lock, Lrc};
use rustc_errors::{
emitter::Emitter,
translation::{to_fluent_args, Translate},
Applicability, Diagnostic, Handler, LazyFallbackBundle,
};
use rustc_parse::parse_stream_from_source_str;
use rustc_session::parse::ParseSess;
use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId};
use rustc_span::source_map::{FilePathMapping, SourceMap};
use rustc_span::{FileName, InnerSpan, DUMMY_SP};
use crate::clean;
use crate::core::DocContext;
use crate::html::markdown::{self, RustCodeBlock};
use crate::passes::Pass;
use crate::visit::DocVisitor;
pub(crate) const CHECK_CODE_BLOCK_SYNTAX: Pass = Pass {
name: "check-code-block-syntax",
run: check_code_block_syntax,
description: "validates syntax inside Rust code blocks",
};
pub(crate) fn check_code_block_syntax(
krate: clean::Crate,
cx: &mut DocContext<'_>,
) -> clean::Crate {
SyntaxChecker { cx }.visit_crate(&krate);
krate
}
struct SyntaxChecker<'a, 'tcx> {
cx: &'a DocContext<'tcx>,
}
impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> {
fn check_rust_syntax(&self, item: &clean::Item, dox: &str, code_block: RustCodeBlock) {
let buffer = Lrc::new(Lock::new(Buffer::default()));
let fallback_bundle =
rustc_errors::fallback_fluent_bundle(rustc_errors::DEFAULT_LOCALE_RESOURCES, false);
let emitter = BufferEmitter { buffer: Lrc::clone(&buffer), fallback_bundle };
let sm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
let handler = Handler::with_emitter(false, None, Box::new(emitter));
let source = dox[code_block.code].to_owned();
let sess = ParseSess::with_span_handler(handler, sm);
let edition = code_block.lang_string.edition.unwrap_or_else(|| self.cx.tcx.sess.edition());
let expn_data = ExpnData::default(
ExpnKind::AstPass(AstPass::TestHarness),
DUMMY_SP,
edition,
None,
None,
);
let expn_id =
self.cx.tcx.with_stable_hashing_context(|hcx| LocalExpnId::fresh(expn_data, hcx));
let span = DUMMY_SP.fresh_expansion(expn_id);
let is_empty = rustc_driver::catch_fatal_errors(|| {
parse_stream_from_source_str(
FileName::Custom(String::from("doctest")),
source,
&sess,
Some(span),
)
.is_empty()
})
.unwrap_or(false);
let buffer = buffer.borrow();
if !buffer.has_errors && !is_empty {
// No errors in a non-empty program.
return;
}
let Some(local_id) = item.item_id.as_def_id().and_then(|x| x.as_local())
else {
// We don't need to check the syntax for other crates so returning
// without doing anything should not be a problem.
return;
};
let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_id);
let empty_block = code_block.lang_string == Default::default() && code_block.is_fenced;
let is_ignore = code_block.lang_string.ignore != markdown::Ignore::None;
// The span and whether it is precise or not.
let (sp, precise_span) = match super::source_span_for_markdown_range(
self.cx.tcx,
dox,
&code_block.range,
&item.attrs,
) {
Some(sp) => (sp, true),
None => (item.attr_span(self.cx.tcx), false),
};
let msg = if buffer.has_errors {
"could not parse code block as Rust code"
} else {
"Rust code block is empty"
};
// Finally build and emit the completed diagnostic.
// All points of divergence have been handled earlier so this can be
// done the same way whether the span is precise or not.
self.cx.tcx.struct_span_lint_hir(
crate::lint::INVALID_RUST_CODEBLOCKS,
hir_id,
sp,
msg,
|lint| {
let explanation = if is_ignore {
"`ignore` code blocks require valid Rust code for syntax highlighting; \
mark blocks that do not contain Rust code as text"
} else {
"mark blocks that do not contain Rust code as text"
};
if precise_span {
if is_ignore {
// giving an accurate suggestion is hard because `ignore` might not have come first in the list.
// just give a `help` instead.
lint.span_help(
sp.from_inner(InnerSpan::new(0, 3)),
&format!("{}: ```text", explanation),
);
} else if empty_block {
lint.span_suggestion(
sp.from_inner(InnerSpan::new(0, 3)).shrink_to_hi(),
explanation,
"text",
Applicability::MachineApplicable,
);
}
} else if empty_block || is_ignore {
lint.help(&format!("{}: ```text", explanation));
}
// FIXME(#67563): Provide more context for these errors by displaying the spans inline.
for message in buffer.messages.iter() {
lint.note(message);
}
lint
},
);
}
}
impl<'a, 'tcx> DocVisitor for SyntaxChecker<'a, 'tcx> {
fn visit_item(&mut self, item: &clean::Item) {
if let Some(dox) = &item.attrs.collapsed_doc_value() {
let sp = item.attr_span(self.cx.tcx);
let extra = crate::html::markdown::ExtraInfo::new_did(
self.cx.tcx,
item.item_id.expect_def_id(),
sp,
);
for code_block in markdown::rust_code_blocks(dox, &extra) {
self.check_rust_syntax(item, dox, code_block);
}
}
self.visit_item_recur(item)
}
}
#[derive(Default)]
struct Buffer {
messages: Vec<String>,
has_errors: bool,
}
struct BufferEmitter {
buffer: Lrc<Lock<Buffer>>,
fallback_bundle: LazyFallbackBundle,
}
impl Translate for BufferEmitter {
fn fluent_bundle(&self) -> Option<&Lrc<rustc_errors::FluentBundle>> {
None
}
fn fallback_fluent_bundle(&self) -> &rustc_errors::FluentBundle {
&**self.fallback_bundle
}
}
impl Emitter for BufferEmitter {
fn emit_diagnostic(&mut self, diag: &Diagnostic) {
let mut buffer = self.buffer.borrow_mut();
let fluent_args = to_fluent_args(diag.args());
let translated_main_message = self.translate_message(&diag.message[0].0, &fluent_args);
buffer.messages.push(format!("error from rustc: {}", translated_main_message));
if diag.is_error() {
buffer.has_errors = true;
}
}
fn source_map(&self) -> Option<&Lrc<SourceMap>> {
None
}
}

33
src/librustdoc/passes/lint.rs Normal file
View file

@ -0,0 +1,33 @@
//! Runs several rustdoc lints, consolidating them into a single pass for
//! efficiency and simplicity.
mod bare_urls;
mod check_code_block_syntax;
mod html_tags;
use super::Pass;
use crate::clean::*;
use crate::core::DocContext;
use crate::visit::DocVisitor;
pub(crate) const RUN_LINTS: Pass =
Pass { name: "run-lints", run: run_lints, description: "runs some of rustdoc's lints" };
struct Linter<'a, 'tcx> {
cx: &'a mut DocContext<'tcx>,
}
pub(crate) fn run_lints(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
Linter { cx }.visit_crate(&krate);
krate
}
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);
self.visit_item_recur(item)
}
}

89
src/librustdoc/passes/lint/bare_urls.rs Normal file
View file

@ -0,0 +1,89 @@
//! Detects links that are not linkified, e.g., in Markdown such as `Go to https://example.com/.`
//! Suggests wrapping the link with angle brackets: `Go to <https://example.com/>.` to linkify it.
use crate::clean::*;
use crate::core::DocContext;
use crate::html::markdown::main_body_opts;
use crate::passes::source_span_for_markdown_range;
use core::ops::Range;
use pulldown_cmark::{Event, Parser, Tag};
use regex::Regex;
use rustc_errors::Applicability;
use std::mem;
use std::sync::LazyLock;
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;
};
let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
if !dox.is_empty() {
let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range<usize>| {
let sp = source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)
.unwrap_or_else(|| item.attr_span(cx.tcx));
cx.tcx.struct_span_lint_hir(crate::lint::BARE_URLS, hir_id, sp, msg, |lint| {
lint.note("bare URLs are not automatically turned into clickable links")
.span_suggestion(
sp,
"use an automatic link instead",
format!("<{}>", url),
Applicability::MachineApplicable,
)
});
};
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) =>
{
break;
}
_ => {}
}
}
}
_ => {}
}
}
}
}
static URL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(concat!(
r"https?://", // url scheme
r"([-a-zA-Z0-9@:%._\+~#=]{2,256}\.)+", // one or more subdomains
r"[a-zA-Z]{2,63}", // root domain
r"\b([-a-zA-Z0-9@:%_\+.~#?&/=]*)" // optional query or url fragments
))
.expect("failed to build regex")
});
fn find_raw_urls(
cx: &DocContext<'_>,
text: &str,
range: Range<usize>,
f: &impl Fn(&DocContext<'_>, &str, &str, Range<usize>),
) {
trace!("looking for raw urls in {}", text);
// For now, we only check "full" URLs (meaning, starting with "http://" or "https://").
for match_ in URL_REGEX.find_iter(text) {
let url = match_.as_str();
let url_range = match_.range();
f(
cx,
"this URL is not a hyperlink",
url,
Range { start: range.start + url_range.start, end: range.start + url_range.end },
);
}
}

170
src/librustdoc/passes/lint/check_code_block_syntax.rs Normal file
View file

@ -0,0 +1,170 @@
//! Validates syntax inside Rust code blocks (\`\`\`rust).
use rustc_data_structures::sync::{Lock, Lrc};
use rustc_errors::{
emitter::Emitter,
translation::{to_fluent_args, Translate},
Applicability, Diagnostic, Handler, LazyFallbackBundle,
};
use rustc_parse::parse_stream_from_source_str;
use rustc_session::parse::ParseSess;
use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId};
use rustc_span::source_map::{FilePathMapping, SourceMap};
use rustc_span::{FileName, InnerSpan, DUMMY_SP};
use crate::clean;
use crate::core::DocContext;
use crate::html::markdown::{self, RustCodeBlock};
use crate::passes::source_span_for_markdown_range;
pub(crate) fn visit_item(cx: &DocContext<'_>, item: &clean::Item) {
if let Some(dox) = &item.attrs.collapsed_doc_value() {
let sp = item.attr_span(cx.tcx);
let extra =
crate::html::markdown::ExtraInfo::new_did(cx.tcx, item.item_id.expect_def_id(), sp);
for code_block in markdown::rust_code_blocks(dox, &extra) {
check_rust_syntax(cx, item, dox, code_block);
}
}
}
fn check_rust_syntax(
cx: &DocContext<'_>,
item: &clean::Item,
dox: &str,
code_block: RustCodeBlock,
) {
let buffer = Lrc::new(Lock::new(Buffer::default()));
let fallback_bundle =
rustc_errors::fallback_fluent_bundle(rustc_errors::DEFAULT_LOCALE_RESOURCES, false);
let emitter = BufferEmitter { buffer: Lrc::clone(&buffer), fallback_bundle };
let sm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
let handler = Handler::with_emitter(false, None, Box::new(emitter));
let source = dox[code_block.code].to_owned();
let sess = ParseSess::with_span_handler(handler, sm);
let edition = code_block.lang_string.edition.unwrap_or_else(|| cx.tcx.sess.edition());
let expn_data =
ExpnData::default(ExpnKind::AstPass(AstPass::TestHarness), DUMMY_SP, edition, None, None);
let expn_id = cx.tcx.with_stable_hashing_context(|hcx| LocalExpnId::fresh(expn_data, hcx));
let span = DUMMY_SP.fresh_expansion(expn_id);
let is_empty = rustc_driver::catch_fatal_errors(|| {
parse_stream_from_source_str(
FileName::Custom(String::from("doctest")),
source,
&sess,
Some(span),
)
.is_empty()
})
.unwrap_or(false);
let buffer = buffer.borrow();
if !buffer.has_errors && !is_empty {
// No errors in a non-empty program.
return;
}
let Some(local_id) = item.item_id.as_def_id().and_then(|x| x.as_local())
else {
// We don't need to check the syntax for other crates so returning
// without doing anything should not be a problem.
return;
};
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_id);
let empty_block = code_block.lang_string == Default::default() && code_block.is_fenced;
let is_ignore = code_block.lang_string.ignore != markdown::Ignore::None;
// The span and whether it is precise or not.
let (sp, precise_span) =
match source_span_for_markdown_range(cx.tcx, dox, &code_block.range, &item.attrs) {
Some(sp) => (sp, true),
None => (item.attr_span(cx.tcx), false),
};
let msg = if buffer.has_errors {
"could not parse code block as Rust code"
} else {
"Rust code block is empty"
};
// Finally build and emit the completed diagnostic.
// All points of divergence have been handled earlier so this can be
// done the same way whether the span is precise or not.
cx.tcx.struct_span_lint_hir(crate::lint::INVALID_RUST_CODEBLOCKS, hir_id, sp, msg, |lint| {
let explanation = if is_ignore {
"`ignore` code blocks require valid Rust code for syntax highlighting; \
mark blocks that do not contain Rust code as text"
} else {
"mark blocks that do not contain Rust code as text"
};
if precise_span {
if is_ignore {
// giving an accurate suggestion is hard because `ignore` might not have come first in the list.
// just give a `help` instead.
lint.span_help(
sp.from_inner(InnerSpan::new(0, 3)),
&format!("{}: ```text", explanation),
);
} else if empty_block {
lint.span_suggestion(
sp.from_inner(InnerSpan::new(0, 3)).shrink_to_hi(),
explanation,
"text",
Applicability::MachineApplicable,
);
}
} else if empty_block || is_ignore {
lint.help(&format!("{}: ```text", explanation));
}
// FIXME(#67563): Provide more context for these errors by displaying the spans inline.
for message in buffer.messages.iter() {
lint.note(message);
}
lint
});
}
#[derive(Default)]
struct Buffer {
messages: Vec<String>,
has_errors: bool,
}
struct BufferEmitter {
buffer: Lrc<Lock<Buffer>>,
fallback_bundle: LazyFallbackBundle,
}
impl Translate for BufferEmitter {
fn fluent_bundle(&self) -> Option<&Lrc<rustc_errors::FluentBundle>> {
None
}
fn fallback_fluent_bundle(&self) -> &rustc_errors::FluentBundle {
&**self.fallback_bundle
}
}
impl Emitter for BufferEmitter {
fn emit_diagnostic(&mut self, diag: &Diagnostic) {
let mut buffer = self.buffer.borrow_mut();
let fluent_args = to_fluent_args(diag.args());
let translated_main_message = self.translate_message(&diag.message[0].0, &fluent_args);
buffer.messages.push(format!("error from rustc: {}", translated_main_message));
if diag.is_error() {
buffer.has_errors = true;
}
}
fn source_map(&self) -> Option<&Lrc<SourceMap>> {
None
}
}

309
src/librustdoc/passes/html_tags.rs → src/librustdoc/passes/lint/html_tags.rs
View file

@ -1,9 +1,8 @@
//! Detects invalid HTML (like an unclosed `<span>`) in doc comments. //! Detects invalid HTML (like an unclosed `<span>`) in doc comments.
use super::Pass;
use crate::clean::*; use crate::clean::*;
use crate::core::DocContext; use crate::core::DocContext;
use crate::html::markdown::main_body_opts; use crate::html::markdown::main_body_opts;
use crate::visit::DocVisitor; use crate::passes::source_span_for_markdown_range;
use pulldown_cmark::{BrokenLink, Event, LinkType, Parser, Tag}; use pulldown_cmark::{BrokenLink, Event, LinkType, Parser, Tag};
@ -11,20 +10,150 @@ use std::iter::Peekable;
use std::ops::Range; use std::ops::Range;
use std::str::CharIndices; use std::str::CharIndices;
pub(crate) const CHECK_INVALID_HTML_TAGS: Pass = Pass { pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
name: "check-invalid-html-tags", let tcx = cx.tcx;
run: check_invalid_html_tags, let Some(hir_id) = DocContext::as_local_hir_id(tcx, item.item_id)
description: "detects invalid HTML tags in doc comments", // If non-local, no need to check anything.
}; else { return };
let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
if !dox.is_empty() {
let report_diag = |msg: &str, range: &Range<usize>, is_open_tag: bool| {
let sp = match source_span_for_markdown_range(tcx, &dox, range, &item.attrs) {
Some(sp) => sp,
None => item.attr_span(tcx),
};
tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, msg, |lint| {
use rustc_lint_defs::Applicability;
// 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'>'
{
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;
}
}
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,
) {
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 lint;
}
// 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,
);
}
struct InvalidHtmlTagsLinter<'a, 'tcx> { lint
cx: &'a mut DocContext<'tcx>, });
} };
pub(crate) fn check_invalid_html_tags(krate: Crate, cx: &mut DocContext<'_>) -> Crate { let mut tags = Vec::new();
let mut coll = InvalidHtmlTagsLinter { cx }; let mut is_in_comment = None;
coll.visit_crate(&krate); let mut in_code_block = false;
krate
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.as_str().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) if !in_code_block => {
extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
}
Event::End(Tag::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", &range, false);
}
}
} }
const ALLOWED_UNCLOSED: &[&str] = &[ const ALLOWED_UNCLOSED: &[&str] = &[
@ -276,155 +405,3 @@ fn extract_tags(
} }
} }
} }
impl<'a, 'tcx> DocVisitor for InvalidHtmlTagsLinter<'a, 'tcx> {
fn visit_item(&mut self, item: &Item) {
let tcx = self.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.attrs.collapsed_doc_value().unwrap_or_default();
if !dox.is_empty() {
let report_diag = |msg: &str, range: &Range<usize>, is_open_tag: bool| {
let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs)
{
Some(sp) => sp,
None => item.attr_span(tcx),
};
tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, msg, |lint| {
use rustc_lint_defs::Applicability;
// 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'>'
{
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;
}
}
if let Some(new_end) = extract_path_forward(&dox, generics_end) {
generics_end = new_end;
}
let generics_sp = match super::source_span_for_markdown_range(
tcx,
&dox,
&(generics_start..generics_end),
&item.attrs,
) {
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 lint;
}
// 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,
);
}
lint
});
};
let mut tags = Vec::new();
let mut is_in_comment = None;
let mut in_code_block = false;
let link_names = item.link_names(&self.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.as_str().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) if !in_code_block => {
extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
}
Event::End(Tag::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", &range, false);
}
}
self.visit_item_recur(item)
}
}

18
src/librustdoc/passes/mod.rs
View file

@ -12,9 +12,6 @@ use crate::core::DocContext;
mod stripper; mod stripper;
pub(crate) use stripper::*; pub(crate) use stripper::*;
mod bare_urls;
pub(crate) use self::bare_urls::CHECK_BARE_URLS;
mod strip_hidden; mod strip_hidden;
pub(crate) use self::strip_hidden::STRIP_HIDDEN; pub(crate) use self::strip_hidden::STRIP_HIDDEN;
@ -36,14 +33,11 @@ pub(crate) use self::check_doc_test_visibility::CHECK_DOC_TEST_VISIBILITY;
mod collect_trait_impls; mod collect_trait_impls;
pub(crate) use self::collect_trait_impls::COLLECT_TRAIT_IMPLS; pub(crate) use self::collect_trait_impls::COLLECT_TRAIT_IMPLS;
mod check_code_block_syntax;
pub(crate) use self::check_code_block_syntax::CHECK_CODE_BLOCK_SYNTAX;
mod calculate_doc_coverage; mod calculate_doc_coverage;
pub(crate) use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE; pub(crate) use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE;
mod html_tags; mod lint;
pub(crate) use self::html_tags::CHECK_INVALID_HTML_TAGS; pub(crate) use self::lint::RUN_LINTS;
/// A single pass over the cleaned documentation. /// A single pass over the cleaned documentation.
/// ///
@ -82,11 +76,9 @@ pub(crate) const PASSES: &[Pass] = &[
STRIP_PRIV_IMPORTS, STRIP_PRIV_IMPORTS,
PROPAGATE_DOC_CFG, PROPAGATE_DOC_CFG,
COLLECT_INTRA_DOC_LINKS, COLLECT_INTRA_DOC_LINKS,
CHECK_CODE_BLOCK_SYNTAX,
COLLECT_TRAIT_IMPLS, COLLECT_TRAIT_IMPLS,
CALCULATE_DOC_COVERAGE, CALCULATE_DOC_COVERAGE,
CHECK_INVALID_HTML_TAGS, RUN_LINTS,
CHECK_BARE_URLS,
]; ];
/// The list of passes run by default. /// The list of passes run by default.
@ -97,10 +89,8 @@ pub(crate) const DEFAULT_PASSES: &[ConditionalPass] = &[
ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate), ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate),
ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate), ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate),
ConditionalPass::always(COLLECT_INTRA_DOC_LINKS), ConditionalPass::always(COLLECT_INTRA_DOC_LINKS),
ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX),
ConditionalPass::always(CHECK_INVALID_HTML_TAGS),
ConditionalPass::always(PROPAGATE_DOC_CFG), ConditionalPass::always(PROPAGATE_DOC_CFG),
ConditionalPass::always(CHECK_BARE_URLS), ConditionalPass::always(RUN_LINTS),
]; ];
/// The list of default passes run when `--doc-coverage` is passed to rustdoc. /// The list of default passes run when `--doc-coverage` is passed to rustdoc.

8
src/test/rustdoc-ui/issue-91713.stdout
View file

@ -5,11 +5,9 @@ check_doc_test_visibility - run various visibility-related lints on doctests
strip-priv-imports - strips all private import statements (`use`, `extern crate`) from a crate strip-priv-imports - strips all private import statements (`use`, `extern crate`) from a crate
propagate-doc-cfg - propagates `#[doc(cfg(...))]` to child items propagate-doc-cfg - propagates `#[doc(cfg(...))]` to child items
collect-intra-doc-links - resolves intra-doc links collect-intra-doc-links - resolves intra-doc links
check-code-block-syntax - validates syntax inside Rust code blocks
collect-trait-impls - retrieves trait impls for items in the crate collect-trait-impls - retrieves trait impls for items in the crate
calculate-doc-coverage - counts the number of items with and without documentation calculate-doc-coverage - counts the number of items with and without documentation
check-invalid-html-tags - detects invalid HTML tags in doc comments run-lints - runs some of rustdoc's lints
check-bare-urls - detects URLs that are not hyperlinks
Default passes for rustdoc: Default passes for rustdoc:
collect-trait-impls collect-trait-impls
@ -18,10 +16,8 @@ check_doc_test_visibility
strip-private (when not --document-private-items) strip-private (when not --document-private-items)
strip-priv-imports (when --document-private-items) strip-priv-imports (when --document-private-items)
collect-intra-doc-links collect-intra-doc-links
check-code-block-syntax
check-invalid-html-tags
propagate-doc-cfg propagate-doc-cfg
check-bare-urls run-lints
Passes run with `--show-coverage`: Passes run with `--show-coverage`:
strip-hidden (when not --document-hidden-items) strip-hidden (when not --document-hidden-items)