about summary refs log tree commit diff
path: root/src/librustdoc
diff options
context:
space:
mode:
authorNicholas Nethercote <n.nethercote@gmail.com>2023-05-04 10:55:21 +1000
committerNicholas Nethercote <n.nethercote@gmail.com>2023-05-29 09:23:43 +1000
commit781111ef3591e4355c115593c4d75964b85555d6 (patch)
tree7417f5be447fb1a099678d197bddca021135fd13 /src/librustdoc
parent1c53407e8c7cc922d718bde61ca34f47b6d2120f (diff)
Use `Cow` in `{D,Subd}iagnosticMessage`.
Each of `{D,Subd}iagnosticMessage::{Str,Eager}` has a comment:
```
// FIXME(davidtwco): can a `Cow<'static, str>` be used here?
```
This commit answers that question in the affirmative. It's not the most
compelling change ever, but it might be worth merging.

This requires changing the `impl<'a> From<&'a str>` impls to `impl
From<&'static str>`, which involves a bunch of knock-on changes that
require/result in call sites being a little more precise about exactly
what kind of string they use to create errors, and not just `&str`. This
will result in fewer unnecessary allocations, though this will not have
any notable perf effects given that these are error paths.

Note that I was lazy within Clippy, using `to_string` in a few places to
preserve the existing string imprecision. I could have used `impl
Into<{D,Subd}iagnosticMessage>` in various places as is done in the
compiler, but that would have required changes to *many* call sites
(mostly changing `&format("...")` to `format!("...")`) which didn't seem
worthwhile.
Diffstat (limited to 'src/librustdoc')
-rw-r--r--src/librustdoc/html/markdown.rs2
-rw-r--r--src/librustdoc/passes/collect_intra_doc_links.rs2
-rw-r--r--src/librustdoc/passes/lint/bare_urls.rs29
-rw-r--r--src/librustdoc/passes/lint/html_tags.rs170
-rw-r--r--src/librustdoc/passes/lint/unescaped_backticks.rs2
5 files changed, 106 insertions, 99 deletions
diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index 9bb20022cfd..b26a5c32ec6 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -781,7 +781,7 @@ impl<'tcx> ExtraInfo<'tcx> {
         ExtraInfo { def_id, sp, tcx }
     }
 
-    fn error_invalid_codeblock_attr(&self, msg: String, help: &str) {
+    fn error_invalid_codeblock_attr(&self, msg: String, help: &'static str) {
         if let Some(def_id) = self.def_id.as_local() {
             self.tcx.struct_span_lint_hir(
                 crate::lint::INVALID_CODEBLOCK_ATTRIBUTES,
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 061a572c46b..2b0fef267e9 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -842,7 +842,7 @@ impl PreprocessingError {
         match self {
             PreprocessingError::MultipleAnchors => report_multiple_anchors(cx, diag_info),
             PreprocessingError::Disambiguator(range, msg) => {
-                disambiguator_error(cx, diag_info, range.clone(), msg.as_str())
+                disambiguator_error(cx, diag_info, range.clone(), msg.clone())
             }
             PreprocessingError::MalformedGenerics(err, path_str) => {
                 report_malformed_generics(cx, diag_info, *err, path_str)
diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs
index a10d5fdb410..e9cee92d22b 100644
--- a/src/librustdoc/passes/lint/bare_urls.rs
+++ b/src/librustdoc/passes/lint/bare_urls.rs
@@ -20,19 +20,20 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item) {
         };
     let dox = item.doc_value();
     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 report_diag =
+            |cx: &DocContext<'_>, msg: &'static 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();
 
@@ -72,7 +73,7 @@ fn find_raw_urls(
     cx: &DocContext<'_>,
     text: &str,
     range: Range<usize>,
-    f: &impl Fn(&DocContext<'_>, &str, &str, Range<usize>),
+    f: &impl Fn(&DocContext<'_>, &'static str, &str, Range<usize>),
 ) {
     trace!("looking for raw urls in {}", text);
     // For now, we only check "full" URLs (meaning, starting with "http://" or "https://").
diff --git a/src/librustdoc/passes/lint/html_tags.rs b/src/librustdoc/passes/lint/html_tags.rs
index f0403647af0..5273f52bc6f 100644
--- a/src/librustdoc/passes/lint/html_tags.rs
+++ b/src/librustdoc/passes/lint/html_tags.rs
@@ -17,90 +17,96 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
     else { return };
     let dox = item.doc_value();
     if !dox.is_empty() {
-        let report_diag = |msg: &str, range: &Range<usize>, is_open_tag: bool| {
+        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) {
                 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'>'
+            tcx.struct_span_lint_hir(
+                crate::lint::INVALID_HTML_TAGS,
+                hir_id,
+                sp,
+                msg.to_string(),
+                |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))
                     {
-                        generics_end += 1;
-                        generics_start -= 1;
-                        if let Some(new_start) = extract_path_backwards(&dox, generics_start) {
-                            generics_start = new_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,
+                        );
                     }
-                    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,
-                    );
-                }
 
-                lint
-            });
+                    lint
+                },
+            );
         };
 
         let mut tags = Vec::new();
@@ -147,11 +153,11 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
             let t = t.to_lowercase();
             !ALLOWED_UNCLOSED.contains(&t.as_str())
         }) {
-            report_diag(&format!("unclosed HTML tag `{}`", tag), range, true);
+            report_diag(format!("unclosed HTML tag `{}`", tag), range, true);
         }
 
         if let Some(range) = is_in_comment {
-            report_diag("Unclosed HTML comment", &range, false);
+            report_diag("Unclosed HTML comment".to_string(), &range, false);
         }
     }
 }
