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

[NOMERGE] Soliciting feedback on an experimental rustdoc feature #1796

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

davisp
Copy link

@davisp davisp commented Jan 20, 2025

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:

    /// Some docs for my_fun
    ///
    /// ```rust,{source="some_module::some_function"}
    /// ```
    fn my_fun() {
    }

    #[cfg(test)]
    mod some_module {
        #[test]
        fn some_function() {
            // Only the body of this function is rendered in the doc comment.
            println!("Hello, full editor support for code examples!");
        }
    }

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 as example_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 the doctests module name and the example_$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:

  1. My librustdoc branch
  2. This nom PR rendered with my branch

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 them no_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 the test 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 using ignore is the integration tests.

main - Test Timing

$ time cargo test
cargo test  35.40s user 13.07s system 101% cpu 47.671 total

main - Coverage

$ cargo llvm-cov clean --workspace
$ cargo llvm-cov --no-report --tests
$ cargo +nightly llvm-cov --no-report --doctests
$ cargo +nightly llvm-cov report --summary-only --doctests
Filename                               Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
nom-language/src/error.rs                   51                51     0.00%          16                16     0.00%         139               139     0.00%           0                 0         -
nom-language/src/precedence/mod.rs         129                53    58.91%          15                 5    66.67%         234                99    57.69%           0                 0         -
src/bits/complete.rs                        60                 9    85.00%          16                 3    81.25%         141                23    83.69%           0                 0         -
src/bits/mod.rs                             55                 9    83.64%          13                 0   100.00%         110                 8    92.73%           0                 0         -
src/bits/streaming.rs                       53                 1    98.11%          14                 0   100.00%         128                 1    99.22%           0                 0         -
src/branch/mod.rs                           81                16    80.25%          24                 5    79.17%         114                29    74.56%           0                 0         -
src/branch/tests.rs                         47                 3    93.62%          20                 3    85.00%         115                 9    92.17%           0                 0         -
src/bytes/complete.rs                      176                 0   100.00%          71                 0   100.00%         335                 0   100.00%           0                 0         -
src/bytes/mod.rs                           306                51    83.33%          91                 5    94.51%         614                87    85.83%           0                 0         -
src/bytes/streaming.rs                     143                 0   100.00%          59                 0   100.00%         280                 0   100.00%           0                 0         -
src/bytes/tests.rs                         243                 1    99.59%          75                 0   100.00%         500                 0   100.00%           0                 0         -
src/character/complete.rs                  508                24    95.28%         125                 1    99.20%         883                29    96.72%           0                 0         -
src/character/mod.rs                       151                46    69.54%          50                13    74.00%         223                67    69.96%           0                 0         -
src/character/streaming.rs                 485                18    96.29%         102                 0   100.00%         863                24    97.22%           0                 0         -
src/character/tests.rs                      23                 0   100.00%           9                 0   100.00%          41                 0   100.00%           0                 0         -
src/combinator/mod.rs                      149                 8    94.63%          61                 0   100.00%         385                12    96.88%           0                 0         -
src/combinator/tests.rs                     85                 9    89.41%          33                 6    81.82%         187                17    90.91%           0                 0         -
src/error.rs                               171               128    25.15%          38                10    73.68%         297               161    45.79%           0                 0         -
src/internal.rs                            187                47    74.87%          64                16    75.00%         300                84    72.00%           0                 0         -
src/lib.rs                                 123                29    76.42%          51                17    66.67%         255               145    43.14%           0                 0         -
src/multi/mod.rs                           497                55    88.93%         140                 9    93.57%        1159                82    92.92%           0                 0         -
src/multi/tests.rs                         250                 3    98.80%          79                 3    96.20%         699                 9    98.71%           0                 0         -
src/number/complete.rs                     708                70    90.11%         227                 3    98.68%        1360                75    94.49%           0                 0         -
src/number/mod.rs                          489                33    93.25%         170                 8    95.29%         948                52    94.51%           0                 0         -
src/number/streaming.rs                    736                73    90.08%         220                 3    98.64%        1468                76    94.82%           0                 0         -
src/sequence/mod.rs                         68                 3    95.59%          20                 2    90.00%         137                 4    97.08%           0                 0         -
src/sequence/tests.rs                       71                 1    98.59%          16                 0   100.00%         209                 0   100.00%           0                 0         -
src/str.rs                                 172                35    79.65%          43                 0   100.00%         324               108    66.67%           0                 0         -
src/traits.rs                              494               178    63.97%         208                84    59.62%         920               327    64.46%           0                 0         -
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                     6711               954    85.78%        2070               212    89.76%       13368              1667    87.53%           0                 0         -

