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/09/25 11:36:44 UTC

[groovy] branch GROOVY_2_5_X updated (f383f2674f -> 370c4e9639)

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

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


    from f383f2674f GROOVY-10700: STC: non-static (not just default) interface/trait methods
     new d84c59e543 GROOVY-10757: STC: don't write inference metadata to `PropertyNode`
     new 370c4e9639 GROOVY-10637: SC: parameterized return type for getter method

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:
 .../VariableExpressionTransformer.java             | 95 +++++++++++++---------
 .../transform/stc/StaticTypeCheckingVisitor.java   | 15 ++--
 .../transform/stc/ConstructorsSTCTest.groovy       | 10 +--
 .../groovy/transform/stc/GenericsSTCTest.groovy    | 17 ++++
 src/test/groovy/transform/stc/MyBean.java          | 13 +--
 5 files changed, 93 insertions(+), 57 deletions(-)


[groovy] 01/02: GROOVY-10757: STC: don't write inference metadata to `PropertyNode`

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

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

commit d84c59e543f319cb739b438925024ea6859c4796
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Sep 25 05:17:44 2022 -0500

    GROOVY-10757: STC: don't write inference metadata to `PropertyNode`
---
 .../VariableExpressionTransformer.java             | 92 +++++++++++++---------
 .../transform/stc/StaticTypeCheckingVisitor.java   | 15 ++--
 2 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
