You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/03/26 15:46:08 UTC
[groovy] 01/02: GROOVY-10540: add `GroovyObject` interface before STC and classgen
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 35502249b3e8885982cdfd857638f3030d2a9cc4
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Mar 22 14:49:05 2022 -0500
GROOVY-10540: add `GroovyObject` interface before STC and classgen
---
.../transform/stc/FromAbstractTypeMethods.java | 29 ++----
.../java/org/codehaus/groovy/ast/ClassHelper.java | 71 ++++++--------
.../java/org/codehaus/groovy/ast/GenericsType.java | 29 +-----
.../codehaus/groovy/control/CompilationUnit.java | 22 +++++
.../transform/stc/StaticTypeCheckingSupport.java | 6 +-
.../transform/stc/StaticTypeCheckingVisitor.java | 5 -
src/spec/test/typing/TypeCheckingTest.groovy | 2 +-
.../groovy/transform/stc/ClosuresSTCTest.groovy | 108 +++++++++++----------
.../stc/FieldsAndPropertiesSTCTest.groovy | 13 ++-
.../ArraysAndCollectionsStaticCompileTest.groovy | 2 +
10 files changed, 131 insertions(+), 156 deletions(-)
diff --git a/src/main/java/groovy/transform/stc/FromAbstractTypeMethods.java b/src/main/java/groovy/transform/stc/FromAbstractTypeMethods.java
index b00544b..5d49e6b 100644
--- a/src/main/java/groovy/transform/stc/FromAbstractTypeMethods.java
+++ b/src/main/java/groovy/transform/stc/FromAbstractTypeMethods.java
@@ -19,6 +19,7 @@
package groovy.transform.stc;
import org.codehaus.groovy.ast.ASTNode;
+import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
@@ -26,7 +27,8 @@ import org.codehaus.groovy.control.CompilationUnit;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.transform.trait.Traits;
-import java.util.LinkedList;
+import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
/**
@@ -40,28 +42,15 @@ public class FromAbstractTypeMethods extends ClosureSignatureHint {
@Override
public List<ClassNode[]> getClosureSignatures(final MethodNode node, final SourceUnit sourceUnit, final CompilationUnit compilationUnit, final String[] options, final ASTNode usage) {
String className = options[0];
- ClassNode cn = findClassNode(sourceUnit, compilationUnit, className);
- return extractSignaturesFromMethods(cn);
- }
+ ClassNode classNode = findClassNode(sourceUnit, compilationUnit, className);
- private static List<ClassNode[]> extractSignaturesFromMethods(final ClassNode cn) {
- List<MethodNode> methods = cn.getAllDeclaredMethods();
- List<ClassNode[]> signatures = new LinkedList<ClassNode[]>();
- for (MethodNode method : methods) {
- if (!method.isSynthetic() && method.isAbstract()) {
- extractParametersFromMethod(signatures, method);
+ List<ClassNode[]> signatures = new ArrayList<>();
+ for (MethodNode method : classNode.getAbstractMethods()) {
+ if (!method.isSynthetic() && !Traits.hasDefaultImplementation(method)
+ && !ClassHelper.isGroovyObjectType(method.getDeclaringClass())) {
+ signatures.add(Arrays.stream(method.getParameters()).map(Parameter::getOriginType).toArray(ClassNode[]::new));
}
}
return signatures;
}
-
- private static void extractParametersFromMethod(final List<ClassNode[]> signatures, final MethodNode method) {
- if (Traits.hasDefaultImplementation(method)) return;
- Parameter[] parameters = method.getParameters();
- ClassNode[] typeParams = new ClassNode[parameters.length];
- for (int i = 0; i < parameters.length; i++) {
- typeParams[i] = parameters[i].getOriginType();
- }
- signatures.add(typeParams);
- }
}
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
index 95acea2..e190ffc 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
@@ -52,7 +52,6 @@ import org.apache.groovy.util.concurrent.ManagedIdentityConcurrentMap;
import org.codehaus.groovy.classgen.asm.util.TypeUtil;
import org.codehaus.groovy.runtime.GeneratedClosure;
import org.codehaus.groovy.runtime.GeneratedLambda;
-import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.trait.Traits;
import org.codehaus.groovy.vmplugin.VMPluginFactory;
import org.objectweb.asm.Opcodes;
@@ -62,7 +61,6 @@ import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.invoke.SerializedLambda;
import java.lang.ref.SoftReference;
-import java.lang.reflect.Modifier;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Collection;
@@ -73,6 +71,8 @@ import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Stream;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
+
/**
* Helper for {@link ClassNode} and classes handling them. Contains a set of
* pre-defined instances for the most used types and some code for cached node
@@ -181,8 +181,6 @@ public class ClassHelper {
GROOVY_OBJECT_TYPE, GROOVY_INTERCEPTABLE_TYPE, Enum_Type, Annotation_TYPE
};
- private static final int ABSTRACT_STATIC_PRIVATE = Opcodes.ACC_ABSTRACT | Opcodes.ACC_STATIC | Opcodes.ACC_PRIVATE;
- private static final int VISIBILITY = 5; // public|protected
private static final String DYNAMIC_TYPE_METADATA = "_DYNAMIC_TYPE_METADATA_";
protected static final ClassNode[] EMPTY_TYPE_ARRAY = {};
@@ -562,52 +560,45 @@ public class ClassHelper {
* @return the method node if type is a SAM type, null otherwise
*/
public static MethodNode findSAM(final ClassNode type) {
- if (!Modifier.isAbstract(type.getModifiers())) return null;
if (type.isInterface()) {
- List<MethodNode> methods;
- if (type.isInterface()) {
- // e.g. BinaryOperator extends BiFunction, BinaryOperator contains no abstract method, but it is really a SAM
- methods = type.redirect().getAllDeclaredMethods();
- } else {
- methods = type.getMethods();
- }
-
- MethodNode found = null;
- for (MethodNode mi : methods) {
- // ignore methods, that are not abstract and from Object
- if (!Modifier.isAbstract(mi.getModifiers())) continue;
- // ignore trait methods which have a default implementation
- if (Traits.hasDefaultImplementation(mi)) continue;
- if (isObjectType(mi.getDeclaringClass())) continue;
- if (OBJECT_TYPE.getDeclaredMethod(mi.getName(), mi.getParameters()) != null) continue;
+ MethodNode sam = null;
+ for (MethodNode mn : type.getAbstractMethods()) {
+ // ignore methods that will have an implementation
+ if (Traits.hasDefaultImplementation(mn)) continue;
+ if (OBJECT_TYPE.getDeclaredMethod(mn.getName(), mn.getParameters()) != null) continue;
// we have two methods, so no SAM
- if (found != null) return null;
- found = mi;
+ if (sam != null) return null;
+ sam = mn;
}
- return found;
-
- } else {
- MethodNode found = null;
- for (MethodNode mi : type.getAbstractMethods()) {
- if (!hasUsableImplementation(type, mi)) {
- if (found != null) return null;
- found = mi;
+ return sam;
+ }
+ if (type.isAbstract()) {
+ MethodNode sam = null;
+ for (MethodNode mn : type.getAbstractMethods()) {
+ if (!hasUsableImplementation(type, mn)) {
+ if (sam != null) return null;
+ sam = mn;
}
}
- return found;
+ return sam;
}
+ return null;
}
- private static boolean hasUsableImplementation(ClassNode c, MethodNode m) {
- if (c == m.getDeclaringClass()) return false;
+ private static boolean hasUsableImplementation(final ClassNode c, final MethodNode m) {
+ ClassNode declaringClass = m.getDeclaringClass();
+ if (c.equals(declaringClass)) return false;
+ // GROOVY-10540: GroovyObject declared and Verifier not run yet
+ if (isGroovyObjectType(declaringClass) && c.getCompileUnit() != null) return true;
+
MethodNode found = c.getDeclaredMethod(m.getName(), m.getParameters());
if (found == null) return false;
- int asp = found.getModifiers() & ABSTRACT_STATIC_PRIVATE;
- int visible = found.getModifiers() & VISIBILITY;
- if (visible != 0 && asp == 0) return true;
- if (isObjectType(c)) return false;
- return hasUsableImplementation(c.getSuperClass(), m);
+
+ int modifiers = (found.getModifiers() & 0x40F);//ACC_ABSTRACT|ACC_STATIC|ACC_PROTECTED|ACC_PRIVATE|ACC_PUBLIC
+ if (modifiers == Opcodes.ACC_PUBLIC || modifiers == Opcodes.ACC_PROTECTED) return true;
+
+ return !isObjectType(c) && hasUsableImplementation(c.getSuperClass(), m);
}
/**
@@ -627,7 +618,7 @@ public class ClassHelper {
if (target.isInterface()) {
for (ClassNode face : source.getUnresolvedInterfaces()) {
- if (StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(face, target)) {
+ if (implementsInterfaceOrIsSubclassOf(face, target)) {
return face;
}
}
diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index 0aae08e..927c939 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -28,8 +28,8 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
-import static org.codehaus.groovy.ast.ClassHelper.isGroovyObjectType;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
/**
* This class is used to describe generic type signatures for ClassNodes.
@@ -250,33 +250,6 @@ public class GenericsType extends ASTNode {
return classNode.equals(getType()) && compareGenericsWithBound(classNode, getType());
}
- private static boolean implementsInterfaceOrIsSubclassOf(final ClassNode type, final ClassNode superOrInterface) {
- if (type.equals(superOrInterface)
- || type.isDerivedFrom(superOrInterface)
- || type.implementsInterface(superOrInterface)) {
- return true;
- }
- if (isGroovyObjectType(superOrInterface) && type.getCompileUnit() != null) {
- // type is being compiled so it will implement GroovyObject later
- return true;
- }
- if (superOrInterface instanceof WideningCategories.LowestUpperBoundClassNode) {
- WideningCategories.LowestUpperBoundClassNode lub = (WideningCategories.LowestUpperBoundClassNode) superOrInterface;
- boolean result = implementsInterfaceOrIsSubclassOf(type, lub.getSuperClass());
- if (result) {
- for (ClassNode face : lub.getInterfaces()) {
- result = implementsInterfaceOrIsSubclassOf(type, face);
- if (!result) break;
- }
- }
- if (result) return true;
- }
- if (type.isArray() && superOrInterface.isArray()) {
- return implementsInterfaceOrIsSubclassOf(type.getComponentType(), superOrInterface.getComponentType());
- }
- return false;
- }
-
/**
* Compares the bounds of this generics specification against the given type
* for compatibility. Ex: String would satisfy <? extends CharSequence>.
diff --git a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
index 24e81cd..2308168 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
@@ -22,6 +22,7 @@ import groovy.lang.GroovyClassLoader;
import groovy.lang.GroovyRuntimeException;
import groovy.transform.CompilationUnitAware;
import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
@@ -280,6 +281,27 @@ public class CompilationUnit extends ProcessingUnit {
}
}, Phases.CANONICALIZATION);
+ addPhaseOperation(source -> {
+ for (ClassNode cn : source.getAST().getClasses()) {
+ // GROOVY-10540: add GroovyObject before STC and classgen
+ if (!cn.isInterface() && !cn.isDerivedFromGroovyObject()) {
+ boolean cs = false, pojo = false, trait = false;
+ for (AnnotationNode an : cn.getAnnotations()) {
+ switch (an.getClassNode().getName()) {
+ case "groovy.transform.CompileStatic":
+ cs = true; break;
+ case "groovy.transform.stc.POJO":
+ pojo = true; break;
+ case "groovy.transform.Trait":
+ trait = true; break;
+ }
+ }
+ if (!(cs && pojo) && !trait)
+ cn.addInterface(ClassHelper.GROOVY_OBJECT_TYPE);
+ }
+ }
+ }, Phases.INSTRUCTION_SELECTION);
+
addPhaseOperation(classgen, Phases.CLASS_GENERATION);
addPhaseOperation(groovyClass -> {
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index 53baef0..7eceff4 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -739,10 +739,6 @@ public abstract class StaticTypeCheckingSupport {
return true;
}
- if (isGroovyObjectType(leftRedirect) && isBeingCompiled(right)) {
- return true;
- }
-
if (right.isDerivedFrom(CLOSURE_TYPE) && isSAMType(left)) {
return true;
}
@@ -908,7 +904,7 @@ public abstract class StaticTypeCheckingSupport {
if (type.isArray() && superOrInterface.isArray()) {
return implementsInterfaceOrIsSubclassOf(type.getComponentType(), superOrInterface.getComponentType());
}
- if (isGroovyObjectType(superOrInterface) && !type.isInterface() && isBeingCompiled(type)) {
+ if (isGroovyObjectType(superOrInterface) && isBeingCompiled(type) && !type.isInterface()) {//TODO: !POJO !Trait
return true;
}
return false;
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 c0cfd3b..3b05d2d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -152,7 +152,6 @@ import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Character_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Double_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Float_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.GROOVY_OBJECT_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Integer_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Iterator_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.LIST_TYPE;
@@ -2457,7 +2456,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
receiverType = wrapTypeIfNecessary(currentReceiver.getType());
candidates = findMethodsWithGenerated(receiverType, nameText);
- if (isBeingCompiled(receiverType)) candidates.addAll(GROOVY_OBJECT_TYPE.getMethods(nameText));
candidates.addAll(findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), receiverType, nameText));
candidates = filterMethodsByVisibility(candidates, typeCheckingContext.getEnclosingClassNode());
@@ -4900,9 +4898,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (isGStringType(receiver)) {
return findMethod(STRING_TYPE, name, args);
}
- if (isBeingCompiled(receiver)) {
- return findMethod(GROOVY_OBJECT_TYPE, name, args);
- }
return EMPTY_METHODNODE_LIST;
}
diff --git a/src/spec/test/typing/TypeCheckingTest.groovy b/src/spec/test/typing/TypeCheckingTest.groovy
index b6bfbd5..75d85b9 100644
--- a/src/spec/test/typing/TypeCheckingTest.groovy
+++ b/src/spec/test/typing/TypeCheckingTest.groovy
@@ -625,7 +625,7 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound
}
// end::least_upper_bound_collection_inference[]
''',
- 'Cannot find matching method (Greeter or Salute)#exit()'
+ 'Cannot find matching method (Greeter or Salute'
}
void testInstanceOfInference() {
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index 5988486..d922bb5 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -692,18 +692,67 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
}
}
- void testSAMVariable() {
+ void testSAMType() {
assertScript '''
- interface SAM { def foo(); }
+ interface I { def m() }
@ASTTest(phase=INSTRUCTION_SELECTION, value={
- assert node.getNodeMetaData(INFERRED_TYPE).name == 'SAM'
+ assert node.getNodeMetaData(INFERRED_TYPE).name == 'I'
})
- SAM s = {1}
- assert s.foo() == 1
- def t = (SAM) {2}
- assert t.foo() == 2
+ I i = { 1 }
+ assert i.m() == 1
+ def x = (I) { 2 }
+ assert x.m() == 2
+ '''
+
+ assertScript '''
+ interface I { int m() }
+ abstract class A implements I { }
+
+ I i = { 1 }
+ assert i.m() == 1
+ A a = { 2 }
+ assert a.m() == 2
+ '''
+
+ shouldFailWithMessages '''
+ interface I {
+ String toString()
+ }
+ I i = { p -> "" }
+ ''',
+ 'Cannot assign'
+
+ shouldFailWithMessages '''
+ interface I {
+ String toString()
+ }
+ abstract class A implements I { }
+
+ A a = { "" } // implicit parameter
+ ''',
+ 'Cannot assign'
+
+ assertScript '''
+ interface I { // non-functional, but every instance extends Object
+ boolean equals(Object)
+ int m()
+ }
+ I i = { 1 }
+ assert i.m() == 1
'''
+
+ shouldFailWithMessages '''
+ interface I {
+ boolean equals(Object)
+ int m()
+ }
+ abstract class A implements I { // no abstract methods
+ int m() { 1 }
+ }
+ A a = { 2 }
+ ''',
+ 'Cannot assign'
}
// GROOVY-7927
@@ -826,51 +875,6 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testSAMType() {
- assertScript '''
- interface Foo {int foo()}
- Foo f = {1}
- assert f.foo() == 1
- abstract class Bar implements Foo {}
- Bar b = {2}
- assert b.foo() == 2
- '''
- shouldFailWithMessages '''
- interface Foo2 {
- String toString()
- }
- Foo2 f2 = {int i->"hi"}
- ''',
- 'Cannot assign'
- shouldFailWithMessages '''
- interface Foo2 {
- String toString()
- }
- abstract class Bar2 implements Foo2 {}
- Bar2 b2 = {"there"}
- ''',
- 'Cannot assign'
- assertScript '''
- interface Foo3 {
- boolean equals(Object)
- int f()
- }
- Foo3 f3 = {1}
- assert f3.f() == 1
- '''
- shouldFailWithMessages '''
- interface Foo3 {
- boolean equals(Object)
- int f()
- }
- abstract class Bar3 implements Foo3 {
- int f(){2}
- }
- Bar3 b3 = {2}
- ''',
- 'Cannot assign'
- }
-
// GROOVY-6238
void testDirectMethodCallOnClosureExpression() {
assertScript '''
diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index c9d56cf..0a445b3 100644
--- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -479,17 +479,20 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
// GROOVY-5517
void testShouldFindStaticPropertyEvenIfObjectImplementsMap() {
assertScript '''
+ @groovy.transform.stc.POJO
+ @groovy.transform.CompileStatic
class MyHashMap extends HashMap {
public static int version = 666
}
def map = new MyHashMap()
- map['foo'] = 123
- Object value = map.foo
+ map.foo = 123
+ def value = map.foo
assert value == 123
+ map['foo'] = 4.5
value = map['foo']
- assert value == 123
- int v = MyHashMap.version
- assert v == 666
+ assert value == 4.5
+ value = MyHashMap.version
+ assert value == 666
'''
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
index 038bcd4..f697026 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/ArraysAndCollectionsStaticCompileTest.groovy
@@ -123,6 +123,8 @@ class ArraysAndCollectionsStaticCompileTest extends ArraysAndCollectionsSTCTest
// GROOVY-8074
void testMapSubclassPropertyStyleAccess() {
assertScript '''
+ @groovy.transform.stc.POJO
+ @groovy.transform.CompileStatic
class MyMap extends LinkedHashMap {
def foo = 1
}