pd/experimental-rustdoc - Test Timing

$ time cargo test
cargo test  18.29s user 8.61s system 206% cpu 13.030 total

pd/experimental-rustdoc - Coverage

$ cargo llvm-cov clean --workspace
$ cargo llvm-cov --no-report --tests
$ cargo +nightly llvm-cov --no-report --doctests
$ cargo +nightly llvm-cov report --summary-only --doctests
Filename                               Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
nom-language/src/error.rs                   51                51     0.00%          16                16     0.00%         139               139     0.00%           0                 0         -
nom-language/src/precedence/mod.rs         129                53    58.91%          15                 5    66.67%         234                99    57.69%           0                 0         -
src/bits/complete.rs                        58                 9    84.48%          15                 3    80.00%         160                23    85.62%           0                 0         -
src/bits/mod.rs                             53                 9    83.02%          12                 0   100.00%         108                 8    92.59%           0                 0         -
src/bits/streaming.rs                       51                 1    98.04%          13                 0   100.00%         132                 1    99.24%           0                 0         -
src/branch/mod.rs                           87                18    79.31%          26                 5    80.77%         131                30    77.10%           0                 0         -
src/branch/tests.rs                         47                 3    93.62%          20                 3    85.00%         115                 9    92.17%           0                 0         -
src/bytes/complete.rs                      174                 0   100.00%          70                 0   100.00%         384                 0   100.00%           0                 0         -
src/bytes/mod.rs                           304                51    83.22%          90                 5    94.44%         654                87    86.70%           0                 0         -
src/bytes/streaming.rs                     141                 0   100.00%          58                 0   100.00%         317                 0   100.00%           0                 0         -
src/bytes/tests.rs                         243                 1    99.59%          75                 0   100.00%         500                 0   100.00%           0                 0         -
src/character/complete.rs                  505                91    81.98%         124                 8    93.55%         980               123    87.45%           0                 0         -
src/character/mod.rs                       145                46    68.28%          49                13    73.47%         254                67    73.62%           0                 0         -
src/character/streaming.rs                 460                93    79.78%         101                 8    92.08%        1008               125    87.60%           0                 0         -
src/character/tests.rs                      23                 0   100.00%           9                 0   100.00%          41                 0   100.00%           0                 0         -
src/combinator/mod.rs                      250                 8    96.80%          87                 0   100.00%         606                12    98.02%           0                 0         -
src/combinator/tests.rs                     85                 9    89.41%          33                 6    81.82%         187                17    90.91%           0                 0         -
src/error.rs                               169               128    24.26%          37                10    72.97%         295               161    45.42%           0                 0         -
src/internal.rs                            187                48    74.33%          64                16    75.00%         300                85    71.67%           0                 0         -
src/lib.rs                                 152                31    79.61%          60                17    71.67%         342               145    57.60%           0                 0         -
src/multi/mod.rs                           495                55    88.89%         139                 9    93.53%        1199                82    93.16%           0                 0         -
src/multi/tests.rs                         250                 3    98.80%          79                 3    96.20%         699                 9    98.71%           0                 0         -
src/number/complete.rs                     706                75    89.38%         226                 4    98.23%        1515                83    94.52%           0                 0         -
src/number/mod.rs                          487                33    93.22%         169                 8    95.27%         987                52    94.73%           0                 0         -
src/number/streaming.rs                    734                82    88.83%         219                 5    97.72%        1486                86    94.21%           0                 0         -
src/sequence/mod.rs                         66                 3    95.45%          19                 2    89.47%         153                 4    97.39%           0                 0         -
src/sequence/tests.rs                       71                 1    98.59%          16                 0   100.00%         209                 0   100.00%           0                 0         -
src/str.rs                                 172                35    79.65%          43                 0   100.00%         324               108    66.67%           0                 0         -
src/traits.rs                              494               179    63.77%         208                85    59.13%         920               330    64.13%           0                 0         -
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                     6789              1116    83.56%        2092               231    88.96%       14379              1885    86.89%           0                 0         -```

For whatever reason, my build of rustdoc was failing to generate docs
due to not knowing about the proptest dependency somehow. No clue what
that's about as it also doesn't understand the `#[cfg(test)]` attribute
either.
@davisp davisp requested a review from Geal as a code owner January 20, 2025 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant