Skip to content

Commit 04e1dab

Browse files
Smaug123claude
andcommitted
Cut the arg-parser generator over to the embedded runtime
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) 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). 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; - 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. Known drift, deliberate: duplicate-argument errors now cluster after scan-order errors rather than appearing at scan position, and their messages distinguish valued occurrences from bare flags. 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>
1 parent c844f1e commit 04e1dab

13 files changed

Lines changed: 8072 additions & 5865 deletions

ConsumePlugin/Args.fs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,11 @@ type WithMultilineTypeHelp =
259259
OutputDir : string
260260
Force : bool
261261
}
262+
263+
/// Regression test: the pre-rewrite generator produced uncompilable code for a non-positional
264+
/// list of booleans (its accumulator was a ResizeArray but the flag machinery assumed an option).
265+
[<ArgParser>]
266+
type NonPositionalBoolList =
267+
{
268+
Flags : bool list
269+
}

ConsumePlugin/ConsumePlugin.fsproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@
100100
<Compile Include="GeneratedArgParserNegationTests.fs">
101101
<MyriadFile>ArgParserNegationTests.fs</MyriadFile>
102102
</Compile>
103+
<Compile Include="Nested/Args.fs" />
104+
<Compile Include="Nested/GeneratedArgs.fs">
105+
<MyriadFile>Nested/Args.fs</MyriadFile>
106+
</Compile>
103107
<Compile Include="OpensLeakRegression.fs" />
104108
<Compile Include="GeneratedOpensLeakRegression.fs">
105109
<MyriadFile>OpensLeakRegression.fs</MyriadFile>

ConsumePlugin/GeneratedArgParserNegationTests.fs

Lines changed: 1120 additions & 890 deletions
Large diffs are not rendered by default.

ConsumePlugin/GeneratedArgs.fs

Lines changed: 4089 additions & 3894 deletions
Large diffs are not rendered by default.

ConsumePlugin/GeneratedOpensLeakRegression.fs

Lines changed: 807 additions & 114 deletions
Large diffs are not rendered by default.

ConsumePlugin/Nested/Args.fs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace ConsumePlugin
2+
3+
open WoofWare.Myriad.Plugins
4+
5+
/// Regression tests, two in one.
6+
/// This file shares its base name with Args.fs and generates parsers into the same namespace:
7+
/// were the embedded runtime module named after the input file, the two generated files would
8+
/// both define it and fail to compile (FS0248). And its positional sink has two long forms,
9+
/// both of which must reach the sink in keyed syntax.
10+
[<ArgParser>]
11+
type SameBaseNameArgs =
12+
{
13+
Value : int
14+
[<ArgumentLongForm "rest">]
15+
[<ArgumentLongForm "others">]
16+
[<PositionalArgs>]
17+
Rest : string list
18+
}

ConsumePlugin/Nested/GeneratedArgs.fs

Lines changed: 877 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 106 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,22 @@
11
namespace WoofWare.Myriad.Plugins.Test
22

3-
open System
43
open NUnit.Framework
54
open FsUnitTyped
65
open ConsumePlugin
76

8-
/// These tests pin down current behaviour of the generated arg parser which we believe to be
9-
/// buggy. Each test asserts what the parser *does today*, with a comment stating the desired
10-
/// behaviour. They exist so that the planned rewrite of the arg parser flips them deliberately,
11-
/// making every observable semantic change reviewable, rather than changing behaviour silently.
7+
/// These tests pin behaviours which were bugs in the original arg parser and were fixed by the
8+
/// runtime rewrite. Each test's original (buggy) behaviour is noted in a comment, so that the
9+
/// deliberate semantic changes remain visible in history.
1210
[<TestFixture>]
1311
module TestArgParserKnownBugs =
1412

1513
let noEnv (_ : string) : string option = None
1614

