[NOMERGE] Soliciting feedback on an experimental rustdoc feature #1796
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Howdy, nom devs!
I'm currently working on an experiment rustdoc feature to improve the ergonomics of authoring doctests. I've gotten far enough that I'm starting to look for feedback from the wider community. I spent a bit of time searching for projects that make extensive use of doctests and nom came up pretty quick based on some random searching.
As such, I've spent a few hours today refactoring nom's extensive doctest suite to use my branch of librustdoc. So far I have the basics down, but after porting all 300 or so nom doctests, I've definitely got a few ideas on things to improve. However, I'd like to hear from folks that aren't me on whether they think this approach is even worth it. With that out of the way, here's my elevator pitch:
Writing doctests is a pain in the rear end. Rather than write Rust in code blocks with all of the issues that entails (non-awesome editor support, one test per binary, tedious setup code elision, etc) I'm proposing that instead we just write example code in a conditionally compiled module similar to unit tests (in fact, for anyone that wants to, they could just reuse unit tests for examples).
My current approach re-uses the existing doctest machinery for specifying which function to render in a code block. This has a number of issues but it at least serves to show the idea. Personally, I think this feature would likely be better having new syntax separate from fenced code blocks, but I'll leave that for the bike shedding thread it'll surely entail as its not extremely important in the grand scheme of things.
The basic idea is that in doc comments we can use syntax like such:
And that's pretty much the gist of it. So far I've ported every nom doc test (that's not marked no_compile) without issue, so that at least shows the basics are there.
After porting the nom doctest suite to my experimental approach, I've certainly noticed a few things. First, I don't know nom internals so when I started this work I ended up just using a
doctests
module in the same source file and then enumerating every example asexample_1, example_2, ... example_$n
. In hindsight I should have mirrored the function name with the function being documented. However, I didn't want to bother going back once I figured that out, and it likely would have added a significant amount of time since I with the numbering scheme I was able to jump up and down pretty quickly and mechanically. However, just note that my use of thedoctests
module name and theexample_$n
pattern are completely arbitrary. If this feature goes anywhere, folks will be able to do whatever they want with their naming schemes.Secondly, there should absolutely be an option to automatically link the function being documented with a function of the same name in some configurable module name (i.e., have a setting for "find all examples in the nested doctests module" or similar). Then we could just add syntax for where to insert the example. For multiple examples in a given doc comment I'm thinking to just add
_0, _1, _2
or similar syntax where_0
is equivalent to having elided the suffix altogether. This obviously won't work for examples on things that aren't functions, but we'd still have the fall back syntax to specify them manually in those cases.Thirdly, I did manage to uncover a few odd issues with various tests. Almost all of the clippy issues were simple unused imports, but there were a few deprecation warnings and the like. So I do think that suggests this approach could be generally useful to keeping code examples compiling and tested. However, I will note that @kpreid pointed out on the rust-lang Zulip chat that one significant drawback of this approach is that we're no longer compiling doctests externally to the crate under test. This means that (as it stands) we'd be losing the assertions that all of the examples work against the public API without using internals accidentally. I'm pretty sure that can be mitigated with either more rustdoc hacking or possibly with new separate tooling. However, I haven't currently got an actual working solution to that issue. If anyone can think of similar issues like that, I'd be happy to hear them.
All that said, here are a few links if anyone is interested:
Also, I collected some timing and coverage numbers before and after this work. I'll paste those down below.
And finally, if the nom devs find this PR an affront to their senses and want to pretend it never existed, feel free to close this PR and we'll never speak of it again. I'm hoping that nom's large doctest suite means y'all would be interested in exploring easier maintenance of it, but I'll be the first to admit I might be tilting at windmills here. Nom was just the first large doctest suite I found on a popular project, so apologies if this is beyond the pale or anything of that nature.
A note on the test timings, it turns out that my branch breaks if I
ignore
the doctest instead of marking themno_run
. I'm still learning the rustc toolchain internals and have not managed to figure out why building rustdoc results in a binary that appears to have no idea about thetest
configuration option which means that I'm running the test suite with the stable toolchain. Point being, of the 13s measured, roughly 11s of that is just compiling the doctests. In real life these won't actually show up there and instead will just be listed in the normal unit test output which executes ~instantly. The rest of the 2s or so when usingignore
is the integration tests.main - Test Timing
main - Coverage
pd/experimental-rustdoc - Test Timing
pd/experimental-rustdoc - Coverage