rustdoc: Experiment: Inject doctests into the host crate

[fmease]

From: https://hackmd.io/@fmease/inject-doctests-into-host-crate:

Motivation

• Have doctest work in binary crate
• Have better warnings and errors reporting by having correct spans
• Handle doctests with different editions in the same compilation unit
• Simplify cross-compilation (deprecating all the --doctest-cross-arg ...)
• Better Cargo integration (parallel test exec, smarter unused_crate_dependencies, ...)

Implementation

• Have a hook in the AST->HIR lowering that is only implemented by rustdoc:
  • It would parse the doctests, create the AST nodes (by calling rustc_parse maybe?) and give back to rustc the AST nodes corresponding to #[test] fn doctest_1234 { ... } with the doctest body inside
• Rustdoc would give spans corresponding to the original doctest code in the doc comment. This could include a different edition for doctests that override that
  • Problem: how would we get the spans to be correct given the presence of /// in the original code but not the new code given to rustc_parse? would it just work or does it need more invasive changes?
• These AST nodes would then be injected into the crate and lowered into HIR and go through the rest of the compilation
• Then we run them using the libtest harness.
• We need to run all rustc passes to make it work

Implementation History

Empty.

[epage]

Thank you for writing this up!

To add notes from the Cargo perspective.

The assumption is that rustdoc would be acting like a rustc driver and Cargo could treat this as a compilation step, one that can produce multiple binaries instead of just one. To this end, we checked and cargo passes --out-dir to rustc rather than -o.

This also assumes that all doctest binaries, standalone or merged, can be run like any other libtest test. I had thought that some are run in a special way but was assured in the All Hands meeting that this wasn't the case.

This would allow Cargo to treat this like any other compilation step, including caching, and we can run the binaries like any other test.

The main downside to this is if only one doctest binary needs to be rebuilt, then all will be rebuilt. However, changing the Edition is rare and the goal is to migrate everyone over to merged_crate (implicitly or explicitly).

[epage]

As for the rustdoc side of this, the question is what to do about doctests against public APIs.

We could make crate:: available as <crate-name>:: using some kind of use extern crate self as that was mentioned in the meeting (yes, I likely butchered that syntax but I hope its enough for people to know what I'm talking about).

The main downside is that public API doctests could make references to private items and this wouldn't be caught. Personally, I'm fine with that. Its not restricting user behavior but allowing them to do more. It could lead to some poor docs due to sloppiness but that is a trade off. This could actually make doctesting easier because you could have shared helper functions to initialize logic with the call hidden by #.

[epage]

There was also the question of how to handle unit tests as those will also be present in the code being compiled and run. This would cause unit tests to run in the lib's test target as well as each doctest binary.

There may be ways to detect this and avoid them but we need to make sure that happens.

[jyn514]

note that this requires type checking function bodies, which rustdoc is currently not allowed to do.

[kpreid]

The main downside is that public API doctests could make references to private items and this wouldn't be caught. Personally, I'm fine with that.

I have always told people that one of the advantages of writing doctests is that they help you make sure your public API is actually usable — that there isn't some little mistake in not making a function, type, or field pub that means it can’t actually be used in the intended way from outside the crate. It would be disappointing to lose that. Such validation can still be done using test targets and example targets, but doctests are in a unique position among tests because, being associated with specific public items, it's relatively clear what coverage they (should) add.

[GuillaumeGomez]

Well, there is no downside. You will have control over whether this a "normal" doctest or unit-test-like one.

[kpreid]

I do not see anywhere that it is said that this would be under user control, so I assumed it would be the one new way of running doctests.

(I do like the idea of doctests being less magic, and built and run more like other kinds of tests.)

[GuillaumeGomez]

The end goal is to have all doctests be injected like this. But we cannot do it if it means losing the current behaviour of the doctests without a way to keep it.

[Nemo157]

One question I just had: how will this handle lint attributes? If the test item is being injected approximately at the location of the item it is testing, then will its parent lint attributes apply to it?

Something else I think I mentioned in some discussion but didn't write down: I don't see any (nice) way to extend this to supporting clippy (#56232), since it's all integrated in one process I assume we'd have to have a "rustdoc-clippy" binary that does this transformation then runs with clippy's lints too.

[jyn514]

at the all hands, I was discussing with @rust-lang/clippy and @Mark-Simulacrum that clippy could instead be a library, not its own driver, which would allow rustc to emit clippy lints at the same time as generating unit tests. that would work here too, rustdoc could link against clippy and run its lints.

[epage]

I'm concerned about the clippy-library route. One of my goals is to de-specialize doctests. I overlooked that this proposal does not do that in it does not respect the configured driver. We could have cargo clippy and rustdoc special case this but then what about other situations, like cargo miri?

[jyn514]

another possible transformation is for rustdoc to do something like this:

• verify that the in-process doctest would compile
• change all functions in the crate to public (preferably by emitting a custom .rmeta, not textually)
• generate a external doctest (either merged or not) which has imports "in the right places" - probably a glob import of the module where the doctest is defined? that might have edge cases though.
• compile and run that external doctest with the driver (normally rustc, could be clippy or miri or a custom driver).