17-
// DESIRED: a malformed value in `--key value` form should record the conversion error and
18-
// continue scanning, exactly as the `--key=value` form does. Instead, the scan stops dead:
19-
// `--bar` and `--baz` below are never processed, so they are spuriously reported missing.
15+
// Previously the scan stopped dead at a malformed `--key value` pair, so `--bar` and `--baz`
16+
// were spuriously reported missing. Now the conversion error is recorded (with the offending
17+
// arg's name) and scanning continues.
2018
[<Test>]
21-
let ``BUG: a malformed space-separated value aborts the remainder of the scan`` () =
19+
let ``A malformed space-separated value does not abort the remainder of the scan`` () =
2220
let exc =
2321
Assert.Throws<exn> (fun () ->
2422
BasicNoPositionals.parse' noEnv [ "--foo" ; "bad" ; "--bar=present" ; "--baz=true" ]
@@ -28,14 +26,10 @@ module TestArgParserKnownBugs =
2826
exc.Message
2927
|> shouldEqual
3028
"""Errors during parse!
31-
The input string 'bad' was not in a correct format.
32-
Required argument '--foo' received no value
33-
Required argument '--bar' received no value
34-
Required argument '--baz' received no value"""
35-
36-
// Contrast case: the same malformed value in `--key=value` form *does* continue the scan.
37-
// This test is here to document the asymmetry with the test above; this behaviour (continue
38-
// scanning after a conversion error) is the desired one.
29+
The input string 'bad' was not in a correct format. (at arg --foo)
30+
Required argument '--foo' received no value"""
31+
32+
// The equals form behaves identically.
3933
[<Test>]
4034
let ``A malformed equals-form value does not abort the scan`` () =
4135
let exc =
@@ -50,11 +44,11 @@ Required argument '--baz' received no value"""
5044
The input string 'bad' was not in a correct format. (at arg --foo=bad)
5145
Required argument '--foo' received no value"""
5246

53-
// DESIRED: a duplicated flag must not consume the next token as its "value". Here the
54-
// duplicate `--baz` swallows `--foo=3`, so on top of the duplicate-arg error we get a
55-
// spurious "'--foo' received no value" error, and `--foo=3` is never parsed.
47+
// Previously the duplicated `--baz` consumed `--foo=3` as its alleged value, so `--foo` was
48+
// spuriously reported missing as well. Now the duplicate is reported on its own and every
49+
// other argument parses normally.
5650
[<Test>]
57-
let ``BUG: a duplicated flag consumes the following option as its value`` () =
51+
let ``A duplicated flag does not consume the following option as its value`` () =
5852
let exc =
5953
Assert.Throws<exn> (fun () ->
6054
BasicNoPositionals.parse' noEnv [ "--baz=true" ; "--baz" ; "--foo=3" ; "--bar=present" ]
@@ -64,75 +58,84 @@ Required argument '--foo' received no value"""
6458
exc.Message
6559
|> shouldEqual
6660
"""Errors during parse!
67-
Argument '--baz' was supplied multiple times: True and --foo=3
68-
Required argument '--foo' received no value"""
61+
Flag '--baz' was supplied multiple times"""
6962

70-
// DESIRED: a conversion failure in a list-typed (repeatable) argument should be recorded as a
71-
// parse error like scalar conversion failures are, not escape as a raw FormatException with
72-
// no indication of which argument was at fault.
63+
// Previously a raw FormatException escaped, with no indication of which argument was at
64+
// fault and no error aggregation. The same applied to positional conversions (before and
65+
// after the `--` separator) and to environment-variable defaults.
7366
[<Test>]
74-
let ``BUG: a malformed list-element value throws a raw FormatException`` () =
75-
Assert.Throws<FormatException> (fun () ->
76-
BasicNoPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "--rest" ; "notanint" ]
77-
|> ignore<BasicNoPositionals>
78-
)
79-
|> fun exc ->
80-
exc.Message
81-
|> shouldEqual "The input string 'notanint' was not in a correct format."
82-
83-
// DESIRED: as above, but for a positional argument.
67+
let ``A malformed list-element value is a parse error, not a raw FormatException`` () =
68+
let exc =
69+
Assert.Throws<exn> (fun () ->
70+
BasicNoPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "--rest" ; "notanint" ]
71+
|> ignore<BasicNoPositionals>
72+
)
73+
74+
exc.Message
75+
|> shouldEqual
76+
"""Errors during parse!
77+
The input string 'notanint' was not in a correct format. (at arg --rest)"""
78+
8479
[<Test>]
85-
let ``BUG: a malformed positional value throws a raw FormatException`` () =
86-
Assert.Throws<FormatException> (fun () ->
87-
BasicWithIntPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "notanint" ]
88-
|> ignore<BasicWithIntPositionals>
89-
)
90-
|> fun exc ->
91-
exc.Message
92-
|> shouldEqual "The input string 'notanint' was not in a correct format."
93-
94-
// DESIRED: as above, but for a positional argument appearing after the `--` separator.
80+
let ``A malformed positional value is a parse error, not a raw FormatException`` () =
81+
let exc =
82+
Assert.Throws<exn> (fun () ->
83+
BasicWithIntPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "notanint" ]
84+
|> ignore<BasicWithIntPositionals>
85+
)
86+
87+
exc.Message
88+
|> shouldEqual
89+
"""Errors during parse!
90+
The input string 'notanint' was not in a correct format. (at arg notanint)"""
91+
9592
[<Test>]
96-
let ``BUG: a malformed positional value after the separator throws a raw FormatException`` () =
97-
Assert.Throws<FormatException> (fun () ->
98-
BasicWithIntPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "--" ; "notanint" ]
99-
|> ignore<BasicWithIntPositionals>
100-
)
101-
|> fun exc ->
102-
exc.Message
103-
|> shouldEqual "The input string 'notanint' was not in a correct format."
104-
105-
// DESIRED: a malformed environment-variable default should be reported through the parser's
106-
// error channel (naming the environment variable), not escape as a raw FormatException.
93+
let ``A malformed positional value after the separator is a parse error, not a raw FormatException`` () =
94+
let exc =
95+
Assert.Throws<exn> (fun () ->
96+
BasicWithIntPositionals.parse' noEnv [ "--foo=1" ; "--bar=x" ; "--baz=true" ; "--" ; "notanint" ]
97+
|> ignore<BasicWithIntPositionals>
98+
)
99+
100+
exc.Message
101+
|> shouldEqual
102+
"""Errors during parse!
103+
The input string 'notanint' was not in a correct format. (at arg notanint)"""
104+
107105
[<Test>]
108-
let ``BUG: a malformed environment-variable default throws a raw FormatException`` () =
109-
Assert.Throws<FormatException> (fun () ->
110-
ContainsBoolEnvVar.parse' (fun _ -> Some "notabool") []
111-
|> ignore<ContainsBoolEnvVar>
112-
)
113-
|> fun exc ->
114-
exc.Message
115-
|> shouldEqual "String 'notabool' was not recognized as a valid Boolean."
116-
117-
// DESIRED: once the parse is known to have failed, no further effects should run: the
118-
// environment should not be consulted for defaults. Today the env lookup runs anyway, so a
119-
// throwing `getEnvironmentVariable` masks the real parse error.
106+
let ``A malformed environment-variable default is a parse error naming the variable`` () =
107+
let exc =
108+
Assert.Throws<exn> (fun () ->
109+
ContainsBoolEnvVar.parse' (fun _ -> Some "notabool") []
110+
|> ignore<ContainsBoolEnvVar>
111+
)
112+
113+
exc.Message
114+
|> shouldEqual
115+
"""Errors during parse!
116+
String 'notabool' was not recognized as a valid Boolean. (from environment variable CONSUMEPLUGIN_THINGS)"""
117+
118+
// Previously the environment was consulted for defaults even after the parse had already
119+
// failed, so a throwing `getEnvironmentVariable` masked the real parse error. Defaults now
120+
// run only when the parse is otherwise clean.
120121
[<Test>]
121-
let ``BUG: environment lookups run even after the parse has already failed`` () =
122+
let ``Environment lookups do not run after the parse has failed`` () =
122123
let exc =
123124
Assert.Throws<exn> (fun () ->
124125
ContainsBoolEnvVar.parse' (fun _ -> failwith "env var was consulted") [ "--bool-var=notabool" ]
125126
|> ignore<ContainsBoolEnvVar>
126127
)
127128

128-
exc.Message |> shouldEqual "env var was consulted"
129+
exc.Message
130+
|> shouldEqual
131+
"""Errors during parse!
132+
String 'notabool' was not recognized as a valid Boolean. (at arg --bool-var=notabool)"""
129133

130-
// DESIRED: help detection should use the same case-insensitive comparison as ordinary
131-
// argument matching. Today `--FOO=1` matches the field `Foo`, but `--HELP` is not help: it
132-
// falls through to ordinary (failed) key processing.
134+
// Previously `--FOO=1` matched the field `Foo` but `--HELP` was not help: ordinary argument
135+
// matching was case-insensitive while help detection was case-sensitive. Help now uses the
136+
// same case-insensitive comparison as everything else.
133137
[<Test>]
134-
let ``BUG: ordinary args match case-insensitively but help is case-sensitive`` () =
135-
// Case-insensitive ordinary match: this parses fine.
138+
let ``Help matches case-insensitively, like ordinary arguments`` () =
136139
BasicNoPositionals.parse' noEnv [ "--FOO=1" ; "--bar=x" ; "--baz=true" ]
137140
|> shouldEqual
138141
{
@@ -142,14 +145,35 @@ Required argument '--foo' received no value"""
142145
Rest = []
143146
}
144147

