ArgParser: escape an argument's spelling when emitting it - #600
Merged
Conversation
Smaug123
force-pushed
the
argparser-escape-emitted-arg-forms
branch
from
July 29, 2026 07:00
21f96f7 to
e5aa1be
Compare
A `SynConst.String` holds *decoded* text, so writing one into the generated file demands the escaping the author's own source supplied -- the hazard `defaultValueExpr` already documents and guards against for default values. Names had no such guard: `[<ArgumentLongForm "back\\tab">]` was emitted as `"back\tab"`, read back with a tab in it, so the argument silently answered to a name other than the one declared. A spelling containing e.g. `\q` could stop the generated file compiling at all. Escape at the emission sites rather than in `ArgForm` itself. `ArgForm` must keep holding the *semantic* spelling, because the generation-time name checks compare it under the scanner's own case-insensitive equality: escaped, they would compare `é` against `É` and miss the collision that `é` and `É` do have. A verbatim spelling was already emitted correctly (Fantomas re-prints it as `@"..."`); it now goes out as an escaped regular string instead, which reads back the same. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found in review. F#'s attribute syntax permits parentheses around the argument,
so `[<ArgumentLongForm ("back\\tab")>]` reaches us as a `SynExpr.Paren` and fell
through the escaping to be emitted verbatim. The generation-time name checks
already strip parentheses (`literalForms`), so without this the same spelling was
checked as one name and emitted as another.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reachable only once [<ArgumentPrefix>] landed. A long form we cannot read at generation time -- an [<ArgumentLongForm>] naming a [<Literal>] -- stays an expression the generated program evaluates, so a prefix is joined to it there rather than here. The prefix half is still a constant we invent and write out, and it was going out unescaped: `[<ArgumentPrefix "viaesc\\tab">]` was emitted as `"viaesc\tab-" + (TheLiteral)`, whose `\t` is a tab. That branch is pure emission -- `literalForms` matches only a bare constant, so nothing it builds reaches the name checks -- so escaping there cannot disturb the semantic spellings those checks compare. Also covers the case deferred from the [<ArgumentPrefix>] PR, which needed this one: a prefixed spelling containing backslashes now survives re-emission, since combining a prefix with a name invents a constant just as much as this does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Smaug123
force-pushed
the
argparser-escape-emitted-arg-forms
branch
from
July 29, 2026 07:18
e5aa1be to
50c97cb
Compare
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
1 participant
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.
A
SynConst.Stringholds decoded text, so writing one into the generated file demands the escaping the author's own source supplied.defaultValueExpralready documents and guards against exactly this for default values (ContainsAwkwardStringDefaultsinConsumePlugin/Args.fsis its regression test). Argument names had no such guard:was emitted as
Forms = [ "back\tab" ]— whose\tis a tab — so the argument silently answered to a name other than the one declared. A spelling containing e.g.\qcould stop the generated file compiling at all.We now escape it at the emission sites (
HumanReadableArgForm, and the erased schema'sForms), not inArgFormitself.ArgFormneeds the semantic spelling, not the escaped spelling, because at generation time we have to compare it to those of other fields to check collisions.Notes
@"back\tab") was already emitted correctly, since Fantomas re-prints it as@"...". It now goes out as an escaped regular string instead, which reads back the same.Found while implementing
[<ArgumentPrefix>](#598), which rebuilds spellings and so hits this immediately.🤖 Generated with Claude Code