-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[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
base: master
Are you sure you want to change the base?
Changes from all commits
5b0e218
9888eb4
7447a70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ | |||||
use serde::Serialize; | ||||||
|
||||||
use super::make::DocTestWrapResult; | ||||||
use super::{BuildDocTestBuilder, ScrapedDocTest}; | ||||||
use crate::config::Options as RustdocOptions; | ||||||
use crate::html::markdown; | ||||||
|
@@ -14,7 +15,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 { | ||||||
|
@@ -42,7 +43,7 @@ impl ExtractedDocTests { | |||||
.edition(edition) | ||||||
.lang_str(&langstr) | ||||||
.build(None); | ||||||
let (full_test_code, size) = doctest.generate_unique_doctest( | ||||||
let (wrapper, _size) = doctest.generate_unique_doctest( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
&text, | ||||||
langstr.test_harness, | ||||||
opts, | ||||||
|
@@ -52,21 +53,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 wrapper { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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, | ||||||
}); | ||||||
} | ||||||
} | ||||||
|
||||||
#[derive(Serialize)] | ||||||
pub(crate) struct WrapperInfo { | ||||||
before: String, | ||||||
after: String, | ||||||
returns_result: bool, | ||||||
} | ||||||
|
||||||
#[derive(Serialize)] | ||||||
pub(crate) struct DocTest { | ||||||
crate_level: String, | ||||||
code: String, | ||||||
wrapper: Option<WrapperInfo>, | ||||||
} | ||||||
|
||||||
#[derive(Serialize)] | ||||||
pub(crate) struct ExtractedDocTest { | ||||||
file: String, | ||||||
line: usize, | ||||||
doctest_attributes: LangString, | ||||||
original_code: String, | ||||||
/// `None` if the code syntax is invalid. | ||||||
doctest_code: Option<String>, | ||||||
doctest_code: Option<DocTest>, | ||||||
name: String, | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,6 +184,73 @@ 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, | ||
wrapper: Option<WrapperInfo>, | ||
code: String, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure If we do rename it, we should probably also rename any local variables whose name is derived from this due to the struct pattern sugar, such as in |
||
}, | ||
/// Contains the original source code. | ||
SyntaxError(String), | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
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( | ||
crate_attrs: String, | ||
|
@@ -210,53 +277,53 @@ impl DocTestBuilder { | |
/// lines before the test code begins. | ||
pub(crate) fn generate_unique_doctest( | ||
&self, | ||
test_code: &str, | ||
doctest_code: &str, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: in |
||
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); | ||
debug!("invalid AST:\n{doctest_code}"); | ||
return (DocTestWrapResult::SyntaxError(doctest_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().to_string(); | ||
if opts.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 the crate root via #![doc(test(attr(...)))]. | ||
for attr in &opts.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'); | ||
} | ||
} | ||
|
||
|
@@ -270,21 +337,24 @@ impl DocTestBuilder { | |
// NOTE: this is terribly inaccurate because it doesn't actually | ||
// parse the source, but only has false positives, not false | ||
// negatives. | ||
test_code.contains(crate_name) | ||
doctest_code.contains(crate_name) | ||
{ | ||
// 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 { | ||
|
@@ -318,28 +388,15 @@ 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, wrapper, crate_level_code }, line_offset) | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,9 +17,9 @@ fn make_test( | |
builder = builder.test_id(test_id.to_string()); | ||
} | ||
let doctest = builder.build(None); | ||
let (code, line_offset) = | ||
let (wrapper, line_offset) = | ||
doctest.generate_unique_doctest(test_code, dont_insert_main, opts, crate_name); | ||
(code, line_offset) | ||
(wrapper.to_string(), line_offset) | ||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: should s/wrapper/wrapped/, same reasoning as before. |
||
} | ||
|
||
/// Default [`GlobalTestOptions`] for these unit tests. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// Test to ensure that it generates expected output for `--output-format=doctest` command-line | ||
// flag. | ||
|
||
//@ compile-flags:-Z unstable-options --output-format=doctest | ||
//@ normalize-stdout: "tests/rustdoc-ui" -> "$$DIR" | ||
//@ check-pass | ||
|
||
//! ``` | ||
//! let x = 12; | ||
//! Ok(()) | ||
//! ``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{"format_version":2,"doctests":[{"file":"$DIR/extract-doctests-result.rs","line":8,"doctest_attributes":{"original":"","should_panic":false,"no_run":false,"ignore":"None","rust":true,"test_harness":false,"compile_fail":false,"standalone_crate":false,"error_codes":[],"edition":null,"added_css_classes":[],"unknown":[]},"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}},"name":"$DIR/extract-doctests-result.rs - (line 8)"}]} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
{"format_version":1,"doctests":[{"file":"$DIR/extract-doctests.rs","line":8,"doctest_attributes":{"original":"ignore (checking attributes)","should_panic":false,"no_run":false,"ignore":"All","rust":true,"test_harness":false,"compile_fail":false,"standalone_crate":false,"error_codes":[],"edition":null,"added_css_classes":[],"unknown":[]},"original_code":"let x = 12;\nlet y = 14;","doctest_code":"#![allow(unused)]\nfn main() {\nlet x = 12;\nlet y = 14;\n}","name":"$DIR/extract-doctests.rs - (line 8)"},{"file":"$DIR/extract-doctests.rs","line":13,"doctest_attributes":{"original":"edition2018,compile_fail","should_panic":false,"no_run":true,"ignore":"None","rust":true,"test_harness":false,"compile_fail":true,"standalone_crate":false,"error_codes":[],"edition":"2018","added_css_classes":[],"unknown":[]},"original_code":"let","doctest_code":null,"name":"$DIR/extract-doctests.rs - (line 13)"}]} | ||
{"format_version":2,"doctests":[{"file":"$DIR/extract-doctests.rs","line":8,"doctest_attributes":{"original":"ignore (checking attributes)","should_panic":false,"no_run":false,"ignore":"All","rust":true,"test_harness":false,"compile_fail":false,"standalone_crate":false,"error_codes":[],"edition":null,"added_css_classes":[],"unknown":[]},"original_code":"let x = 12;\nlet y = 14;","doctest_code":{"crate_level":"#![allow(unused)]\n","code":"let x = 12;\nlet y = 14;","wrapper":{"before":"fn main() {\n","after":"\n}","returns_result":false}},"name":"$DIR/extract-doctests.rs - (line 8)"},{"file":"$DIR/extract-doctests.rs","line":13,"doctest_attributes":{"original":"edition2018,compile_fail","should_panic":false,"no_run":true,"ignore":"None","rust":true,"test_harness":false,"compile_fail":true,"standalone_crate":false,"error_codes":[],"edition":"2018","added_css_classes":[],"unknown":[]},"original_code":"let","doctest_code":null,"name":"$DIR/extract-doctests.rs - (line 13)"}]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: s/wrapper/wrapped/, same reasoning as before