Skip to content

Commit

Permalink
rustdoc: point at span in include_str!-ed md file
Browse files Browse the repository at this point in the history
  • Loading branch information
notriddle committed Mar 29, 2024
1 parent d748046 commit 98642da
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 42 deletions.
10 changes: 6 additions & 4 deletions compiler/rustc_builtin_macros/src/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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)
}
Expand All @@ -238,7 +240,7 @@ fn load_binary_file(
original_path: &Path,
macro_span: Span,
path_span: Span,
) -> Result<Lrc<[u8]>, Box<dyn MacResult>> {
) -> Result<(Lrc<[u8]>, Span), Box<dyn MacResult>> {
let resolved_path = match resolve_path(&cx.sess, original_path, macro_span) {
Ok(path) => path,
Err(err) => {
Expand Down
45 changes: 38 additions & 7 deletions compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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.
///
Expand Down Expand Up @@ -482,15 +492,36 @@ pub fn span_of_fragments(fragments: &[DocFragment]) -> Option<Span> {

/// 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<usize>,
fragments: &[DocFragment],
) -> Option<Span> {
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 {
Expand Down
14 changes: 11 additions & 3 deletions compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ impl SourceMap {
///
/// Unlike `load_file`, guarantees that no normalization like BOM-removal
/// takes place.
pub fn load_binary_file(&self, path: &Path) -> io::Result<Lrc<[u8]>> {
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
Expand All @@ -227,8 +227,16 @@ impl SourceMap {
// 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
Expand Down
10 changes: 10 additions & 0 deletions tests/rustdoc-ui/auxiliary/include-str-bare-urls.md
Original file line number Diff line number Diff line change
@@ -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!
15 changes: 15 additions & 0 deletions tests/rustdoc-ui/include-str-bare-urls.rs
Original file line number Diff line number Diff line change
@@ -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")]
15 changes: 15 additions & 0 deletions tests/rustdoc-ui/include-str-bare-urls.stderr
Original file line number Diff line number Diff line change
@@ -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: `<https://example.com>`
|
= 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

4 changes: 2 additions & 2 deletions tests/rustdoc-ui/intra-doc/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
*
Expand Down
19 changes: 8 additions & 11 deletions tests/rustdoc-ui/intra-doc/warning.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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:

Expand All @@ -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:

Expand All @@ -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:

Expand Down
7 changes: 4 additions & 3 deletions tests/rustdoc-ui/invalid-syntax.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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: \
Expand Down
24 changes: 12 additions & 12 deletions tests/rustdoc-ui/unescaped_backticks.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -640,21 +640,21 @@ 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
change: `
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: \`
Expand All @@ -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)`.
Expand All @@ -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)`.
Expand All @@ -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)`.
Expand All @@ -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).
Expand Down

0 comments on commit 98642da

Please sign in to comment.