diff options
| author | bors <bors@rust-lang.org> | 2023-05-29 07:10:44 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-05-29 07:10:44 +0000 |
| commit | 70e04bd88d85cab8ed110ace5a278fab106d0ef5 (patch) | |
| tree | 958f31b79a6c3c0edc39f07eb9bc99b04f0064da /src/librustdoc | |
| parent | dc0943d2eebf139e7e509882b803f77bfd25656d (diff) | |
| parent | 781111ef3591e4355c115593c4d75964b85555d6 (diff) | |
Auto merge of #111748 - nnethercote:Cow-DiagnosticMessage, r=WaffleLapkin
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.
r? `@WaffleLapkin`
Diffstat (limited to 'src/librustdoc')
| -rw-r--r-- | src/librustdoc/html/markdown.rs | 2 | ||||
| -rw-r--r-- | src/librustdoc/passes/collect_intra_doc_links.rs | 2 | ||||
| -rw-r--r-- | src/librustdoc/passes/lint/bare_urls.rs | 29 | ||||
| -rw-r--r-- | src/librustdoc/passes/lint/html_tags.rs | 170 | ||||
| -rw-r--r-- | src/librustdoc/passes/lint/unescaped_backticks.rs | 2 |
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; |
