fix(derive): emit the tables beside the user's types, not in a module above them - #938
Conversation
|
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 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 48a7214. Configure here.
Greptile SummaryThe PR moves derive-generated tables and implementations from hidden modules into anonymous const blocks so function-local derives retain access to their surrounding scope.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "fix(derive): emit the tables beside the ..." | Re-trigger Greptile |
48a7214 to
029f3a6
Compare
… above them Closes #935. A derive written in a function body did not compile. The tables went into a `mod` of their own, and a module cannot see the scope it was written in, so every reference to the user's type said `super::` — which from a module nested in a `fn` is the enclosing *module*, not the body. The function-local types were invisible, and the error named a generated identifier the author never wrote: error[E0425]: cannot find type `__Usage5LocalInner` in module `super` Every derive shape, not one of them: an enum with a wrapping variant and no bare variant anywhere failed identically. The tables now go in a `const _: () = { … }` with the impls inside it. A const block *is* the surrounding scope, so a name means what the author meant, and the whole `super::` question stops being asked. That deletes the path-rewriting pass — `in_module`/`path_in_module`, which shifted `self::` by one and `super::` by two and left `crate::` and `::` alone — along with the `inside_module` parameter that told `partial_defaults` which side of the wall it was emitting on, and the two spellings a subcommand type needed. Getting that rewriting wrong was never an error here; it was one in the adopter's crate, at a line they did not write. Sharing the scope has a cost the module was paying: the tables used to `use` argv's own types, and an import that was private to the module now sits beside the user's declarations. A subcommand enum called `Command` resolved to `::usage_argv::Command` — three of this repo's own fixtures broke exactly that way. So the tables name every type in full and import nothing. Both halves are tested and mutation-checked: re-importing `Command` fails the shadowing test with `usage_argv::Command<'_>: Subcommands is not satisfied`, and putting a `super::` back on a completer path fails to compile. Generated output is otherwise unchanged — the gate over mise's 211 commands and `gen-shadow` both agree byte for byte.
029f3a6 to
0609d25
Compare
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes. Shadow comparisonParsing
|

Closes #935.
A derive written in a function body did not compile. The tables went into a
modof their own, and a module cannot see the scope it was written in, soevery reference to the user's type said
super::— which from a modulenested in a
fnis the enclosing module, not the body. The function-localtypes were invisible, and the error named a generated identifier the author
never wrote:
Every derive shape, not one of them: an enum with a wrapping variant and no
bare variant anywhere failed identically.
The tables now go in a
const _: () = { … }with the impls inside it. A constblock is the surrounding scope, so a name means what the author meant, and
the whole
super::question stops being asked. That deletes the path-rewritingpass —
in_module/path_in_module, which shiftedself::by one andsuper::by two and left
crate::and::alone — along with theinside_moduleparameter that told
partial_defaultswhich side of the wall it was emittingon, and the two spellings a subcommand type needed. Getting that rewriting
wrong was never an error here; it was one in the adopter's crate, at a line
they did not write.
Sharing the scope has a cost the module was paying: the tables used to
useargv's own types, and an import that was private to the module now sits beside
the user's declarations. A subcommand enum called
Commandresolved to::usage_argv::Command— three of this repo's own fixtures broke exactly thatway. So the tables name every type in full and import nothing.
Both halves are tested and mutation-checked: re-importing
Commandfails theshadowing test with
usage_argv::Command<'_>: Subcommands is not satisfied,and putting a
super::back on a completer path fails to compile.Generated output is otherwise unchanged — the gate over mise's 211 commands
and
gen-shadowboth agree byte for byte.Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Medium Risk
This is a broad change to proc-macro expansion and name resolution for every derived CLI; behavior is intended to stay byte-identical for existing adopters, but any mistake in scoping or qualification would surface as compile errors or subtle path bugs in consumer crates.
Overview
Derive output no longer nests generated tables in a private
mod __usage_*. Parse tables, metadata, helpers, and the inherentimpl(forCli) now live inside aconst _: () = { … }block at the same scope as the user's type, so local types in a function body resolve correctly andcomplete/ flatten / subcommand paths are emitted as written (nosuper::rewriting).Removed the
in_module/path_in_modulemachinery and theinside_moduleflag onpartial_defaults, plus the unit tests that locked in the old rewriting rules.Shadowing fix: generated code drops
use usage_argv::…imports and refers to::usage_argv::Command,Flag,Arg,Spec,*Meta, etc. explicitly so user types namedCommand,Flag, and similar no longer hijack the tables.Conformance adds
where_a_derive_is_written.rscovering function-local derives, shadowed argv type names, and completer paths (coloursvsself::colours).Reviewed by Cursor Bugbot for commit 0609d25. Bugbot is set up for automated code reviews on this repo. Configure here.
AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.