Repository navigation
Conversation
📊 Automated PR Analysis
SummaryFixes gradlew task detection to use a proper argument tokenizer instead of a naive 'last non-dash argument' heuristic, so flag values (e.g. Review Checklist
Analyzed automatically by wshm · This is an automated analysis, not a human review. |
KuSh
marked this pull request as draft
September 12, 2026 22:22
KuSh
force-pushed
the
feat/gradlew-arg-tokenizer
branch
2 times, most recently
from
September 18, 2026 21:11
6c552fd to
6f03235
Compare
KuSh
marked this pull request as ready for review
September 18, 2026 21:15
KuSh
force-pushed
the
feat/tokenizer-dialect-axes
branch
from
October 6, 2026 22:35
a41cc10 to
cd9d7a6
Compare
KuSh
force-pushed
the
feat/gradlew-arg-tokenizer
branch
2 times, most recently
from
October 6, 2026 22:49
a1a85eb to
bf81426
Compare
KuSh
force-pushed
the
feat/gradlew-arg-tokenizer
branch
from
October 6, 2026 22:54
bf81426 to
d6546df
Compare
…the task `detect_task` picked the last argument that did not start with `-`, with no notion of a flag consuming the next token. `gradlew test --tests com.example.Foo` therefore resolved the task to `com.example.Foo`, which matches no branch, and the run fell through to unfiltered passthrough; `gradlew assembleDebug -p ../other` did the same. Verified against gradle 9.7.1 and 8.14 that `--tests`, `-p`, `-x`, `-P`, `-D`, `--console`, `--configuration` and the rest of the global option table take a separate value, and that a clustered value-taker does not (`-mx compileJava` is rejected), hence `solo_only` for shorts. Task detection and the verbose-logging bypass now go through `tokenize_grammar` under `Dialect::Posix`. The bypass gains gradle's short spellings (`-i`/`-d`/`-s`/`-S`) and stops firing when `--info` is another flag's value. Gradle's `--` ends built-in option parsing but keeps parsing tasks and their options, so the region past it is re-tokenized with the same grammar; `restore_double_dash` hands back the `--` clap strips, which gradle treats as meaningful. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`tasks --group build` still read `build` as the task and routed the listing into the build filter, which swallowed it. `gradle help --task` on 9.7.1 confirms `--group`, `--groups`, `--args` and `--dependency` take values too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Consuming a flag's value can leave zero task names, and `gradlew -p ../other` then fell to the `Build` fallback and had gradle's default-task output stripped by the build filter. That fallback is now reserved for a genuinely task-less invocation. `gradle help --task init` also lists eight value-taking options; without them `init --test-framework testng` read `testng` as the task and routed to the test filter, which swallowed the scaffolding output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`gradlew -p ../other` already stayed unfiltered, but bare `gradlew` still hit the `Build` fallback and had its default-task output stripped. Same reason, so it takes the same branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`task_names` re-tokenized the region past `--` by hand, because no dialect modelled a `--` that ends only the *global* option region while tasks and their options keep parsing. `DashDashRole::EndsGlobalOptions` models exactly that, so one pass over the whole arg vector replaces the split, and `free_positionals` folds into its only remaining caller. Gradle's parser is `org.gradle.cli`, shared with no other tool, so the dialect is composed from `Posix` at this call site instead of becoming a preset. The one behaviour the single pass would have changed is a repeated `--`, which is an ordinary positional rather than a second boundary. Gradle 9.7.1 runs `gradle -- test --` as `test`, so it is dropped from the task names and pinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gradlew grammar test named CARGO_BIN_EXE_rtk directly, so the child it spawned wrote to the contributor's real ~/.local/share/rtk/ (tracking rows, tee spool). Build the command with common::rtk_command(), which redirects rtk's data to the test binary's scratch directory, as the other integration tests do. core::test_isolation::tests_that_spawn_rtk_isolate_their_data rejects a test file that names the binary itself. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
KuSh
force-pushed
the
feat/gradlew-arg-tokenizer
branch
from
October 7, 2026 00:36
d6546df to
044dca1
Compare
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.
The bug
detect_taskpicked the gradle task as "the last argument that does not start with-, ignoringclean". It has no notion of a flag consuming the next token, so a flag's value became the task:Routing is substring-based (
task.contains("test")), so some wrong values still land on the right branch by luck (--tests MyTestcontains "test"); the reproducers above are where the luck runs out. Confirmed on the develop binary against a./gradlewstub: both print the daemon banner and thePASSEDline, i.e. no filter ran at all.Flag facts, verified against real gradle
gradle --helpongradle:jdk21(9.7.1) andgradle:8.14-jdk21, plus actual runs against a throwaway project, both underLC_ALL=C:test --tests com.example.Foo--tests,Selection failed: Task 'com.example.Foo' not found-P myprop=v/-Pmyprop=v-D sysprop=v/-Dsysprop=v--console plain,-x compileJava,-M sha256,--update-locks org.foo:bardependencies --configuration compileClasspath,help --task showPropstasks --group build,run --args x,dependencyInsight --dependency x,init --test-framework testnggradle help --task <name>lists them;initalone has eight)-qm-mx compileJava-xcompileJava-b/-cDialect
Gradle's parser is
org.gradle.cli—gradle:jdk21shipsgradle-cli-9.7.1.jarand nocommons-cli— and no other tool uses it, so under #4008's preset naming rule it gets no preset. It is a one-axis derivation ofPosix, composed as aconstat its call site ingradlew_cmd.rs:Every other axis is
Posix, and each is measured:gradle -qi tasksworks, so short flags cluster; only=attaches; every short is single-char; there is no/flagspelling. Shorts useValueSpec::solo_only()—gradle -qp /w tasksis rejected whilegradle -q -p /wandgradle -q -p=/wwork, which is exactly whatsolo_onlymodels, and the same shape git's-nalready uses.DashDashRole::EndsGlobalOptionsis the axis value gradle needs.gradle --helpcalls--"Signals the end of built-in options. Parses subsequent parameters as tasks or task options only", and that is measurably what it does —gradle -- test --tests com.example.Foostill consumes the filter value, whilegradle -- showProps --offlinefails. That is neitherEndsOptions(git, cargo, rg) norForwards(dotnet).So task detection is one pass under
GRADLE. A repeated--needs one extra rule: where a task is expected it is a task name (gradle -- -- help→Task '--' not found), but after a task it separates that task's options (gradle -- help --→BUILD SUCCESSFUL), so a--is dropped from the task names.wants_verbose_loggingstill slices withbefore_dashdash, because gradle rejects its own global options past--.#[allow(dead_code)]comes offDashDashRole::EndsGlobalOptions, which gradlew is the in-tree caller of.Also migrated
The verbose-logging bypass was an
arg == "--flag"scan. It now runs on tokens, which gains two things:-i/-d/-s/-Sturn filtering off the same way--infoetc. already did;--infositting in a value position (--exclude-task --info) no longer trips it.restore_double_dashis now called: clap strips the user's--when it is the first token of the trailing region (measured:rtk gradlew -- test …reached the stub astest …, whilertk gradlew test -- …kept its--). Gradle's--is meaningful, so dropping a token the user typed is a silent argv mutation.gradlewis the only command routing through this module —gradle,./gradlewandgradlew.batall rewrite tortk gradlew(src/discover/rules.rs), so nothing is left behind.When no task resolves
Consuming a flag's value can leave no task name at all, and gradle then runs its default
helptask. Barertk gradlewand flags-only invocations (-p ../other,--console plain,-g <home>,-I <script>) therefore stay on passthrough instead of the build filter, which would strip that output.The value-taking task options cover the built-in tasks rtk routes on rather than claiming to be exhaustive: plugins define task options freely.
Tests
tests/gradlew_arg_grammar_test.rsstubs./gradlewwith a script that records its argv and prints gradle-shaped output, then asserts on the resolved routing (did a filter run) and the child argv, not on the predicate, which reports green for this whole bug family. It spawns rtk throughcommon::rtk_command(), so it never writes to the developer's real~/.local/share/rtk/. Unit tests coverdetect_task's resolved task, including the repeated--and the no-task cases.Each behavioural test fails against the previous implementation and passes after.
test_detect_last_task_still_winsandtest_detect_attached_project_prop_is_not_the_taskpass both ways by design: they pin the existing "last task wins" intent and a case the oldstarts_with('-')scan already got right.