From ea5843cc7781dd572679c69fd008bfe71279b03d Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Fri, 16 Nov 2018 14:30:52 -0500 Subject: [PATCH] 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. --- src/librustdoc/clean/mod.rs | 2 - .../passes/collect_intra_doc_links.rs | 81 ++++++++++++------ src/test/rustdoc-ui/intra-links-warning.rs | 23 +++++ .../rustdoc-ui/intra-links-warning.stderr | 84 +++++++++++++------ 4 files changed, 140 insertions(+), 50 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 0518d73e1e30f..50b17f59b47d9 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -708,8 +708,6 @@ impl> NestedAttributesExt for I { /// kept separate because of issue #42760. #[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Debug, Hash)] pub enum DocFragment { - // FIXME #44229 (misdreavus): sugared and raw doc comments can be brought back together once - // hoedown is completely removed from rustdoc. /// A doc fragment created from a `///` or `//!` doc comment. SugaredDoc(usize, syntax_pos::Span, String), /// A doc fragment created from a "raw" `#[doc=""]` attribute. diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f5056cead567e..31047ab353a47 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -497,6 +497,19 @@ fn span_of_attrs(attrs: &Attributes) -> syntax_pos::Span { start.to(end) } +/// Reports a resolution failure diagnostic. +/// +/// Ideally we can report the diagnostic with the actual span in the source where the link failure +/// occurred. However, there's a mismatch between the span in the source code and the span in the +/// markdown, so we have to do a bit of work to figure out the correspondence. +/// +/// It's not too hard to find the span for sugared doc comments (`///` and `/**`), because the +/// source will match the markdown exactly, excluding the comment markers. However, it's much more +/// difficult to calculate the spans for unsugared docs, because we have to deal with escaping and +/// other source features. So, we attempt to find the exact source span of the resolution failure +/// in sugared docs, but use the span of the documentation attributes themselves for unsugared +/// docs. Because this span might be overly large, we display the markdown line containing the +/// failure as a note. fn resolution_failure( cx: &DocContext, attrs: &Attributes, @@ -507,34 +520,50 @@ fn resolution_failure( let sp = span_of_attrs(attrs); let msg = format!("`[{}]` cannot be resolved, ignoring it...", path_str); - let code_dox = sp.to_src(cx); - - let doc_comment_padding = 3; let mut diag = if let Some(link_range) = link_range { - // blah blah blah\nblah\nblah [blah] blah blah\nblah blah - // ^ ~~~~~~ - // | link_range - // last_new_line_offset - let mut diag; - if dox.lines().count() == code_dox.lines().count() { - let line_offset = dox[..link_range.start].lines().count(); - // The span starts in the `///`, so we don't have to account for the leading whitespace - let code_dox_len = if line_offset <= 1 { - doc_comment_padding - } else { - // The first `///` - doc_comment_padding + - // Each subsequent leading whitespace and `///` - code_dox.lines().skip(1).take(line_offset - 1).fold(0, |sum, line| { - sum + doc_comment_padding + line.len() - line.trim_start().len() - }) - }; - // Extract the specific span + if attrs.doc_strings.iter().all(|frag| match frag { + DocFragment::SugaredDoc(..) => true, + _ => false, + }) { + let source_dox = sp.to_src(cx); + let mut source_lines = source_dox.lines().peekable(); + let mut md_lines = dox.lines().peekable(); + + // The number of bytes from the start of the source span to the resolution failure that + // are *not* part of the markdown, like comment markers. + let mut source_offset = 0; + + // Eat any source lines before the markdown starts (e.g., `/**` on its own line). + while let Some(source_line) = source_lines.peek() { + if source_line.contains(md_lines.peek().unwrap()) { + break; + } + + // Include the newline. + source_offset += source_line.len() + 1; + source_lines.next().unwrap(); + } + + // The number of lines up to and including the resolution failure. + let num_lines = dox[..link_range.start].lines().count(); + + // Consume inner comment markers (e.g., `///` or ` *`). + for (source_line, md_line) in source_lines.zip(md_lines).take(num_lines) { + source_offset += if md_line.is_empty() { + // If there is no markdown on this line, then the whole line is a comment + // marker. We don't have to count the newline here because it's in the markdown + // too. + source_line.len() + } else { + source_line.find(md_line).unwrap() + }; + } + let sp = sp.from_inner_byte_pos( - link_range.start + code_dox_len, - link_range.end + code_dox_len, + link_range.start + source_offset, + link_range.end + source_offset, ); diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE, @@ -548,6 +577,10 @@ fn resolution_failure( sp, &msg); + // blah blah blah\nblah\nblah [blah] blah blah\nblah blah + // ^ ~~~~ + // | link_range + // last_new_line_offset let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1); let line = dox[last_new_line_offset..].lines().next().unwrap_or(""); diff --git a/src/test/rustdoc-ui/intra-links-warning.rs b/src/test/rustdoc-ui/intra-links-warning.rs index d6bc275b57a6a..3051bec88d24d 100644 --- a/src/test/rustdoc-ui/intra-links-warning.rs +++ b/src/test/rustdoc-ui/intra-links-warning.rs @@ -55,3 +55,26 @@ macro_rules! f { } } f!("Foo\nbar [BarF] bar\nbaz"); + +/** # for example, + * + * time to introduce a link [error]*/ +pub struct A; + +/** + * # for example, + * + * time to introduce a link [error] + */ +pub struct B; + +#[doc = "single line [error]"] +pub struct C; + +#[doc = "single line with \"escaping\" [error]"] +pub struct D; + +/// Item docs. +#[doc="Hello there!"] +/// [error] +pub struct E; diff --git a/src/test/rustdoc-ui/intra-links-warning.stderr b/src/test/rustdoc-ui/intra-links-warning.stderr index c05f99fadc9e4..543919938cea7 100644 --- a/src/test/rustdoc-ui/intra-links-warning.stderr +++ b/src/test/rustdoc-ui/intra-links-warning.stderr @@ -55,6 +55,60 @@ LL | /// [Qux:Y] | = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` +warning: `[error]` cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:61:30 + | +LL | * time to introduce a link [error]*/ + | ^^^^^ cannot be resolved, ignoring + | + = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` + +warning: `[error]` cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:67:30 + | +LL | * time to introduce a link [error] + | ^^^^^ cannot be resolved, ignoring + | + = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` + +warning: `[error]` cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:71:1 + | +LL | #[doc = "single line [error]"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the link appears in this line: + + single line [error] + ^^^^^ + = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` + +warning: `[error]` cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:74:1 + | +LL | #[doc = "single line with /"escaping/" [error]"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the link appears in this line: + + single line with "escaping" [error] + ^^^^^ + = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` + +warning: `[error]` cannot be resolved, ignoring it... + --> $DIR/intra-links-warning.rs:77:1 + | +LL | / /// Item docs. +LL | | #[doc="Hello there!"] +LL | | /// [error] + | |___________^ + | + = note: the link appears in this line: + + [error] + ^^^^^ + = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` + warning: `[BarA]` cannot be resolved, ignoring it... --> $DIR/intra-links-warning.rs:24:10 | @@ -64,37 +118,19 @@ LL | /// bar [BarA] bar = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` warning: `[BarB]` cannot be resolved, ignoring it... - --> $DIR/intra-links-warning.rs:28:1 + --> $DIR/intra-links-warning.rs:30:9 | -LL | / /** -LL | | * Foo -LL | | * bar [BarB] bar -LL | | * baz -LL | | */ - | |___^ +LL | * bar [BarB] bar + | ^^^^ cannot be resolved, ignoring | - = note: the link appears in this line: - - bar [BarB] bar - ^^^^ = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` warning: `[BarC]` cannot be resolved, ignoring it... - --> $DIR/intra-links-warning.rs:35:1 + --> $DIR/intra-links-warning.rs:37:6 | -LL | / /** Foo -LL | | -LL | | bar [BarC] bar -LL | | baz -... | -LL | | -LL | | */ - | |__^ +LL | bar [BarC] bar + | ^^^^ cannot be resolved, ignoring | - = note: the link appears in this line: - - bar [BarC] bar - ^^^^ = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]` warning: `[BarD]` cannot be resolved, ignoring it...