145-
// ...but --HELP does not produce the help text.
146148
let exc =
147149
Assert.Throws<exn> (fun () -> BasicNoPositionals.parse' noEnv [ "--HELP" ] |> ignore<BasicNoPositionals>)
148150

149151
exc.Message
150152
|> shouldEqual
151-
"""Errors during parse!
152-
Trailing argument --HELP had no value. Use a double-dash to separate positional args from key-value args.
153-
Required argument '--foo' received no value
154-
Required argument '--bar' received no value
155-
Required argument '--baz' received no value"""
153+
"""Help text requested.
154+
--foo int32
155+
--bar string
156+
--baz bool
157+
--rest int32 (can be repeated)"""
158+
159+
// The pre-rewrite generator emitted uncompilable code for a non-positional `bool list`
160+
// (its accumulator was a ResizeArray, but the flag machinery assumed an option), so the
161+
// NonPositionalBoolList type could not previously exist at all.
162+
[<Test>]
163+
let ``A non-positional list of booleans parses, in all three syntaxes`` () =
164+
NonPositionalBoolList.parse' noEnv [ "--flags" ; "true" ; "--flags=false" ; "--flags" ]
165+
|> shouldEqual
166+
{
167+
Flags = [ true ; false ; true ]
168+
}
169+
170+
// The positional sink is addressable in keyed form under every one of its long forms, not
171+
// just the first (Rest here has both "rest" and "others").
172+
[<Test>]
173+
let ``Every long form of a positional sink reaches the sink in keyed syntax`` () =
174+
SameBaseNameArgs.parse' noEnv [ "--value=1" ; "--rest=a" ; "--others=b" ; "c" ]
175+
|> shouldEqual
176+
{
177+
Value = 1
178+
Rest = [ "a" ; "b" ; "c" ]
179+
}

0 commit comments

Comments
 (0)