groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [2/2] groovy git commit: GROOVY-8220: SC GroovyCastException on parameter flow typing (closes #605)
Date Wed, 27 Sep 2017 08:07:21 GMT
GROOVY-8220: SC GroovyCastException on parameter flow typing (closes #605)

GROOVY-8157 introduced flow typing for parameters and this fix is
required in order to track their assignments in `if` branches for
temporary type assignments.


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/639896b6
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/639896b6
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/639896b6

Branch: refs/heads/GROOVY_2_6_X
Commit: 639896b6920dee99cdf8814c9ceb094086a0bcbd
Parents: 10e0fd9
Author: John Wagenleitner <jwagenleitner@apache.org>
Authored: Thu Sep 7 17:27:06 2017 -0700
Committer: paulk <paulk@asert.com.au>
Committed: Wed Sep 27 18:07:08 2017 +1000

----------------------------------------------------------------------
 .../stc/StaticTypeCheckingVisitor.java          | 67 ++++++++++++++-
 .../transform/stc/STCAssignmentTest.groovy      | 89 +++++++++++++++++++-
 2 files changed, 151 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/639896b6/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 2890428..8ed723d 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -131,9 +131,6 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.*;
 /**
  * The main class code visitor responsible for static type checking. It will perform various
inspections like checking
  * assignment types, type inference, ... Eventually, class nodes may be annotated with inferred
type information.
- *
- * @author Cedric Champeau
- * @author Jochen Theodorou
  */
 public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
@@ -668,6 +665,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
                 if (typeCheckingContext.ifElseForWhileAssignmentTracker != null &&
leftExpression instanceof VariableExpression
                         && !isNullConstant(rightExpression)) {
                     Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
+                    if (accessedVariable instanceof Parameter) {
+                        accessedVariable = new ParameterVariableExpression((Parameter) accessedVariable);
+                    }
                     if (accessedVariable instanceof VariableExpression) {
                         VariableExpression var = (VariableExpression) accessedVariable;
                         List<ClassNode> types = typeCheckingContext.ifElseForWhileAssignmentTracker.get(var);
@@ -4847,4 +4847,65 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         }
     }
 
+    /**
+     * Wrapper for a Parameter so it can be treated like a VariableExpression
+     * and tracked in the ifElseForWhileAssignmentTracker.
+     *
+     * This class purposely does not adhere to the normal equals and hashCode
+     * contract on the Object class and delegates those calls to the wrapped
+     * variable.
+     */
+    private static class ParameterVariableExpression extends VariableExpression {
+
+        private final Parameter parameter;
+
+        ParameterVariableExpression(Parameter parameter) {
+            super(parameter);
+            this.parameter = parameter;
+            ClassNode inferred = parameter.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+            if (inferred == null) {
+                parameter.setNodeMetaData(StaticTypesMarker.INFERRED_TYPE, parameter.getOriginType());
+            }
+        }
+
+        @Override
+        public void copyNodeMetaData(ASTNode other) {
+            parameter.copyNodeMetaData(other);
+        }
+
+        @Override
+        public Object putNodeMetaData(Object key, Object value) {
+            return parameter.putNodeMetaData(key, value);
+        }
+
+        @Override
+        public void removeNodeMetaData(Object key) {
+            parameter.removeNodeMetaData(key);
+        }
+
+        @Override
+        public Map<?, ?> getNodeMetaData() {
+            return parameter.getNodeMetaData();
+        }
+
+        @Override
+        public <T> T getNodeMetaData(Object key) {
+            return parameter.getNodeMetaData(key);
+        }
+
+        @Override
+        public void setNodeMetaData(Object key, Object value) {
+            parameter.setNodeMetaData(key, value);
+        }
+
+        @Override
+        public int hashCode() {
+            return parameter.hashCode();
+        }
+
+        @Override
+        public boolean equals(Object other) {
+            return parameter.equals(other);
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/639896b6/src/test/groovy/transform/stc/STCAssignmentTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index acb1c75..645a6de 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -20,8 +20,6 @@ package groovy.transform.stc
 
 /**
  * Unit tests for static type checking : assignments.
- *
- * @author Cedric Champeau
  */
 class STCAssignmentTest extends StaticTypeCheckingTestCase {
 
@@ -446,6 +444,21 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         ''', 'Cannot find matching method java.io.Serializable#toInteger()'
     }
 
+    void testIfElseBranchParameter() {
+        shouldFailWithMessages '''
+            def foo(x) {
+                def y = 'foo'
+                if (y) {
+                    x = new HashSet()
+                } else {
+                    x = '123'
+                }
+                x.toInteger()
+            }
+            foo('bar')
+        ''', 'Cannot find matching method java.lang.Object#toInteger()'
+    }
+
     void testIfOnly() {
         shouldFailWithMessages '''
             def x = '123'
@@ -457,6 +470,20 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         ''', 'Cannot find matching method java.io.Serializable#toInteger()'
     }
 
+    void testIfOnlyParameter() {
+        shouldFailWithMessages '''
+            def foo(x) {
+                def y = 'foo'
+                if (y) {
+                    x = new HashSet()
+                    assert x.isEmpty()
+                }
+                x.toInteger()
+            }
+            foo('123')
+        ''', 'Cannot find matching method java.lang.Object#toInteger()'
+    }
+
     void testIfWithCommonInterface() {
         assertScript '''
             interface Foo { void foo() }
@@ -873,6 +900,64 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-8220
+    void testFlowTypingParameterTempTypeAssignmentTracking() {
+        assertScript '''
+            class Foo {
+                CharSequence makeEnv( env, StringBuilder result = new StringBuilder() ) {
+                    if (env instanceof File) {
+                        env = env.toPath()
+                    }
+                    if (env instanceof String && env.contains('=')) {
+                        result << 'export ' << env << ';'
+                    }
+                    return result.toString()
+                }
+            }
+            assert new Foo().makeEnv('X=1') == 'export X=1;'
+        '''
+        // GROOVY-8237
+        assertScript '''
+            class Foo {
+                String parse(Reader reader) {
+                    if (reader == null)
+                        reader = new BufferedReader(reader)
+                    int i = reader.read()
+                    return (i != -1) ? 'bar' : 'baz'
+                }
+            }
+            assert new Foo().parse(new StringReader('foo')) == 'bar'
+        '''
+    }
+
+    void testFlowTypingParameterTempTypeAssignmentTrackingWithGenerics() {
+        assertScript '''
+            class M {
+                Map<String, List<Object>> mvm = new HashMap<String, List<Object>>()
+                void setProperty(String name, value) {
+                    if (value instanceof File) {
+                        value = new File(value, 'bar.txt')
+                    }
+                    else if (value instanceof URL) {
+                        value = value.toURI()
+                    }
+                    else if (value instanceof InputStream) {
+                        value = new BufferedInputStream(value)
+                    }
+                    else if (value instanceof GString) {
+                        value = value.toString()
+                    }
+                    if (mvm[name]) {
+                        mvm[name].add value
+                    } else {
+                        mvm.put(name, [value])
+                    }
+                }
+            }
+            new M().setProperty('foo', 'bar')
+        '''
+    }
+
     void testNarrowingConversion() {
         assertScript '''
         interface A1{}


Mime
View raw message