roc-lang
Parser docs examples
I've found it a bit tricky to understand the primitives of the parser. My goal here is to add some examples for new people to have a decent, self-contained place to get started. Also, I'd be nice for more experienced people to have a good reminder of what the function does.
I love these code quality improvements @roboteng :) Just a few nits
@Anton-4, Thanks for the feedback, and thanks for looking at this! I'll clean it up some more before I make it a real PR
@Anton-4 , Thanks!
These are already tests! cargo test compiles and runs each example. So as long as we're testing this crate, these are guaranteed to be compiler-checked, and passing. That's what the # do in the examples. Those lines are included in the tests but excluded in the cargo doc examples. Also, sorry for the 1000+ line PR
Also, sorry for the 1000+ line PR
No problem :)
These are already tests!
Aha, I didn't realize that. Currently, I still think regular tests would be better for these reasons:
- Our CI does not run when changes are made to rust comments, so breaking changes could go unnoticed.
- Rust-analyzer seems to work poorly on the code when it is embedded like this.
- We currently do not generate rust HTML docs for our crates, so regular tests would be roughly equally visible.
- I'm not sure if the imports could be simplified compared to what they are right now but they definitely could be if we used regular tests.
What do you think?
Also, no big deal, but it's not necessary to merge every time the update button appears.
... so breaking changes could go unnoticed.
I agree, this would be bad. If we do pull out the code into full tests, I think that should be automated, so it doesn't fall behind, in a similar manner.
Rust-analyzer seems to work poorly on the code when it is embedded like this.
rustc/cargo does some preprocessing to allow things like allowing unused variables and imports. I know these examples aren't as minimal as they could be. I could, at the very least, manually go through and clean them up. We could probably add some annotations to have those emit warnings so the examples could stay clean.
We currently do not generate rust HTML docs for our crates, so regular tests would be roughly equally visible
I'm not sure what other people's normal flow is, but I normally use the docs/examples in my editor (VSCode). Docs are generated on the fly when I hover over a function, no HTML/browser needed. I've also manually run the cargo doc command locally to get the docs that docs.rs would normally provide, but that takes a little more setup.
I can start working on removing unused things in the examples, and see if there's a way to ensure that CI always checks the docs are up to date with source. Would it be possible to run CI on example code changes? Otherwise, I can see if there's a way to extract the examples in a way that will flag for the usual warnings/errors. I'd like to avoid having copy/pasted code sitting alongside the code, as that seems more susceptible to getting out-of-date.
I looked at the check for running tests, and it's just a big regex. It seems doable to change it so that it would consider lines like "/// ```" as 'non-comment' so that CI would still run if a new example were added. However, it seems much trickier to detect changes that don't include the start/end of the example code... It's better than no check at all, but it's still not a guarantee.
If we do pull out the code into full tests, I think that should be automated
With "that should be automated", do you mean the conversion from examples in doc comments to normal tests?
Docs are generated on the fly when I hover over a function
Yeah I do that too, but I forgot about it here :smile:. I do think that if the docs are as large as they are in this case I prefer to go-to-def and read them full size instead of in the pop-up.
I'd like to avoid having copy/pasted code sitting alongside the code
Do you mean have the same code duplicated in the doc comments and as a normal test?
I want to back up a bit and make sure we're on the same page.
The problem is making sure that code examples always stay up to date with the source code. As far as we know, this general problem hasn't been solved yet in main.
Background / Context
When code blocks are written in docs, they serve two function:
- Shown in the docs as a code example
- Embedded in other code, and ran as a test
This means that there is a mechanism to keep example code up to date with the source code its documenting.
When cargo test is run on a project all doc tests are included in that. However, since the roc repo has a lot of tests, we only run tests if needed. Right now, that means a change to rust source code, excluding comments.
The implication is that PR's like this one that only have comments don't trigger cargo test. Examples could get out of sync with source code.
Ideal solution
Every time a doc-test/example is added or changed, it is tested and is always kept up to date with its source code.
Possible solutions
These are ones I could think of that seemed somewhat reasonable. There are probably others tat I haven't thought of, and would be willing to look in to.
Current solution
Writing docs in the same file as the source code.
This doesn't meet our requirements because tests are only run if the PR contains changes to (rust) files that aren't only comments.
Change CI to run tests if /// ``` is present
This would (probably) only require changing some regex in the CI yaml. This would be able to run tests on new examples added, but wouldn't trigger tests on a change to examples.
This is probably the simplest solution.
Use #[doc = ] for all the docs
We can use the include_str!() macro to move the docs form inline to their own files. We would put docs that we want compile checked into files with .doc.md extension (or similar). We could then add extension to trigger tests. This would cut down on excessive tests but would still give some false positives.
However, rust-analyzer seems like it can't process the include_str!() macro, and doesn't show that content
Use #[doc = ] for only the doc-test part of the docs
Similar to the above approach, but we only put doc-tests in their own files. This would ensure that the false positives are cut down for the most part.
These would be close to .rs files, but would have a few differences:
- No main function
-
#at the start of some lines - non-standard formatting, to make the examples look good. (E.G. not indenting the body of a function)
This is probably the one I'd prefer, if our current solution doesn't cut it.
Use a custom macro that is more flexible than include_str!
Some of the above solutions use include_str!(), but we could write our own macro that's better! I'm not 100% sure on all the details, but I'm pretty sure we could write a proc macro to solve all of our problems... but also have a proc macro...
This is probably overkill
What do you think about these? I'm open to suggestions, and let me know if I'm offbase anywhere!
I appreciate your structured approach @roboteng!
In CI, we can make it run for changes to lines with /// but still skip it for //. That seems like the best trade-off overall.
What do you think about combining "Success Case" and "Error Case" examples into one? We could use rust comments // success case: and // error case: inside the example instead and reduce the lines of code significantly.
In CI, we can make it run for changes to lines with /// but still skip it for //. That seems like the best trade-off overall.
I think I've made that change correctly, but it's regex, so who knows...
I'll work on condensing the examples. I don't think there's a big difference in splitting them out, so I'll combine them 😄
Thanks for your feedback @Anton-4, it's helped make this PR clearer and more consistent!
I've gone through and updated as you've suggested