index 63249b5b3a..f8bff91f7a 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
@@ -20,14 +20,16 @@ package org.codehaus.groovy.transform.sc.transformers;
 
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.expr.Expression;
+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.sc.StaticCompilationMetadataKeys;
 import org.codehaus.groovy.transform.stc.StaticTypesMarker;
 
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
 
 /**
@@ -36,62 +38,76 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
  */
 public class VariableExpressionTransformer {
 
-    public Expression transformVariableExpression(final VariableExpression expr) {
-        Expression trn = tryTransformDelegateToProperty(expr);
-        if (trn != null) {
-            return trn;
+    public Expression transformVariableExpression(final VariableExpression ve) {
+        Expression xe = tryTransformImplicitReceiver(ve);
+        if (xe == null) {
+            xe = tryTransformPrivateFieldAccess(ve);
         }
-        trn = tryTransformPrivateFieldAccess(expr);
-        if (trn != null) {
-            return trn;
+        if (xe == null) {
+            xe = tryTransformDirectMethodTarget(ve);
         }
-        return expr;
+        if (xe != null) {
+            return xe;
+        }
+        return ve;
     }
 
-    private static Expression tryTransformDelegateToProperty(final VariableExpression expr) {
+    private static Expression tryTransformImplicitReceiver(final VariableExpression ve) {
         // we need to transform variable expressions that go to a delegate
         // to a property expression, as ACG would lose the information in
         // processClassVariable before it reaches any makeCall, that could handle it
-        Object val = expr.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
-        if (val == null || val.equals(expr.getName())) return null;
+        Object val = ve.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+        if (val == null || val.equals(ve.getName())) return null;
 
-        // TODO handle the owner and delegate cases better for nested scenarios and potentially remove the need for the implicit this case
-        VariableExpression receiver = new VariableExpression("owner".equals(val) ? (String) val : "delegate".equals(val) ? (String) val : "this");
-        // GROOVY-9136 -- object expression should not overlap source range of property; property stands in for original variable expression
-        receiver.setLineNumber(expr.getLineNumber());
-        receiver.setColumnNumber(expr.getColumnNumber());
+        // TODO: handle the owner and delegate cases better for nested scenarios and potentially remove the need for the implicit this case
+        Expression receiver = new VariableExpression("owner".equals(val) ? (String) val : "delegate".equals(val) ? (String) val : "this");
+        // GROOVY-9136: object expression should not overlap source range of property; property stands in for original variable expression
+        receiver.setColumnNumber(ve.getColumnNumber());
+        receiver.setLineNumber(ve.getLineNumber());
 
-        PropertyExpression pexp = new PropertyExpression(receiver, expr.getName());
-        pexp.getProperty().setSourcePosition(expr);
-        pexp.copyNodeMetaData(expr);
-        pexp.setImplicitThis(true);
+        PropertyExpression pe = new PropertyExpression(receiver, ve.getName());
+        pe.getProperty().setSourcePosition(ve);
+        pe.setImplicitThis(true);
+        pe.copyNodeMetaData(ve);
 
-        ClassNode owner = expr.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
+        ClassNode owner = ve.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER);
         if (owner != null) {
             receiver.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, owner);
             receiver.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, val);
         }
-        pexp.removeNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
+        pe.removeNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
 
-        return pexp;
+        return pe;
     }
 
-    private static Expression tryTransformPrivateFieldAccess(final VariableExpression expr) {
-        FieldNode field = expr.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
+    private static Expression tryTransformPrivateFieldAccess(final VariableExpression ve) {
+        FieldNode field = ve.getNodeMetaData(StaticTypesMarker.PV_FIELDS_ACCESS);
         if (field == null) {
-            field = expr.getNodeMetaData(StaticTypesMarker.PV_FIELDS_MUTATION);
+            field = ve.getNodeMetaData(StaticTypesMarker.PV_FIELDS_MUTATION);
         }
-        if (field != null) {
-            // access to a private field from a section of code that normally doesn't have access to it, like a
-            // closure or an inner class
-            PropertyExpression pexp = !field.isStatic() ? thisPropX(true, expr.getName())
-                    : (PropertyExpression) propX(classX(field.getDeclaringClass()), expr.getName());
-            // store the declaring class so that the class writer knows that it will have to call a bridge method
-            pexp.getObjectExpression().putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getDeclaringClass());
-            pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getOriginType());
-            pexp.getProperty().setSourcePosition(expr);
-            return pexp;
+        if (field == null) {
+            return null;
         }
-        return null;
+
+        // access to a private field from a section of code that normally doesn't have access to it, like a closure block or an inner class
+        PropertyExpression pe = !field.isStatic() ? thisPropX(true, ve.getName()) : new PropertyExpression(classX(field.getDeclaringClass()), ve.getName());
+        // store the declaring class so that the class writer knows that it will have to call a bridge method
+        pe.getObjectExpression().putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, field.getDeclaringClass());
+        pe.putNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE, field.getOriginType());
+        pe.getProperty().setSourcePosition(ve);
+        return pe;
+    }
+
+    private static Expression tryTransformDirectMethodTarget(final VariableExpression ve) {
+        MethodNode dmct = ve.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
+        // NOTE: BinaryExpressionTransformer handles the setter
+        if (dmct == null || dmct.getParameters().length != 0) {
+            return null;
+        }
+
+        MethodCallExpression mce = callThisX(dmct.getName());
+        mce.getMethod().setSourcePosition(ve);
+        mce.setMethodTarget(dmct);
+        return mce;
     }
 }
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 303dd596a0..686fd047c3 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4301,16 +4301,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 exp.putNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE, cn == null ? null : lowestUpperBound(oldValue, cn));
             }
         }
+
         if (exp instanceof VariableExpression) {
             VariableExpression var = (VariableExpression) exp;
-            final Variable accessedVariable = var.getAccessedVariable();
+            Variable accessedVariable = var.getAccessedVariable();
             if (accessedVariable instanceof VariableExpression) {
-                if (accessedVariable != exp)
+                if (accessedVariable != var)
                     storeType((VariableExpression) accessedVariable, cn);
-            } else if (accessedVariable instanceof Parameter
-                    || accessedVariable instanceof PropertyNode
-                    && ((PropertyNode) accessedVariable).getField().isSynthetic()) {
-                ((AnnotatedNode) accessedVariable).putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, cn);
+            } else if (accessedVariable instanceof Parameter) {
+                ((Parameter) accessedVariable).putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, cn);
             }
             if (cn != null && var.isClosureSharedVariable()) {
                 List<ClassNode> assignedTypes = typeCheckingContext.closureSharedVariablesAssignmentTypes.get(var);
@@ -4321,10 +4320,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 assignedTypes.add(cn);
             }
             if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
-                List<ClassNode> temporaryTypesForExpression = getTemporaryTypesForExpression(exp);
+                List<ClassNode> temporaryTypesForExpression = getTemporaryTypesForExpression(var);
                 if (temporaryTypesForExpression != null && !temporaryTypesForExpression.isEmpty()) {
                     // a type inference has been made on a variable whose type was defined in an instanceof block
-                    // we erase available information with the new type
+                    // erase available information with the new type
                     temporaryTypesForExpression.clear();
                 }
             }


[groovy] 02/02: GROOVY-10637: SC: parameterized return type for getter method

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

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

commit 370c4e9639c4c9507438060cd3c0f851850de9b9
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu May 26 15:17:52 2022 -0500

    GROOVY-10637: SC: parameterized return type for getter method
---
 .../sc/transformers/VariableExpressionTransformer.java  |  3 +++
 .../groovy/transform/stc/ConstructorsSTCTest.groovy     | 10 +++++-----
 src/test/groovy/transform/stc/GenericsSTCTest.groovy    | 17 +++++++++++++++++
 src/test/groovy/transform/stc/MyBean.java               | 13 +++++++------
 4 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
index f8bff91f7a..5cc8228017 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/VariableExpressionTransformer.java
@@ -108,6 +108,9 @@ public class VariableExpressionTransformer {
         MethodCallExpression mce = callThisX(dmct.getName());
         mce.getMethod().setSourcePosition(ve);
         mce.setMethodTarget(dmct);
+        // GROOVY-10637: return type might be parameterized
+        mce.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE,
+         ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE));
         return mce;
     }
 }
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index 734fd48177..950fa79957 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -338,17 +338,17 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
     void testConstructJavaBeanFromMap() {
         assertScript '''import groovy.transform.stc.MyBean
 
-        MyBean bean = new MyBean(name:'Cedric')
-        assert bean.name == 'Cedric'
+        MyBean bean = new MyBean<String>(value:'Cedric')
+        assert bean.value == 'Cedric'
         '''
     }
     void testConstructJavaBeanFromMapAndSubclass() {
         assertScript '''import groovy.transform.stc.MyBean
-        class MyBean2 extends MyBean {
+        class MyBean2 extends MyBean<String> {
             int age
         }
-        MyBean2 bean = new MyBean2(name:'Cedric', age:33)
-        assert bean.name == 'Cedric'
+        MyBean2 bean = new MyBean2(value:'Cedric', age:33)
+        assert bean.value == 'Cedric'
         assert bean.age == 33
         '''
     }
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 064807fe4c..c9fe57dd82 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -377,6 +377,23 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         }
     }
 
+    // GROOVY-10637
+    void testReturnTypeInferenceWithMethodGenerics27() {
+        assertScript '''
+            class Outer extends groovy.transform.stc.MyBean<Inner> {
+                static class Inner {
+                    String string
+                }
+                def bar() {
+                    { -> value.string }.call()
+                }
+            }
+
+            def foo = new Outer(value: new Outer.Inner(string:'hello world'))
+            assert foo.bar() == 'hello world'
+        '''
+    }
+
     // GROOVY-10749
     void testReturnTypeInferenceWithMethodGenerics29() {
         if (!GroovyAssert.isAtLeastJdk('1.8')) return
diff --git a/src/test/groovy/transform/stc/MyBean.java b/src/test/groovy/transform/stc/MyBean.java
index c50c8e2b65..7471e5b4e2 100644
--- a/src/test/groovy/transform/stc/MyBean.java
+++ b/src/test/groovy/transform/stc/MyBean.java
@@ -21,14 +21,15 @@ package groovy.transform.stc;
 /**
  * A simple Java bean, used by unit test for GROOVY-5578
  */
-public class MyBean {
-    private String name;
+public class MyBean<T> {
 
-    public String getName() {
-        return name;
+    private T value;
+
+    public T getValue() {
+        return value;
     }
 
-    public void setName(final String name) {
-        this.name = name;
+    public void setValue(final T value) {
+        this.value = value;
     }
 }