Skip to content

Commit f0fbfe2

Browse files
committed
Rollup merge of #56010 - euclio:intra-doc-spans, r=QuietMisdreavus
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.
2 parents 7021f15 + ea5843c commit f0fbfe2

File tree

4 files changed

+140
-50
lines changed

4 files changed

+140
-50
lines changed

src/librustdoc/clean/mod.rs

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

src/librustdoc/passes/collect_intra_doc_links.rs

+57-24
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,19 @@ pub fn span_of_attrs(attrs: &Attributes) -> syntax_pos::Span {
460460
start.to(end)
461461
}
462462

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

473-
let code_dox = sp.to_src(cx);
474-
475-
let doc_comment_padding = 3;
476486
let mut diag = if let Some(link_range) = link_range {
477-
// blah blah blah\nblah\nblah [blah] blah blah\nblah blah
478-
// ^ ~~~~~~
479-
// | link_range
480-
// last_new_line_offset
481-
482487
let mut diag;
483-
if dox.lines().count() == code_dox.lines().count() {
484-
let line_offset = dox[..link_range.start].lines().count();
485-
// The span starts in the `///`, so we don't have to account for the leading whitespace
486-
let code_dox_len = if line_offset <= 1 {
487-
doc_comment_padding
488-
} else {
489-
// The first `///`
490-
doc_comment_padding +
491-
// Each subsequent leading whitespace and `///`
492-
code_dox.lines().skip(1).take(line_offset - 1).fold(0, |sum, line| {
493-
sum + doc_comment_padding + line.len() - line.trim_start().len()
494-
})
495-
};
496488

497-
// Extract the specific span
489+
if attrs.doc_strings.iter().all(|frag| match frag {
490+
DocFragment::SugaredDoc(..) => true,
491+
_ => false,
492+
}) {
493+
let source_dox = sp.to_src(cx);
494+
let mut source_lines = source_dox.lines().peekable();
495+
let mut md_lines = dox.lines().peekable();
496+
497+
// The number of bytes from the start of the source span to the resolution failure that
498+
// are *not* part of the markdown, like comment markers.
499+
let mut source_offset = 0;
500+
501+
// Eat any source lines before the markdown starts (e.g., `/**` on its own line).
502+
while let Some(source_line) = source_lines.peek() {
503+
if source_line.contains(md_lines.peek().unwrap()) {
504+
break;
505+
}
506+
507+
// Include the newline.
508+
source_offset += source_line.len() + 1;
509+
source_lines.next().unwrap();
510+
}
511+
512+
// The number of lines up to and including the resolution failure.
513+
let num_lines = dox[..link_range.start].lines().count();
514+
515+
// Consume inner comment markers (e.g., `///` or ` *`).
516+
for (source_line, md_line) in source_lines.zip(md_lines).take(num_lines) {
517+
source_offset += if md_line.is_empty() {
518+
// If there is no markdown on this line, then the whole line is a comment
519+
// marker. We don't have to count the newline here because it's in the markdown
520+
// too.
521+
source_line.len()
522+
} else {
523+
source_line.find(md_line).unwrap()
524+
};
525+
}
526+
498527
let sp = sp.from_inner_byte_pos(
499-
link_range.start + code_dox_len,
500-
link_range.end + code_dox_len,
528+
link_range.start + source_offset,
529+
link_range.end + source_offset,
501530
);
502531

503532
diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
@@ -511,6 +540,10 @@ fn resolution_failure(
511540
sp,
512541
&msg);
513542

543+
// blah blah blah\nblah\nblah [blah] blah blah\nblah blah
544+
// ^ ~~~~
545+
// | link_range
546+
// last_new_line_offset
514547
let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1);
515548
let line = dox[last_new_line_offset..].lines().next().unwrap_or("");
516549

src/test/rustdoc-ui/intra-links-warning.rs

+23
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,26 @@ 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;

src/test/rustdoc-ui/intra-links-warning.stderr

+60-24
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,60 @@ 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+
58112
warning: `[BarA]` cannot be resolved, ignoring it...
59113
--> $DIR/intra-links-warning.rs:24:10
60114
|
@@ -64,37 +118,19 @@ LL | /// bar [BarA] bar
64118
= help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
65119

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

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

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

0 commit comments

Comments
 (0)