Skip to content

fix(coreutils-port): accept localized-Command let-binding in uu_app#1628

Open
chaliy wants to merge 1 commit into
mainfrom
fix/coreutils-port-truncate-let-binding
Open

fix(coreutils-port): accept localized-Command let-binding in uu_app#1628
chaliy wants to merge 1 commit into
mainfrom
fix/coreutils-port-truncate-let-binding

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented May 12, 2026

Summary

Yesterday's Coreutils Args Drift workflow on main went red at the regenerating truncate step:

error: unsafe uu_app body: expected a single clap Command builder expression, found 2 statements

Upstream truncate (rev 94b06d97b) now routes the Command through uucore::clap_localization::configure_localized_command(cmd), so the body has the shape:

pub fn uu_app() -> Command {
    let cmd = Command::new("truncate");
    uucore::clap_localization::configure_localized_command(cmd)
        .arg()}

The rewriter elides the localization helper (it pulls in Fluent; bashkit doesn't need it), leaving the body as a 2-stmt let cmd = …; cmd.arg(…).arg(…) form. PR #1617 had tightened the validator to a single expression, which is what this case trips. The on-disk truncate_args.rs was generated before #1617 and already uses this exact shape, so this PR only re-enables the codegen path that produced it.

Fix

Extend validate_uu_app_body in crates/bashkit-coreutils-port/src/args.rs to also accept the 2-stmt shape [Stmt::Local(local), Stmt::Expr(tail, None)] with strict guards so TM-INF-025 does not weaken:

  • The let pattern must be a plain Pat::Ident — no mut, no ref, no subpattern, no type ascription, no let … else ….
  • The initializer must pass the same per-expression validator (no closures, loops, match, blocks, unsafe, async; only allowlisted macros; only allowlisted method names) AND chain-root at Command::new(...), identical to the single-expression path.
  • The tail must pass the per-expression validator AND its receiver chain must bottom out at exactly the binder identifier — not at another Command::new, not at a function call, not at a different variable.

Anything richer (tuple patterns, refutable matches, type ascription, mutability, an extra prefix statement) still fails.

Test plan

  • 5 new tests in args::tests cover both directions:
    • positive: accepts_localized_command_let_binding mirroring truncate's exact shape, asserting the wrapper is elided in the emitted body and the tail continues at cmd.arg(…)
    • negative: rejects_let_mut_binding_in_uu_app, rejects_three_statement_uu_app, rejects_tail_rooted_at_other_ident, rejects_let_init_not_rooted_at_command_new
  • cargo test -p bashkit-coreutils-port green (37 tests)
  • cargo build -p bashkit green
  • cargo clippy -p bashkit-coreutils-port --all-targets -- -D warnings clean
  • cargo fmt --check clean
  • Manually dispatch Coreutils Args Drift after merge and confirm regenerating truncate is green

Generated by Claude Code

Yesterday's `Coreutils Args Drift` workflow on main went red at the
`regenerating truncate` step:

    error: unsafe uu_app body: expected a single clap Command builder
           expression, found 2 statements

Upstream `truncate` (rev 94b06d97b) now routes the Command through
`uucore::clap_localization::configure_localized_command(cmd)`, so the
body has the shape

    pub fn uu_app() -> Command {
        let cmd = Command::new("truncate")…;
        uucore::clap_localization::configure_localized_command(cmd)
            .arg(…)…
    }

The rewriter elides the localization helper (it pulls in Fluent;
bashkit doesn't need it), leaving the body as a 2-stmt `let cmd = …;
cmd.arg(…).arg(…)` form. PR #1617 had tightened the validator to a
single expression, which is what this case trips.

## Fix

Extend `validate_uu_app_body` to also accept the 2-stmt shape

    [Stmt::Local(local), Stmt::Expr(tail, None)]

with strict guards so the safety story does not weaken:

- The `let` pattern must be a plain `Pat::Ident` — no `mut`, no `ref`,
  no subpattern, no type ascription, no `let … else …`.
- The initializer must pass the same per-expression validator
  (no closures, loops, match, blocks, unsafe, async; only allowlisted
  macros; only allowlisted method names) AND chain-root at
  `Command::new(...)`, identical to the single-expression path.
- The tail must pass the per-expression validator AND its receiver
  chain must bottom out at exactly the binder identifier — not at
  another Command::new, not at a function call, not at a different
  variable. This proves the tail is just continuing the same builder
  chain.

Anything richer (tuple patterns, refutable matches, type ascription,
mutability, an extra prefix statement) still fails, keeping the
TM-INF-025 boundary closed.

## Test plan

- [x] 5 new tests in `args::tests` cover both directions: positive
  (`accepts_localized_command_let_binding` mirroring truncate's exact
  shape, asserting the wrapper is elided in the emitted body and the
  tail starts at `cmd.arg(…)`); negative (`rejects_let_mut_binding…`,
  `rejects_three_statement_uu_app`, `rejects_tail_rooted_at_other_ident`,
  `rejects_let_init_not_rooted_at_command_new`).
- [x] `cargo test -p bashkit-coreutils-port` green (32 + 5 → 37 tests).
- [x] `cargo build -p bashkit` green (existing generated
  truncate_args.rs already uses this shape; this PR only re-enables
  the codegen path that produced it).
- [x] `cargo clippy -p bashkit-coreutils-port --all-targets -- -D warnings`
  clean.
- [x] `cargo fmt --check` clean.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit d6d2d6c Commit Preview URL

Branch Preview URL
May 12 2026, 09:26 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant