fix: @CompileStatic on @Service abstract class with injected @Service properties fails to compile#15396
Conversation
…stract classes
ServiceTransformation generated lazy getters on abstract class PropertyNodes
that referenced varX('datastore') — a field only present on the generated
$ServiceImplementation class. Under @CompileStatic, this caused:
1. Unresolvable variable reference (datastore field not in abstract class scope)
2. StaticTypeCheckingVisitor 'Unexpected return statement' in property getter blocks
Fix: Remove lazy getter block from abstract class PropertyNodes entirely. The
generated setDatastore() method on the impl class already eagerly populates all
@Service-typed properties during initialization, making the lazy fallback redundant.
Adds 6 Spock tests covering:
- @CompileStatic + single/multiple @Service-typed properties
- @CompileStatic + custom methods with complex return statements
- Regression: dynamic mode still works
- Regression: no datastore infrastructure when no @Service-typed properties
- Impl class has correct setDatastore/getDatastore/datastore field
jdaugherty
left a comment
There was a problem hiding this comment.
We should add a functional test for this scenario too.
There was a problem hiding this comment.
Pull request overview
This pull request fixes a compilation bug where @CompileStatic (or @GrailsCompileStatic) on a @Service abstract class causes compilation failure when the class has properties whose types are annotated with @Service. The root cause was that ServiceTransformation was setting a lazy getter block on the abstract class's PropertyNode that referenced a datastore field only present on the generated implementation class, causing "Unexpected return statement" errors under @CompileStatic.
Changes:
- Removed lazy getter block generation from abstract class PropertyNode in ServiceTransformation
- Added comprehensive test suite covering @CompileStatic service injection scenarios
- Added explanatory comment documenting why lazy getter block is not set
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ServiceTransformation.groovy | Removed lazy getter block that referenced non-existent 'datastore' field; added comment explaining the fix relies on eager initialization in setDatastore() |
| CompileStaticServiceInjectionSpec.groovy | New comprehensive test suite with 6 tests covering @CompileStatic + service injection, dynamic mode regression, multiple services, and datastore infrastructure verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...re/src/main/groovy/org/grails/datastore/gorm/services/transform/ServiceTransformation.groovy
Outdated
Show resolved
Hide resolved
...tamapping-core/src/test/groovy/grails/gorm/services/CompileStaticServiceInjectionSpec.groovy
Outdated
Show resolved
Hide resolved
…ervice property Add AuthorDataService interface, CompileStaticBookService abstract class with @CompileStatic and AuthorDataService injection, and 7 integration tests in GormDataServicesSpec that exercise cross-service method calls under static compilation. Assisted-by: OpenCode <opencode@opencode.ai>
Functional tests added to existing test app
Fix inaccurate comment in ServiceTransformation that said 'lazy getter methods are generated on the impl class' when the code actually uses eager assignment in setDatastore(). Rename test from 'setDatastore eagerly populates service properties' to 'impl has datastore infrastructure' since it only verifies structural presence, not runtime behavior. Assisted-by: OpenCode <opencode@opencode.ai>
Remove assignX, equalsNullX, and ifS static imports from ServiceTransformation that became unused after the lazy getter code was removed from @service abstract classes. Assisted-by: Claude Code <Claude@Claude.ai>
…iTenant routing, and CRUD connection fixes Add documentation reflecting the post-fix state after PRs apache#15393, apache#15395, and apache#15396 are merged: - Add @CompileStatic + injected @service property example (PR apache#15396) - Add Multi-Tenancy with explicit datasource section (PR apache#15393) - List all CRUD methods that respect connection routing (PR apache#15395) - Soften IMPORTANT boxes to NOTE with authoritative tone Assisted-by: Claude Code <Claude@Claude.ai>
Summary
@CompileStatic(or@GrailsCompileStatic) on a@Serviceabstract class causes a compilation failure when the class has properties whose types are annotated with@Service. The error is:This forces users to omit
@CompileStaticon any@Serviceabstract class that injects other Data Services, losing static compilation benefits (compile-time type checking, better performance).Bug Description
Given two Data Services where one injects the other:
Compilation fails with:
Without
@CompileStatic, the code compiles and runs correctly.Root Cause
ServiceTransformation(SEMANTIC_ANALYSIS phase) iterates over the abstract class's properties and, for each property whose type has@Service, sets a lazy getter block on the abstract class's PropertyNode:This causes two problems under
@CompileStatic:varX('datastore')is unresolvable - thedatastorefield only exists on the generated$BookServiceImplementationclass, not on the abstractBookServiceclass where the getter block is being set.ReturnStatementin property getter block -StaticTypeCheckingVisitor.visitProperty()does not expectReturnStatementnodes inside property getter blocks, causing the "Unexpected return statement" error during instruction selection.Under dynamic Groovy (no
@CompileStatic), neither issue surfaces because variable references resolve at runtime and the static type checker is not invoked.Fix
Removed the lazy getter block from the abstract class's
PropertyNodeentirely. The getter block was redundant because the generatedsetDatastore()method on the impl class already eagerly populates all@Service-typed properties during initialization:This eager initialization runs when the Spring context wires the
Datastorebean, so by the time any user code accesses the property, it's already populated. The lazy fallback was unnecessary.Changes: 1 file modified -
ServiceTransformation.groovy(8 insertions, 9 deletions). Replaced thepn.setGetterBlock(...)block with a comment explaining why it was removed.Test Coverage
Unit Tests - CompileStaticServiceInjectionSpec (6 tests)
@CompileStaticand one@Service-typed property compiles without error@CompileStaticstill compiles (regression check)@Service-typed properties doesn't get datastore field (regression check)@Service-typed properties compiles without errorFunctional Tests - GormDataServicesSpec (7 new tests in
grails-test-examples/gorm)Added
AuthorDataService(interface@Service(Author)) andCompileStaticBookService(@CompileStaticabstract@Service(Book)injectingAuthorDataService) to the gorm functional test app, with integration tests:@Service-typed property is wired correctlyAll tests pass. All 30 existing
ServiceTransformSpectests pass (no regressions).Example Application
https://github.com/jamesfredley/grails-compile-static-service-bug
A minimal Grails 7 app demonstrating the bug.
BookServiceis a@Service(Book)abstract class with anAuthorDataServiceproperty.@CompileStaticis commented out as a workaround.To reproduce the bug:
@CompileStaticingrails-app/services/com/example/BookService.groovy./gradlew compileGroovyBUG! exception in phase 'instruction selection' ... Unexpected return statement at -1:-1 return authorDataServiceTo run with workaround:
./gradlew bootRunhttp://localhost:8080/bugDemo/index{"bug_present": true, "workaround_applied": true, ...}Environment Information
Version
7.0.7