about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Howell <michael@notriddle.com>2024-09-04 16:39:23 -0700
committerMichael Howell <michael@notriddle.com>2024-09-04 20:08:49 -0700
commitd4b246bb117d451e34f1692fa34f9574b3d169e4 (patch)
tree8f264e0d6ff9d0cbe3770bededed947ab7e8d829
parentd6c8169c186ab16a3404cd0d0866674018e8a19e (diff)
downloadrust-d4b246bb117d451e34f1692fa34f9574b3d169e4.tar.gz
rust-d4b246bb117d451e34f1692fa34f9574b3d169e4.zip
rustdoc: unify the short-circuit on all lints
This is a bit of an experiment to see if it improves perf.
-rw-r--r--src/librustdoc/passes/lint.rs33
-rw-r--r--src/librustdoc/passes/lint/bare_urls.rs68
-rw-r--r--src/librustdoc/passes/lint/check_code_block_syntax.rs6
-rw-r--r--src/librustdoc/passes/lint/html_tags.rs248
-rw-r--r--src/librustdoc/passes/lint/redundant_explicit_links.rs7
-rw-r--r--src/librustdoc/passes/lint/unescaped_backticks.rs12
-rw-r--r--src/librustdoc/passes/lint/unportable_markdown.rs12
7 files changed, 184 insertions, 202 deletions
diff --git a/src/librustdoc/passes/lint.rs b/src/librustdoc/passes/lint.rs
index bc804a340bf..4da5d8f0e06 100644
--- a/src/librustdoc/passes/lint.rs
+++ b/src/librustdoc/passes/lint.rs
@@ -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)
     }
diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs
index 22051dd954d..bac0e07f1c1 100644
--- a/src/librustdoc/passes/lint/bare_urls.rs
+++ b/src/librustdoc/passes/lint/bare_urls.rs
@@ -8,6 +8,7 @@ use std::sync::LazyLock;
 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::clean::*;
 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;
                         }
+                        _ => {}
                     }
                 }
-                _ => {}
             }
+            _ => {}
         }
     }
 }
diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs
index 977c0953336..1b2431a629b 100644
--- a/src/librustdoc/passes/lint/check_code_block_syntax.rs
+++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs
@@ -15,10 +15,8 @@ use crate::clean;
 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) {
diff --git a/src/librustdoc/passes/lint/html_tags.rs b/src/librustdoc/passes/lint/html_tags.rs
index 6f9e9d36a5c..223174838ad 100644
--- a/src/librustdoc/passes/lint/html_tags.rs
+++ b/src/librustdoc/passes/lint/html_tags.rs
@@ -5,159 +5,149 @@ use std::ops::Range;
 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)
-            {
-                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;
+    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);
+            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))
+            // 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,
-                    );
                 }
-            });
-        };
+                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 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 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 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();
+    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 (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);
-        }
+    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);
-        }
+    if let Some(range) = is_in_comment {
+        report_diag("Unclosed HTML comment".to_string(), &range, false);
     }
 }
 
diff --git a/src/librustdoc/passes/lint/redundant_explicit_links.rs b/src/librustdoc/passes/lint/redundant_explicit_links.rs
index 0a90c039dfb..9c37e11349a 100644
--- a/src/librustdoc/passes/lint/redundant_explicit_links.rs
+++ b/src/librustdoc/passes/lint/redundant_explicit_links.rs
@@ -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())
diff --git a/src/librustdoc/passes/lint/unescaped_backticks.rs b/src/librustdoc/passes/lint/unescaped_backticks.rs
index a6c8db16f82..d79f682a580 100644
--- a/src/librustdoc/passes/lint/unescaped_backticks.rs
+++ b/src/librustdoc/passes/lint/unescaped_backticks.rs
@@ -4,6 +4,7 @@ use std::ops::Range;
 
 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::clean::Item;
 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<'_>| {
diff --git a/src/librustdoc/passes/lint/unportable_markdown.rs b/src/librustdoc/passes/lint/unportable_markdown.rs
index 87fe0055883..f8368a866c8 100644
--- a/src/librustdoc/passes/lint/unportable_markdown.rs
+++ b/src/librustdoc/passes/lint/unportable_markdown.rs
@@ -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 {pulldown_cmark as cmarkn, pulldown_cmark_old as cmarko};
 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