Skip to content

Commit 56413ec

Browse files
euclioeuclio
committed
fix intra-link resolution spans in block comments
This commit improves the calculation of code spans for intra-doc resolution failures. All sugared doc comments should now have the correct spans, including those where the comment is longer than the docs. It also fixes an issue where the spans were calculated incorrectly for certain unsugared doc comments. The diagnostic will now always use the span of the attributes, as originally intended. Fixes #55964.
1 parent b755501 commit 56413ec

7 files changed

+238
-60
lines changed

Diff for: ‎src/librustdoc/clean/mod.rs

-2
Original file line numberDiff line numberDiff line change
@@ -707,8 +707,6 @@ impl<I: IntoIterator<Item=ast::NestedMetaItem>> NestedAttributesExt for I {
707707
/// kept separate because of issue #42760.
708708
#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Debug, Hash)]
709709
pub enum DocFragment {
710-
// FIXME #44229 (misdreavus): sugared and raw doc comments can be brought back together once
711-
// hoedown is completely removed from rustdoc.
712710
/// A doc fragment created from a `///` or `//!` doc comment.
713711
SugaredDoc(usize, syntax_pos::Span, String),
714712
/// A doc fragment created from a "raw" `#[doc=""]` attribute.

Diff for: ‎src/librustdoc/passes/collect_intra_doc_links.rs

+75-34
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,19 @@ pub fn span_of_attrs(attrs: &Attributes) -> syntax_pos::Span {
459459
start.to(end)
460460
}
461461

462+
/// Reports a resolution failure diagnostic.
463+
///
464+
/// Ideally we can report the diagnostic with the actual span in the source where the link failure
465+
/// occurred. However, there's a mismatch between the span in the source code and the span in the
466+
/// markdown, so we have to do a bit of work to figure out the correspondence.
467+
///
468+
/// It's not too hard to find the span for sugared doc comments (`///` and `/**`), because the
469+
/// source will match the markdown exactly, excluding the comment markers. However, it's much more
470+
/// difficult to calculate the spans for unsugared docs, because we have to deal with escaping and
471+
/// other source features. So, we attempt to find the exact source span of the resolution failure
472+
/// in sugared docs, but use the span of the documentation attributes themselves for unsugared
473+
/// docs. Because this span might be overly large, we display the markdown line containing the
474+
/// failure as a note.
462475
fn resolution_failure(
463476
cx: &DocContext,
464477
attrs: &Attributes,
@@ -469,47 +482,75 @@ fn resolution_failure(
469482
let sp = span_of_attrs(attrs);
470483
let msg = format!("`[{}]` cannot be resolved, ignoring it...", path_str);
471484

472-
let code_dox = sp.to_src(cx);
473-
474-
let doc_comment_padding = 3;
475485
let mut diag = if let Some(link_range) = link_range {
476-
// blah blah blah\nblah\nblah [blah] blah blah\nblah blah
477-
// ^ ~~~~~~
478-
// | link_range
479-
// last_new_line_offset
480-
481-
let mut diag;
482-
if dox.lines().count() == code_dox.lines().count() {
483-
let line_offset = dox[..link_range.start].lines().count();
484-
// The span starts in the `///`, so we don't have to account for the leading whitespace.
485-
let code_dox_len = if line_offset <= 1 {
486-
doc_comment_padding
487-
} else {
488-
// The first `///`.
489-
doc_comment_padding +
490-
// Each subsequent leading whitespace and `///`.
491-
code_dox.lines().skip(1).take(line_offset - 1).fold(0, |sum, line| {
492-
sum + doc_comment_padding + line.len() - line.trim_start().len()
493-
})
494-
};
486+
let src = cx.sess().source_map().span_to_snippet(sp);
487+
let is_all_sugared_doc = attrs.doc_strings.iter().all(|frag| match frag {
488+
DocFragment::SugaredDoc(..) => true,
489+
_ => false,
490+
});
491+
492+
if let (Ok(src), true) = (src, is_all_sugared_doc) {
493+
// The number of markdown lines up to and including the resolution failure.
494+
let num_lines = dox[..link_range.start].lines().count();
495+
496+
// We use `split_terminator('\n')` instead of `lines()` when counting bytes to ensure
497+
// that DOS-style line endings do not cause the spans to be calculated incorrectly.
498+
let mut src_lines = src.split_terminator('\n');
499+
let mut md_lines = dox.split_terminator('\n').take(num_lines).peekable();
500+
501+
// The number of bytes from the start of the source span to the resolution failure that
502+
// are *not* part of the markdown, like comment markers.
503+
let mut extra_src_bytes = 0;
504+
505+
while let Some(md_line) = md_lines.next() {
506+
loop {
507+
let source_line = src_lines
508+
.next()
509+
.expect("could not find markdown line in source");
510+
511+
match source_line.find(md_line) {
512+
Some(offset) => {
513+
extra_src_bytes += if md_lines.peek().is_some() {
514+
source_line.len() - md_line.len()
515+
} else {
516+
offset
517+
};
518+
break;
519+
}
520+
None => {
521+
// Since this is a source line that doesn't include a markdown line,
522+
// we have to count the newline that we split from earlier.
523+
extra_src_bytes += source_line.len() + 1;
524+
}
525+
}
526+
}
527+
}
495528

496-
// Extract the specific span.
497529
let sp = sp.from_inner_byte_pos(
498-
link_range.start + code_dox_len,
499-
link_range.end + code_dox_len,
530+
link_range.start + extra_src_bytes,
531+
link_range.end + extra_src_bytes,
500532
);
501533

502-
diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
503-
NodeId::from_u32(0),
504-
sp,
505-
&msg);
534+
let mut diag = cx.tcx.struct_span_lint_node(
535+
lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
536+
NodeId::from_u32(0),
537+
sp,
538+
&msg,
539+
);
506540
diag.span_label(sp, "cannot be resolved, ignoring");
541+
diag
507542
} else {
508-
diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
509-
NodeId::from_u32(0),
510-
sp,
511-
&msg);
543+
let mut diag = cx.tcx.struct_span_lint_node(
544+
lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
545+
NodeId::from_u32(0),
546+
sp,
547+
&msg,
548+
);
512549

550+
// blah blah blah\nblah\nblah [blah] blah blah\nblah blah
551+
// ^ ~~~~
552+
// | link_range
553+
// last_new_line_offset
513554
let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1);
514555
let line = dox[last_new_line_offset..].lines().next().unwrap_or("");
515556

@@ -522,8 +563,8 @@ fn resolution_failure(
522563
before=link_range.start - last_new_line_offset,
523564
found=link_range.len(),
524565
));
566+
diag
525567
}
526-
diag
527568
} else {
528569
cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
529570
NodeId::from_u32(0),

Diff for: ‎src/test/rustdoc-ui/.gitattributes

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
intra-links-warning-crlf.rs eol=crlf

Diff for: ‎src/test/rustdoc-ui/intra-links-warning-crlf.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// ignore-tidy-cr
2+
3+
// compile-pass
4+
5+
// This file checks the spans of intra-link warnings in a file with CRLF line endings. The
6+
// .gitattributes file in this directory should enforce it.
7+
8+
/// [error]
9+
pub struct A;
10+
11+
///
12+
/// docs [error1]
13+
14+
/// docs [error2]
15+
///
16+
pub struct B;
17+
18+
/**
19+
* This is a multi-line comment.
20+
*
21+
* It also has an [error].
22+
*/
23+
pub struct C;

Diff for: ‎src/test/rustdoc-ui/intra-links-warning-crlf.stderr

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
warning: `[error]` cannot be resolved, ignoring it...
2+
--> $DIR/intra-links-warning-crlf.rs:8:6
3+
|
4+
LL | /// [error]
5+
| ^^^^^ cannot be resolved, ignoring
6+
|
7+
= note: #[warn(intra_doc_link_resolution_failure)] on by default
8+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
9+
10+
warning: `[error1]` cannot be resolved, ignoring it...
11+
--> $DIR/intra-links-warning-crlf.rs:12:11
12+
|
13+
LL | /// docs [error1]
14+
| ^^^^^^ cannot be resolved, ignoring
15+
|
16+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
17+
18+
warning: `[error2]` cannot be resolved, ignoring it...
19+
--> $DIR/intra-links-warning-crlf.rs:14:11
20+
|
21+
LL | /// docs [error2]
22+
| ^^^^^^ cannot be resolved, ignoring
23+
|
24+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
25+
26+
warning: `[error]` cannot be resolved, ignoring it...
27+
--> $DIR/intra-links-warning-crlf.rs:21:20
28+
|
29+
LL | * It also has an [error].
30+
| ^^^^^ cannot be resolved, ignoring
31+
|
32+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
33+

Diff for: ‎src/test/rustdoc-ui/intra-links-warning.rs

+30
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,33 @@ macro_rules! f {
5555
}
5656
}
5757
f!("Foo\nbar [BarF] bar\nbaz");
58+
59+
/** # for example,
60+
*
61+
* time to introduce a link [error]*/
62+
pub struct A;
63+
64+
/**
65+
* # for example,
66+
*
67+
* time to introduce a link [error]
68+
*/
69+
pub struct B;
70+
71+
#[doc = "single line [error]"]
72+
pub struct C;
73+
74+
#[doc = "single line with \"escaping\" [error]"]
75+
pub struct D;
76+
77+
/// Item docs.
78+
#[doc="Hello there!"]
79+
/// [error]
80+
pub struct E;
81+
82+
///
83+
/// docs [error1]
84+
85+
/// docs [error2]
86+
///
87+
pub struct F;

Diff for: ‎src/test/rustdoc-ui/intra-links-warning.stderr

+76-24
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,76 @@ LL | /// [Qux:Y]
5555
|
5656
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
5757

58+
warning: `[error]` cannot be resolved, ignoring it...
59+
--> $DIR/intra-links-warning.rs:61:30
60+
|
61+
LL | * time to introduce a link [error]*/
62+
| ^^^^^ cannot be resolved, ignoring
63+
|
64+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
65+
66+
warning: `[error]` cannot be resolved, ignoring it...
67+
--> $DIR/intra-links-warning.rs:67:30
68+
|
69+
LL | * time to introduce a link [error]
70+
| ^^^^^ cannot be resolved, ignoring
71+
|
72+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
73+
74+
warning: `[error]` cannot be resolved, ignoring it...
75+
--> $DIR/intra-links-warning.rs:71:1
76+
|
77+
LL | #[doc = "single line [error]"]
78+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
79+
|
80+
= note: the link appears in this line:
81+
82+
single line [error]
83+
^^^^^
84+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
85+
86+
warning: `[error]` cannot be resolved, ignoring it...
87+
--> $DIR/intra-links-warning.rs:74:1
88+
|
89+
LL | #[doc = "single line with /"escaping/" [error]"]
90+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
91+
|
92+
= note: the link appears in this line:
93+
94+
single line with "escaping" [error]
95+
^^^^^
96+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
97+
98+
warning: `[error]` cannot be resolved, ignoring it...
99+
--> $DIR/intra-links-warning.rs:77:1
100+
|
101+
LL | / /// Item docs.
102+
LL | | #[doc="Hello there!"]
103+
LL | | /// [error]
104+
| |___________^
105+
|
106+
= note: the link appears in this line:
107+
108+
[error]
109+
^^^^^
110+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
111+
112+
warning: `[error1]` cannot be resolved, ignoring it...
113+
--> $DIR/intra-links-warning.rs:83:11
114+
|
115+
LL | /// docs [error1]
116+
| ^^^^^^ cannot be resolved, ignoring
117+
|
118+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
119+
120+
warning: `[error2]` cannot be resolved, ignoring it...
121+
--> $DIR/intra-links-warning.rs:85:11
122+
|
123+
LL | /// docs [error2]
124+
| ^^^^^^ cannot be resolved, ignoring
125+
|
126+
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
127+
58128
warning: `[BarA]` cannot be resolved, ignoring it...
59129
--> $DIR/intra-links-warning.rs:24:10
60130
|
@@ -64,37 +134,19 @@ LL | /// bar [BarA] bar
64134
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
65135

66136
warning: `[BarB]` cannot be resolved, ignoring it...
67-
--> $DIR/intra-links-warning.rs:28:1
137+
--> $DIR/intra-links-warning.rs:30:9
68138
|
69-
LL | / /**
70-
LL | | * Foo
71-
LL | | * bar [BarB] bar
72-
LL | | * baz
73-
LL | | */
74-
| |___^
139+
LL | * bar [BarB] bar
140+
| ^^^^ cannot be resolved, ignoring
75141
|
76-
= note: the link appears in this line:
77-
78-
bar [BarB] bar
79-
^^^^
80142
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
81143

82144
warning: `[BarC]` cannot be resolved, ignoring it...
83-
--> $DIR/intra-links-warning.rs:35:1
145+
--> $DIR/intra-links-warning.rs:37:6
84146
|
85-
LL | / /** Foo
86-
LL | |
87-
LL | | bar [BarC] bar
88-
LL | | baz
89-
... |
90-
LL | |
91-
LL | | */
92-
| |__^
147+
LL | bar [BarC] bar
148+
| ^^^^ cannot be resolved, ignoring
93149
|
94-
= note: the link appears in this line:
95-
96-
bar [BarC] bar
97-
^^^^
98150
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
99151

100152
warning: `[BarD]` cannot be resolved, ignoring it...

0 commit comments

Comments
 (0)