From 9a273d5a3e0c40be28a53f037182b2e6bb26e889 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Tue, 13 May 2025 09:48:07 -0500 Subject: [PATCH 1/3] GROOVY-11663: STC: error for class or trait property expression --- .../stc/StaticTypeCheckingVisitor.java | 41 ++++++++++++------- .../packageScope/DifferentPackageTest.groovy | 2 +- .../traitx/TraitASTTransformationTest.groovy | 9 +++- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index f375a1ea7d3..fa7df3da1ee 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -763,28 +763,39 @@ private boolean tryVariableExpressionAsProperty(final VariableExpression vexp, f @Override public void visitPropertyExpression(final PropertyExpression expression) { - if (existsProperty(expression, !typeCheckingContext.isTargetOfEnclosingAssignment(expression))) return; - - if (!extension.handleUnresolvedProperty(expression)) { - var objectExpression = expression.getObjectExpression(); - var objectExpressionType = (objectExpression instanceof ClassExpression - ? objectExpression.getType() : wrapTypeIfNecessary(getType(objectExpression))); - objectExpressionType = findCurrentInstanceOfClass(objectExpression, objectExpressionType); - addStaticTypeError("No such property: " + expression.getPropertyAsString() + " for class: " + prettyPrintTypeName(objectExpressionType), expression); + boolean readOnly = !typeCheckingContext.isTargetOfEnclosingAssignment(expression); + if (existsProperty(expression, readOnly) + || extension.handleUnresolvedProperty(expression)) { + return; // resolved or excused } + recordMissingProperty(expression); } @Override public void visitAttributeExpression(final AttributeExpression expression) { - if (existsProperty(expression, true)) return; + boolean readOnly = true; + if (existsProperty(expression, readOnly) + || extension.handleUnresolvedAttribute(expression)) { + return; // resolved or excused + } + recordMissingProperty(expression); + } - if (!extension.handleUnresolvedAttribute(expression)) { - var objectExpression = expression.getObjectExpression(); - var objectExpressionType = (objectExpression instanceof ClassExpression - ? objectExpression.getType() : wrapTypeIfNecessary(getType(objectExpression))); - objectExpressionType = findCurrentInstanceOfClass(objectExpression, objectExpressionType); - addStaticTypeError("No such attribute: " + expression.getPropertyAsString() + " for class: " + prettyPrintTypeName(objectExpressionType), expression); + private void recordMissingProperty(final PropertyExpression expression) { + var objectExpression = expression.getObjectExpression(); + var objectExpressionType = findCurrentInstanceOfClass(objectExpression, getType(objectExpression)); + + String pattern; + if (!isClassClassNodeWrappingConcreteType(objectExpressionType)) { + pattern = "No such {0,choice,1#attribute|2#property}: {1} for class: {2}"; + } else { + objectExpressionType = objectExpressionType.getGenericsTypes()[0].getType(); + pattern = "No such {0,choice,1#attribute|2#property}: {1} for Class or static {0,choice,1#field|2#property} for class: {2}"; } + + String error = java.text.MessageFormat.format(pattern, expression instanceof AttributeExpression ? 1 : 2, expression.getPropertyAsString(), prettyPrintTypeName(wrapTypeIfNecessary(objectExpressionType))); + ASTNode node = expression.getLineNumber() > 0 ? expression : expression.getProperty(); // GROOVY-11663 + addStaticTypeError(error, node); } @Override diff --git a/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy index ba1644879e9..135dd8d3646 100644 --- a/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy @@ -447,6 +447,6 @@ final class DifferentPackageTest { ''' ) } - assert err =~ /No such property: answer for class: p.One/ // TODO: Cannot access p.One#getAnswer? + assert err =~ /No such property: answer for Class or static property for class: p.One/ // TODO: Cannot access p.One#getAnswer? } } diff --git a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy index e0bfd2b087c..c1dd00f55e4 100644 --- a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy @@ -3710,10 +3710,10 @@ final class TraitASTTransformationTest { """ } - // GROOVY-11641 + // GROOVY-11641, GROOVY-11663 @CompileModesTest void testTraitAccessToInheritedStaticMethods4(String mode) { - shouldFail shell, """ + var err = shouldFail shell, """ $mode trait Foo { public static final String BANG = '!' @@ -3739,6 +3739,11 @@ final class TraitASTTransformationTest { Main.test1() new Main().test2() """ + if (mode.endsWith('Dynamic')) { + assert err =~ /MissingPropertyException/ + } else { + assert err =~ /\[Static type checking\] - No such property: BANG for Class or static property for class: Bar/ + } } // GROOVY-9386 From 83358811c7514a5195935b2e75a010aacf554986 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 14 May 2025 09:48:57 -0500 Subject: [PATCH 2/3] GROOVY-11663: support static trait field referenced in static context --- .../groovy/control/StaticVerifier.java | 42 ++++--- .../traitx/TraitASTTransformationTest.groovy | 115 ++++++++++++++---- 2 files changed, 113 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/control/StaticVerifier.java b/src/main/java/org/codehaus/groovy/control/StaticVerifier.java index 3d7d27758ab..83b1646363d 100644 --- a/src/main/java/org/codehaus/groovy/control/StaticVerifier.java +++ b/src/main/java/org/codehaus/groovy/control/StaticVerifier.java @@ -18,6 +18,7 @@ */ package org.codehaus.groovy.control; +import org.apache.groovy.ast.tools.ClassNodeUtils; import org.codehaus.groovy.ast.ClassCodeVisitorSupport; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.DynamicVariable; @@ -27,9 +28,8 @@ import org.codehaus.groovy.ast.expr.ClosureExpression; import org.codehaus.groovy.ast.expr.ConstructorCallExpression; import org.codehaus.groovy.ast.expr.VariableExpression; +import org.codehaus.groovy.transform.trait.Traits; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; /** @@ -88,34 +88,40 @@ public void visitConstructorOrMethod(MethodNode node, boolean isConstructor) { @Override public void visitVariableExpression(VariableExpression ve) { if (ve.getAccessedVariable() instanceof DynamicVariable && (ve.isInStaticContext() || inSpecialConstructorCall) && !inClosure) { + String variableName = ve.getName(); // GROOVY-5687: interface constants not visible to implementing subclass in static context if (methodNode != null && methodNode.isStatic()) { - FieldNode fieldNode = getDeclaredOrInheritedField(methodNode.getDeclaringClass(), ve.getName()); + ClassNode classNode = methodNode.getDeclaringClass(); + FieldNode fieldNode = getDeclaredOrInheritedField(classNode, variableName); if (fieldNode != null && fieldNode.isStatic()) { return; } } - addError("Apparent variable '" + ve.getName() + "' was found in a static scope but doesn't refer to a local variable, static field or class. Possible causes:\n" + + addError("Apparent variable '" + variableName + "' was found in a static scope but doesn't refer to a local variable, static field or class. Possible causes:\n" + "You attempted to reference a variable in the binding or an instance variable from a static context.\n" + "You misspelled a classname or statically imported field. Please check the spelling.\n" + - "You attempted to use a method '" + ve.getName() + "' but left out brackets in a place not allowed by the grammar.", ve); + "You attempted to use a method '" + variableName + "' but left out brackets in a place not allowed by the grammar.", + ve); } } - private static FieldNode getDeclaredOrInheritedField(ClassNode cn, String fieldName) { - ClassNode node = cn; - while (node != null) { - FieldNode fn = node.getDeclaredField(fieldName); - if (fn != null) return fn; - List interfacesToCheck = new ArrayList<>(Arrays.asList(node.getInterfaces())); - while (!interfacesToCheck.isEmpty()) { - ClassNode nextInterface = interfacesToCheck.remove(0); - fn = nextInterface.getDeclaredField(fieldName); - if (fn != null) return fn; - interfacesToCheck.addAll(Arrays.asList(nextInterface.getInterfaces())); + private static FieldNode getDeclaredOrInheritedField(ClassNode classNode, String fieldName) { + FieldNode fieldNode = ClassNodeUtils.getField(classNode, fieldName); + if (fieldNode == null && fieldName.contains("__")) { // GROOVY-11663 + List traits = Traits.findTraits(classNode); + traits.remove(classNode); // included if it is a trait + for (ClassNode cn : traits) { + cn = Traits.findFieldHelper(cn); + if (cn != null) { + for (FieldNode fn : cn.getFields()) { + if (fn.getName().endsWith(fieldName)) { // prefix for modifiers + fieldNode = fn; + break; + } + } + } } - node = node.getSuperClass(); } - return null; + return fieldNode; } } diff --git a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy index c1dd00f55e4..7caea447979 100644 --- a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy @@ -1997,7 +1997,7 @@ final class TraitASTTransformationTest { void testShouldCompileTraitMethodStatically() { def err = shouldFail shell, ''' @CompileStatic - trait Foo { + trait T { int foo() { 1+'foo'} } ''' @@ -2018,10 +2018,13 @@ final class TraitASTTransformationTest { class D extends C { } - assert C.foo() == 'static method' - assert D.foo() == 'static method' - assert new C().foo() == 'static method' - assert new D().foo() == 'static method' + $mode void m() { + assert C.foo() == 'static method' + assert D.foo() == 'static method' + assert new C().foo() == 'static method' + assert new D().foo() == 'static method' + } + m() """ // GROOVY-7322 @@ -2038,10 +2041,13 @@ final class TraitASTTransformationTest { class D extends C { } - assert C.foo() == 'static method' - assert D.foo() == 'static method' - assert new C().foo() == 'static method' - assert new D().foo() == 'static method' + $mode void m() { + assert C.foo() == 'static method' + assert D.foo() == 'static method' + assert new C().foo() == 'static method' + assert new D().foo() == 'static method' + } + m() """ // GROOVY-7191 @@ -2058,8 +2064,11 @@ final class TraitASTTransformationTest { class D extends C { } - assert new C().foo() == 1 - assert new D().foo() == 1 + $mode void m() { + assert new C().foo() == 1 + assert new D().foo() == 1 + } + m() """ // GROOVY-8854 @@ -2084,11 +2093,14 @@ final class TraitASTTransformationTest { class D extends C { } - def c = new C(name:'name') - c.audit(); assert c.passes + $mode void m() { + def c = new C(name:'name') + c.audit(); assert c.passes - def d = new D(name:'name') - d.audit(); assert d.passes + def d = new D(name:'name') + d.audit(); assert d.passes + } + m() """ } @@ -2103,7 +2115,10 @@ final class TraitASTTransformationTest { class C implements T { } - assert C.T__VAL == 123 + $mode void m() { + assert C.T__VAL == 123 + } + m() """ assertScript shell, """ @@ -2116,9 +2131,12 @@ final class TraitASTTransformationTest { class C implements T { } - assert C.T__VAL == 123 - C.update(456) - assert C.T__VAL == 456 + $mode void m() { + assert C.T__VAL == 123 + C.update(456) + assert C.T__VAL == 456 + } + m() """ } @@ -2134,9 +2152,12 @@ final class TraitASTTransformationTest { class C implements T { } - assert C.VAL == 123 - C.update(456) - assert C.VAL == 456 + $mode void m() { + assert C.VAL == 123 + C.update(456) + assert C.VAL == 456 + } + m() """ // GROOVY-7255 @@ -2153,8 +2174,11 @@ final class TraitASTTransformationTest { class C implements T { } - C.initStuff([4,5,6]) - assert C.stuff == [1,2,3,4,5,6] + $mode void m() { + C.initStuff([4,5,6]) + assert C.stuff == [1,2,3,4,5,6] + } + m() """ assertScript shell, """ @@ -2171,7 +2195,10 @@ final class TraitASTTransformationTest { } } - assert C.m() == 3 + $mode void m() { + assert C.m() == 3 + } + m() """ // GROOVY-9678 @@ -2189,7 +2216,10 @@ final class TraitASTTransformationTest { } } - assert C.m() == 3 + $mode void m() { + assert C.m() == 3 + } + m() """ } @@ -3746,6 +3776,39 @@ final class TraitASTTransformationTest { } } + // GROOVY-11663 + @CompileModesTest + void testTraitAccessToInheritedStaticConstant(String mode) { + assertScript shell, """ + $mode + trait A { + public static final String BANG = '!' + } + $mode + trait B extends A { + static one(String string) { + string// + A__BANG + } + Object two(String string) { + string// + A__BANG + } + } + $mode + class C implements B { + static test1() { + assert A__BANG + one('works') == '!works' + } + void test2() { + assert A__BANG + one('works') == '!works' + assert A__BANG + two('works') == '!works' + } + } + + C.test1() + new C().test2() + """ + } + // GROOVY-9386 @CompileModesTest void testTraitPropertyInitializedByTap(String mode) { From bb5b1f16439329301fbaa661e94bc3f8ebd619cc Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 14 May 2025 11:06:45 -0500 Subject: [PATCH 3/3] GROOVY-11663: STC: resolve `$self.pack_Type__name` to a static access --- .../stc/TraitTypeCheckingExtension.java | 32 +++++++++++++++++++ .../traitx/TraitASTTransformationTest.groovy | 10 +++--- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/stc/TraitTypeCheckingExtension.java b/src/main/java/org/codehaus/groovy/transform/stc/TraitTypeCheckingExtension.java index f0b2b9157ec..926582a9675 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/TraitTypeCheckingExtension.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/TraitTypeCheckingExtension.java @@ -25,6 +25,8 @@ import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.MethodCall; import org.codehaus.groovy.ast.expr.MethodCallExpression; +import org.codehaus.groovy.ast.expr.PropertyExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.transform.trait.TraitASTTransformation; import org.codehaus.groovy.transform.trait.Traits; @@ -93,4 +95,34 @@ public List handleMissingMethod(final ClassNode receiver, final Stri return Collections.emptyList(); } + + @Override + public boolean handleUnresolvedProperty(final PropertyExpression pexp) { + var objectExpression = pexp.getObjectExpression(); + if (objectExpression instanceof VariableExpression + && objectExpression.getText().endsWith(Traits.THIS_OBJECT)) { + String propertyName = pexp.getPropertyAsString(); + if (propertyName != null) { + ClassNode objectExpressionType = getType(objectExpression); + if (isClassClassNodeWrappingConcreteType(objectExpressionType)) { + objectExpressionType = objectExpressionType.getGenericsTypes()[0].getType(); + } + for (ClassNode trait : Traits.findTraits(objectExpressionType)) { + if (propertyName.startsWith(trait.getName().replace('.', '_') + "__")) { + ClassNode staticFieldHelper = Traits.findStaticFieldHelper(trait); + if (staticFieldHelper != null) { + MethodNode getter = staticFieldHelper.getDeclaredMethod(propertyName + "$get", Parameter.EMPTY_ARRAY); + if (getter != null) { // GROOVY-11663: resolve "$self.pack_Type__name" to a static field access method + ClassNode returnType = typeCheckingVisitor.inferReturnTypeGenerics(objectExpressionType, getter, ArgumentListExpression.EMPTY_ARGUMENTS); + pexp.putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, returnType); + storeType(pexp, returnType); + return true; + } + } + } + } + } + } + return false; + } } diff --git a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy index 7caea447979..32a21c5cdb2 100644 --- a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy @@ -3787,20 +3787,20 @@ final class TraitASTTransformationTest { $mode trait B extends A { static one(String string) { - string// + A__BANG + string + A__BANG } Object two(String string) { - string// + A__BANG + string + A__BANG } } $mode class C implements B { static test1() { - assert A__BANG + one('works') == '!works' + assert A__BANG + one('works') == '!works!' } void test2() { - assert A__BANG + one('works') == '!works' - assert A__BANG + two('works') == '!works' + assert A__BANG + one('works') == '!works!' + assert A__BANG + two('works') == '!works!' } }