Skip to content

Commit 75eefd1

Browse files
borsbors
committed
Auto merge of #136400 - lolbinarycat:rustdoc-link-lint-135851, r=<try>
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 #135851
2 parents 79de6c0 + c6d7696 commit 75eefd1

File tree

9 files changed

+146
-39
lines changed

9 files changed

+146
-39
lines changed

‎compiler/rustc_resolve/src/rustdoc.rs‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,20 +514,25 @@ pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {
514514
/// This method does not always work, because markdown bytes don't necessarily match source bytes,
515515
/// like if escapes are used in the string. In this case, it returns `None`.
516516
///
517-
/// This method will return `Some` only if:
517+
/// `markdown` is typically the entire documentation for an item,
518+
/// after combining fragments.
519+
///
520+
/// This method will return `Some` only if one of the following is true:
518521
///
519522
/// - The doc is made entirely from sugared doc comments, which cannot contain escapes
520523
/// - The doc is entirely from a single doc fragment, with a string literal, exactly equal
521524
/// - The doc comes from `include_str!`
525+
/// - The doc includes exactly one substring matching `markdown[md_range]` which is contained in a single doc fragment.
522526
pub fn source_span_for_markdown_range(
523527
tcx: TyCtxt<'_>,
524528
markdown: &str,
525529
md_range: &Range<usize>,
526530
fragments: &[DocFragment],
527531
) -> Option<Span> {
532+
let span_to_snippet = |span| tcx.sess.source_map().span_to_snippet(span);
528533
if let &[fragment] = &fragments
529534
&& fragment.kind == DocFragmentKind::RawDoc
530-
&& let Ok(snippet) = tcx.sess.source_map().span_to_snippet(fragment.span)
535+
&& let Ok(snippet) = span_to_snippet(fragment.span)
531536
&& snippet.trim_end() == markdown.trim_end()
532537
&& let Ok(md_range_lo) = u32::try_from(md_range.start)
533538
&& let Ok(md_range_hi) = u32::try_from(md_range.end)
@@ -544,6 +549,38 @@ pub fn source_span_for_markdown_range(
544549
let is_all_sugared_doc = fragments.iter().all(|frag| frag.kind == DocFragmentKind::SugaredDoc);
545550

546551
if !is_all_sugared_doc {
552+
// this case ignores the markdown outside of the range so that it can
553+
// work in cases where the markdown is made from several different
554+
// doc fragments, but the target range does not span across multiple
555+
// fragments.
556+
let mut match_data = None;
557+
let pat = &markdown[md_range.clone()];
558+
// this heirustic doesn't make sense with a zero-sized range.
559+
if pat.len() == 0 {
560+
return None;
561+
}
562+
for (i, fragment) in fragments.iter().enumerate() {
563+
if let Ok(snippet) = span_to_snippet(fragment.span)
564+
&& let Some(match_start) = snippet.find(pat)
565+
{
566+
// if there is either a match in a previous fragment,
567+
// or multiple matches in this fragment,
568+
// there is ambiguity.
569+
if match_data.is_none() && !snippet[match_start + 1..].contains(pat) {
570+
match_data = Some((i, match_start));
571+
} else {
572+
// heirustic produced ambiguity, return nothing.
573+
return None;
574+
}
575+
}
576+
}
577+
if let Some((i, match_start)) = match_data {
578+
use rustc_span::BytePos;
579+
let mut sp = fragments[i].span;
580+
sp = sp.with_lo(sp.lo() + BytePos(match_start as u32));
581+
sp = sp.with_hi(sp.lo() + BytePos((md_range.end - md_range.start) as u32));
582+
return Some(sp);
583+
}
547584
return None;
548585
}
549586

‎src/librustdoc/passes/lint/bare_urls.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,23 @@ use crate::html::markdown::main_body_opts;
1818

1919
pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) {
2020
let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>| {
21-
let sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings)
22-
.unwrap_or_else(|| item.attr_span(cx.tcx));
21+
let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings);
22+
let sp = maybe_sp.unwrap_or_else(|| item.attr_span(cx.tcx));
2323
cx.tcx.node_span_lint(crate::lint::BARE_URLS, hir_id, sp, |lint| {
2424
lint.primary_message(msg)
25-
.note("bare URLs are not automatically turned into clickable links")
26-
.multipart_suggestion(
25+
.note("bare URLs are not automatically turned into clickable links");
26+
// the fallback of using the item span is suitible for
27+
// highlighting where the error is, but not for placing the < and >
28+
if let Some(sp) = maybe_sp {
29+
lint.multipart_suggestion(
2730
"use an automatic link instead",
2831
vec![
2932
(sp.shrink_to_lo(), "<".to_string()),
3033
(sp.shrink_to_hi(), ">".to_string()),
3134
],
3235
Applicability::MachineApplicable,
3336
);
37+
}
3438
});
3539
};
3640

‎tests/rustdoc-ui/intra-doc/warning.stderr‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -69,29 +69,19 @@ LL | bar [BarC] bar
6969
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
7070

7171
warning: unresolved link to `BarD`
72-
--> $DIR/warning.rs:45:9
72+
--> $DIR/warning.rs:45:20
7373
|
7474
LL | #[doc = "Foo\nbar [BarD] bar\nbaz"]
75-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
75+
| ^^^^ no item named `BarD` in scope
7676
|
77-
= note: the link appears in this line:
78-
79-
bar [BarD] bar
80-
^^^^
81-
= note: no item named `BarD` in scope
8277
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
8378

8479
warning: unresolved link to `BarF`
85-
--> $DIR/warning.rs:54:4
80+
--> $DIR/warning.rs:54:15
8681
|
8782
LL | f!("Foo\nbar [BarF] bar\nbaz");
88-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
83+
| ^^^^ no item named `BarF` in scope
8984
|
90-
= note: the link appears in this line:
91-
92-
bar [BarF] bar
93-
^^^^
94-
= note: no item named `BarF` in scope
9585
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
9686
= note: this warning originates in the macro `f` (in Nightly builds, run with -Z macro-backtrace for more info)
9787

@@ -112,29 +102,19 @@ LL | * time to introduce a link [error]
112102
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
113103

114104
warning: unresolved link to `error`
115-
--> $DIR/warning.rs:68:9
105+
--> $DIR/warning.rs:68:23
116106
|
117107
LL | #[doc = "single line [error]"]
118-
| ^^^^^^^^^^^^^^^^^^^^^
108+
| ^^^^^ no item named `error` in scope
119109
|
120-
= note: the link appears in this line:
121-
122-
single line [error]
123-
^^^^^
124-
= note: no item named `error` in scope
125110
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
126111

127112
warning: unresolved link to `error`
128-
--> $DIR/warning.rs:71:9
113+
--> $DIR/warning.rs:71:41
129114
|
130115
LL | #[doc = "single line with \"escaping\" [error]"]
131-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
116+
| ^^^^^ no item named `error` in scope
132117
|
133-
= note: the link appears in this line:
134-
135-
single line with "escaping" [error]
136-
^^^^^
137-
= note: no item named `error` in scope
138118
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
139119

140120
warning: unresolved link to `error`

‎tests/rustdoc-ui/lints/bare-urls-limit.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//@ check-fail
2+
3+
#![deny(rustdoc::bare_urls)]
4+
5+
// examples of bare urls that are beyond our ability to generate suggestions for
6+
7+
// this falls through every heirustic in `source_span_for_markdown_range`,
8+
// and thus does not get any suggestion.
9+
#[doc = "good: <https://example.com/> \n\n"]
10+
//~^ ERROR this URL is not a hyperlink
11+
#[doc = "bad: https://example.com/"]
12+
pub fn duplicate_raw() {}

‎tests/rustdoc-ui/lints/bare-urls-limit.stderr‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error: this URL is not a hyperlink
2+
--> $DIR/bare-urls-limit.rs:9:9
3+
|
4+
LL | #[doc = "good: <https://example.com/> \n\n"]
5+
| _________^
6+
LL | |
7+
LL | | #[doc = "bad: https://example.com/"]
8+
| |___________________________________^
9+
|
10+
= note: bare URLs are not automatically turned into clickable links
11+
note: the lint level is defined here
12+
--> $DIR/bare-urls-limit.rs:3:9
13+
|
14+
LL | #![deny(rustdoc::bare_urls)]
15+
| ^^^^^^^^^^^^^^^^^^
16+
17+
error: aborting due to 1 previous error
18+

‎tests/rustdoc-ui/lints/bare-urls.fixed‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@
3838
//~^ ERROR this URL is not a hyperlink
3939
pub fn c() {}
4040

41+
#[doc = "here's a thing: <https://example.com/>"]
42+
//~^ ERROR this URL is not a hyperlink
43+
pub fn f() {}
44+
45+
/// <https://example.com/sugar>
46+
//~^ ERROR this URL is not a hyperlink
47+
#[doc = "<https://example.com/raw>"]
48+
//~^ ERROR this URL is not a hyperlink
49+
pub fn mixed() {}
50+
4151
/// <https://somewhere.com>
4252
/// [a](http://a.com)
4353
/// [b]

‎tests/rustdoc-ui/lints/bare-urls.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@
3838
//~^ ERROR this URL is not a hyperlink
3939
pub fn c() {}
4040

41+
#[doc = "here's a thing: https://example.com/"]
42+
//~^ ERROR this URL is not a hyperlink
43+
pub fn f() {}
44+
45+
/// https://example.com/sugar
46+
//~^ ERROR this URL is not a hyperlink
47+
#[doc = "https://example.com/raw"]
48+
//~^ ERROR this URL is not a hyperlink
49+
pub fn mixed() {}
50+
4151
/// <https://somewhere.com>
4252
/// [a](http://a.com)
4353
/// [b]

‎tests/rustdoc-ui/lints/bare-urls.stderr‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,5 +207,41 @@ help: use an automatic link instead
207207
LL | /// hey! <https://somewhere.com/a?hello=12&bye=11#xyz>
208208
| + +
209209

210-
error: aborting due to 17 previous errors
210+
error: this URL is not a hyperlink
211+
--> $DIR/bare-urls.rs:41:26
212+
|
213+
LL | #[doc = "here's a thing: https://example.com/"]
214+
| ^^^^^^^^^^^^^^^^^^^^
215+
|
216+
= note: bare URLs are not automatically turned into clickable links
217+
help: use an automatic link instead
218+
|
219+
LL | #[doc = "here's a thing: <https://example.com/>"]
220+
| + +
221+
222+
error: this URL is not a hyperlink
223+
--> $DIR/bare-urls.rs:45:5
224+
|
225+
LL | /// https://example.com/sugar
226+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
227+
|
228+
= note: bare URLs are not automatically turned into clickable links
229+
help: use an automatic link instead
230+
|
231+
LL | /// <https://example.com/sugar>
232+
| + +
233+
234+
error: this URL is not a hyperlink
235+
--> $DIR/bare-urls.rs:47:10
236+
|
237+
LL | #[doc = "https://example.com/raw"]
238+
| ^^^^^^^^^^^^^^^^^^^^^^^
239+
|
240+
= note: bare URLs are not automatically turned into clickable links
241+
help: use an automatic link instead
242+
|
243+
LL | #[doc = "<https://example.com/raw>"]
244+
| + +
245+
246+
error: aborting due to 20 previous errors
211247

‎tests/rustdoc-ui/unescaped_backticks.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -628,21 +628,21 @@ LL | /// or even to add a number `n` to 42 (`add(42, n)\`)!
628628
| +
629629

630630
error: unescaped backtick
631-
--> $DIR/unescaped_backticks.rs:108:9
631+
--> $DIR/unescaped_backticks.rs:108:10
632632
|
633633
LL | #[doc = "`"]
634-
| ^^^
634+
| ^
635635
|
636636
= help: the opening or closing backtick of an inline code may be missing
637637
= help: if you meant to use a literal backtick, escape it
638638
change: `
639639
to this: \`
640640

641641
error: unescaped backtick
642-
--> $DIR/unescaped_backticks.rs:115:9
642+
--> $DIR/unescaped_backticks.rs:115:26
643643
|
644644
LL | #[doc = concat!("\\", "`")]
645-
| ^^^^^^^^^^^^^^^^^^^^
645+
| ^
646646
|
647647
= help: the opening backtick of an inline code may be missing
648648
change: \`

0 commit comments

Comments
 (0)