From 59b02b54d8d8458b4be6bc2f1f16c641ae077def Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 18 Sep 2024 08:54:39 -0500 Subject: [PATCH] GROOVY-11480: SC: check for `Type.this.name` within lambda --- .../asm/sc/StaticTypesLambdaWriter.java | 16 +++++-- .../groovy/transform/stc/LambdaTest.groovy | 35 ++++++++++++++++ .../classgen/FinalVariableAnalyzerTest.groovy | 42 ++++++++++--------- 3 files changed, 71 insertions(+), 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 index d661c9bcf8d..916805755ac 100644 --- 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 index 902c0f8d9a5..c7efa53ab22 100644 --- 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 index 64caae3a658..a8044746367 100644 --- 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 expectations, final String script) throws Exception { + protected void assertFinals(Map 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 vars, final String script, boolean unInitialized = false) { + protected void assertFinalCompilationErrors(List vars, String script, boolean unInitialized = false) { Set 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 assertionsToCheck private Set variablesToCheck - private Map assertionsToCheck - AssertionFinalVariableAnalyzer(final SourceUnit sourceUnit, final Map assertions) { + AssertionFinalVariableAnalyzer(SourceUnit sourceUnit, Map 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 } - } } }