fix: reject --key=value form in unknown-flag guard#373
Merged
Conversation
split each arg on '=' before the shape check so --engine=tree and --foo=bar are caught the same way as --engine tree. before this the shape predicate excluded any token containing '=', so the equals form slipped past the guard, got silently consumed as a positional, and surfaced as a misleading ILO-R012/R004 downstream. allowlist also matches against the --key head, so callers that pre-approve a known flag accept both --bench and --bench=on. the pre-existing unit test that intentionally accepted --foo=bar was flipped to assert rejection; the doc comment justifying the old behaviour was itself the bug. new tests cover --engine=tree, --foo= (empty value), allowlist-head acceptance, and --foo=bar after the -- separator.
cover the bare-positional path, the run subcommand path, the -- separator escape still working on the equals form, and key=value (no leading --) staying as data. update the file-level contract comment to spell out that the --word=value shape is now caught. examples/unknown-flag-equals-form.ilo gives the engine harness a happy-path file so the guard's no-op behaviour on plain positional args stays under cross-engine regression coverage.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Summary
ab-tester rerun9 noticed the unknown-flag guard catches
--bogusand--engine treebut lets--engine=treeand--foo=barthrough. The equals form gets silently consumed as a positional and surfaces as a misleadingILO-R012 no functions definedorILO-R004 main: expected N args, got N+1downstream. Same retry-tax trap as the bare-flag variant six personas burned minutes on in rerun8, just on a shape the guard didn't cover.Manifesto framing: zero-token-cost diagnostics on the highest-frequency CLI mistake. The fix is one
split_once('=')away from symmetric coverage with the space-separated form, and it turns a confused arity error into a clear "unrecognised flag" with the--escape hint already in place.Repro
Before (on
main, 0.11.8):After:
Symmetric with the already-working
ilo m.ilo main --engine tree 5error.What's in the diff
cf23109 reject --key=value form in unknown-flag guard
reject_unknown_flags_with_allowlistnowsplit_once('=')on each arg and runslooks_like_clean_long_flagagainst the--keyhead.--bench=onas well as--bench.--foo=barwas flipped to assert rejection. The doc comment justifying the old "err on the side of accepting it" behaviour was itself the bug.--engine=tree,--foo=barrejection, allowlist-head acceptance,--separator escape on equals form,--foo=empty-value rejection,key=value(no leading--) data passthrough.a0997e9 add cross-CLI regression tests for --key=value rejection
tests/regression_unknown_flag_guard.rscovering bare-positional path,runsubcommand path,--separator escape on the equals form, andkey=valuedata passthrough.--word=valueshape is now caught.examples/unknown-flag-equals-form.iloexercises the guard's no-op behaviour on plain positionals across every engine via the existingtests/examples_engines.rsharness.Test plan
cargo test --release --features craneliftgreen locally (full suite)cargo test --release --features cranelift -- unknown_flagall green, including the four new equals-form casescargo test --release --features cranelift --test examples_enginesgreen (new example file picked up automatically)cargo fmt --checkcleancargo clippy --release --features cranelift --all-targets -- -D warningsclean--engine=tree,--foo=barboth now error cleanly with exit 1;--engine tree/--bogusstill error;-- --foo=barstill passes through as dataFollow-ups
None. Behaviour parity restored with the space-separated form; nothing else in the CLI path expects the equals form to slip through.