Auto 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.

r? @QuietMisdreavus

src/librustdoc/clean/mod.rs

@@ -707,8 +707,6 @@ impl<I: IntoIterator<Item=ast::NestedMetaItem>> 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.

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -459,6 +459,19 @@ pub 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,
@@ -469,47 +482,75 @@ 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()
-                    })
-            };
+        let src = cx.sess().source_map().span_to_snippet(sp);
+        let is_all_sugared_doc = attrs.doc_strings.iter().all(|frag| match frag {
+            DocFragment::SugaredDoc(..) => true,
+            _ => false,
+        });
+
+        if let (Ok(src), true) = (src, is_all_sugared_doc) {
+            // The number of markdown lines up to and including the resolution failure.
+            let num_lines = dox[..link_range.start].lines().count();
+
+            // We use `split_terminator('\n')` instead of `lines()` when counting bytes to ensure
+            // that DOS-style line endings do not cause the spans to be calculated incorrectly.
+            let mut src_lines = src.split_terminator('\n');
+            let mut md_lines = dox.split_terminator('\n').take(num_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 extra_src_bytes = 0;
+
+            while let Some(md_line) = md_lines.next() {
+                loop {
+                    let source_line = src_lines
+                        .next()
+                        .expect("could not find markdown line in source");
+
+                    match source_line.find(md_line) {
+                        Some(offset) => {
+                            extra_src_bytes += if md_lines.peek().is_some() {
+                                source_line.len() - md_line.len()
+                            } else {
+                                offset
+                            };
+                            break;
+                        }
+                        None => {
+                            // Since this is a source line that doesn't include a markdown line,
+                            // we have to count the newline that we split from earlier.
+                            extra_src_bytes += source_line.len() + 1;
+                        }
+                    }
+                }
+            }
 
-            // Extract the specific span.
             let sp = sp.from_inner_byte_pos(
-                link_range.start + code_dox_len,
-                link_range.end + code_dox_len,
+                link_range.start + extra_src_bytes,
+                link_range.end + extra_src_bytes,
             );
 
-            diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
-                                                NodeId::from_u32(0),
-                                                sp,
-                                                &msg);
+            let mut diag = cx.tcx.struct_span_lint_node(
+                lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
+                NodeId::from_u32(0),
+                sp,
+                &msg,
+            );
             diag.span_label(sp, "cannot be resolved, ignoring");
+            diag
         } else {
-            diag = cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
-                                                NodeId::from_u32(0),
-                                                sp,
-                                                &msg);
+            let mut diag = cx.tcx.struct_span_lint_node(
+                lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
+                NodeId::from_u32(0),
+                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("");
 
@@ -522,8 +563,8 @@ fn resolution_failure(
                 before=link_range.start - last_new_line_offset,
                 found=link_range.len(),
             ));
+            diag
         }
-        diag
     } else {
         cx.tcx.struct_span_lint_node(lint::builtin::INTRA_DOC_LINK_RESOLUTION_FAILURE,
                                      NodeId::from_u32(0),

src/test/rustdoc-ui/.gitattributes

@@ -0,0 +1 @@
+intra-links-warning-crlf.rs eol=crlf

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

@@ -0,0 +1,23 @@
+// ignore-tidy-cr
+
+// compile-pass
+
+// This file checks the spans of intra-link warnings in a file with CRLF line endings. The
+// .gitattributes file in this directory should enforce it.
+
+/// [error]
+pub struct A;
+
+///
+/// docs [error1]
+
+/// docs [error2]
+///
+pub struct B;
+
+/**
+ * This is a multi-line comment.
+ *
+ * It also has an [error].
+ */
+pub struct C;

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

@@ -0,0 +1,33 @@
+warning: `[error]` cannot be resolved, ignoring it...
+  --> $DIR/intra-links-warning-crlf.rs:8:6
+   |
+LL | /// [error]
+   |      ^^^^^ cannot be resolved, ignoring
+   |
+   = note: #[warn(intra_doc_link_resolution_failure)] on by default
+   = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
+
+warning: `[error1]` cannot be resolved, ignoring it...
+  --> $DIR/intra-links-warning-crlf.rs:12:11
+   |
+LL | /// docs [error1]
+   |           ^^^^^^ cannot be resolved, ignoring
+   |
+   = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
+
+warning: `[error2]` cannot be resolved, ignoring it...
+  --> $DIR/intra-links-warning-crlf.rs:14:11
+   |
+LL | /// docs [error2]
+   |           ^^^^^^ 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-crlf.rs:21:20
+   |
+LL |  * It also has an [error].
+   |                    ^^^^^ cannot be resolved, ignoring
+   |
+   = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
+

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

@@ -55,3 +55,33 @@ 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;
+
+///
+/// docs [error1]
+
+/// docs [error2]
+///
+pub struct F;

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

@@ -55,6 +55,76 @@ 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: `[error1]` cannot be resolved, ignoring it...
+  --> $DIR/intra-links-warning.rs:83:11
+   |
+LL | /// docs [error1]
+   |           ^^^^^^ cannot be resolved, ignoring
+   |
+   = help: to escape `[` and `]` characters, just add '/' before them like `/[` or `/]`
+
+warning: `[error2]` cannot be resolved, ignoring it...
+  --> $DIR/intra-links-warning.rs:85:11
+   |
+LL | /// docs [error2]
+   |           ^^^^^^ cannot be resolved, ignoring
+   |
+   = 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 +134,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...