go/shadow - #956
Closed
jdx wants to merge 2 commits into
Closed
Conversation
…ft unnoticed The generator landed in the commit before this one with snapshot tests over small fixtures, which prove it emits what it meant to and nothing about whether the result works. This is the other half: mise's committed spec, generated into `go/internal/shadow/mise`, checked in, and parsed against. Checked in rather than built by the test, for the two reasons the Rust shadows are: a reviewer can read the diff when the emitter's vocabulary changes, and CI runs `mise run gen-go` and fails if regenerating produces one. A change to the generator that nobody meant now has to be committed rather than discovered. mise is the fixture because it is the largest usage CLI there is — 211 commands, 711 flags, 128 positionals, four deep — and because every shape that has been awkward to express came from it. The cases are invocations out of its own docs, hand-written on purpose: what the generator produces is only worth checking if it parses the words users actually type. They cover the `[ARGS]… [-- ARGS_LAST]…` split that made the Rust derive's validation wrong, a hidden alias selecting a command, and a root global reaching a command two levels down. Two properties are measured here rather than at fixture scale, because scale is what would break them: Keys are unique and dense. Generated code dispatches on a Key, so two entries sharing one would bind the wrong field. The Rust derive hashes its way around this because two macro expansions cannot see each other; a generator sees the whole spec and can count, so a collision would be inexcusable rather than unlucky — and this checks it across all 989 entries. A parse still allocates nothing. A scope lookup that collected flags into a slice, or a walk that built one per token, is invisible on a spec with four flags and obvious on one with 711. 110ns and 0 allocations for `mise use -g node@20`. One case pins a hole rather than a property: `run --wat` is `unexpected_arg`, because mise's spec gives `run` no positional at all — it clears them and adds `mount run="mise tasks --usage"`, so task names come from running that, and binding does not resolve mounts. When mounts are answered that case changes, and pinning it means it changes loudly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t wrong The one-line diff is the whole point of checking this file in: `DefaultSubcommand` went from `cmdOciRun` to `cmdRun`, which is what the fix in the commit before this one produces. A reviewer reading the generated diff is how the bug was found, and regenerating is how the fix is shown to be real. Also pins it as a test. Nothing in the parse of an ordinary command line shows the difference — `mise build` reports `unexpected_arg` either way, because mise's spec gives `run` no positional and task names come from a mount — so the pointer is asserted directly, against `cmdRun` and specifically not against `cmdOciRun`, the one a whole-tree search used to win with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
Contributor
Greptile SummaryThe PR adds a checked-in Go parser shadow generated from mise’s specification and extends CI to verify that regeneration is reproducible.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or build failures identified. The generated fixture’s inspected parser relationships match the source spec, its tests follow existing parser contracts, and the new CI task uses the repository’s established generation setup. Important Files Changed
Reviews (1): Last reviewed commit: "fix(go): regenerate mise's tables, and p..." | Re-trigger Greptile |
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.
Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Low Risk
Mostly generated data, docs, and CI guards; runtime behavior changes only if the checked-in tables or emitter output differ from expectations.
Overview
Introduces a mise-scale Go shadow (
go/internal/shadow/mise): checked-in binding tables frombenches/mise.usage.kdland tests that exercise real command lines, default-subcommand resolution, dense uniqueKeys, and zero-allocation parsing at full CLI size.Adds
mise run gen-goto regeneratetables.go, and extends the test workflow sorender,gen-shadow, andgen-gomust all leave a clean git tree.go/README.mdnow documentsusage generate go///go:generate, the shadow package, updated corpus binding counts (122 binding / 30 post-binding skipped), and drops the generator from “what is missing.”Reviewed by Cursor Bugbot for commit dcd3446. Bugbot is set up for automated code reviews on this repo. Configure here.