@@ -165,7 +171,7 @@ fn drop_tag(
     tags: &mut Vec<(String, Range<usize>)>,
     tag_name: String,
     range: Range<usize>,
-    f: &impl Fn(&str, &Range<usize>, bool),
+    f: &impl Fn(String, &Range<usize>, bool),
 ) {
     let tag_name_low = tag_name.to_lowercase();
     if let Some(pos) = tags.iter().rposition(|(t, _)| t.to_lowercase() == tag_name_low) {
@@ -186,14 +192,14 @@ fn drop_tag(
             // `tags` is used as a queue, meaning that everything after `pos` is included inside it.
             // So `<h2><h3></h2>` will look like `["h2", "h3"]`. So when closing `h2`, we will still
             // have `h3`, meaning the tag wasn't closed as it should have.
-            f(&format!("unclosed HTML tag `{}`", last_tag_name), &last_tag_span, true);
+            f(format!("unclosed HTML tag `{}`", last_tag_name), &last_tag_span, true);
         }
         // Remove the `tag_name` that was originally closed
         tags.pop();
     } else {
         // It can happen for example in this case: `<h2></script></h2>` (the `h2` tag isn't required
         // but it helps for the visualization).
-        f(&format!("unopened HTML tag `{}`", tag_name), &range, false);
+        f(format!("unopened HTML tag `{}`", tag_name), &range, false);
     }
 }
 
@@ -261,7 +267,7 @@ fn extract_html_tag(
     range: &Range<usize>,
     start_pos: usize,
     iter: &mut Peekable<CharIndices<'_>>,
-    f: &impl Fn(&str, &Range<usize>, bool),
+    f: &impl Fn(String, &Range<usize>, bool),
 ) {
     let mut tag_name = String::new();
     let mut is_closing = false;
@@ -347,7 +353,7 @@ fn extract_html_tag(
                     if let Some(quote_pos) = quote_pos {
                         let qr = Range { start: quote_pos, end: quote_pos };
                         f(
-                            &format!("unclosed quoted HTML attribute on tag `{}`", tag_name),
+                            format!("unclosed quoted HTML attribute on tag `{}`", tag_name),
                             &qr,
                             false,
                         );
@@ -360,7 +366,7 @@ fn extract_html_tag(
                                 at == "svg" || at == "math"
                             });
                         if !valid {
-                            f(&format!("invalid self-closing HTML tag `{}`", tag_name), &r, false);
+                            f(format!("invalid self-closing HTML tag `{}`", tag_name), &r, false);
                         }
                     } else {
                         tags.push((tag_name, r));
@@ -378,7 +384,7 @@ fn extract_tags(
     text: &str,
     range: Range<usize>,
     is_in_comment: &mut Option<Range<usize>>,
-    f: &impl Fn(&str, &Range<usize>, bool),
+    f: &impl Fn(String, &Range<usize>, bool),
 ) {
     let mut iter = text.char_indices().peekable();
 
diff --git a/src/librustdoc/passes/lint/unescaped_backticks.rs b/src/librustdoc/passes/lint/unescaped_backticks.rs
index 865212205ed..256958d7160 100644
--- a/src/librustdoc/passes/lint/unescaped_backticks.rs
+++ b/src/librustdoc/passes/lint/unescaped_backticks.rs
@@ -373,7 +373,7 @@ fn suggest_insertion(
     lint: &mut DiagnosticBuilder<'_, ()>,
     insert_index: usize,
     suggestion: char,
-    message: &str,
+    message: &'static str,
 ) {
     /// Maximum bytes of context to show around the insertion.
     const CONTEXT_MAX_LEN: usize = 80;