rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.23k stars 12.7k forks source link

Add ability to gate doctests on cfgs #93083

Open Manishearth opened 2 years ago

Manishearth commented 2 years ago

Currently, if a doctest uses specific cargo features, you have one of two options for dealing with it:

(A) You can decide to always use --all-features when running cargo test. This is not great: if you want to test specific features you cannot do so. and also not all crates work with --all-features since features may be mutually exclusive.

(B) You can do the following:

/// 
/// ```
/// # #[cfg(feature = "foo)]
/// # {
///  ... doctest goes here ...
/// # }
/// ```

Here, you're conditionally gating the test on the cfg. This works, but it has a major downside: the test will be reported as passing (as opposed to "ignored") in the cargo test output, which is misleading and not at all the behavior you want around tests. It's also hacky boilerplate that is necessary for a rather common task.

rustdoc has access to cfg-parsing, it should be possible for it to compile-time-ignore tests based on a cfg() spec put on top of the code block.

mqudsi commented 2 years ago

There's an alternative approach C that might (or might not!) be considered cleaner, which is to manually lift (just a few) doc comments to doc attributes then left them again via cfg_attr to be contingent on the presence/absence of a feature.

The approach I have been using to include a doctest if feature std is present and exclude it otherwise:

/// This is the documentation for `foo`.
///
/// An example (that's also validated via doctests):
#[cfg_attr(not(feature = "std"), doc = "```ignore")]
#[cfg_attr(feature = "std", doc = "```")]
/// use size::Size;
///
/// // Create a strongly-typed size object. We don't even need to specify a numeric type!
/// let file1_size = Size::from_bytes(200);
/// // Create another Size instance, this time from a floating-point literal:
/// let file2_size = Size::from_kb(20.1);
/// ```
fn foo() {}

which has the added benefit of cargo test reporting that the test was skipped if it wasn't run.

It works well enough by reusing the existing constructs that I would say there's no need to implement any new features if it weren't for the fact that this runs afoul of tools that operate on the source code as-is before any macro/attribute expansions or evaluations; see for example how this approach breaks rustfmt (while @Manishearth's approach B from above doesn't).

Kixunil commented 2 years ago

@mqudsi cool trick! rustfmt issue seems to be good enough reason to support gating in language though.