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 2021/11/11 17:09:21 UTC

[groovy] branch master updated (11fe90d -> 8d19017)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git.


    from 11fe90d  minor refactor
     new 4e7f4f4  GROOVY-10347: STC: add test cases
     new 8d19017  minor edits

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/main/java/groovy/lang/GroovyShell.java         |   4 +-
 .../java/org/codehaus/groovy/ast/ClassNode.java    |  35 ++-
 .../groovy/classgen/VariableScopeVisitor.java      |   8 +-
 .../org/codehaus/groovy/classgen/Verifier.java     |   2 +-
 .../codehaus/groovy/control/CompilationUnit.java   |   8 +-
 .../codehaus/groovy/control/GenericsVisitor.java   |   5 +-
 .../TupleConstructorASTTransformation.java         |  62 +++---
 .../transform/stc/StaticTypeCheckingSupport.java   |  37 ++--
 .../transform/stc/StaticTypeCheckingVisitor.java   |  27 +--
 src/spec/test/DesignPatternsTest.groovy            |   2 +-
 src/test/groovy/ClosureComposeTest.groovy          | 238 ++++++++++++---------
 src/test/groovy/bugs/groovy6742/Groovy6742.groovy  |  60 ++----
 .../groovy/transform/stc/GenericsSTCTest.groovy    |  52 ++++-
 13 files changed, 292 insertions(+), 248 deletions(-)

[groovy] 02/02: minor edits

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 8d1901775668874575a92275304546949a3cd9c8
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Nov 11 10:40:58 2021 -0600

    minor edits
---
 src/main/java/groovy/lang/GroovyShell.java         |  4 +-
 .../java/org/codehaus/groovy/ast/ClassNode.java    | 35 ++++++------
 .../groovy/classgen/VariableScopeVisitor.java      |  8 +--
 .../org/codehaus/groovy/classgen/Verifier.java     |  2 +-
 .../codehaus/groovy/control/CompilationUnit.java   |  8 +--
 .../codehaus/groovy/control/GenericsVisitor.java   |  5 +-
 .../TupleConstructorASTTransformation.java         | 62 ++++++++++------------
 .../transform/stc/StaticTypeCheckingSupport.java   | 30 +++++------
 .../transform/stc/StaticTypeCheckingVisitor.java   | 27 +++-------
 src/spec/test/DesignPatternsTest.groovy            |  2 +-
 10 files changed, 86 insertions(+), 97 deletions(-)

diff --git a/src/main/java/groovy/lang/GroovyShell.java b/src/main/java/groovy/lang/GroovyShell.java
index 14b1145..8cc43dd 100644
--- a/src/main/java/groovy/lang/GroovyShell.java
+++ b/src/main/java/groovy/lang/GroovyShell.java
@@ -142,8 +142,10 @@ public class GroovyShell extends GroovyObjectSupport {
         setVariable(property, newValue);
         try {
             super.setProperty(property, newValue);
+        } catch (ReadOnlyPropertyException e) {
+            throw e;
         } catch (GroovyRuntimeException e) {
-            // ignore, was probably a dynamic property
+            // probably a dynamic property
         }
     }
 
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassNode.java b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
index 43869d6..340efb4 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
@@ -164,8 +164,7 @@ public class ClassNode extends AnnotatedNode {
     protected List<InnerClassNode> innerClasses;
     private List<ClassNode> permittedSubclasses = new ArrayList<>(4);
     private List<AnnotationNode> typeAnnotations = Collections.emptyList();
-    private List<RecordComponentNode> recordComponentNodes = Collections.emptyList();
-    private boolean isRecord = false;
+    private List<RecordComponentNode> recordComponents = Collections.emptyList();
 
     /**
      * The AST Transformations to be applied during compilation.
@@ -1087,9 +1086,7 @@ public class ClassNode extends AnnotatedNode {
         if (compileUnit != null) compileUnit = cu;
     }
 
-    /**
-     * @return {@code true} if the two arrays are of the same size and have the same contents
-     */
+    @Deprecated
     protected boolean parametersEqual(Parameter[] a, Parameter[] b) {
         return ParameterUtils.parametersEqual(a, b);
     }
@@ -1365,48 +1362,50 @@ public class ClassNode extends AnnotatedNode {
      * Check instead for the {@code RecordType} annotation if looking for records and record-like classes.
      *
      * @return {@code true} if the instance represents a native {@code record}
+     *
      * @since 4.0.0
      */
     public boolean isRecord() {
         return getUnresolvedSuperClass() != null && "java.lang.Record".equals(getUnresolvedSuperClass().getName());
     }
 
-    @Deprecated
-    public List<RecordComponentNode> getRecordComponentNodes() {
-        return getRecordComponents();
-    }
-
     /**
-     * Get the record components of record type
+     * Gets the record components of record type.
      *
      * @return {@code RecordComponentNode} instances
+     *
      * @since 4.0.0
      */
     public List<RecordComponentNode> getRecordComponents() {
         if (redirect != null)
             return redirect.getRecordComponents();
         lazyClassInit();
-        return recordComponentNodes;
+        return recordComponents;
     }
 
     @Deprecated
-    public void setRecordComponentNodes(List<RecordComponentNode> recordComponentNodes) {
-        setRecordComponents(recordComponentNodes);
+    public List<RecordComponentNode> getRecordComponentNodes() {
+        return getRecordComponents();
     }
 
     /**
-     * Set the record components for record type
+     * Sets the record components for record type.
      *
      * @since 4.0.0
      */
-    public void setRecordComponents(List<RecordComponentNode> recordComponentNodes) {
+    public void setRecordComponents(List<RecordComponentNode> recordComponents) {
         if (redirect != null) {
-            redirect.setRecordComponents(recordComponentNodes);
+            redirect.setRecordComponents(recordComponents);
         } else {
-            this.recordComponentNodes = recordComponentNodes;
+            this.recordComponents = recordComponents;
         }
     }
 
+    @Deprecated
+    public void setRecordComponentNodes(List<RecordComponentNode> recordComponentNodes) {
+        setRecordComponents(recordComponentNodes);
+    }
+
     public boolean isAbstract() {
         return (getModifiers() & ACC_ABSTRACT) != 0;
     }
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index df4deb6..b605b63 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -186,7 +186,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
                         if (name.equals(pn.getName())) return pn;
                     }
 
-                    FieldNode fn = new FieldNode(name, mn.getModifiers() & 0xF, ClassHelper.OBJECT_TYPE, cn, null);
+                    FieldNode fn = new FieldNode(name, mn.getModifiers() & 0xF, ClassHelper.dynamicType(), cn, null);
                     fn.setHasNoRealSourcePosition(true);
                     fn.setDeclaringClass(cn);
                     fn.setSynthetic(true);
@@ -198,9 +198,11 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
                 }
             }
 
-            for (ClassNode face : cn.getAllInterfaces()) {
-                FieldNode fn = face.getDeclaredField(name);
+            for (ClassNode in : cn.getAllInterfaces()) {
+                FieldNode fn = in.getDeclaredField(name);
                 if (fn != null) return fn;
+                PropertyNode pn = in.getProperty(name);
+                if (pn != null) return pn;
             }
         }
 
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 41e5047..f0f228e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -209,7 +209,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             return ret;
         }
         ClassNode current = node;
