Cut the arg-parser generator over to the embedded runtime - #567
Merged
Conversation
Smaug123
force-pushed
the
argparser-cutover
branch
2 times, most recently
from
July 14, 2026 22:34
04e1dab to
4e3160b
Compare
Owner
Author
|
Review round 2 addressed:
🤖 Generated with Claude Code |
Smaug123
force-pushed
the
argparser-cutover
branch
from
July 14, 2026 22:49
4e3160b to
0d98b98
Compare
Smaug123
force-pushed
the
argparser-erased-core
branch
from
July 15, 2026 08:07
02bec5b to
e9c5f43
Compare
Smaug123
force-pushed
the
argparser-cutover
branch
2 times, most recently
from
July 15, 2026 18:29
5fd4cf0 to
e5740f0
Compare
Smaug123
force-pushed
the
argparser-erased-core
branch
from
July 15, 2026 19:08
3748b8e to
f80f036
Compare
Smaug123
force-pushed
the
argparser-cutover
branch
from
July 15, 2026 19:08
e5740f0 to
247fe1f
Compare
Generated parsers no longer contain a hand-emitted scanning state machine. Instead the generator embeds ArgParserRuntime verbatim (parsed from an embedded resource and spliced in as a private module, one per namespace per input file, named after the first tagged type there) and emits, per type: an erased schema value describing the parser's shape, typed converter/storage callbacks, and record assembly. The runtime's runParse orchestrates scanning, routing, duplicate/missing detection, message rendering and defaults; by construction a conversion failure can never change how tokens are routed. The exact code shipping inside every generated file is the code property-tested in this repository. All pre-existing contract behaviour is preserved (the entire arg-parser test suite passes unchanged, including the deliberately-pinned eccentricities, keyed positionals under every long form, and argv-ordered diagnostics). The deliberate semantic changes are exactly the known-bug fixes, with TestArgParserKnownBugs flipped to assert them: - a malformed space-separated value no longer aborts the scan; - a duplicated flag no longer swallows the following token (and a duplicate of a slot whose first occurrence failed conversion is not an error at all: the later occurrence fills the slot, as before); - list, positional, post-separator and env-var conversions now feed the aggregated error channel (with argument context) instead of escaping as raw FormatException; - environment lookups no longer run once the parse has failed; - --help matches case-insensitively, like every other argument; - a non-positional `bool list` field now generates compiling code. The `ArgParserRuntime_` module-name prefix is reserved by the generator in namespaces containing parsers, in the same way the generated per-type module names always have been. Not addressed here (unchanged from before): generation-time collision validation is still case-sensitive; nested-record default functions are still invoked on the root type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The scanner matches argument names with OrdinalIgnoreCase, but generation-time validation compared them case-sensitively: `foo` alongside `FOO` passed validation, and at parse time whichever leaf was declared first silently claimed every spelling. The validation now canonicalises every effective spelling (declared forms, `--no-` variants, positional forms, the reserved `help`) under the scanner's equality. Forms the untyped AST cannot see (e.g. [<Literal>] constants) are invisible to that check, so runParse now takes a WellFormedSchema and generated code re-checks the assembled schema at runtime via WellFormedSchema.checkOrFail. The conflict cases in ConsumePlugin/ArgParserConflictTests.fs (plus case-permuted variants) are now asserted automatically by a new GeneratorTest project which drives the generator over in-memory source. It is a separate assembly because these tests need the Fantomas.FCS that WoofWare.Whippet.Fantomas was compiled against, and the main test project's WoofWare.Expect forces a binary-incompatible newer version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Use StringComparer.OrdinalIgnoreCase (the scanner's equality) to group name claims rather than ToUpperInvariant keying, which is strictly coarser and falsely rejected e.g. "s" alongside "ſ". Reject names no token can address (empty, or containing '='). Fix [<ArgumentDefaultFunction>] on nested records: the generated call now resolves against the record which declares the field, not the [<ArgParser>]-tagged root type, against which it did not compile. ArgumentDefaultSpec.FunctionCall carries the owning type through the parse specification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A [<PositionalArgs>] field may carry several [<ArgumentLongForm>] aliases; every one of them must route values to the sink at its own --key. The kernel models this (ErasedPositional.Forms) and checks every alias for collisions; this pins the behaviour through a generated parser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review edits which just missed the #566 squash: the isFatal and scan grammar docstrings no longer describe the pre-rewrite parser's behaviour as the reason for their shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Smaug123
force-pushed
the
argparser-cutover
branch
from
July 15, 2026 19:53
247fe1f to
1ea032c
Compare
The embedded runtime module is named ArgParserRuntime_<firstTaggedType> and generated parser modules are named after their tagged types, so a tagged type named ArgParserRuntime_Foo alongside a tagged type Foo generated two modules with one name, which does not compile. The prefix was already documented as reserved; enforce the reservation at generation time for the names the untyped AST can see. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous check covered only [<ArgParser>]-tagged types, but an untagged type ArgParserRuntime_Foo declared alongside a tagged Foo is equally visible to the generator (it arrives in the same recursive group) and collides with the emitted runtime module all the same. Check the whole group. Declarations the untyped AST does not surface (other input files; user modules) remain covered by the documented reservation only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Smaug123
commented
Jul 16, 2026
Every case lives in working, automated form in WoofWare.Myriad.Plugins/Test/TestArgParserRejection.fs, which drives the generator over in-memory source; the stubs could never be part of a build (rejection at generation time was their entire point). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Everything in this file was fixed by the runtime rewrite, so the name and the was-versus-now narration were stale; the tests now state what the parser does, and Git history records what it used to do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Smaug123
enabled auto-merge (squash)
July 16, 2026 07:54
Smaug123
added a commit
that referenced
this pull request
Jul 25, 2026
…anges (#587) WoofWare.Myriad.Plugins/version.json's "./" path filter covers the whole package directory, including the WoofWare.Myriad.Plugins/Test test project added in #567. A Dependabot bump of Microsoft.NET.Test.Sdk (32e196a) touched only test .fsproj files and nix/deps.json, yet bumped the NBGV height and cut release WoofWare.Myriad.Plugins.10.2.4. Exclude Test the same way WoofWare.Myriad.Plugins.Attributes/version.json already does. Fixes #586 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stacked on #566. The cutover PR of the arg-parser rewrite.
What changes
Generated parsers no longer contain a hand-emitted scanning state machine (~670 lines of quoted-AST emission deleted). Instead:
ArgParserRuntime.fsis carried as an embedded resource, parsed at generation time, and spliced into generated output as a private module (one per namespace per input file, namedArgParserRuntime_<first tagged type>— a name that cannot collide across generated files, since the type itself would collide first; the prefix is reserved in the same sense the generated per-type module names always have been). Generated code stays dependency-free, and the exact text shipping in every consumer is the code the FsCheck suites exercise in this repository.ErasedSchemavalue describing the parser's shape, converter/storage callbacks over the typed slots, and record assembly. The runtime'srunParse(new in this PR, unit-tested over fake callbacks) drives scanning, routing, duplicate/missing detection, structural message rendering, and defaults, keeping diagnostics in argv order. Selection is shape-first by construction: a conversion failure can never change how a token was routed.What doesn't change
Every pre-existing arg-parser test passes unchanged — help formats, negation semantics, keyed positionals (under every long form), env-var defaults,
ParseExact, extension-methods mode, and the deliberately-pinned eccentricities (greedy value consumption etc.).Deliberate semantic changes (= the triaged bug fixes;
TestArgParserKnownBugsflipped accordingly)--key valueno longer aborts the scan (errors carry(at arg ...)context).Errors during parse!channel instead of escaping as rawFormatException.--helpmatches case-insensitively, like every other argument.bool listfield now generates compiling code (newNonPositionalBoolListregression type; previously uncompilable).Argument '--foo' was supplied multiple times: 3 and 4) from bare flags (Flag '--baz' was supplied multiple times).Explicitly not addressed here (unchanged behaviour, follow-ups)
_.membershorthand in the runtime file) is enforced end-to-end by CI's regenerate-and-build check.🤖 Generated with Claude Code