Skip to content
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

Modify doctest's auto-fn main() to allow Results #56470

Merged
merged 1 commit into from Feb 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/doc/rustdoc/src/documentation-tests.md
Expand Up @@ -236,6 +236,23 @@ appears to the reader as the initial idea but works with doc tests:
/// ```
```

As of version 1.34.0, one can also omit the `fn main()`, but you will have to
disambiguate the error type:

```ignore
/// ```
/// use std::io;
/// let mut input = String::new();
/// io::stdin().read_line(&mut input)?;
/// # Ok::<(), io:Error>(())
Copy link

Choose a reason for hiding this comment

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

Typo: io:Error should be io::Error :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. WIll fix in a follow-up.

/// ```
```

This is an unfortunate consequence of the `?` operator adding an implicit
conversion, so type inference fails because the type is not unique. Please note
that you must write the `(())` in one sequence without intermediate whitespace
so that rustdoc understands you want an implicit `Result`-returning function.

## Documenting macros

Here’s an example of documenting a macro:
Expand Down
14 changes: 10 additions & 4 deletions src/librustdoc/test.rs
Expand Up @@ -470,13 +470,19 @@ pub fn make_test(s: &str,
}
}

if dont_insert_main || already_has_main {
// FIXME: This code cannot yet handle no_std test cases yet
if dont_insert_main || already_has_main || prog.contains("![no_std]") {
prog.push_str(everything_else);
} else {
prog.push_str("fn main() {\n");
let returns_result = everything_else.trim_end().ends_with("(())");
let (main_pre, main_post) = if returns_result {
("fn main() { fn _inner() -> Result<(), impl core::fmt::Debug> {",
"}\n_inner().unwrap() }")
} else {
("fn main() {\n", "\n}")
};
prog.extend([main_pre, everything_else, main_post].iter().cloned());
line_offset += 1;
prog.push_str(everything_else);
prog.push_str("\n}");
}

debug!("final doctest:\n{}", prog);
Expand Down
24 changes: 24 additions & 0 deletions src/test/rustdoc/process-termination.rs
@@ -0,0 +1,24 @@
// compile-flags:--test

Copy link
Member

@ollie27 ollie27 Dec 3, 2018

Choose a reason for hiding this comment

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

Suggested change
// compile-flags:--test

will be needed to actually run the examples as doctests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! Thank you!

/// A check of using various process termination strategies
Copy link
Member

Choose a reason for hiding this comment

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

To actually run any of the doctests in this file it needs to contain // compile-flags:--test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops.

///
/// # Examples
///
/// ```rust
/// assert!(true); // this returns `()`, all is well
/// ```
///
/// You can also simply return `Ok(())`, but you'll need to disambiguate the
/// type using turbofish, because we cannot infer the type:
///
/// ```rust
/// Ok::<(), &'static str>(())
/// ```
///
/// You can err with anything that implements `Debug`:
///
/// ```rust,should_panic
/// Err("This is returned from `main`, leading to panic")?;
Copy link
Member

Choose a reason for hiding this comment

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

I find this use of should_panic confusing, since that's not how #[test]s work: #49909 #48854 (comment) #49911

I think that Err paths should look for Errs directly, and that should_panic tests should only pass if the code block raised a panic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, but how should I test this particular function then? I could replace the should_panic with a no_run, but that would fail to catch a change that swallows the panic.

/// Ok::<(), &'static str>(())
/// ```
pub fn check_process_termination() {}