groovy git commit: GROOVY-8355: Instanceof inference does not work on field assigning(closes #706)

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

groovy git commit: GROOVY-8355: Instanceof inference does not work on field assigning(closes #706)

Daniel.Sun
Repository: groovy
Updated Branches:
  refs/heads/master 1f0c0ac6e -> 5df88c8f0


GROOVY-8355: Instanceof inference does not work on field assigning(closes #706)


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

Branch: refs/heads/master
Commit: 5df88c8f05242e828038652b0f6f762fe9b84840
Parents: 1f0c0ac
Author: sunlan <[hidden email]>
Authored: Tue May 15 21:59:00 2018 +0800
Committer: sunlan <[hidden email]>
Committed: Tue May 15 21:59:00 2018 +0800

----------------------------------------------------------------------
 .../asm/sc/StaticTypesCallSiteWriter.java       |  27 ++++-
 .../stc/StaticTypeCheckingVisitor.java          |  93 +++++++++++++++
 src/spec/test/typing/TypeCheckingTest.groovy    |   2 +
 src/test/groovy/bugs/Groovy8355Bug.groovy       | 112 +++++++++++++++++++
 4 files changed, 232 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index b717c42..61e239c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -670,8 +670,15 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
         rType = receiver.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
         if (receiver instanceof VariableExpression && rType == null) {
             // TODO: can STCV be made smarter to avoid this check?
-            VariableExpression ve = (VariableExpression) ((VariableExpression)receiver).getAccessedVariable();
-            rType = ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+            Variable accessedVariable = ((VariableExpression)receiver).getAccessedVariable();
+            VariableExpression ve;
+            if (accessedVariable instanceof FieldNode) {
+                FieldNode fieldNode = (FieldNode) accessedVariable;
+                rType = getInferredTypeOfField(fieldNode);
+            } else {
+                ve = (VariableExpression) accessedVariable;
+                rType = ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
+            }
         }
         if (rType!=null && trySubscript(receiver, message, arguments, rType, aType, safe)) {
             return;
@@ -684,6 +691,22 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
                 "this error and file a bug report at https://issues.apache.org/jira/browse/GROOVY");
     }
 
+    private ClassNode getInferredTypeOfField(FieldNode fieldNode) {
+        ClassNode rType = null;
+
+        Map nodeMetaData = fieldNode.getNodeMetaData(controller.getMethodNode());
+        if (null != nodeMetaData) {
+            rType = (ClassNode) nodeMetaData.get(StaticTypesMarker.INFERRED_TYPE);
+//            if (null == rType) {
+//                nodeMetaData = fieldNode.getNodeMetaData(controller.getClassNode());
+//                if (null != nodeMetaData) {
+//                    rType = (ClassNode) nodeMetaData.get(StaticTypesMarker.INFERRED_TYPE);
+//                }
+//            }
+        }
+        return rType;
+    }
+
     private boolean trySubscript(final Expression receiver, final String message, final Expression arguments, ClassNode rType, final ClassNode aType, boolean safe) {
         if (getWrapper(rType).isDerivedFrom(Number_TYPE)
                 && getWrapper(aType).isDerivedFrom(Number_TYPE)) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
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 0ab1ac5..490d924 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -595,6 +595,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
             }
         } else if (accessedVariable instanceof FieldNode) {
             FieldNode fieldNode = (FieldNode) accessedVariable;
+            FieldVariableExpression fieldVariableExpression = new FieldVariableExpression(fieldNode);
 
             ClassNode parameterizedType = GenericsUtils.findParameterizedType(fieldNode.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode());
             if (null != parameterizedType) {
@@ -602,6 +603,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 ClassNode actualType = findActualTypeByPlaceholderName(originalType.getUnresolvedName(), GenericsUtils.extractPlaceholders(parameterizedType));
 
                 if (null != actualType) {
+                    fieldVariableExpression.storeInferredType(actualType);
                     storeType(vexp, actualType);
                     return;
                 }
@@ -845,6 +847,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
                 // if right expression is a ClosureExpression, store parameter type information
                 if (leftExpression instanceof VariableExpression) {
+                    VariableExpression variableExpression = (VariableExpression) leftExpression;
+
+                    final Variable accessedVariable = variableExpression.getAccessedVariable();
+                    if (accessedVariable instanceof FieldNode) {
+                        new FieldVariableExpression((FieldNode) accessedVariable).storeInferredType(resultType);
+                    }
+
                     if (rightExpression instanceof ClosureExpression) {
                         Parameter[] parameters = ((ClosureExpression) rightExpression).getParameters();
                         leftExpression.putNodeMetaData(StaticTypesMarker.CLOSURE_ARGUMENTS, parameters);
@@ -1751,6 +1760,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
                 if (init instanceof ConstructorCallExpression) {
                     inferDiamondType((ConstructorCallExpression) init, node.getOriginType());
                 }
+
+//                new FieldVariableExpression(node).storeInferredType(getType(init));
             }
         } finally {
             currentField = null;
@@ -4519,6 +4530,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         if (cn != null) {
             return cn;
         }
+
+        if (exp instanceof FieldNode) {
+            ClassNode inferredType = new FieldVariableExpression((FieldNode) exp).getInferredType();
+
+            if (null != inferredType) {
+                return inferredType;
+            }
+        }
+
         if (exp instanceof ClassExpression) {
             ClassNode node = CLASS_Type.getPlainNodeReference();
             node.setGenericsTypes(new GenericsType[]{
@@ -5457,4 +5477,77 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
     }
 
+    private class FieldVariableExpression extends VariableExpression {
+        private final FieldNode fieldNode;
+
+        FieldVariableExpression(FieldNode fieldNode) {
+            super(fieldNode);
+            this.fieldNode = fieldNode;
+
+            ClassNode inferred = getInferredType();
+            if (inferred == null) {
+                storeInferredType(fieldNode.getOriginType());
+            }
+        }
+
+        @Override
+        public ListHashMap getMetaDataMap() {
+            AnnotatedNode node = getKey();
+
+            return getMetaDataMap(node);
+        }
+
+        private ListHashMap getMetaDataMap(AnnotatedNode key) {
+            if (null == key) {
+                return new ListHashMap(); // all valid cases should have a methodNode as key. return a empty map is just to avoid NPE, the map will be abandoned finally
+            }
+
+            ListHashMap metaDataMap = fieldNode.getNodeMetaData(key);
+
+            if (null == metaDataMap) {
+                metaDataMap = new ListHashMap();
+                fieldNode.putNodeMetaData(key, metaDataMap);
+            }
+
+            return metaDataMap;
+        }
+
+        @Override
+        public int hashCode() {
+            return fieldNode.hashCode();
+        }
+
+        @Override
+        public boolean equals(Object other) {
+            return fieldNode.equals(other);
+        }
+
+        @SuppressWarnings("unchecked")
+        private ClassNode getInferredType() {
+            Map<Object, Object> enclosingMetaData = getMetaDataMap();
+
+            ClassNode inferredType = (ClassNode) enclosingMetaData.get(StaticTypesMarker.INFERRED_TYPE);
+
+            return inferredType;
+//            return null == inferredType ? (ClassNode) getMetaDataMap(getDefaultKey()).get(StaticTypesMarker.INFERRED_TYPE) : inferredType;
+        }
+
+        @SuppressWarnings("unchecked")
+        private void storeInferredType(ClassNode inferredType) {
+            Map<Object, Object> enclosingMetaData = getMetaDataMap();
+
+            enclosingMetaData.put(StaticTypesMarker.INFERRED_TYPE, inferredType);
+        }
+
+        private AnnotatedNode getKey() {
+            AnnotatedNode node = typeCheckingContext.getEnclosingMethod();
+
+            return node;
+//            return null == node ? getDefaultKey() : node;
+        }
+
+//        private ClassNode getDefaultKey() {
+//            return typeCheckingContext.getEnclosingClassNode();
+//        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/spec/test/typing/TypeCheckingTest.groovy
----------------------------------------------------------------------
diff --git a/src/spec/test/typing/TypeCheckingTest.groovy b/src/spec/test/typing/TypeCheckingTest.groovy
index fc87413..6574c37 100644
--- a/src/spec/test/typing/TypeCheckingTest.groovy
+++ b/src/spec/test/typing/TypeCheckingTest.groovy
@@ -518,6 +518,7 @@ class TypeCheckingTest extends StaticTypeCheckingTestCase {
 
     }
 
+    /*
     void testTypeInferenceFieldVsLocalVariable() {
         shouldFailWithMessages '''
             // tag::typeinference_field_vs_local_variable[]
@@ -544,6 +545,7 @@ class TypeCheckingTest extends StaticTypeCheckingTestCase {
             SomeClass
         ''', 'Cannot find matching method java.lang.Object#toUpperCase()'
     }
+    */
 
     void testLeastUpperBound() {
         assertScript '''import org.codehaus.groovy.ast.ClassHelper

http://git-wip-us.apache.org/repos/asf/groovy/blob/5df88c8f/src/test/groovy/bugs/Groovy8355Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8355Bug.groovy b/src/test/groovy/bugs/Groovy8355Bug.groovy
new file mode 100644
index 0000000..b512249
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8355Bug.groovy
@@ -0,0 +1,112 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+
+
+
+
+
+package groovy.bugs
+
+class Groovy8355Bug extends GroovyTestCase {
+    void testGroovy8355() {
+        assertScript '''
+        import groovy.transform.CompileStatic
+        
+        @CompileStatic
+        class Foo {
+            Object str = new Object()
+            def bar() {
+                str = "str"
+                str.toUpperCase()
+            }
+        }
+        
+        assert "STR" == new Foo().bar()
+        '''
+    }
+
+    void test2() {
+        assertScript '''
+        import groovy.transform.CompileStatic
+        
+        @CompileStatic
+        class Foo {
+            Object str = new Object()
+            def bar() {
+                str = "str"
+                str.toUpperCase()
+            }
+            
+            def bar2() {
+                str = 1
+                str + 2
+            }
+        }
+        
+        Foo foo = new Foo()
+        assert "STR" == foo.bar()
+        assert 3 == foo.bar2()
+        '''
+    }
+
+    void testTypeInferenceFieldVsLocalVariable() {
+        assertScript '''
+            // tag::typeinference_field_vs_local_variable[]
+            class SomeClass {
+                def someUntypedField                                                                // <1>
+                String someTypedField                                                               // <2>
+
+                void someMethod() {
+                    someUntypedField = '123'                                                        // <3>
+                    someUntypedField = someUntypedField.toUpperCase()  // compile-time error        // <4>
+                }
+
+                void someSafeMethod() {
+                    someTypedField = '123'                                                          // <5>
+                    someTypedField = someTypedField.toUpperCase()                                   // <6>
+                }
+
+                void someMethodUsingLocalVariable() {
+                    def localVariable = '123'                                                       // <7>
+                    someUntypedField = localVariable.toUpperCase()                                  // <8>
+                }
+            }
+            // end::typeinference_field_vs_local_variable[]
+            SomeClass
+        '''
+    }
+
+    /*
+    void test3() {
+        assertScript '''
+        import groovy.transform.CompileStatic
+        
+        @CompileStatic
+        class Foo {
+            Object str = "str"
+            def bar() {
+                str.toUpperCase()
+            }
+        }
+        
+        assert "STR" == new Foo().bar()
+        '''
+    }
+    */
+}

Daniel Sun
Apache Groovy committer

Blog: http://blog.sunlan.me
Twitter: @daniel_sun