feat(quickcheck): add char_range and zip/zip_with/zip_with3 generator combinators - #3977
Conversation
… combinators Migrating a real generator suite (bobzhang/toml's round-trip model) to this package required re-deriving these from flat_map/map ~15 times: - char_range generates characters in an inclusive range, validated at construction (non-empty, no surrogate code points) so the per-sample conversion needs no unsafe cast. - zip / zip_with / zip_with3 combine generators applicatively, following the naming of Array's zip_with. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds new generator combinators and a character-range generator to the quickcheck package, extending the property-testing generator API with higher-level composition helpers and a Unicode-oriented primitive generator.
Changes:
- Add
Generator::zip,Generator::zip_with, andGenerator::zip_with3combinators built from existingflat_map/mapprimitives. - Add
char_range(lower, upper)generator for inclusive character ranges with upfront validation. - Add unit/snapshot tests covering
char_rangebehavior and the new zip combinators.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| quickcheck/pkg.generated.mbti | Exposes char_range and Generator::{zip, zip_with, zip_with3} in the generated public interface. |
| quickcheck/generator.mbt | Implements char_range and the new zip combinators on Generator. |
| quickcheck/generator_test.mbt | Adds tests for char_range bounds/panic behavior and deterministic samples for zip combinators. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| guard lower <= upper else { | ||
| abort("char_range: lower bound exceeds upper bound") | ||
| } | ||
| guard upper.to_int() < 0xD800 || lower.to_int() > 0xDFFF else { | ||
| abort("char_range: range contains surrogate code points") | ||
| } | ||
| int_range(lower.to_int(), upper.to_int() + 1).map(code => { | ||
| code.to_char().unwrap() | ||
| }) |
There was a problem hiding this comment.
Good catch — fixed in 4318770: bounds are now validated as Unicode scalar values (0..=0x10FFFF, non-surrogate), so an invalid Char produced via unsafe_to_char aborts at construction instead of panicking per sample.
| /// The bounds must form a non-empty range of Unicode scalar values: this | ||
| /// function aborts if `lower` exceeds `upper` or if the range contains | ||
| /// surrogate code points, so every generated code point is a valid `Char`. | ||
| pub fn char_range(lower : Char, upper : Char) -> Generator[Char] { |
There was a problem hiding this comment.
can we make char_range smarter to handle cases
lower .. 0xD800 0xDFFF..uper
instead of failing
There was a problem hiding this comment.
Done in 4318770 — a range spanning the surrogate block now draws from the contiguous index space of its valid scalars and shifts past the block, so char_range('\u{D000}', '\u{E800}') works and never emits a surrogate (covered by a new test asserting both sides of the block are reached).
| guard upper.to_int() < 0xD800 || lower.to_int() > 0xDFFF else { | ||
| abort("char_range: range contains surrogate code points") | ||
| } | ||
| int_range(lower.to_int(), upper.to_int() + 1).map(code => { |
There was a problem hiding this comment.
not reusing int_range?
There was a problem hiding this comment.
It does reuse int_range — the draw is a single int_range over the collapsed index space (range width minus the surrogate gap), then a shift past the block in map. Kept that shape in the fix.
…unds as scalar values Review feedback: a range spanning the surrogate block now draws from the contiguous index space of its valid scalars (still a single int_range) and shifts past the block, instead of failing. Bounds are validated as Unicode scalar values so unsafe_to_char-produced invalid bounds abort at construction rather than panicking per sample. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Coverage Report for CI Build 5766Coverage increased (+0.001%) to 90.313%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Follow-up to #3955, first of three scoped PRs from the feedback in #3955 (comment) (migrating bobzhang/toml's property tests to this package — moonbit-community/toml-parser#122).
char_range(lower, upper): characters in an inclusive range. Bounds are validated once at construction (non-empty, no surrogate code points), so the per-sample conversion uses safeto_char()— nounsafe_to_char.zip/zip_with/zip_with3: applicative combination of two or three generators, named afterArray'szip_with. Implemented viaflat_map/mapso they follow the existing state-splitting discipline.Pure addition — no existing behavior or snapshot changes.
Test plan: new bounds/panic/snapshot tests in
generator_test.mbt;moon test quickcheck60/60;moon check --deny-warn,moon fmt,moon infoclean.🤖 Generated with Claude Code