-        while (!(isObjectType(current))) {
+        while (!isObjectType(current)) {
             current = current.getSuperClass();
             if (current == null) break;
             ret = current.getDeclaredField("metaClass");
diff --git a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
index e79db18..9e77602 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
@@ -87,7 +87,6 @@ import static org.codehaus.groovy.transform.stc.StaticTypesMarker.SWITCH_CONDITI
  * This is commonly used when you want to wire a new AST Transformation into the compilation.
  */
 public class CompilationUnit extends ProcessingUnit {
-    private static final int COMPUTE_MAX_STACK_AND_FRAMES = ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES;
 
     /** The overall AST for this CompilationUnit. */
     protected CompileUnit ast; // TODO: Switch to private and access through getAST().
@@ -772,8 +771,9 @@ public class CompilationUnit extends ProcessingUnit {
             //
             // Handle any callback that's been set
             //
-            Optional.ofNullable(getClassgenCallback())
-                .ifPresent(callback -> callback.call(classVisitor, classNode));
+            if (classgenCallback != null) {
+                classgenCallback.call(classVisitor, classNode);
+            }
 
             //
             // Recurse for inner classes
@@ -791,7 +791,7 @@ public class CompilationUnit extends ProcessingUnit {
     };
 
     protected ClassVisitor createClassVisitor() {
-        return new ClassWriter(COMPUTE_MAX_STACK_AND_FRAMES) {
+        return new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS) {
             private ClassNode getClassNode(String name) {
                 // try classes under compilation
                 CompileUnit cu = getAST();
diff --git a/src/main/java/org/codehaus/groovy/control/GenericsVisitor.java b/src/main/java/org/codehaus/groovy/control/GenericsVisitor.java
index 4742b71..c596e03 100644
--- a/src/main/java/org/codehaus/groovy/control/GenericsVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/GenericsVisitor.java
@@ -30,6 +30,7 @@ import org.codehaus.groovy.ast.expr.CastExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.transform.trait.Traits;
 
 import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isUnboundedWildcard;
@@ -106,10 +107,10 @@ public class GenericsVisitor extends ClassCodeVisitorSupport {
     public void visitDeclarationExpression(final DeclarationExpression expression) {
         if (expression.isMultipleAssignmentDeclaration()) {
             for (Expression e : expression.getTupleExpression().getExpressions()) {
-                checkGenericsUsage(e.getType());
+                checkGenericsUsage(((VariableExpression) e).getOriginType());
             }
         } else {
-            checkGenericsUsage(expression.getVariableExpression().getType());
+            checkGenericsUsage(expression.getVariableExpression().getOriginType());
         }
 
         super.visitDeclarationExpression(expression);
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index 7f64fba..97d8058 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -44,7 +44,6 @@ import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.EmptyStatement;
 import org.codehaus.groovy.ast.stmt.ExpressionStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
-import org.codehaus.groovy.ast.tools.GenericsUtils;
 import org.codehaus.groovy.classgen.VariableScopeVisitor;
 import org.codehaus.groovy.control.CompilationUnit;
 import org.codehaus.groovy.control.CompilePhase;
@@ -61,10 +60,6 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor
 import static org.apache.groovy.ast.tools.ClassNodeUtils.hasExplicitConstructor;
 import static org.apache.groovy.ast.tools.ConstructorNodeUtils.checkPropNamesS;
 import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
-import static org.codehaus.groovy.ast.ClassHelper.MAP_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
-import static org.codehaus.groovy.ast.ClassHelper.make;
-import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.assignS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
@@ -94,12 +89,14 @@ import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
 public class TupleConstructorASTTransformation extends AbstractASTTransformation implements CompilationUnitAware {
 
     private CompilationUnit compilationUnit;
-    static final Class MY_CLASS = TupleConstructor.class;
-    static final ClassNode MY_TYPE = make(MY_CLASS);
+
+    static final Class<?> MY_CLASS = TupleConstructor.class;
+    static final ClassNode MY_TYPE = ClassHelper.make(MY_CLASS);
     static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
+
     private static final String NAMED_ARGS = "__namedArgs";
-    private static final ClassNode LHMAP_TYPE = makeWithoutCaching(LinkedHashMap.class, false);
-    private static final ClassNode POJO_TYPE = make(POJO.class);
+    private static final ClassNode LHMAP_TYPE = ClassHelper.makeWithoutCaching(LinkedHashMap.class, false);
+    private static final ClassNode POJO_TYPE = ClassHelper.make(POJO.class);
 
     @Override
     public String getAnnotationName() {
@@ -107,12 +104,12 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
     }
 
     @Override
-    public void setCompilationUnit(CompilationUnit unit) {
+    public void setCompilationUnit(final CompilationUnit unit) {
         compilationUnit = unit;
     }
 
     @Override
-    public void visit(ASTNode[] nodes, SourceUnit source) {
+    public void visit(final ASTNode[] nodes, final SourceUnit source) {
         init(nodes, source);
         AnnotatedNode parent = (AnnotatedNode) nodes[1];
         AnnotationNode anno = (AnnotationNode) nodes[0];
@@ -134,8 +131,8 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
                 return;
             if (!checkPropertyList(cNode, excludes, "excludes", anno, MY_TYPE_NAME, includeFields, includeSuperProperties, allProperties, includeSuperFields, false))
                 return;
-            final GroovyClassLoader classLoader = compilationUnit != null ? compilationUnit.getTransformLoader() : source.getClassLoader();
-            final PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader, cNode);
+            GroovyClassLoader classLoader = compilationUnit != null ? compilationUnit.getTransformLoader() : source.getClassLoader();
+            PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader, cNode);
             if (handler == null) return;
             if (!handler.validateAttributes(this, anno)) return;
 
@@ -163,13 +160,13 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         }
     }
 
-    private static void createConstructor(AbstractASTTransformation xform, AnnotationNode anno, ClassNode cNode, boolean includeFields,
-                                          boolean includeProperties, boolean includeSuperFields, boolean includeSuperProperties,
-                                          List<String> excludes, final List<String> includes, boolean allNames, boolean allProperties,
-                                          SourceUnit sourceUnit, PropertyHandler handler, ClosureExpression pre, ClosureExpression post) {
+    private static void createConstructor(final AbstractASTTransformation xform, final AnnotationNode anno, final ClassNode cNode, final boolean includeFields,
+                                          final boolean includeProperties, final boolean includeSuperFields, final boolean includeSuperProperties,
+                                          final List<String> excludes, final List<String> includes, final boolean allNames, final boolean allProperties,
+                                          final SourceUnit sourceUnit, final PropertyHandler handler, final ClosureExpression pre, final ClosureExpression post) {
         boolean callSuper = xform.memberHasValue(anno, "callSuper", true);
-        boolean force = xform.memberHasValue(anno, "force", true);
         boolean defaults = !xform.memberHasValue(anno, "defaults", false);
+        boolean force = xform.memberHasValue(anno, "force", true);
         boolean namedVariant = xform.memberHasValue(anno, "namedVariant", true);
         Set<String> names = new HashSet<>();
         List<PropertyNode> superList;
@@ -188,9 +185,9 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         // no processing if existing constructors found unless forced or ImmutableBase in play
         if (hasExplicitConstructor(null, cNode) && !force && !makeImmutable) return;
 
-        final List<Parameter> params = new ArrayList<>();
-        final List<Expression> superParams = new ArrayList<>();
-        final BlockStatement preBody = new BlockStatement();
+        List<Parameter> params = new ArrayList<>();
+        List<Expression> superParams = new ArrayList<>();
+        BlockStatement preBody = new BlockStatement();
         boolean superInPre = false;
         if (pre != null) {
             superInPre = copyStatementsWithSuperAdjustment(pre, preBody);
@@ -200,7 +197,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
             }
         }
 
-        final BlockStatement body = new BlockStatement();
+        BlockStatement body = new BlockStatement();
 
         List<PropertyNode> tempList = new ArrayList<>(list);
         tempList.addAll(superList);
@@ -246,16 +243,14 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         }
 
         if (includes != null) {
-            Comparator<Parameter> includeComparator = Comparator.comparingInt(p -> includes.indexOf(p.getName()));
-            params.sort(includeComparator);
+            params.sort(Comparator.comparingInt(p -> includes.indexOf(p.getName())));
         }
 
-        boolean hasMapCons = AnnotatedNodeUtils.hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE);
         int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC);
         ConstructorNode consNode = addGeneratedConstructor(cNode, modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body);
         if (namedVariant) {
             BlockStatement inner = new BlockStatement();
-            Parameter mapParam = param(GenericsUtils.nonGeneric(MAP_TYPE), NAMED_ARGS);
+            Parameter mapParam = param(ClassHelper.MAP_TYPE.getPlainNodeReference(), NAMED_ARGS);
             List<Parameter> genParams = new ArrayList<>();
             genParams.add(mapParam);
             ArgumentListExpression args = new ArgumentListExpression();
@@ -280,16 +275,17 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         // we don't do it for LinkedHashMap for now (would lead to duplicate signature)
         // or if there is only one Map property (for backwards compatibility)
         // or if there is already a @MapConstructor annotation
-        if (!params.isEmpty() && defaults && !hasMapCons && specialNamedArgCase) {
+        if (defaults && specialNamedArgCase && !params.isEmpty()
+                && !AnnotatedNodeUtils.hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE)) {
             ClassNode firstParamType = params.get(0).getType();
-            if (params.size() > 1 || isObjectType(firstParamType)) {
+            if (params.size() > 1 || ClassHelper.isObjectType(firstParamType)) {
                 String message = "The class " + cNode.getName() + " was incorrectly initialized via the map constructor with null.";
                 addSpecialMapConstructors(modifiers, cNode, message, false);
             }
         }
     }
 
-    private static Parameter createParam(FieldNode fNode, String name, boolean defaults, AbstractASTTransformation xform, boolean makeImmutable) {
+    private static Parameter createParam(final FieldNode fNode, final String name, final boolean defaults, final AbstractASTTransformation xform, final boolean makeImmutable) {
         Parameter param = new Parameter(fNode.getType(), name);
         if (defaults) {
             param.setInitialExpression(providedOrDefaultInitialValue(fNode));
@@ -312,7 +308,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         return init;
     }
 
-    public static void addSpecialMapConstructors(int modifiers, ClassNode cNode, String message, boolean addNoArg) {
+    public static void addSpecialMapConstructors(final int modifiers, final ClassNode cNode, final String message, final boolean addNoArg) {
         Parameter[] parameters = params(new Parameter(LHMAP_TYPE, NAMED_ARGS));
         BlockStatement code = new BlockStatement();
         VariableExpression namedArgs = varX(NAMED_ARGS);
@@ -329,11 +325,11 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
         }
     }
 
-    private static BlockStatement illegalArgumentBlock(String message) {
-        return block(throwS(ctorX(make(IllegalArgumentException.class), args(constX(message)))));
+    private static BlockStatement illegalArgumentBlock(final String message) {
+        return block(throwS(ctorX(ClassHelper.make(IllegalArgumentException.class), args(constX(message)))));
     }
 
-    private static BlockStatement processArgsBlock(ClassNode cNode, VariableExpression namedArgs) {
+    private static BlockStatement processArgsBlock(final ClassNode cNode, final VariableExpression namedArgs) {
         BlockStatement block = new BlockStatement();
         List<PropertyNode> props = new ArrayList<>();
         for (PropertyNode pNode : cNode.getProperties()) {
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 5f3d915..31c8d79 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1662,19 +1662,19 @@ public abstract class StaticTypeCheckingSupport {
         return replacementType;
     }
 
-    private static boolean equalIncludingGenerics(final GenericsType orig, final GenericsType copy) {
-        if (orig == copy) return true;
-        if (orig.isPlaceholder() != copy.isPlaceholder()) return false;
-        if (orig.isWildcard() != copy.isWildcard()) return false;
-        if (!equalIncludingGenerics(orig.getType(), copy.getType())) return false;
-        ClassNode lower1 = orig.getLowerBound();
-        ClassNode lower2 = copy.getLowerBound();
+    private static boolean equalIncludingGenerics(final GenericsType one, final GenericsType two) {
+        if (one == two) return true;
+        if (one.isWildcard() != two.isWildcard()) return false;
+        if (one.isPlaceholder() != two.isPlaceholder()) return false;
+        if (!equalIncludingGenerics(one.getType(), two.getType())) return false;
+        ClassNode lower1 = one.getLowerBound();
+        ClassNode lower2 = two.getLowerBound();
         if ((lower1 == null) ^ (lower2 == null)) return false;
         if (lower1 != lower2) {
             if (!equalIncludingGenerics(lower1, lower2)) return false;
         }
-        ClassNode[] upper1 = orig.getUpperBounds();
-        ClassNode[] upper2 = copy.getUpperBounds();
+        ClassNode[] upper1 = one.getUpperBounds();
+        ClassNode[] upper2 = two.getUpperBounds();
         if ((upper1 == null) ^ (upper2 == null)) return false;
         if (upper1 != upper2) {
             if (upper1.length != upper2.length) return false;
@@ -1685,12 +1685,12 @@ public abstract class StaticTypeCheckingSupport {
         return true;
     }
 
-    private static boolean equalIncludingGenerics(final ClassNode orig, final ClassNode copy) {
-        if (orig == copy) return true;
-        if (orig.isGenericsPlaceHolder() != copy.isGenericsPlaceHolder()) return false;
-        if (!orig.equals(copy)) return false;
-        GenericsType[] gt1 = orig.getGenericsTypes();
-        GenericsType[] gt2 = copy.getGenericsTypes();
+    private static boolean equalIncludingGenerics(final ClassNode one, final ClassNode two) {
+        if (one == two) return true;
+        if (one.isGenericsPlaceHolder() != two.isGenericsPlaceHolder()) return false;
+        if (!one.equals(two)) return false;
+        GenericsType[] gt1 = one.getGenericsTypes();
+        GenericsType[] gt2 = two.getGenericsTypes();
         if ((gt1 == null) ^ (gt2 == null)) return false;
         if (gt1 != gt2) {
             if (gt1.length != gt2.length) 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 4a26bf9..59aa5fb 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3654,24 +3654,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
     }
 
-    private LambdaExpression constructLambdaExpressionForMethodReference(final ClassNode paramType) {
-        Parameter[] newParameters = createParametersForConstructedLambdaExpression(paramType);
-        return new LambdaExpression(newParameters, block());
-    }
-
-    private Parameter[] createParametersForConstructedLambdaExpression(final ClassNode functionalInterfaceType) {
-        MethodNode abstractMethodNode = findSAM(functionalInterfaceType);
-
-        Parameter[] abstractMethodNodeParameters = abstractMethodNode.getParameters();
-        if (abstractMethodNodeParameters == null) {
-            abstractMethodNodeParameters = Parameter.EMPTY_ARRAY;
+    private LambdaExpression constructLambdaExpressionForMethodReference(final ClassNode functionalInterfaceType) {
+        Parameter[] parameters = findSAM(functionalInterfaceType).getParameters().clone();
+        for (int i = 0, n = parameters.length; i < n; i += 1) {
+            parameters[i] = new Parameter(dynamicType(), "p" + System.nanoTime());
         }
 
-        Parameter[] newParameters = new Parameter[abstractMethodNodeParameters.length];
-        for (int i = 0; i < newParameters.length; i += 1) {
-            newParameters[i] = new Parameter(dynamicType(), "p" + System.nanoTime());
-        }
-        return newParameters;
+        return new LambdaExpression(parameters, EmptyStatement.INSTANCE);
     }
 
     /**
@@ -4920,7 +4909,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             }
         }
 
-        if (isClassClassNodeWrappingConcreteType(receiver)) {
+        if (isClassClassNodeWrappingConcreteType(receiver)) { // GROOVY-6802, GROOVY-6803
             List<MethodNode> result = findMethod(receiver.getGenericsTypes()[0].getType(), name, args);
             if (!result.isEmpty()) return result;
         }
@@ -5344,7 +5333,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
                         if (isClosureWithType(argumentType)) {
                             MethodNode sam = findSAM(paramType);
-                            if (sam != null) { // implicit closure coerce
+                            if (sam != null) { // adapt closure to functional interface or other single-abstract-method class
                                 argumentType = convertClosureTypeToSAMType(expressions.get(i), argumentType, sam, paramType);
                             }
                         }
@@ -5716,7 +5705,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 }
             }
 
-            addStaticTypeError("Cannot call " + Optional.ofNullable(gt).map(GenericsUtils::toGenericTypesString).orElse("") +
+            addStaticTypeError("Cannot call " + (gt == null ? "" : GenericsUtils.toGenericTypesString(gt)) +
                     prettyPrintTypeName(r) + "#" + toMethodParametersString(m.getName(), paramTypes) + " with arguments " + formatArgumentList(at), location);
             return false;
         }
diff --git a/src/spec/test/DesignPatternsTest.groovy b/src/spec/test/DesignPatternsTest.groovy
index 98a7444..b429223 100644
--- a/src/spec/test/DesignPatternsTest.groovy
+++ b/src/spec/test/DesignPatternsTest.groovy
@@ -905,7 +905,7 @@ class DesignPatternsTest extends CompilableTestSupport {
 
     void testDecoratorSql() {
         shouldCompile '''
-            @Grab('org.codehaus.groovy:groovy-sql:2.1.6')
+            @Grab('org.apache.groovy:groovy-sql:4.0.0-beta-2')
             import groovy.sql.Sql
             import java.lang.reflect.InvocationHandler
             import java.sql.Connection

[groovy] 01/02: GROOVY-10347: STC: add test cases

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 4e7f4f45a31e06b8d14fb62e7b14c3216a76cddd
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Nov 11 10:08:38 2021 -0600

    GROOVY-10347: STC: add test cases
---
 .../transform/stc/StaticTypeCheckingSupport.java   |   7 +-
 src/test/groovy/ClosureComposeTest.groovy          | 238 ++++++++++++---------
 src/test/groovy/bugs/groovy6742/Groovy6742.groovy  |  60 ++----
 .../groovy/transform/stc/GenericsSTCTest.groovy    |  52 ++++-
 4 files changed, 206 insertions(+), 151 deletions(-)

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 a07b1b1..5f3d915 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1773,11 +1773,12 @@ public abstract class StaticTypeCheckingSupport {
             if (di.isPlaceholder()) {
                 connections.put(new GenericsTypeName(di.getName()), ui);
             } else if (di.isWildcard()) {
+                ClassNode lowerBound = di.getLowerBound(), upperBounds[] = di.getUpperBounds();
                 if (ui.isWildcard()) {
-                    extractGenericsConnections(connections, ui.getLowerBound(), di.getLowerBound());
-                    extractGenericsConnections(connections, ui.getUpperBounds(), di.getUpperBounds());
+                    extractGenericsConnections(connections, ui.getLowerBound(), lowerBound);
+                    extractGenericsConnections(connections, ui.getUpperBounds(), upperBounds);
                 } else if (!isUnboundedWildcard(di)) {
-                    ClassNode boundType = di.getLowerBound() != null ? di.getLowerBound() : di.getUpperBounds()[0];
+                    ClassNode boundType = lowerBound != null ? lowerBound : upperBounds[0];
                     if (boundType.isGenericsPlaceHolder()) { // GROOVY-9998
                         String placeholderName = boundType.getUnresolvedName();
                         ui = new GenericsType(ui.getType()); ui.setWildcard(true);
diff --git a/src/test/groovy/ClosureComposeTest.groovy b/src/test/groovy/ClosureComposeTest.groovy
index 6814105..5005209 100644
--- a/src/test/groovy/ClosureComposeTest.groovy
+++ b/src/test/groovy/ClosureComposeTest.groovy
@@ -18,39 +18,47 @@
  */
 package groovy
 
-import groovy.test.GroovyTestCase
-import groovy.transform.CompileStatic
+import org.junit.Test
 
-import java.util.function.Function
+import static groovy.test.GroovyAssert.assertScript
 
 /**
  * Tests for Closure composition
  */
-class ClosureComposeTest extends GroovyTestCase {
+final class ClosureComposeTest {
 
+    @Test
     void testComposeMultiply() {
-        def twice = { a -> a * 2 }
-        def thrice = { a -> a * 3 }
-        def times6 = twice >> thrice
-        assert times6(3) == 18
+        assertScript '''
+            def twice = { a -> a * 2 }
+            def thrice = { a -> a * 3 }
+            def times6 = twice >> thrice
+            assert times6(3) == 18
+        '''
     }
 
+    @Test
     void testReverseComposeAndCallShortcut() {
-        def twice = { a -> a * 2 }
-        def thrice = { a -> a * 3 }
-        assert twice << thrice << 3 == 18
+        assertScript '''
+            def twice = { a -> a * 2 }
+            def thrice = { a -> a * 3 }
+            assert twice << thrice << 3 == 18
+        '''
     }
 
+    @Test
     void testComposeAndLonghand() {
-        def twice = { a -> a * 2 }
-        def inc = { b -> b + 1 }
-        def f = inc >> twice
-        def g = { x -> twice(inc(x)) }
-        assert f(10) == 22
-        assert g(10) == 22
+        assertScript '''
+            def twice = { a -> a * 2 }
+            def inc = { b -> b + 1 }
+            def f = inc >> twice
+            def g = { x -> twice(inc(x)) }
+            assert f(10) == 22
+            assert g(10) == 22
+        '''
     }
 
-    // GROOVY-4512
+    @Test // GROOVY-4512
     void testClosureCompositionInstance() {
         def inst = new ComposeTestHelper()
         assert inst.composedA.call() == 42
@@ -59,38 +67,58 @@ class ClosureComposeTest extends GroovyTestCase {
         assert inst.composedB(3) == 122
     }
 
+    static class ComposeTestHelper {
+        def closure1 = { 40 }
+        def closure2 = { it * 40 }
+        def closure3 = { it + 2 }
+        def composedA = closure1 >> closure3
+        def composedB = closure2 >> closure3
+    }
+
+    @Test
     void testComposeWithMethodClosure() {
-        def s2c = { it.chars[0] }
-        def p = Integer.&toHexString >> s2c >> Character.&toUpperCase
-        assert p(15) == 'F'
+        assertScript '''
+            def s2c = { it.chars[0] }
+            def p = Integer.&toHexString >> s2c >> Character.&toUpperCase
+            assert p(15) == 'F'
+        '''
     }
 
+    @Test
     void testComposeWithMultipleArgs() {
-        def multiply = { a, b -> a * b }
-        def identity = { a -> [a, a] }
-        def sq = identity >> multiply
-        assert (1..5).collect { sq(it) } == [1, 4, 9, 16, 25]
+        assertScript '''
+            def multiply = { a, b -> a * b }
+            def identity = { a -> [a, a] }
+            def sq = identity >> multiply
+            assert (1..5).collect { sq(it) } == [1, 4, 9, 16, 25]
+        '''
     }
 
+    @Test
     void testReverseCompositionWithMultipleArgs() {
-        def multiply = { a, b -> a * b }
-        def identity = { a -> [a, a] }
-        def sq = multiply << identity
-        assert (1..5).collect { sq(it) } == [1, 4, 9, 16, 25]
+        assertScript '''
+            def multiply = { a, b -> a * b }
+            def identity = { a -> [a, a] }
+            def sq = multiply << identity
+            assert (1..5).collect { sq(it) } == [1, 4, 9, 16, 25]
+        '''
     }
 
+    @Test
     void testComposeWithCurriedClosures() {
-        def add3 = { a, b, c -> a + b + c }
-        def add2plus10 = add3.curry(10)
-        def multBoth = { a, b, c -> [a * c, b * c] }
-        def twiceBoth = multBoth.rcurry(2)
-        def twiceBothPlus10 = twiceBoth >> add2plus10
-        assert twiceBothPlus10(5, 10) == 40
+        assertScript '''
+            def add3 = { a, b, c -> a + b + c }
+            def add2plus10 = add3.curry(10)
+            def multBoth = { a, b, c -> [a * c, b * c] }
+            def twiceBoth = multBoth.rcurry(2)
+            def twiceBothPlus10 = twiceBoth >> add2plus10
+            assert twiceBothPlus10(5, 10) == 40
+        '''
     }
 
+    @Test // GROOVY-4994: failed with MissingPropertyException
     void testDelegate() {
-        // Groovy-4994 failed with MissingPropertyException
-        assertScript """
+        assertScript '''
             def a = { foo }
             def b = { bar }
 
@@ -102,78 +130,96 @@ class ClosureComposeTest extends GroovyTestCase {
             def ab = a >> b
             ab.delegate = new O()
             ab()
-        """
+        '''
     }
 
-    @CompileStatic
+    @Test
     void testAndThenCS() {
-        Function<String, String> lower = String::toLowerCase
-        Function<String, String> upper = String::toUpperCase
-        Function<String, String> lu = lower.andThen(upper)
-        Function<? super String, String> ul = upper.andThen(lower)
-        assert lower('Hi') == ul('Hi')
-        assert upper('Hi') == lu('Hi')
+        assertScript '''
+            import java.util.function.Function
+            @groovy.transform.CompileStatic
+            void test() {
+                Function<String, String> lower = String::toLowerCase
+                Function<String, String> upper = String::toUpperCase
+                Function<String, String> lu = lower.andThen(upper)
+                Function<? super String, String> ul = upper.andThen(lower)
+                assert lower('Hi') == ul('Hi')
+                assert upper('Hi') == lu('Hi')
+            }
+            test()
+        '''
     }
 
+    @Test
     void testAndThen() {
-        def lower = String::toLowerCase
-        def upper = String::toUpperCase
-        def lu1 = lower.rightShift(upper)
-        def ul1 = upper.rightShift(lower)
-        assert lower('Hi') == ul1('Hi')
-        assert upper('Hi') == lu1('Hi')
-        def lu2 = lower.andThen(upper)
-        def ul2 = upper.andThen(lower)
-        assert lower('Hi') == ul2('Hi')
-        assert upper('Hi') == lu2('Hi')
-    }
-
+        assertScript '''
+            def lower = String::toLowerCase
+            def upper = String::toUpperCase
+            def lu1 = lower.rightShift(upper)
+            def ul1 = upper.rightShift(lower)
+            assert lower('Hi') == ul1('Hi')
+            assert upper('Hi') == lu1('Hi')
+            def lu2 = lower.andThen(upper)
+            def ul2 = upper.andThen(lower)
+            assert lower('Hi') == ul2('Hi')
+            assert upper('Hi') == lu2('Hi')
+        '''
+    }
+
+    @Test
     void testAndThenSelf() {
-        def inc = String::next
-        def inc2 = inc.andThenSelf()
-        def inc4 = inc.andThenSelf(3)
-        assert inc('abc') == 'abd'
-        assert inc2('abc') == 'abe'
-        assert inc4('abc') == 'abg'
+        assertScript '''
+            def inc = String::next
+            def inc2 = inc.andThenSelf()
+            def inc4 = inc.andThenSelf(3)
+            assert inc('abc') == 'abd'
+            assert inc2('abc') == 'abe'
+            assert inc4('abc') == 'abg'
+        '''
     }
 
-    @CompileStatic
+    @Test
     void testComposeCS() {
-        Function<String, String> lower = String::toLowerCase
-        Function<String, String> upper = String::toUpperCase
-        Function<String, String> ul = lower.compose(upper)
-        Function<String, ? extends String> lu = upper.compose(lower)
-        assert lower('Hi') == ul('Hi')
-        assert upper('Hi') == lu('Hi')
+        assertScript '''
+            import java.util.function.Function
+            @groovy.transform.CompileStatic
+            void test() {
+                Function<String, String> lower = String::toLowerCase
+                Function<String, String> upper = String::toUpperCase
+                Function<String, String> ul = lower.compose(upper)
+                Function<String, ? extends String> lu = upper.compose(lower)
+                assert lower('Hi') == ul('Hi')
+                assert upper('Hi') == lu('Hi')
+            }
+            test()
+        '''
     }
 
+    @Test
     void testCompose() {
-        def lower = String::toLowerCase
-        def upper = String::toUpperCase
-        def ul1 = lower.leftShift(upper)
-        def lu1 = upper.leftShift(lower)
-        assert lower('Hi') == ul1('Hi')
-        assert upper('Hi') == lu1('Hi')
-        def ul2 = lower.compose(upper)
-        def lu2 = upper.compose(lower)
-        assert lower('Hi') == ul2('Hi')
-        assert upper('Hi') == lu2('Hi')
-    }
-
+        assertScript '''
+            def lower = String::toLowerCase
+            def upper = String::toUpperCase
+            def ul1 = lower.leftShift(upper)
+            def lu1 = upper.leftShift(lower)
+            assert lower('Hi') == ul1('Hi')
+            assert upper('Hi') == lu1('Hi')
+            def ul2 = lower.compose(upper)
+            def lu2 = upper.compose(lower)
+            assert lower('Hi') == ul2('Hi')
+            assert upper('Hi') == lu2('Hi')
+        '''
+    }
+
+    @Test
     void testComposeSelf() {
-        def inc = String::next
-        def inc2 = inc.composeSelf()
-        def inc4 = inc.composeSelf(3)
-        assert inc('abc') == 'abd'
-        assert inc2('abc') == 'abe'
-        assert inc4('abc') == 'abg'
-    }
-
-    class ComposeTestHelper {
-        def closure1 = { 40 }
-        def closure2 = { it * 40 }
-        def closure3 = { it + 2 }
-        def composedA = closure1 >> closure3
-        def composedB = closure2 >> closure3
+        assertScript '''
+            def inc = String::next
+            def inc2 = inc.composeSelf()
+            def inc4 = inc.composeSelf(3)
+            assert inc('abc') == 'abd'
+            assert inc2('abc') == 'abe'
+            assert inc4('abc') == 'abg'
+        '''
     }
 }
diff --git a/src/test/groovy/bugs/groovy6742/Groovy6742.groovy b/src/test/groovy/bugs/groovy6742/Groovy6742.groovy
index 6246719..38770ef 100644
--- a/src/test/groovy/bugs/groovy6742/Groovy6742.groovy
+++ b/src/test/groovy/bugs/groovy6742/Groovy6742.groovy
@@ -18,75 +18,43 @@
  */
 package groovy.bugs.groovy6742
 
-import groovy.transform.CompileStatic
 import org.junit.Test
 
 import static groovy.test.GroovyAssert.assertScript
 
-@CompileStatic
 final class Groovy6742 {
 
     @Test
-    void test1() {
+    void testAssignAIC() {
         assertScript '''
             package groovy.bugs.groovy6742
 
             @groovy.transform.TypeChecked
-            class Issue1 {
-                public void issue(){
-                    Function<String,String> function = new Function<String,String>() {
-                        @Override
-                        String apply(String input) {
-                            return "ok"
-                        }
+            def test() {
+                Function<String,String> function = new Function<String,String>() {
+                    @Override
+                    String apply(String input) {
+                        return input + ' world'
                     }
                 }
+                function.apply('hello')
             }
 
-            assert true
-        '''
-    }
-
-    @Test
-    void test2() {
-        assertScript '''
-            package groovy.bugs.groovy6742
-
-            @groovy.transform.TypeChecked
-            class Issue2 {
-                public void issue() {
-                    transform(new Function<String, String>() {
-                        @Override
-                        String apply(String input) {
-                            return "ok"
-                        }
-                    })
-                }
-
-                public <I, O> void transform(Function<? super I, ? extends O> function) {
-                }
-            }
-
-            assert true
+            assert test() == 'hello world'
         '''
     }
 
     @Test
-    void test3() {
+    void testReturnAIC() {
         assertScript '''
             package groovy.bugs.groovy6742
 
             @groovy.transform.TypeChecked
-            class Issue3 {
-                public static <F, T> FutureCallback<F> deferredCallback(DeferredResult<T> deferredResult, final Function<F, T> function) {
-                    return new FutureCallback<F>() {
-                        private F f = null
-                        F f2 = null
-
-                        @Override
-                        void onSuccess(F result) {
-                            deferredResult.setResult(function.apply(result))
-                        }
+            static <R,T> FutureCallback<R> deferredCallback(DeferredResult<R> deferredResult, final Function<R,T> transformation) {
+                new FutureCallback<R>() {
+                    @Override
+                    void onSuccess(R result) {
+                        deferredResult.setResult(transformation.apply(result))
                     }
                 }
             }
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 31a13a4..41ecf03 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -561,14 +561,54 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
 
     // GROOVY-10339
     void testReturnTypeInferenceWithMethodGenerics21() {
-        shouldFailWithMessages '''
-            String foo() {
+        for (type in ['Character', 'Integer']) {
+            shouldFailWithMessages """
+                Character foo() {
+                }
+                def <T> T bar(T x, T y) {
+                }
+                $type z = bar(foo(), 1)
+            """,
+            'Cannot assign value of type'
+        }
+    }
+
+    // GROOVY-10339
+    void testReturnTypeInferenceWithMethodGenerics22() {
+        for (type in ['Comparable', 'Serializable']) {
+            assertScript """
+                String foo() {
+                }
+                def <T> T bar(T x, T y) {
+                }
+                $type z = bar(foo(), 1)
+            """
+        }
+    }
+
+    // GROOVY-10347
+    void testReturnTypeInferenceWithMethodGenerics23() {
+        assertScript '''
+            String[] one = ['foo','bar'], two = ['baz']
+            Map<String,Integer> map = one.collectEntries{[it,1]} + two.collectEntries{[it,2]}
+            assert map == [foo:1, bar:1, baz:2]
+        '''
+    }
+
+    // GROOVY-10347
+    void testReturnTypeInferenceWithMethodGenerics24() {
+        assertScript '''
+            class Pogo {
+                String s
             }
-            def <T> T bar(T x, T y) {
+            class Sorter implements Comparator<Pogo>, Serializable {
+                int compare(Pogo p1, Pogo p2) { p1.s <=> p2.s }
             }
-            Integer i = bar(foo(), 1)
-        ''',
-        'Cannot assign value of type'
+
+            Pogo[] pogos = [new Pogo(s:'foo'), new Pogo(s:'bar')]
+            List<String> strings = pogos.sort(true, new Sorter())*.s // sort(T[],boolean,Comparator<? super T>)
+            assert strings == ['bar', 'foo']
+        '''
     }
 
     void testDiamondInferrenceFromConstructor1() {