GROOVY-11480: SC: check for Type.this.name within lambda

apache · Sep 18, 2024 · 59b02b5 · 59b02b5
1 parent 168bb9b
commit 59b02b5
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 22 deletions.
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesLambdaWriter.java
@@ -28,6 +28,7 @@
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.builder.AstStringCompiler;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.LambdaExpression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
@@ -141,13 +142,22 @@ private static Parameter[] createDeserializeLambdaMethodParams() {
     private static boolean isAccessingInstanceMembersOfEnclosingClass(final MethodNode lambdaMethod) {
         boolean[] result = new boolean[1];
 
-        ClassNode enclosingClass = lambdaMethod.getDeclaringClass().getOuterClass();
-
         lambdaMethod.getCode().visit(new CodeVisitorSupport() {
+            @Override
+            public void visitConstantExpression(final ConstantExpression expression) {
+                if ("this".equals(expression.getValue())) { // as in Type.this.name
+                    result[0] = true;
+                }
+            }
             @Override
             public void visitVariableExpression(final VariableExpression expression) {
-                if (expression.isThisExpression() || enclosingClass.equals(expression.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER))) {
+                if ("this".equals(expression.getName()) || "thisObject".equals(expression.getName())) {
                     result[0] = true;
+                } else {
+                    var owner = expression.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
+                    if (owner != null && lambdaMethod.getDeclaringClass().getOuterClasses().contains(owner)) {
+                        result[0] = true;
+                    }
                 }
             }
         });

diff --git a/src/test/groovy/transform/stc/LambdaTest.groovy b/src/test/groovy/transform/stc/LambdaTest.groovy
@@ -1020,6 +1020,41 @@ final class LambdaTest {
         '''
     }
 
+    @Test
+    void testNestedLambdaAccessingInstanceFields2() {
+        for (qual in ['','this.','thisObject.','getThisObject().']) {
+            assertScript shell, """
+                class Test1 {
+                    def m() {
+                        Optional.empty().orElseGet(() -> Optional.empty().orElseGet(() -> ${qual}n))
+                    }
+                    private n = 123
+                }
+
+                assert new Test1().m() == 123
+            """
+        }
+    }
+
+    // GROOVY-11480
+    @Test
+    void testNestedLambdaAccessingInstanceFields3() {
+        for (qual in ['','Test1.this.']) {
+            assertScript shell, """
+                class Test1 {
+                    class Test2 {
+                        def m() {
+                            Optional.empty().orElseGet(() -> Optional.empty().orElseGet(() -> ${qual}n))
+                        }
+                    }
+                    private n = 123
+                }
+
+                assert new Test1.Test2(new Test1()).m() == 123
+            """
+        }
+    }
+
     @Test
     void testInitializeBlocks() {
         assertScript shell, '''

diff --git a/src/test/org/codehaus/groovy/classgen/FinalVariableAnalyzerTest.groovy b/src/test/org/codehaus/groovy/classgen/FinalVariableAnalyzerTest.groovy
@@ -19,6 +19,7 @@
 package org.codehaus.groovy.classgen
 
 import groovy.test.GroovyTestCase
+import groovy.transform.AutoFinal
 import groovy.transform.CompileStatic
 import org.codehaus.groovy.ast.ClassNode
 import org.codehaus.groovy.ast.InnerClassNode
@@ -29,9 +30,10 @@ import org.codehaus.groovy.control.MultipleCompilationErrorsException
 import org.codehaus.groovy.control.SourceUnit
 import org.codehaus.groovy.control.customizers.builder.CompilerCustomizationBuilder
 
-class FinalVariableAnalyzerTest extends GroovyTestCase {
+@AutoFinal
+final class FinalVariableAnalyzerTest extends GroovyTestCase {
 
-    protected void assertFinals(final Map<String, Boolean> expectations, final String script) throws Exception {
+    protected void assertFinals(Map<String, Boolean> expectations, String script) throws Exception {
         def cc = new CompilerConfiguration()
         CompilerCustomizationBuilder.withConfig(cc) {
             inline(phase: 'SEMANTIC_ANALYSIS') { source, context, classNode ->
@@ -43,7 +45,7 @@ class FinalVariableAnalyzerTest extends GroovyTestCase {
         shell.parse(script)
     }
 
-    protected void assertFinalCompilationErrors(List<String> vars, final String script, boolean unInitialized = false) {
+    protected void assertFinalCompilationErrors(List<String> vars, String script, boolean unInitialized = false) {
         Set<String> checked = []
         try {
             assertFinals [:], script
@@ -58,7 +60,7 @@ class FinalVariableAnalyzerTest extends GroovyTestCase {
                 checked << var
             }
         }
-        assert (vars - checked).empty
+        assert (vars - checked).isEmpty()
     }
 
     void testVariableShouldBeEffectivelyFinal() {
@@ -293,7 +295,6 @@ class FinalVariableAnalyzerTest extends GroovyTestCase {
         ''')
     }
 
-
     void testDirectlyAssignedAICSharedVariableShouldBeConsideredFinal() {
         assertFinals x: true, '''
             def x = 1
@@ -574,19 +575,19 @@ class FinalVariableAnalyzerTest extends GroovyTestCase {
         ''')
     }
 
-    @CompileStatic
+    @AutoFinal @CompileStatic
     private static class AssertionFinalVariableAnalyzer extends FinalVariableAnalyzer {
 
+        private final Map<String, Boolean> assertionsToCheck
         private Set<Variable> variablesToCheck
-        private Map<String, Boolean> assertionsToCheck
 
-        AssertionFinalVariableAnalyzer(final SourceUnit sourceUnit, final Map<String, Boolean> assertions) {
+        AssertionFinalVariableAnalyzer(SourceUnit sourceUnit, Map<String, Boolean> assertions) {
             super(sourceUnit)
             assertionsToCheck = assertions
         }
 
         @Override
-        void visitVariableExpression(final VariableExpression expression) {
+        void visitVariableExpression(VariableExpression expression) {
             super.visitVariableExpression(expression)
             if (assertionsToCheck.containsKey(expression.name)) {
                 variablesToCheck << expression
@@ -595,21 +596,24 @@ class FinalVariableAnalyzerTest extends GroovyTestCase {
         }
 
         @Override
-        void visitClass(final ClassNode node) {
-            def old = variablesToCheck
-            variablesToCheck = []
-            super.visitClass(node)
-            if (!(node instanceof InnerClassNode)) {
-                checkAssertions()
-            }
-            variablesToCheck = old
+        void visitClass(ClassNode node) {
+            def vars = variablesToCheck
+            try {
+                variablesToCheck = []
+                super.visitClass(node)
+                if (node !instanceof InnerClassNode) {
+                    checkAssertions()
+                }
+            } finally {
+                variablesToCheck = vars
+            }
         }
 
         private void checkAssertions() {
-            assertionsToCheck.each { name, shouldBeFinal ->
+            for (entry in assertionsToCheck) {
+                def name = entry.key, shouldBeFinal = entry.value
                 def candidates = variablesToCheck.findAll { it.name == name }
                 assert candidates.any { isEffectivelyFinal(it) == shouldBeFinal }
-
             }
         }
     }