[daniellansun]

https://issues.apache.org/jira/browse/GROOVY-12441

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.19658% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.5095%. Comparing base (6b9812e) to head (a791aac).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...ransform/stc/SharedVariableExpressionRewriter.java	80.0000%	4 Missing and 18 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #3068        +/-   ##
==================================================
+ Coverage     74.5006%   74.5095%   +0.0089%     
- Complexity      44947      44991        +44     
==================================================
  Files            1703       1704         +1     
  Lines          152337     152453       +116     
  Branches        29187      29223        +36     
==================================================
+ Hits           113492     113592       +100     
- Misses          28413      28414         +1     
- Partials        10432      10447        +15     

Files with missing lines	Coverage Δ
...g/codehaus/groovy/ast/expr/VariableExpression.java	98.4615% <ø> (ø)
...roovy/transform/stc/StaticTypeCheckingVisitor.java	87.9633% <100.0000%> (-0.0055%)	⬇️
...ransform/stc/SharedVariableExpressionRewriter.java	80.0000% <80.0000%> (ø)

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[daniellansun]

Friendly ping: CI is green and this is ready for review. If there are no objections, I plan to merge it this weekend. Thanks!

[paulk-asert]

I agree something is needed here beyond documentation: MethodPointerExpression.getExpression() hands out THIS_EXPRESSION for a null receiver and transformExpression bakes it in, so after any ClassCodeExpressionTransformer pass the compiler's own AST carries the shared node, and groovy-contracts uses it in generated code too. On master 7 of the 16 cases in Groovy12441 fail, plus one the test doesn't cover yet: this::tag inside a closure fails with "Failed to find method 'tag()' for the type: C$_run_closure1". This PR fixes that one as well, so it's worth adding.

I also tried the other obvious fix, making SC resolve an untyped this in a closure to the enclosing class (StaticTypesTypeChooser + StaticInvocationWriter, 14 lines). It passes all 16 cases but breaks 8 existing tests (Groovy7996, 9063, 9288, 9293, testMapPropertyAccess10, Memoized record serialization), because classgen itself creates untyped this nodes inside closure classes that really do mean the closure. So typing each use before classgen, as done here, is the right layer.

On Jochen's point about the amount of visitor code: the same four hook points work with a ClassCodeExpressionTransformer subclass whose only override is transform() (fresh node for either sentinel; descend into closures/lambdas as its Javadoc shows), plus a small CodeVisitorSupport presence check so members without a shared node are never rebuilt. That's about 100 lines including the header, and gives the same result: STC/SC/bugs/gls/transform/typing suites green (13,323 tests), all 18 cases passing. It also removes the getText() special case: MethodPointerExpression.transformExpression never routes a truly-null receiver through transform(), so implicit pointers stay implicit on their own. (As written, the startsWith("::") branch is dead code: MethodReferenceExpression.getText() goes through getExpression(), which already substitutes the shared node, so an implicit ::m would currently be rewritten to an explicit this::m.) The trade-off is that transformExpression rebuilds the composite nodes of the affected member rather than mutating in place; metadata and positions are copied, as StaticCompilationTransformer already relies on. With that, the rewriter's own unit test could shrink to the bugs test.

Happy to share the patch if useful. Separately, a Javadoc on THIS_EXPRESSION/SUPER_EXPRESSION pointing transform authors at varX("this") would address Björn's request, either here or as a follow-up.

[daniellansun]

@paulk-asert Thank you — that analysis was exactly what we needed. Tip 17ee5d34 follows your sketch:

• SharedVariableExpressionRewriter is now a thin ClassCodeExpressionTransformer + presence check (same four hook points).
• Added the missing this::tag case under @CompileStatic, plus switch-arm and classic for-loop cases inside closures.
• Dropped the dead getText() / startsWith("::") path; implicit pointers/references stay implicit.
• Javadoc on THIS_EXPRESSION / SUPER_EXPRESSION now points transform authors at varX("this") / varX("super").

Locally: Groovy12441 + rewriter unit tests green, and a broader STC/SC smoke (groovy.transform.stc.* + asm.sc.*) also green. CI on this tip is re-running. Appreciate the detailed write-up — it saved a lot of round-trips.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: a791aac
▶️ Tests: 132076 executed
⚪️ Checks: 26/26 completed

---

Learn more about TestLens at testlens.app/docs.

[daniellansun]

Ping @paulk-asert ;-)