Skip to content

[rustdoc] Give more information into extracted doctest information #141399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/doc/rustdoc/src/unstable-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ For this rust code:
```rust
/// ```
/// let x = 12;
/// Ok(())
/// ```
pub trait Trait {}
```
Expand All @@ -590,10 +591,10 @@ The generated output (formatted) will look like this:

```json
{
"format_version": 1,
"format_version": 2,
"doctests": [
{
"file": "foo.rs",
"file": "src/lib.rs",
"line": 1,
"doctest_attributes": {
"original": "",
Expand All @@ -609,9 +610,17 @@ The generated output (formatted) will look like this:
"added_css_classes": [],
"unknown": []
},
"original_code": "let x = 12;",
"doctest_code": "#![allow(unused)]\nfn main() {\nlet x = 12;\n}",
"name": "foo.rs - Trait (line 1)"
"original_code": "let x = 12;\nOk(())",
"doctest_code": {
"crate_level": "#![allow(unused)]\n",
"code": "let x = 12;\nOk(())",
"wrapper": {
"before": "fn main() { fn _inner() -> core::result::Result<(), impl core::fmt::Debug> {\n",
"after": "\n} _inner().unwrap() }",
"returns_result": true
}
},
Comment on lines +614 to +622
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we explain these new fields in the docs below?

At least the difference between original_code and code isn't clear (since at least in this example it is the same value).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good point, gonna add explanations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It looks much better. Does "naked" mean anything different from original_code? If yes, perhaps we should mention how it differs (or for what one would use one or the other). If not, should we keep both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me look more in depth if both fields are required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stupidly enough, I just re-read the documentation I wrote to realize the difference: it doesn't have anything crate-level related. So if your doctest has crate-level attribute (#![]) or extern crate items, it won't be in this code... like I wrote in the docs 🤣

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, the test-provided ones, thanks! I think "that will be added" may be understood as the ones added by rustdoc, i.e. only those. An allow attribute or similar in the example could perhaps make it easier to remember those will be moved too.

"name": "src/lib.rs - (line 1)"
}
]
}
Expand All @@ -624,6 +633,10 @@ The generated output (formatted) will look like this:
* `doctest_attributes` contains computed information about the attributes used on the doctests. For more information about doctest attributes, take a look [here](write-documentation/documentation-tests.html#attributes).
* `original_code` is the code as written in the source code before rustdoc modifies it.
* `doctest_code` is the code modified by rustdoc that will be run. If there is a fatal syntax error, this field will not be present.
* `crate_level` is the crate level code (like attributes or `extern crate`) that will be added at the top-level of the generated doctest.
* `code` is "naked" doctest without anything from `crate_level` and `wrapper` content.
* `wrapper` contains extra code that will be added before and after `code`.
* `returns_result` is a boolean. If `true`, it means that the doctest returns a `Result` type.
* `name` is the name generated by rustdoc which represents this doctest.

### html
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,14 +1053,14 @@ fn doctest_run_fn(
let report_unused_externs = |uext| {
unused_externs.lock().unwrap().push(uext);
};
let (full_test_code, full_test_line_offset) = doctest.generate_unique_doctest(
let (wrapped, full_test_line_offset) = doctest.generate_unique_doctest(
&scraped_test.text,
scraped_test.langstr.test_harness,
&global_opts,
Some(&global_opts.crate_name),
);
let runnable_test = RunnableDocTest {
full_test_code,
full_test_code: wrapped.to_string(),
full_test_line_offset,
test_opts,
global_opts,
Expand Down
55 changes: 50 additions & 5 deletions src/librustdoc/doctest/extracted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
//! This module contains the logic to extract doctests and output a JSON containing this
//! information.

use rustc_span::edition::Edition;
use serde::Serialize;

use super::make::DocTestWrapResult;
use super::{BuildDocTestBuilder, ScrapedDocTest};
use crate::config::Options as RustdocOptions;
use crate::html::markdown;
Expand All @@ -14,7 +16,7 @@ use crate::html::markdown;
/// This integer is incremented with every breaking change to the API,
/// and is returned along with the JSON blob into the `format_version` root field.
/// Consuming code should assert that this value matches the format version(s) that it supports.
const FORMAT_VERSION: u32 = 1;
const FORMAT_VERSION: u32 = 2;

#[derive(Serialize)]
pub(crate) struct ExtractedDocTests {
Expand All @@ -34,7 +36,16 @@ impl ExtractedDocTests {
options: &RustdocOptions,
) {
let edition = scraped_test.edition(options);
self.add_test_with_edition(scraped_test, opts, edition)
}

/// This method is used by unit tests to not have to provide a `RustdocOptions`.
pub(crate) fn add_test_with_edition(
&mut self,
scraped_test: ScrapedDocTest,
opts: &super::GlobalTestOptions,
edition: Edition,
) {
let ScrapedDocTest { filename, line, langstr, text, name, global_crate_attrs, .. } =
scraped_test;

Expand All @@ -44,8 +55,7 @@ impl ExtractedDocTests {
.edition(edition)
.lang_str(&langstr)
.build(None);

let (full_test_code, size) = doctest.generate_unique_doctest(
let (wrapped, _size) = doctest.generate_unique_doctest(
&text,
langstr.test_harness,
opts,
Expand All @@ -55,11 +65,46 @@ impl ExtractedDocTests {
file: filename.prefer_remapped_unconditionaly().to_string(),
line,
doctest_attributes: langstr.into(),
doctest_code: if size != 0 { Some(full_test_code) } else { None },
doctest_code: match wrapped {
DocTestWrapResult::Valid { crate_level_code, wrapper, code } => Some(DocTest {
crate_level: crate_level_code,
code,
wrapper: wrapper.map(
|super::make::WrapperInfo { before, after, returns_result, .. }| {
WrapperInfo { before, after, returns_result }
},
),
}),
DocTestWrapResult::SyntaxError { .. } => None,
},
original_code: text,
name,
});
}

#[cfg(test)]
pub(crate) fn doctests(&self) -> &[ExtractedDocTest] {
&self.doctests
}
}

#[derive(Serialize)]
pub(crate) struct WrapperInfo {
before: String,
after: String,
returns_result: bool,
}

#[derive(Serialize)]
pub(crate) struct DocTest {
crate_level: String,
code: String,
/// This field can be `None` if one of the following conditions is true:
///
/// * The doctest's codeblock has the `test_harness` attribute.
/// * The doctest has a `main` function.
/// * The doctest has the `![no_std]` attribute.
pub(crate) wrapper: Option<WrapperInfo>,
}

#[derive(Serialize)]
Expand All @@ -69,7 +114,7 @@ pub(crate) struct ExtractedDocTest {
doctest_attributes: LangString,
original_code: String,
/// `None` if the code syntax is invalid.
doctest_code: Option<String>,
pub(crate) doctest_code: Option<DocTest>,
name: String,
}

Expand Down
148 changes: 109 additions & 39 deletions src/librustdoc/doctest/make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,80 @@ pub(crate) struct DocTestBuilder {
pub(crate) can_be_merged: bool,
}

/// Contains needed information for doctest to be correctly generated with expected "wrapping".
pub(crate) struct WrapperInfo {
pub(crate) before: String,
pub(crate) after: String,
pub(crate) returns_result: bool,
insert_indent_space: bool,
}

impl WrapperInfo {
fn len(&self) -> usize {
self.before.len() + self.after.len()
}
}

/// Contains a doctest information. Can be converted into code with the `to_string()` method.
pub(crate) enum DocTestWrapResult {
Valid {
crate_level_code: String,
/// This field can be `None` if one of the following conditions is true:
///
/// * The doctest's codeblock has the `test_harness` attribute.
/// * The doctest has a `main` function.
/// * The doctest has the `![no_std]` attribute.
wrapper: Option<WrapperInfo>,
/// Contains the doctest processed code without the wrappers (which are stored in the
/// `wrapper` field).
code: String,
},
/// Contains the original source code.
SyntaxError(String),
}

impl std::string::ToString for DocTestWrapResult {
fn to_string(&self) -> String {
match self {
Self::SyntaxError(s) => s.clone(),
Self::Valid { crate_level_code, wrapper, code } => {
let mut prog_len = code.len() + crate_level_code.len();
if let Some(wrapper) = wrapper {
prog_len += wrapper.len();
if wrapper.insert_indent_space {
prog_len += code.lines().count() * 4;
}
}
let mut prog = String::with_capacity(prog_len);

prog.push_str(crate_level_code);
if let Some(wrapper) = wrapper {
prog.push_str(&wrapper.before);

// add extra 4 spaces for each line to offset the code block
if wrapper.insert_indent_space {
write!(
prog,
"{}",
fmt::from_fn(|f| code
.lines()
.map(|line| fmt::from_fn(move |f| write!(f, " {line}")))
.joined("\n", f))
)
.unwrap();
} else {
prog.push_str(code);
}
prog.push_str(&wrapper.after);
} else {
prog.push_str(code);
}
prog
}
}
}
}

impl DocTestBuilder {
fn invalid(
global_crate_attrs: Vec<String>,
Expand Down Expand Up @@ -228,50 +302,49 @@ impl DocTestBuilder {
dont_insert_main: bool,
opts: &GlobalTestOptions,
crate_name: Option<&str>,
) -> (String, usize) {
) -> (DocTestWrapResult, usize) {
if self.invalid_ast {
// If the AST failed to compile, no need to go generate a complete doctest, the error
// will be better this way.
debug!("invalid AST:\n{test_code}");
return (test_code.to_string(), 0);
return (DocTestWrapResult::SyntaxError(test_code.to_string()), 0);
}
let mut line_offset = 0;
let mut prog = String::new();
let everything_else = self.everything_else.trim();

let mut crate_level_code = String::new();
let processed_code = self.everything_else.trim();
if self.global_crate_attrs.is_empty() {
// If there aren't any attributes supplied by #![doc(test(attr(...)))], then allow some
// lints that are commonly triggered in doctests. The crate-level test attributes are
// commonly used to make tests fail in case they trigger warnings, so having this there in
// that case may cause some tests to pass when they shouldn't have.
prog.push_str("#![allow(unused)]\n");
crate_level_code.push_str("#![allow(unused)]\n");
line_offset += 1;
}

// Next, any attributes that came from #![doc(test(attr(...)))].
for attr in &self.global_crate_attrs {
prog.push_str(&format!("#![{attr}]\n"));
crate_level_code.push_str(&format!("#![{attr}]\n"));
line_offset += 1;
}

// Now push any outer attributes from the example, assuming they
// are intended to be crate attributes.
if !self.crate_attrs.is_empty() {
prog.push_str(&self.crate_attrs);
crate_level_code.push_str(&self.crate_attrs);
if !self.crate_attrs.ends_with('\n') {
prog.push('\n');
crate_level_code.push('\n');
}
}
if !self.maybe_crate_attrs.is_empty() {
prog.push_str(&self.maybe_crate_attrs);
crate_level_code.push_str(&self.maybe_crate_attrs);
if !self.maybe_crate_attrs.ends_with('\n') {
prog.push('\n');
crate_level_code.push('\n');
}
}
if !self.crates.is_empty() {
prog.push_str(&self.crates);
crate_level_code.push_str(&self.crates);
if !self.crates.ends_with('\n') {
prog.push('\n');
crate_level_code.push('\n');
}
}

Expand All @@ -289,17 +362,20 @@ impl DocTestBuilder {
{
// rustdoc implicitly inserts an `extern crate` item for the own crate
// which may be unused, so we need to allow the lint.
prog.push_str("#[allow(unused_extern_crates)]\n");
crate_level_code.push_str("#[allow(unused_extern_crates)]\n");

prog.push_str(&format!("extern crate r#{crate_name};\n"));
crate_level_code.push_str(&format!("extern crate r#{crate_name};\n"));
line_offset += 1;
}

// FIXME: This code cannot yet handle no_std test cases yet
if dont_insert_main || self.has_main_fn || prog.contains("![no_std]") {
prog.push_str(everything_else);
let wrapper = if dont_insert_main
|| self.has_main_fn
|| crate_level_code.contains("![no_std]")
{
None
} else {
let returns_result = everything_else.ends_with("(())");
let returns_result = processed_code.ends_with("(())");
// Give each doctest main function a unique name.
// This is for example needed for the tooling around `-C instrument-coverage`.
let inner_fn_name = if let Some(ref test_id) = self.test_id {
Expand Down Expand Up @@ -333,28 +409,22 @@ impl DocTestBuilder {
// /// ``` <- end of the inner main
line_offset += 1;

prog.push_str(&main_pre);

// add extra 4 spaces for each line to offset the code block
if opts.insert_indent_space {
write!(
prog,
"{}",
fmt::from_fn(|f| everything_else
.lines()
.map(|line| fmt::from_fn(move |f| write!(f, " {line}")))
.joined("\n", f))
)
.unwrap();
} else {
prog.push_str(everything_else);
};
prog.push_str(&main_post);
}

debug!("final doctest:\n{prog}");
Some(WrapperInfo {
before: main_pre,
after: main_post,
returns_result,
insert_indent_space: opts.insert_indent_space,
})
};

(prog, line_offset)
(
DocTestWrapResult::Valid {
code: processed_code.to_string(),
wrapper,
crate_level_code,
},
line_offset,
)
}
}

Expand Down
Loading
Loading