-
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
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 | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -184,6 +184,64 @@ 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 DocTestWrapper { | ||||||||
Valid { crate_level_code: String, wrapper: Option<WrapperInfo>, code: String }, | ||||||||
SyntaxError(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.
Suggested change
It wasn't immediately obvious that the I assume we do this partially for compile_fail tests and also because rustc provides better diagnostics? |
||||||||
} | ||||||||
|
||||||||
impl std::string::ToString for DocTestWrapper { | ||||||||
fn to_string(&self) -> String { | ||||||||
match self { | ||||||||
Self::SyntaxError(s) => s.clone(), | ||||||||
Self::Valid { crate_level_code, wrapper, code } => { | ||||||||
let prog_len = code.len() | ||||||||
+ crate_level_code.len() | ||||||||
+ wrapper.as_ref().map(|w| w.len()).unwrap_or(0); | ||||||||
Comment on lines
+212
to
+214
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. If we're doing all this to estimate capacity, should we also do |
||||||||
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, | ||||||||
|
@@ -214,49 +272,49 @@ impl DocTestBuilder { | |||||||
dont_insert_main: bool, | ||||||||
opts: &GlobalTestOptions, | ||||||||
crate_name: Option<&str>, | ||||||||
) -> (String, usize) { | ||||||||
) -> (DocTestWrapper, 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 (DocTestWrapper::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 code = self.everything_else.trim().to_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. Nit: could we call this |
||||||||
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'); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
|
@@ -274,17 +332,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 = 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 +379,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) | ||||||||
(DocTestWrapper::Valid { code, wrapper, crate_level_code }, line_offset) | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -308,7 +308,8 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> { | |||||||||
builder = builder.crate_name(krate); | ||||||||||
} | ||||||||||
let doctest = builder.build(None); | ||||||||||
let (test, _) = doctest.generate_unique_doctest(&test, false, &opts, krate); | ||||||||||
let (wrapper, _) = doctest.generate_unique_doctest(&test, false, &opts, krate); | ||||||||||
let test = wrapper.to_string(); | ||||||||||
Comment on lines
+311
to
+312
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
Nit: it's not just a wrapper, but the wrapper in combination with the inner code that has been wrapped. we make this distinction in the JSON output, so maybe we should stick to that terminology here. |
||||||||||
let channel = if test.contains("#![feature(") { "&version=nightly" } else { "" }; | ||||||||||
|
||||||||||
let test_escaped = small_url_encode(test); | ||||||||||
|
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.
DocTestWrapper
seems a bit confusing since we already haveDocTest
andWrapperInfo
(and unlikeWrapperInfo
, this also includes the code itself), maybe this could beDocTestWrapResult
as it is the result of wrapping the doctest, and it has an error variant likeResult
?Valid(DocTest)