0.8.0 (older version) Thoroughness: High Understanding: High
by git.jcg.re/jcgruenhage/crev-proofs.git on 2022-07-07
These reviews are from Crev, a distributed system for code reviews. To add your review, set up cargo-crev
.
The current version of Yap is 0.12.0.
0.8.0 (older version) Thoroughness: High Understanding: High
by git.jcg.re/jcgruenhage/crev-proofs.git on 2022-07-07
0.7.2 (older version) Thoroughness: High Understanding: High
by git.jcg.re/jcgruenhage/crev-proofs.git on 2022-06-27
Simple enough parser combinator library that comes with most of the bells and whistles I was looking for. I've noticed a few minor things that could use improving, but nothing critical.
First of all, why did I review this? I implemented parsing a specific HTTP
auth header credential, which is non-trivial to do with just string splitting.
I had gathered some experience with yap
during last years Advent of Code,
and I'm relatively happy with the implementation now. The only thing I was
missing is something like Tokens::optional
that was based on Result
instead of Option
. I'll make sure to contribute that later though.
As for problems I've found while reviewing this crate:
There's one sound use of unsafe, that could probably be avoided, just for
good measure. It's about going from a str
to the next char
. It
increases a counter by one, and then by another one until it's at a char
boundary. This is explicitly checked before it then does
str::get_unchecked
, which is the unsafe call. It seems sound to me, but
without tests this isn't a great look.
The formatting is a bit off in some places. A few extra spaces, nothing big, just made reviewing a bit more of a chore.
A previous version of this review claimed that the logos
dev-dependency
wasn't used anywhere. That's true for the releases on crates.io, but the
repository contains examples, where logos
is actually used.
Things I specifically liked:
The documentation is amazing. Every single thing one could hope for has a doc comment, and the documentation really goes above and beyond to demonstrate how yap can be used.
Leveraging iterators was a very good choice. Using yap
feels super
natural, and even without any prior parser-combinator experience, I was
able to quickly get started. Not using regular expressions and string
splitting feels really good.
In general, I'm extremely happy with yap, and even happier now that I've done my due diligence with regards to supply chain security. Thanks to the authors.
Lib.rs has been able to verify that all files in the crate's tarball, except Cargo.lock
,
are in the crate's repository. Please note that this check is still in beta, and absence of this confirmation does not mean that the files don't match.
Crates in the crates.io registry are tarball snapshots uploaded by crates' publishers. The registry is not using crates' git repositories, so there is a possibility that published crates have a misleading repository URL, or contain different code from the code in the repository.
To review the actual code of the crate, it's best to use cargo crev open yap
. Alternatively, you can download the tarball of yap v0.12.0 or view the source online.
Follow-up from my previous review for version 0.7.2: The sound use of unsafe has now gotten a test case to show that it's actually working as expected with exotic characters, and the
Tokens::optional_err
function I was missing last time is something I've contributed since. Still very happy with yap, and looking forward to seeing this used more.For a more detailed review, read my previous coverage of 0.7.2.