Rollup merge of #136400 - lolbinarycat:rustdoc-link-lint-135851, r=GuillaumeGomez
Improve handling of rustdoc lints when used with raw doc fragments. 1. `rustdoc::bare_urls` no longer outputs incoherent suggestions if `source_span_for_markdown_range` returns None, instead outputting no suggestion 2. `source_span_for_markdown_range` has one more heuristic, so it will return `None` less often. 3. add ui test to make sure we don't emit nonsense suggestions. fixes https://github.com/rust-lang/rust/issues/135851
This commit is contained in:
Matthias Krüger
GitHub
commit
1c2ea28727
9 changed files with 154 additions and 41 deletions
|
|
@ -514,20 +514,30 @@ pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
|
|||
/// This method does not always work, because markdown bytes don't necessarily match source bytes,
|
||||
/// like if escapes are used in the string. In this case, it returns `None`.
|
||||
///
|
||||
/// This method will return `Some` only if:
|
||||
/// `markdown` is typically the entire documentation for an item,
|
||||
/// after combining fragments.
|
||||
///
|
||||
/// This method will return `Some` only if one of the following is true:
|
||||
///
|
||||
/// - The doc is made entirely from sugared doc comments, which cannot contain escapes
|
||||
/// - The doc is entirely from a single doc fragment, with a string literal, exactly equal
|
||||
/// - The doc is entirely from a single doc fragment with a string literal exactly equal to `markdown`.
|
||||
/// - The doc comes from `include_str!`
|
||||
/// - The doc includes exactly one substring matching `markdown[md_range]` which is contained in a single doc fragment.
|
||||
///
|
||||
/// This function is defined in the compiler so it can be used by
|
||||
/// both `rustdoc` and `clippy`.
|
||||
pub fn source_span_for_markdown_range(
|
||||
tcx: TyCtxt<'_>,
|
||||
markdown: &str,
|
||||
md_range: &Range<usize>,
|
||||
fragments: &[DocFragment],
|
||||
) -> Option<Span> {
|
||||
use rustc_span::BytePos;
|
||||
|
||||
let map = tcx.sess.source_map();
|
||||
if let &[fragment] = &fragments
|
||||
&& fragment.kind == DocFragmentKind::RawDoc
|
||||
&& let Ok(snippet) = tcx.sess.source_map().span_to_snippet(fragment.span)
|
||||
&& let Ok(snippet) = map.span_to_snippet(fragment.span)
|
||||
&& snippet.trim_end() == markdown.trim_end()
|
||||
&& let Ok(md_range_lo) = u32::try_from(md_range.start)
|
||||
&& let Ok(md_range_hi) = u32::try_from(md_range.end)
|
||||
|
|
@ -544,10 +554,43 @@ pub fn source_span_for_markdown_range(
|
|||
let is_all_sugared_doc = fragments.iter().all(|frag| frag.kind == DocFragmentKind::SugaredDoc);
|
||||
|
||||
if !is_all_sugared_doc {
|
||||
// This case ignores the markdown outside of the range so that it can
|
||||
// work in cases where the markdown is made from several different
|
||||
// doc fragments, but the target range does not span across multiple
|
||||
// fragments.
|
||||
let mut match_data = None;
|
||||
let pat = &markdown[md_range.clone()];
|
||||
// This heirustic doesn't make sense with a zero-sized range.
|
||||
if pat.is_empty() {
|
||||
return None;
|
||||
}
|
||||
for (i, fragment) in fragments.iter().enumerate() {
|
||||
if let Ok(snippet) = map.span_to_snippet(fragment.span)
|
||||
&& let Some(match_start) = snippet.find(pat)
|
||||
{
|
||||
// If there is either a match in a previous fragment, or
|
||||
// multiple matches in this fragment, there is ambiguity.
|
||||
if match_data.is_none() && !snippet[match_start + 1..].contains(pat) {
|
||||
match_data = Some((i, match_start));
|
||||
} else {
|
||||
// Heirustic produced ambiguity, return nothing.
|
||||
return None;
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some((i, match_start)) = match_data {
|
||||
let sp = fragments[i].span;
|
||||
// we need to calculate the span start,
|
||||
// then use that in our calulations for the span end
|
||||
let lo = sp.lo() + BytePos(match_start as u32);
|
||||
return Some(
|
||||
sp.with_lo(lo).with_hi(lo + BytePos((md_range.end - md_range.start) as u32)),
|
||||
);
|
||||
}
|
||||
return None;
|
||||
}
|
||||
|
||||
let snippet = tcx.sess.source_map().span_to_snippet(span_of_fragments(fragments)?).ok()?;
|
||||
let snippet = map.span_to_snippet(span_of_fragments(fragments)?).ok()?;
|
||||
|
||||
let starting_line = markdown[..md_range.start].matches('\n').count();
|
||||
let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count();
|
||||
|
|
|
|||
|
|
@ -18,12 +18,15 @@ use crate::html::markdown::main_body_opts;
|
|||
|
||||
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));
|
||||
let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings);
|
||||
let sp = maybe_sp.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(
|
||||
.note("bare URLs are not automatically turned into clickable links");
|
||||
// The fallback of using the attribute span is suitable for
|
||||
// highlighting where the error is, but not for placing the < and >
|
||||
if let Some(sp) = maybe_sp {
|
||||
lint.multipart_suggestion(
|
||||
"use an automatic link instead",
|
||||
vec![
|
||||
(sp.shrink_to_lo(), "<".to_string()),
|
||||
|
|
@ -31,6 +34,7 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &
|
|||
],
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -69,29 +69,19 @@ LL | bar [BarC] bar
|
|||
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
|
||||
|
||||
warning: unresolved link to `BarD`
|
||||
--> $DIR/warning.rs:45:9
|
||||
--> $DIR/warning.rs:45:20
|
||||
|
|
||||
LL | #[doc = "Foo\nbar [BarD] bar\nbaz"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^ no item named `BarD` in scope
|
||||
|
|
||||
= note: the link appears in this line:
|
||||
|
||||
bar [BarD] bar
|
||||
^^^^
|
||||
= note: no item named `BarD` in scope
|
||||
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
|
||||
|
||||
warning: unresolved link to `BarF`
|
||||
--> $DIR/warning.rs:54:4
|
||||
--> $DIR/warning.rs:54:15
|
||||
|
|
||||
LL | f!("Foo\nbar [BarF] bar\nbaz");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^ no item named `BarF` in scope
|
||||
|
|
||||
= note: the link appears in this line:
|
||||
|
||||
bar [BarF] bar
|
||||
^^^^
|
||||
= note: no item named `BarF` in scope
|
||||
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
|
||||
= note: this warning originates in the macro `f` (in Nightly builds, run with -Z macro-backtrace for more info)
|
||||
|
||||
|
|
@ -112,29 +102,19 @@ LL | * time to introduce a link [error]
|
|||
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
|
||||
|
||||
warning: unresolved link to `error`
|
||||
--> $DIR/warning.rs:68:9
|
||||
--> $DIR/warning.rs:68:23
|
||||
|
|
||||
LL | #[doc = "single line [error]"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^ no item named `error` in scope
|
||||
|
|
||||
= note: the link appears in this line:
|
||||
|
||||
single line [error]
|
||||
^^^^^
|
||||
= note: no item named `error` in scope
|
||||
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
|
||||
|
||||
warning: unresolved link to `error`
|
||||
--> $DIR/warning.rs:71:9
|
||||
--> $DIR/warning.rs:71:41
|
||||
|
|
||||
LL | #[doc = "single line with \"escaping\" [error]"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
| ^^^^^ no item named `error` in scope
|
||||
|
|
||||
= note: the link appears in this line:
|
||||
|
||||
single line with "escaping" [error]
|
||||
^^^^^
|
||||
= note: no item named `error` in scope
|
||||
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
|
||||
|
||||
warning: unresolved link to `error`
|
||||
|
|
|
|||
12
tests/rustdoc-ui/lints/bare-urls-limit.rs
Normal file
12
tests/rustdoc-ui/lints/bare-urls-limit.rs
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
//@ check-fail
|
||||
|
||||
#![deny(rustdoc::bare_urls)]
|
||||
|
||||
// examples of bare urls that are beyond our ability to generate suggestions for
|
||||
|
||||
// this falls through every heuristic in `source_span_for_markdown_range`,
|
||||
// and thus does not get any suggestion.
|
||||
#[doc = "good: <https://example.com/> \n\n"]
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
#[doc = "bad: https://example.com/"]
|
||||
pub fn duplicate_raw() {}
|
||||
18
tests/rustdoc-ui/lints/bare-urls-limit.stderr
Normal file
18
tests/rustdoc-ui/lints/bare-urls-limit.stderr
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
error: this URL is not a hyperlink
|
||||
--> $DIR/bare-urls-limit.rs:9:9
|
||||
|
|
||||
LL | #[doc = "good: <https://example.com/> \n\n"]
|
||||
| _________^
|
||||
LL | |
|
||||
LL | | #[doc = "bad: https://example.com/"]
|
||||
| |___________________________________^
|
||||
|
|
||||
= note: bare URLs are not automatically turned into clickable links
|
||||
note: the lint level is defined here
|
||||
--> $DIR/bare-urls-limit.rs:3:9
|
||||
|
|
||||
LL | #![deny(rustdoc::bare_urls)]
|
||||
| ^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
|
|
@ -38,6 +38,16 @@
|
|||
//~^ ERROR this URL is not a hyperlink
|
||||
pub fn c() {}
|
||||
|
||||
#[doc = "here's a thing: <https://example.com/>"]
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
pub fn f() {}
|
||||
|
||||
/// <https://example.com/sugar>
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
#[doc = "<https://example.com/raw>"]
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
pub fn mixed() {}
|
||||
|
||||
/// <https://somewhere.com>
|
||||
/// [a](http://a.com)
|
||||
/// [b]
|
||||
|
|
|
|||
|
|
@ -38,6 +38,16 @@
|
|||
//~^ ERROR this URL is not a hyperlink
|
||||
pub fn c() {}
|
||||
|
||||
#[doc = "here's a thing: https://example.com/"]
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
pub fn f() {}
|
||||
|
||||
/// https://example.com/sugar
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
#[doc = "https://example.com/raw"]
|
||||
//~^ ERROR this URL is not a hyperlink
|
||||
pub fn mixed() {}
|
||||
|
||||
/// <https://somewhere.com>
|
||||
/// [a](http://a.com)
|
||||
/// [b]
|
||||
|
|
|
|||
|
|
@ -207,5 +207,41 @@ help: use an automatic link instead
|
|||
LL | /// hey! <https://somewhere.com/a?hello=12&bye=11#xyz>
|
||||
| + +
|
||||
|
||||
error: aborting due to 17 previous errors
|
||||
error: this URL is not a hyperlink
|
||||
--> $DIR/bare-urls.rs:41:26
|
||||
|
|
||||
LL | #[doc = "here's a thing: https://example.com/"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: bare URLs are not automatically turned into clickable links
|
||||
help: use an automatic link instead
|
||||
|
|
||||
LL | #[doc = "here's a thing: <https://example.com/>"]
|
||||
| + +
|
||||
|
||||
error: this URL is not a hyperlink
|
||||
--> $DIR/bare-urls.rs:45:5
|
||||
|
|
||||
LL | /// https://example.com/sugar
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: bare URLs are not automatically turned into clickable links
|
||||
help: use an automatic link instead
|
||||
|
|
||||
LL | /// <https://example.com/sugar>
|
||||
| + +
|
||||
|
||||
error: this URL is not a hyperlink
|
||||
--> $DIR/bare-urls.rs:47:10
|
||||
|
|
||||
LL | #[doc = "https://example.com/raw"]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: bare URLs are not automatically turned into clickable links
|
||||
help: use an automatic link instead
|
||||
|
|
||||
LL | #[doc = "<https://example.com/raw>"]
|
||||
| + +
|
||||
|
||||
error: aborting due to 20 previous errors
|
||||
|
||||
|
|
|
|||
|
|
@ -628,10 +628,10 @@ LL | /// or even to add a number `n` to 42 (`add(42, n)\`)!
|
|||
| +
|
||||
|
||||
error: unescaped backtick
|
||||
--> $DIR/unescaped_backticks.rs:108:9
|
||||
--> $DIR/unescaped_backticks.rs:108:10
|
||||
|
|
||||
LL | #[doc = "`"]
|
||||
| ^^^
|
||||
| ^
|
||||
|
|
||||
= help: the opening or closing backtick of an inline code may be missing
|
||||
= help: if you meant to use a literal backtick, escape it
|
||||
|
|
@ -639,10 +639,10 @@ LL | #[doc = "`"]
|
|||
to this: \`
|
||||
|
||||
error: unescaped backtick
|
||||
--> $DIR/unescaped_backticks.rs:115:9
|
||||
--> $DIR/unescaped_backticks.rs:115:26
|
||||
|
|
||||
LL | #[doc = concat!("\\", "`")]
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
| ^
|
||||
|
|
||||
= help: the opening backtick of an inline code may be missing
|
||||
change: \`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue