From 98642da6a9dda39e711b7f3520b0b6d40c1cd043 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 29 Mar 2024 12:31:34 -0700 Subject: [PATCH] rustdoc: point at span in `include_str!`-ed md file --- .../rustc_builtin_macros/src/source_util.rs | 10 +++-- compiler/rustc_resolve/src/rustdoc.rs | 45 ++++++++++++++++--- compiler/rustc_span/src/source_map.rs | 14 ++++-- .../auxiliary/include-str-bare-urls.md | 10 +++++ tests/rustdoc-ui/include-str-bare-urls.rs | 15 +++++++ tests/rustdoc-ui/include-str-bare-urls.stderr | 15 +++++++ tests/rustdoc-ui/intra-doc/warning.rs | 4 +- tests/rustdoc-ui/intra-doc/warning.stderr | 19 ++++---- tests/rustdoc-ui/invalid-syntax.stderr | 7 +-- tests/rustdoc-ui/unescaped_backticks.stderr | 24 +++++----- 10 files changed, 121 insertions(+), 42 deletions(-) create mode 100644 tests/rustdoc-ui/auxiliary/include-str-bare-urls.md create mode 100644 tests/rustdoc-ui/include-str-bare-urls.rs create mode 100644 tests/rustdoc-ui/include-str-bare-urls.stderr diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index abcdfabcaed..51975568981 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -196,10 +196,10 @@ pub fn expand_include_str( Err(guar) => return ExpandResult::Ready(DummyResult::any(sp, guar)), }; ExpandResult::Ready(match load_binary_file(cx, path.as_str().as_ref(), sp, path_span) { - Ok(bytes) => match std::str::from_utf8(&bytes) { + Ok((bytes, bsp)) => match std::str::from_utf8(&bytes) { Ok(src) => { let interned_src = Symbol::intern(src); - MacEager::expr(cx.expr_str(sp, interned_src)) + MacEager::expr(cx.expr_str(cx.with_def_site_ctxt(bsp), interned_src)) } Err(_) => { let guar = cx.dcx().span_err(sp, format!("`{path}` wasn't a utf-8 file")); @@ -225,7 +225,9 @@ pub fn expand_include_bytes( Err(guar) => return ExpandResult::Ready(DummyResult::any(sp, guar)), }; ExpandResult::Ready(match load_binary_file(cx, path.as_str().as_ref(), sp, path_span) { - Ok(bytes) => { + Ok((bytes, _bsp)) => { + // Don't care about getting the span for the raw bytes, + // because the console can't really show them anyway. let expr = cx.expr(sp, ast::ExprKind::IncludedBytes(bytes)); MacEager::expr(expr) } @@ -238,7 +240,7 @@ fn load_binary_file( original_path: &Path, macro_span: Span, path_span: Span, -) -> Result, Box> { +) -> Result<(Lrc<[u8]>, Span), Box> { let resolved_path = match resolve_path(&cx.sess, original_path, macro_span) { Ok(path) => path, Err(err) => { diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index 0ebcad3cdb8..0bc7579918c 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -194,12 +194,12 @@ pub fn attrs_to_doc_fragments<'a>( for (attr, item_id) in attrs { if let Some((doc_str, comment_kind)) = attr.doc_str_and_comment_kind() { let doc = beautify_doc_string(doc_str, comment_kind); - let kind = if attr.is_doc_comment() { - DocFragmentKind::SugaredDoc + let (span, kind) = if attr.is_doc_comment() { + (attr.span, DocFragmentKind::SugaredDoc) } else { - DocFragmentKind::RawDoc + (span_for_value(attr), DocFragmentKind::RawDoc) }; - let fragment = DocFragment { span: attr.span, doc, kind, item_id, indent: 0 }; + let fragment = DocFragment { span, doc, kind, item_id, indent: 0 }; doc_fragments.push(fragment); } else if !doc_only { other_attrs.push(attr.clone()); @@ -211,6 +211,16 @@ pub fn attrs_to_doc_fragments<'a>( (doc_fragments, other_attrs) } +fn span_for_value(attr: &ast::Attribute) -> Span { + if let ast::AttrKind::Normal(normal) = &attr.kind + && let ast::AttrArgs::Eq(_, ast::AttrArgsEq::Hir(meta)) = &normal.item.args + { + meta.span.with_ctxt(attr.span.ctxt()) + } else { + attr.span + } +} + /// Return the doc-comments on this item, grouped by the module they came from. /// The module can be different if this is a re-export with added documentation. /// @@ -482,15 +492,36 @@ pub fn span_of_fragments(fragments: &[DocFragment]) -> Option { /// Attempts to match a range of bytes from parsed markdown to a `Span` in the source code. /// -/// This method will return `None` if we cannot construct a span from the source map or if the -/// fragments are not all sugared doc comments. It's difficult to calculate the correct span in -/// that case due to escaping and other source features. +/// This method does not always work, because markdown bytes don't necessarily match source bytes, +/// like if escapes are used in the string. In this case, it returns `None`. +/// +/// This method will return `Some` only if: +/// +/// - The doc is made entirely from sugared doc comments, which cannot contain escapes +/// - The doc is entirely from a single doc fragment, with a string literal, exactly equal +/// - The doc comes from `include_str!` pub fn source_span_for_markdown_range( tcx: TyCtxt<'_>, markdown: &str, md_range: &Range, fragments: &[DocFragment], ) -> Option { + if let &[fragment] = &fragments + && fragment.kind == DocFragmentKind::RawDoc + && let Ok(snippet) = tcx.sess.source_map().span_to_snippet(fragment.span) + && snippet.trim_end() == markdown.trim_end() + && let Ok(md_range_lo) = u32::try_from(md_range.start) + && let Ok(md_range_hi) = u32::try_from(md_range.end) + { + // Single fragment with string that contains same bytes as doc. + return Some(Span::new( + fragment.span.lo() + rustc_span::BytePos(md_range_lo), + fragment.span.lo() + rustc_span::BytePos(md_range_hi), + fragment.span.ctxt(), + fragment.span.parent(), + )); + } + let is_all_sugared_doc = fragments.iter().all(|frag| frag.kind == DocFragmentKind::SugaredDoc); if !is_all_sugared_doc { diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index df7635e447d..cac4d6064e5 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -218,7 +218,7 @@ pub fn load_file(&self, path: &Path) -> io::Result> { /// /// Unlike `load_file`, guarantees that no normalization like BOM-removal /// takes place. - pub fn load_binary_file(&self, path: &Path) -> io::Result> { + pub fn load_binary_file(&self, path: &Path) -> io::Result<(Lrc<[u8]>, Span)> { let bytes = self.file_loader.read_binary_file(path)?; // We need to add file to the `SourceMap`, so that it is present @@ -227,8 +227,16 @@ pub fn load_binary_file(&self, path: &Path) -> io::Result> { // via `mod`, so we try to use real file contents and not just an // empty string. let text = std::str::from_utf8(&bytes).unwrap_or("").to_string(); - self.new_source_file(path.to_owned().into(), text); - Ok(bytes) + let file = self.new_source_file(path.to_owned().into(), text); + Ok(( + bytes, + Span::new( + file.start_pos, + BytePos(file.start_pos.0 + file.source_len.0), + SyntaxContext::root(), + None, + ), + )) } // By returning a `MonotonicVec`, we ensure that consumers cannot invalidate diff --git a/tests/rustdoc-ui/auxiliary/include-str-bare-urls.md b/tests/rustdoc-ui/auxiliary/include-str-bare-urls.md new file mode 100644 index 00000000000..b07717d8f02 --- /dev/null +++ b/tests/rustdoc-ui/auxiliary/include-str-bare-urls.md @@ -0,0 +1,10 @@ +HEADS UP! https://example.com MUST SHOW UP IN THE STDERR FILE! + +Normally, a line with errors on it will also have a comment +marking it up as something that needs to generate an error. + +The test harness doesn't gather hot comments from this file. +Rustdoc will generate an error for the line, and the `.stderr` +snapshot includes this error, but Compiletest doesn't see it. + +If the stderr file changes, make sure the warning points at the URL! diff --git a/tests/rustdoc-ui/include-str-bare-urls.rs b/tests/rustdoc-ui/include-str-bare-urls.rs new file mode 100644 index 00000000000..c452c88cdd3 --- /dev/null +++ b/tests/rustdoc-ui/include-str-bare-urls.rs @@ -0,0 +1,15 @@ +// https://github.com/rust-lang/rust/issues/118549 +// +// HEADS UP! +// +// Normally, a line with errors on it will also have a comment +// marking it up as something that needs to generate an error. +// +// The test harness doesn't gather hot comments from the `.md` file. +// Rustdoc will generate an error for the line, and the `.stderr` +// snapshot includes this error, but Compiletest doesn't see it. +// +// If the stderr file changes, make sure the warning points at the URL! + +#![deny(rustdoc::bare_urls)] +#![doc=include_str!("auxiliary/include-str-bare-urls.md")] diff --git a/tests/rustdoc-ui/include-str-bare-urls.stderr b/tests/rustdoc-ui/include-str-bare-urls.stderr new file mode 100644 index 00000000000..a4234196b23 --- /dev/null +++ b/tests/rustdoc-ui/include-str-bare-urls.stderr @@ -0,0 +1,15 @@ +error: this URL is not a hyperlink + --> $DIR/auxiliary/include-str-bare-urls.md:1:11 + | +LL | HEADS UP! https://example.com MUST SHOW UP IN THE STDERR FILE! + | ^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` + | + = note: bare URLs are not automatically turned into clickable links +note: the lint level is defined here + --> $DIR/include-str-bare-urls.rs:14:9 + | +LL | #![deny(rustdoc::bare_urls)] + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/rustdoc-ui/intra-doc/warning.rs b/tests/rustdoc-ui/intra-doc/warning.rs index 96b5c2b36a1..ed51b2fa41b 100644 --- a/tests/rustdoc-ui/intra-doc/warning.rs +++ b/tests/rustdoc-ui/intra-doc/warning.rs @@ -47,11 +47,11 @@ pub fn d() {} macro_rules! f { ($f:expr) => { - #[doc = $f] //~ WARNING `BarF` + #[doc = $f] pub fn f() {} } } -f!("Foo\nbar [BarF] bar\nbaz"); +f!("Foo\nbar [BarF] bar\nbaz"); //~ WARNING `BarF` /** # for example, * diff --git a/tests/rustdoc-ui/intra-doc/warning.stderr b/tests/rustdoc-ui/intra-doc/warning.stderr index 19399a0df5b..3a06f1787e0 100644 --- a/tests/rustdoc-ui/intra-doc/warning.stderr +++ b/tests/rustdoc-ui/intra-doc/warning.stderr @@ -69,10 +69,10 @@ LL | bar [BarC] bar = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarD` - --> $DIR/warning.rs:45:1 + --> $DIR/warning.rs:45:9 | LL | #[doc = "Foo\nbar [BarD] bar\nbaz"] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: the link appears in this line: @@ -82,13 +82,10 @@ LL | #[doc = "Foo\nbar [BarD] bar\nbaz"] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `BarF` - --> $DIR/warning.rs:50:9 + --> $DIR/warning.rs:54:4 | -LL | #[doc = $f] - | ^^^^^^^^^^^ -... LL | f!("Foo\nbar [BarF] bar\nbaz"); - | ------------------------------ in this macro invocation + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: the link appears in this line: @@ -115,10 +112,10 @@ LL | * time to introduce a link [error] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` - --> $DIR/warning.rs:68:1 + --> $DIR/warning.rs:68:9 | LL | #[doc = "single line [error]"] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^ | = note: the link appears in this line: @@ -128,10 +125,10 @@ LL | #[doc = "single line [error]"] = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `error` - --> $DIR/warning.rs:71:1 + --> $DIR/warning.rs:71:9 | LL | #[doc = "single line with \"escaping\" [error]"] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: the link appears in this line: diff --git a/tests/rustdoc-ui/invalid-syntax.stderr b/tests/rustdoc-ui/invalid-syntax.stderr index 6140a06c555..46d7cdb4f7e 100644 --- a/tests/rustdoc-ui/invalid-syntax.stderr +++ b/tests/rustdoc-ui/invalid-syntax.stderr @@ -90,12 +90,13 @@ LL | | /// ``` = note: error from rustc: unknown start of token: \ warning: could not parse code block as Rust code - --> $DIR/invalid-syntax.rs:70:1 + --> $DIR/invalid-syntax.rs:70:9 | -LL | / #[doc = "```"] +LL | #[doc = "```"] + | _________^ LL | | /// \_ LL | | #[doc = "```"] - | |______________^ + | |_____________^ | = help: mark blocks that do not contain Rust code as text: ```text = note: error from rustc: unknown start of token: \ diff --git a/tests/rustdoc-ui/unescaped_backticks.stderr b/tests/rustdoc-ui/unescaped_backticks.stderr index 000a5b597d2..67b87f353a1 100644 --- a/tests/rustdoc-ui/unescaped_backticks.stderr +++ b/tests/rustdoc-ui/unescaped_backticks.stderr @@ -640,10 +640,10 @@ LL | /// or even to add a number `n` to 42 (`add(42, n)\`)! | + error: unescaped backtick - --> $DIR/unescaped_backticks.rs:108:1 + --> $DIR/unescaped_backticks.rs:108:9 | LL | #[doc = "`"] - | ^^^^^^^^^^^^ + | ^^^ | = help: the opening or closing backtick of an inline code may be missing = help: if you meant to use a literal backtick, escape it @@ -651,10 +651,10 @@ LL | #[doc = "`"] to this: \` error: unescaped backtick - --> $DIR/unescaped_backticks.rs:115:1 + --> $DIR/unescaped_backticks.rs:115:9 | LL | #[doc = concat!("\\", "`")] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^ | = help: the opening backtick of an inline code may be missing change: \` @@ -664,10 +664,10 @@ LL | #[doc = concat!("\\", "`")] to this: \\` error: unescaped backtick - --> $DIR/unescaped_backticks.rs:119:1 + --> $DIR/unescaped_backticks.rs:119:9 | LL | #[doc = "Addition is commutative, which means that add(a, b)` is the same as `add(b, a)`."] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: the opening backtick of a previous inline code may be missing change: Addition is commutative, which means that add(a, b)` is the same as `add(b, a)`. @@ -677,10 +677,10 @@ LL | #[doc = "Addition is commutative, which means that add(a, b)` is the same a to this: Addition is commutative, which means that add(a, b)` is the same as `add(b, a)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:123:1 + --> $DIR/unescaped_backticks.rs:123:9 | LL | #[doc = "Addition is commutative, which means that `add(a, b) is the same as `add(b, a)`."] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: a previous inline code might be longer than expected change: Addition is commutative, which means that `add(a, b) is the same as `add(b, a)`. @@ -690,10 +690,10 @@ LL | #[doc = "Addition is commutative, which means that `add(a, b) is the same a to this: Addition is commutative, which means that `add(a, b) is the same as `add(b, a)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:127:1 + --> $DIR/unescaped_backticks.rs:127:9 | LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same as add(b, a)`."] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: the opening backtick of an inline code may be missing change: Addition is commutative, which means that `add(a, b)` is the same as add(b, a)`. @@ -703,10 +703,10 @@ LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same to this: Addition is commutative, which means that `add(a, b)` is the same as add(b, a)\`. error: unescaped backtick - --> $DIR/unescaped_backticks.rs:131:1 + --> $DIR/unescaped_backticks.rs:131:9 | LL | #[doc = "Addition is commutative, which means that `add(a, b)` is the same as `add(b, a)."] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: the closing backtick of an inline code may be missing change: Addition is commutative, which means that `add(a, b)` is the same as `add(b, a).