Support final local variables in where-blocks (#138)#2370
Merged
leonard84 merged 18 commits intoJun 13, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (22)
✅ Files skipped from review due to trivial changes (9)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughAdds support for Changeswhere-block local variables feature
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2370 +/- ##
============================================
+ Coverage 82.18% 82.33% +0.15%
- Complexity 4832 4878 +46
============================================
Files 473 474 +1
Lines 15051 15205 +154
Branches 1912 1935 +23
============================================
+ Hits 12369 12519 +150
- Misses 1991 1993 +2
- Partials 691 693 +2
🚀 New features to boost your workflow:
|
leonard84
commented
Jun 7, 2026
leonard84
left a comment
leonard84
left a comment
Member
Author
There was a problem hiding this comment.
Can we support automatic cleanup for AutoCloseable types for where vars? Is there a good location to place a hook/register a cleanup?
Member
Author
Done. Effectively a where-block variable now behaves like a feature-local |
leonard84
force-pushed
the
where-block-variables
branch
from
10c209b to
833222f
Compare
Greptile SummaryThis PR implements
Confidence Score: 3/5The feature is well-designed and broadly tested, but the multiple-assignment validation loop in the compiler leaves The single-variable path and the happy path for multiple assignment are solid. The inconsistency in The validation loop in Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Compiler (WhereBlockRewriter)
participant WV as wherevars() method
participant P as Provider methods
participant PR as Processor method
participant F as Filter method
Note over C: Compile time
C->>C: collectWhereBlockVariables()
C->>WV: generateWhereVariablesMethod()
C->>P: append whereVar params to provider signatures
C->>PR: append whereVar params to processor signature
C->>F: append whereVar params to filter signature
Note over WV,F: Runtime (once per feature)
WV-->>WV: evaluate all final initializers
loop Each iteration
WV->>P: appendArgs(prevTableArgs, whereVariableValues)
P-->>P: return data provider object
P->>PR: appendArgs(rawValues, whereVariableValues)
PR-->>PR: return processed Object[]
PR->>F: appendArgs(processedData, whereVariableValues)
F-->>F: assert filter condition
end
Note over WV: Feature teardown
WV->>WV: closeWhereVariables() LIFO
Reviews (1): Last reviewed commit: "Auto-close AutoCloseable where-block var..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/data_driven_testing.adoc`:
- Around line 429-431: Reflow the newly added AsciiDoc text so every sentence is
on its own line: find the section that explains declaring a `final` local
variable at the start of the `where:` block (the paragraph starting "Sometimes
you need a helper value to build your data...") and break any wrapped sentences
so each sentence occupies a single line; apply the same one-sentence-per-line
fix to the nearby paragraphs mentioned (around lines described in the comment:
the blocks containing the phrases "final local variable", "start of the `where:`
block", and the subsequent explanatory sentences at 443-450, 455-456, 464-465).
Ensure punctuation is preserved and no sentences are merged or split
incorrectly.
In
`@spock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.java`:
- Around line 956-975: The generated where-vars method
(createWhereVariablesMethod) must assign each initializer into the result array
immediately and ensure already-initialized AutoCloseable values are cleaned up
if a later initializer throws: modify the method built in
createWhereVariablesMethod so it constructs an Object[] up front, for each
whereBlockVariables entry evaluate the initializer into the corresponding array
slot (e.g., result[i] = <expr>) instead of collecting expressions separately,
and surround the per-element sequence with a try/catch that on exception
iterates the already-assigned slots and closes any AutoCloseable values before
rethrowing; also ensure the method signature and return (currently in MethodNode
created here) still returns the filled Object[] so
FeatureDataProviderIterator.createWhereVariableValues() receives actual values
or sees proper cleanup on failure.
- Around line 225-247: In collectMultipleAssignmentWhereBlockVariables, skip
wildcard placeholders named "_" so they are not treated as exported where-block
variables: when iterating targets in the second loop (the one that currently
does the InternalIdentifiers.isInternalName and isFinalLocal checks and then
adds to whereBlockVariableNames), detect if varExpr.getName().equals("_") and
simply continue (do not perform reserved-name or final checks and do not add to
whereBlockVariableNames) so that patterns like final (_, x) = ... only export x
and duplicate "_" entries are avoided; leave whereBlockVariables.add(stat)
unchanged so the statement is still recorded.
In
`@spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java`:
- Around line 390-405: The close() path can double-close the same AutoCloseable
when a where-variable is also present in dataProviders or dataProviderIterators;
modify close() / closeWhereVariables() to record identities already closed from
dataProviders and dataProviderIterators (e.g., an Identity-based Set populated
when calling closeQuietly on elements of dataProviders and
dataProviderIterators) and then in closeWhereVariables() skip any
whereVariableValues entries whose identity is present in that set, preserving
the reverse-declaration closing order and still ignoring close failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1dc2a863-143a-4435-9aaa-c39726cced32
📒 Files selected for processing (18)
docs/data_driven_testing.adocdocs/release_notes.adocspock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.javaspock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.javaspock-core/src/main/java/org/spockframework/runtime/IDataIterator.javaspock-core/src/main/java/org/spockframework/runtime/SpecInfoBuilder.javaspock-core/src/main/java/org/spockframework/runtime/model/FeatureInfo.javaspock-core/src/main/java/org/spockframework/runtime/model/MethodKind.javaspock-core/src/main/java/org/spockframework/util/InternalIdentifiers.javaspock-specs/src/test/groovy/org/spockframework/docs/datadriven/DataSpec.groovyspock-specs/src/test/groovy/org/spockframework/runtime/StoreSpec.groovyspock-specs/src/test/groovy/org/spockframework/smoke/ast/DataAstSpec.groovyspock-specs/src/test/groovy/org/spockframework/smoke/parameterization/InvalidWhereBlocks.groovyspock-specs/src/test/groovy/org/spockframework/smoke/parameterization/WhereBlockVariables.groovyspock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/DataAstSpec/where_block_variables.groovyspock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/DataAstSpec/where_block_variables_combined_with_data_table__data_pipe__derived_variables__cross_multiplication_and_filter.groovyspock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/DataAstSpec/where_block_variables_with_multiple_assignment-groovy3_4.groovyspock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/DataAstSpec/where_block_variables_with_multiple_assignment-groovy5.groovy
AndreasTu
requested changes
Jun 11, 2026
AndreasTu
approved these changes
Jun 12, 2026
AndreasTu
left a comment
AndreasTu
left a comment
Member
There was a problem hiding this comment.
LGTM.
Thanks for the feature.
…able, pipe, combine and filter (spockframework#138)
…k#138) The filter-block previously could not reference where-block variables: such references compiled but failed at runtime with a MissingPropertyException. Extend access so filter-blocks see where-block variables, consistent with data tables, data pipes and derived data variables. - WhereBlockRewriter: append the where-variable parameters to the filter method signature, like the provider and processor methods already do. - DataIteratorFactory: propagate getWhereVariableValues() through the iterator chain and pass them when invoking the filter method. - FeatureInfo: mark whereVariablesMethod @nullable for consistency with the neighbouring optional method fields. - Document filter-block visibility and add a smoke test; regenerate the combined AST snapshot.
Allow 'final (a, b) = ...' tuple declarations for where-block variables, in addition to single 'final x = ...'. Each target becomes its own where-block variable; the declaration is emitted verbatim so Groovy handles the destructuring and the existing name-driven machinery covers the rest. Non-final tuples now report the regular must-be-final error. The tuple syntax requires the Parrot parser (Groovy 3.0+), so the new runtime, AST and docs tests are gated with @requires; the AST snapshot is keyed by Groovy version since 5.0 renders the declaration differently. Also address review feedback: return a shared empty-array constant from IDataIterator.getWhereVariableValues(), and tighten the data-variable collision assertion to check the full message.
Close where-block variables that implement AutoCloseable once the feature has finished, in reverse declaration order. Cleanup is quiet: errors thrown while closing are ignored. This hooks into FeatureDataProviderIterator.close(), which already runs after all iterations of the feature complete. A where-block variable thus behaves like a feature-local '@shared @AutoCleanup(quiet = true)' field.
- reject the '_' wildcard in multiple-assignment where-block variables with a clear error - validate all multiple-assignment targets before exposing any name, avoiding partial-state corruption - document NO_WHERE_VARIABLE_VALUES as a shared, must-not-mutate sentinel - reflow new docs section to one sentence per line and document known auto-close limitations
…kframework#138) Without a data variable the data processor method is never generated, so the where-variables method would exist in bytecode but never be invoked: the declarations were silently skipped, and an AutoCloseable variable would never be created or closed. Reject this with a compile error. Also from the same review pass: - test that a throwing initializer fails the feature with the original exception - tests locking in that duplicate where-block variable names are rejected by Groovy's own scope check - use NO_WHERE_VARIABLE_VALUES instead of allocating empty arrays - add @SInCE 2.5 to the new IDataIterator members - docs wording fix for the multiple-assignment Groovy version note
…pockframework#138) RecordingCloseable.closed is shared static state, but spock-specs runs with parallel execution enabled, so one feature's findAll could iterate the list while a concurrently running feature appends to it, which intermittently threw ConcurrentModificationException. Serialize the features touching the list with @ResourceLock.
…framework#138) - reword the awkward intro sentence of the docs section - move isFinalLocal to AstUtil.isFinal, next to setFinal - replace the parallel statement/name lists with a WhereBlockVariable holder that owns the declaration statement and the names it introduces - extract the shared target validation for single and multiple-assignment declarations so their semantics cannot drift - reject the '_' wildcard for single declarations too; it was only rejected in multiple assignments and a single 'final _ = ...' would have registered a variable shadowing Spock's wildcard - clear RecordingCloseable.closed in the features locking it and tighten the findAll assertions to exact equality
…spockframework#138) Previously all initializers ran as a single unit inside the generated where-variables method, so when a later initializer threw, the values created by earlier ones never reached the runtime and leaked unclosed. This was documented as a known limitation. The generated method now declares an Object[] up front and fills one slot per variable as the initializers run (a multiple assignment fills one slot per name). When an initializer throws, a catch block calls the new SpockRuntime.closeWhereBlockVariablesAfterFailure, which closes the already-created AutoCloseable values in reverse declaration order, attaches close failures to the original failure as suppressed exceptions, and lets the catch block rethrow it. Slots whose initializer never ran are still null and are skipped. Values that never made it into a variable remain out of reach, for example a resource created inside the failing initializer expression itself; the docs now describe this inherent remainder instead of the old limitation.
leonard84
force-pushed
the
where-block-variables
branch
from
b6c3a9f to
4766fef
Compare
🚨 TestLens detected 1 failed test 🚨Here is what you can do:
Test Summary
🏷️ Commit: 4766fef Test FailuresAsyncConditionsSpec > passing example - check passes (:spock-specs:test in Verify Branches and PRs / Build and Verify (4.0, 8, windows-latest))Muted TestsNote Checks are currently running using the configuration below. Select tests to mute in this pull request: 🔲 AsyncConditionsSpec > passing example - check passes Reuse successful test results: 🔲 ♻️ Only rerun the tests that failed or were muted before Click the checkbox to trigger a rerun: ☑️ Rerun jobs Learn more about TestLens at testlens.app. |
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
2 participants
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
Implements #138:
finallocal variables inwhere:blocks, following the design discussed on the issue.You can declare
finallocals at the start of awhere:block as scoped helper values that are not turned into data variables / method parameters:Semantics
finalis the marker.final x = …declares a where-block variable; a barex = …remains a derived data variable. Non-finaldeclarations are a compile error pointing atfinal.final fixture = new Fixture()is a single shared instance.filter:block; not feature-method parameters and not visible in the feature body.@Shared/staticfields, not instance fields.$spock_.Implementation
The compiler collects the leading
finaldeclarations into a single generated…wherevars()method that returns their values as anObject[]. The runtime invokes it once per feature and passes the values into every data provider, the data processor, and the filter method as trailing parameters named after the user's variables (mirroring the existing previous-data-table-variable mechanism). No AST duplication; single evaluation.Touched:
WhereBlockRewriter,InternalIdentifiers,MethodKind,FeatureInfo,SpecInfoBuilder,IDataIterator,DataIteratorFactory.Tests & docs
WhereBlockVariables): data tables, data pipes, derived variables, single-evaluation across providers, cross-multiplication (combined:),filter:,@Sharedaccess, not-visible-in-body.InvalidWhereBlocks): non-final, misplaced, multiple assignment, reserved prefix, name collision, instance-field access.DataAstSpec): a focused case and a comprehensive one combining table + pipe + derived + combine + filter.data_driven_testing.adoc(with a tested example) and a release note.Closes #138