go/generate - #955
Conversation
…ead of a derive
Go has no macros. What a Rust CLI gets from `#[derive(Cli)]` at compile time, a
Go CLI has to get from a generator at build time, so `usage generate go` is the
same milestone for usage-go that usage-derive is for the Rust side — the point
at which a spec, rather than a hand-written table, is what a CLI is declared in.
//go:generate usage generate go -f mycli.usage.kdl -o tables.go
It emits binding tables and nothing else. Help text, choices, defaults and `env`
are absent for the same reason they are absent from the Rust hot path: a
successful parse never reads them, and mise's several hundred kilobytes of help
strings do not belong in front of a parser. A cold table is separate work.
The output is gofmt-clean as it comes out, which took some care and is worth it:
the alternative is every adopter running a formatter before they can commit, and
this repo's own CI failing `gofmt -l` on the table it will check in. gofmt pads
within runs of consecutive single-line entries and starts a new run after
anything spanning lines, so the emitter models a literal as a list of fields and
blocks rather than printing strings. Verified against mise's spec, usage's own,
and the examples spec: `gofmt -w` changes nothing in any of them.
What the generated file gives an author beyond speed is the key constants. An
event carries a Key, so generated code dispatches on `mise.FlagUseGlobal`
instead of comparing strings, and a flag that is renamed in the spec fails to
compile rather than silently never matching.
Three things resolved at generation time, so the parser reads one field per
command instead of walking: `unknown_flags` inheritance, `default_subcommand`
into a pointer at the node it names, and hidden aliases folded in beside visible
ones — hiding is a help-output concern and binding never reads it.
Identifier collisions are ordinary rather than exotic, and are handled: mise
declares both a `macos-defaults` command and a `macos defaults` path, and both
want to be spelled `CmdMacosDefaults`.
Checked end to end before committing, though the checked-in fixture that will
keep it honest is the next commit in this stack: the generated mise tables
compile, parse `mise use -g node@20`, resolve `x` to `exec` through its alias,
split `tasks run build extra --dry-run -- --verbose` across ARGS and ARGS_LAST,
and produce a package with no init function whose Root is a type D symbol — 211
commands and 711 flags that cost nothing before main.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… tree Three review findings on the emitter, one of which the checked-in mise tables in the next commit had already made visible: `Root.DefaultSubcommand` pointed at `oci run` rather than the top-level `run`. `default_subcommand` names a subcommand *of the root* — the spec declares it once at the top — but the resolution scanned every command in the tree and took the first match in depth-first order, which for mise is `oci run`. A parse would then have descended into a command that is not the root's child at all, so `mise build` would have run something under `oci`. The comment above the code already said "the root's own subcommands"; the code just did not do it, which is the version of this mistake that survives review easiest. The root's key was spelled rather than claimed, so a subcommand named `root` was handed `CmdRoot` too and the file declared the constant twice and would not compile. It goes through the same counter as everything else now, and gets `CmdRoot2` as any other collision would. A package name that is not a Go identifier produced a file that would not compile either — `--package my-pkg`, or a spec whose bin is `go` or `type`, both of which are plausible names for a CLI. Names derived from the spec are sanitized, since the author did not choose `bin` for this purpose and cannot be asked to fix it; an explicit `--package` is refused with a message instead, because quietly turning `my-pkg` into `mypkg` is a surprise waiting in somebody's build script. `is_valid_package` is public so the check and the sanitizing cannot drift. Not changed: the fourth finding, that a variadic flag's `var_max` is dropped. The two spellings are different questions and the corpus pins them apart. On the flag's *argument* it bounds one occurrence's values and belongs in the binding table, which is the corpus vector `a-bound-stops-a-variadic-flag`, and the emitter does emit it. On the flag itself it counts *occurrences*, which no single token can decide, so it is a post-binding check — `flag-var-too-many`, labelled post-binding for exactly that reason. usage-argv's tables and the derive's `counts_occurrences` split it the same way. A test now pins both halves so the question does not have to be re-derived next time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… names Two follow-up findings on the emitter, both about emitting a file that does not compile. `unique` counted per base and never claimed the spelling it handed out, so a third command could be given an identifier the second had already taken: `macos-defaults` and `macos defaults` produce `CmdMacosDefaults` and `CmdMacosDefaults2`, and a command named `macos-defaults2` then asked for `CmdMacosDefaults2` directly and got it. It searches for a free candidate now and reserves it. The third lands on `CmdMacosDefaults22`, which is unlovely and unique; the test asserts no constant is declared twice rather than pinning the suffix scheme, which is the property that actually matters. `_` and `init` are refused as package names, for two different reasons that are worth keeping straight. `package _` is rejected where it is written — `invalid package name _`. `package init` declares perfectly well and cannot be *imported*: an import binds the package name as an identifier in file scope, and `init` may only be a func, so an importer gets `cannot import package as init - init must be a func`. A table package exists to be imported, so it is out either way. Both checked against the compiler rather than taken from the citation offered, which is about the import and would have had this rejecting `package init` for a reason that is not true. `__` stays valid, and only the exact names are reserved, so `initialize` is fine. `package_ident` now asks `is_valid_package` instead of repeating its conditions, so the sanitizer cannot come to disagree with the check about what is acceptable — and a test asserts that every name the sanitizer produces is one the validator accepts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 4 minutes Limit details: You’ve used all 4 included reviews currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (12)
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 |
Greptile SummaryThe PR adds a
Confidence Score: 4/5The PR should not merge until spec-derived names are safely rendered in comments so valid inputs cannot produce uncompilable Go files. The new generator directly embeds decoded spec names in Go line comments, allowing an embedded newline to terminate the comment and expose the remaining name as invalid source. Files Needing Attention: lib/src/go/mod.rs Important Files Changed
Reviews (1): Last reviewed commit: "fix(go): reserve the suffixed identifier..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let doc = if e.root { | ||
| format!( | ||
| "// Root is the command tree for `{}`. Pass it to argv.New.", | ||
| self.spec.bin | ||
| ) | ||
| } else { | ||
| format!("// {}", e.cmd.full_cmd.join(" ")) |
There was a problem hiding this comment.
When a valid KDL spec contains an escaped newline in its bin or command name, the decoded newline terminates this generated line comment and emits the remaining name as bare Go source, causing the generated file to fail formatting or compilation.
Knowledge Base Used: Spec Model and KDL Parsing
1 participant
Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Medium Risk
Large new codegen path with many CLI-tree edge cases; mistakes would break Go consumers' builds, but it is additive and does not change existing Rust parsing behavior.
Overview
Adds
usage generate go, a build-time generator that turns a usage KDL spec into gofmt-clean Go source forgithub.com/jdx/usage/go/argv(intended for//go:generate). The CLI accepts-f/--spec, optional-o/--out-file, and-p/--package(validated; invalid names error instead of silent mangling).The core is a new
usage::gomodule that emits binding-only tables: command tree, flags, args, stableuint64keys, inheritedunknown_flags, rootdefault_subcommand(root direct children only), version metadata, and package-name sanitization for spec-derived names. It handles real-world edge cases such as identifier collisions, alias deduplication, and separating flag repeat counts from variadic arg bounds.Specs, man page, Fig completion, docs, and command effect metadata (
read/out-file→write) are updated to document and expose the new subcommand.Reviewed by Cursor Bugbot for commit 858445b. Bugbot is set up for automated code reviews on this repo. Configure here.