groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject groovy git commit: GROOVY-7014: A Java compile time error results in a runtime exception in Groovy (closes #381)
Date Wed, 17 Aug 2016 04:41:28 GMT
Repository: groovy
Updated Branches:
  refs/heads/master 9572de356 -> b0a75519d


GROOVY-7014: A Java compile time error results in a runtime exception in Groovy (closes #381)


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

Branch: refs/heads/master
Commit: b0a75519d2055c850aeef76e7475f4e969ebbc91
Parents: 9572de3
Author: paulk <paulk@asert.com.au>
Authored: Wed Aug 10 17:49:17 2016 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Wed Aug 17 14:39:09 2016 +1000

----------------------------------------------------------------------
 src/main/org/codehaus/groovy/ast/ClassNode.java |  35 +----
 .../groovy/ast/tools/ClassNodeUtils.java        | 133 +++++++++++++++++++
 .../groovy/control/StaticImportVisitor.java     |  89 +++++++------
 .../codehaus/groovy/control/StaticVerifier.java |  12 ++
 .../groovy/bugs/ConstructorThisCallBug.groovy   |  68 ++++++++++
 5 files changed, 260 insertions(+), 77 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/b0a75519/src/main/org/codehaus/groovy/ast/ClassNode.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/ClassNode.java b/src/main/org/codehaus/groovy/ast/ClassNode.java
index c6e8254..97d5a98 100644
--- a/src/main/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/org/codehaus/groovy/ast/ClassNode.java
@@ -1296,40 +1296,7 @@ public class ClassNode extends AnnotatedNode implements Opcodes {
      * @return true if a matching method was found
      */
     public boolean hasPossibleStaticMethod(String name, Expression arguments) {
-        int count = 0;
-
-        if (arguments instanceof TupleExpression) {
-            TupleExpression tuple = (TupleExpression) arguments;
-            // TODO this won't strictly be true when using list expansion in argument calls
-            count = tuple.getExpressions().size();
-        } else if (arguments instanceof MapExpression) {
-            count = 1;
-        }
-
-        for (MethodNode method : getMethods(name)) {
-            if(method.isStatic()) {
-                Parameter[] parameters = method.getParameters();
-                if (parameters.length == count) return true;
-
-                // handle varargs case
-                if (parameters.length > 0 && parameters[parameters.length - 1].getType().isArray())
{
-                    if (count >= parameters.length - 1) return true;
-                }
-                
-                // handle parameters with default values
-                int nonDefaultParameters = 0;
-                for (Parameter parameter : parameters) {
-                    if (!parameter.hasInitialExpression()) {
-                        nonDefaultParameters++;
-                    }
-                }
-
-                if (count < parameters.length && nonDefaultParameters <= count)
{
-                    return true;
-                }
-            }
-        }
-        return false;
+        return ClassNodeUtils.hasPossibleStaticMethod(this, name, arguments, false);
     }
 
     public boolean isInterface(){

http://git-wip-us.apache.org/repos/asf/groovy/blob/b0a75519/src/main/org/codehaus/groovy/ast/tools/ClassNodeUtils.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/tools/ClassNodeUtils.java b/src/main/org/codehaus/groovy/ast/tools/ClassNodeUtils.java
index 82cc190..3c6377a 100644
--- a/src/main/org/codehaus/groovy/ast/tools/ClassNodeUtils.java
+++ b/src/main/org/codehaus/groovy/ast/tools/ClassNodeUtils.java
@@ -23,12 +23,20 @@ package org.codehaus.groovy.ast.tools;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.PropertyNode;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MapExpression;
+import org.codehaus.groovy.ast.expr.SpreadExpression;
+import org.codehaus.groovy.ast.expr.TupleExpression;
 
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import static org.codehaus.groovy.ast.ClassHelper.boolean_TYPE;
+
 public class ClassNodeUtils {
     public static void addInterfaceMethods(ClassNode cnode, Map<String, MethodNode>
methodsMap) {
         // add in unimplemented abstract methods from the interfaces
@@ -65,4 +73,129 @@ public class ClassNodeUtils {
             sn = sn.getSuperClass();
         }
     }
+
+    /**
+     * Returns true if the given method has a possibly matching static method with the given
name and arguments.
+     *
+     * @param cNode     the ClassNode of interest
+     * @param name      the name of the method of interest
+     * @param arguments the arguments to match against
+     * @param trySpread whether to try to account for SpreadExpressions within the arguments
+     * @return true if a matching method was found
+     */
+    public static boolean hasPossibleStaticMethod(ClassNode cNode, String name, Expression
arguments, boolean trySpread) {
+        int count = 0;
+        boolean foundSpread = false;
+
+        if (arguments instanceof TupleExpression) {
+            TupleExpression tuple = (TupleExpression) arguments;
+            for (Expression arg : tuple.getExpressions()) {
+                if (arg instanceof SpreadExpression) {
+                    foundSpread = true;
+                } else {
+                    count++;
+                }
+            }
+        } else if (arguments instanceof MapExpression) {
+            count = 1;
+        }
+
+        for (MethodNode method : cNode.getMethods(name)) {
+            if (method.isStatic()) {
+                Parameter[] parameters = method.getParameters();
+                // do fuzzy match for spread case: count will be number of non-spread args
+                if (trySpread && foundSpread && parameters.length >= count)
return true;
+
+                if (parameters.length == count) return true;
+
+                // handle varargs case
+                if (parameters.length > 0 && parameters[parameters.length - 1].getType().isArray())
{
+                    if (count >= parameters.length - 1) return true;
+                    // fuzzy match any spread to a varargs
+                    if (trySpread && foundSpread) return true;
+                }
+
+                // handle parameters with default values
+                int nonDefaultParameters = 0;
+                for (Parameter parameter : parameters) {
+                    if (!parameter.hasInitialExpression()) {
+                        nonDefaultParameters++;
+                    }
+                }
+
+                if (count < parameters.length && nonDefaultParameters <= count)
{
+                    return true;
+                }
+                // TODO handle spread with nonDefaultParams?
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Return true if we have a static accessor
+     */
+    public static boolean hasPossibleStaticProperty(ClassNode candidate, String methodName)
{
+        // assume explicit static method call checked first so we can assume a simple check
here
+        if (!methodName.startsWith("get") && !methodName.startsWith("is")) {
+            return false;
+        }
+        String propName = getPropNameForAccessor(methodName);
+        PropertyNode pNode = getStaticProperty(candidate, propName);
+        return pNode != null && (methodName.startsWith("get") || boolean_TYPE.equals(pNode.getType()));
+    }
+
+    /**
+     * Returns the property name, e.g. age, given an accessor name, e.g. getAge.
+     * Returns the original if a valid prefix cannot be removed.
+     *
+     * @param accessorName the accessor name of interest, e.g. getAge
+     * @return the property name, e.g. age, or original if not a valid property accessor
name
+     */
+    public static String getPropNameForAccessor(String accessorName) {
+        if (!isValidAccessorName(accessorName)) return accessorName;
+        int prefixLength = accessorName.startsWith("is") ? 2 : 3;
+        return String.valueOf(accessorName.charAt(prefixLength)).toLowerCase() + accessorName.substring(prefixLength
+ 1);
+    }
+
+    /**
+     * Detect whether the given accessor name starts with "get", "set" or "is" followed by
at least one character.
+     *
+     * @param accessorName the accessor name of interest, e.g. getAge
+     * @return true if a valid prefix is found
+     */
+    public static boolean isValidAccessorName(String accessorName) {
+        if (accessorName.startsWith("get") || accessorName.startsWith("is") || accessorName.startsWith("set"))
{
+            int prefixLength = accessorName.startsWith("is") ? 2 : 3;
+            return accessorName.length() > prefixLength;
+        };
+        return false;
+    }
+
+    public static boolean hasStaticProperty(ClassNode cNode, String propName) {
+        return getStaticProperty(cNode, propName) != null;
+    }
+
+    /**
+     * Detect whether a static property with the given name is within the class
+     * or a super class.
+     *
+     * @param cNode the ClassNode of interest
+     * @param propName the property name
+     * @return the static property if found or else null
+     */
+    public static PropertyNode getStaticProperty(ClassNode cNode, String propName) {
+        ClassNode classNode = cNode;
+        while (classNode != null) {
+            for (PropertyNode pn : classNode.getProperties()) {
+                if (pn.getName().equals(propName) && pn.isStatic()) return pn;
+            }
+            classNode = classNode.getSuperClass();
+        }
+        return null;
+    }
+
+    public static boolean isInnerClass(ClassNode currentClass) {
+        return currentClass.getOuterClass() != null && !currentClass.isStaticClass();
+    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/b0a75519/src/main/org/codehaus/groovy/control/StaticImportVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/control/StaticImportVisitor.java b/src/main/org/codehaus/groovy/control/StaticImportVisitor.java
index 945cc86..44f77f4 100644
--- a/src/main/org/codehaus/groovy/control/StaticImportVisitor.java
+++ b/src/main/org/codehaus/groovy/control/StaticImportVisitor.java
@@ -27,7 +27,6 @@ import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.ImportNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.ModuleNode;
-import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.Variable;
 import org.codehaus.groovy.ast.expr.AnnotationConstantExpression;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
@@ -52,6 +51,12 @@ import org.codehaus.groovy.syntax.Types;
 import java.util.List;
 import java.util.Map;
 
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.getPropNameForAccessor;
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.hasPossibleStaticMethod;
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.hasPossibleStaticProperty;
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.hasStaticProperty;
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.isInnerClass;
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.isValidAccessorName;
 import static org.codehaus.groovy.runtime.MetaClassHelper.capitalize;
 
 /**
@@ -289,19 +294,35 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer
{
                 ConstantExpression ce = (ConstantExpression) method;
                 Object value = ce.getValue();
                 if (value instanceof String) {
+                    boolean foundInstanceMethod = false;
                     String methodName = (String) value;
-                    boolean lookForPossibleStaticMethod = !methodName.equals("call");
+                    boolean inInnerClass = isInnerClass(currentClass);
                     if (currentMethod != null && !currentMethod.isStatic()) {
                         if (currentClass.hasPossibleMethod(methodName, args)) {
-                            lookForPossibleStaticMethod = false;
+                            foundInstanceMethod = true;
                         }
                     }
-                    if (!inClosure && (inSpecialConstructorCall ||
-                            (lookForPossibleStaticMethod && currentClass.hasPossibleStaticMethod(methodName,
args)))) {
+                    boolean lookForPossibleStaticMethod = !methodName.equals("call");
+                    lookForPossibleStaticMethod &= !foundInstanceMethod;
+                    lookForPossibleStaticMethod |= inSpecialConstructorCall;
+                    lookForPossibleStaticMethod &= !inInnerClass;
+                    if (!inClosure && lookForPossibleStaticMethod &&
+                            (hasPossibleStaticMethod(currentClass, methodName, args, true))
+                            || hasPossibleStaticProperty(currentClass, methodName)) {
                         StaticMethodCallExpression smce = new StaticMethodCallExpression(currentClass,
methodName, args);
                         setSourcePosition(smce, mce);
                         return smce;
                     }
+                    if (!inClosure && inInnerClass && inSpecialConstructorCall
&& mce.isImplicitThis() && !foundInstanceMethod) {
+                        if (currentClass.getOuterClass().hasPossibleMethod(methodName, args))
{
+                            object = new PropertyExpression(new ClassExpression(currentClass.getOuterClass()),
new ConstantExpression("this"));
+                        } else if (hasPossibleStaticMethod(currentClass.getOuterClass(),
methodName, args, true)
+                                || hasPossibleStaticProperty(currentClass.getOuterClass(),
methodName)) {
+                            StaticMethodCallExpression smce = new StaticMethodCallExpression(currentClass.getOuterClass(),
methodName, args);
+                            setSourcePosition(smce, mce);
+                            return smce;
+                        }
+                    }
                 }
             }
         }
@@ -395,19 +416,13 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer
{
         //   import static MyClass.getProp [as getOtherProp]
         // when resolving prop reference
         if (importNodes.containsKey(accessorName)) {
-            ImportNode importNode = importNodes.get(accessorName);
-            expression = findStaticPropertyAccessorByFullName(importNode.getType(), importNode.getFieldName());
-            if (expression != null) return expression;
-            expression = findStaticPropertyAccessor(importNode.getType(), getPropNameForAccessor(importNode.getFieldName()));
+            expression = findStaticProperty(importNodes, accessorName);
             if (expression != null) return expression;
         }
         if (accessorName.startsWith("get")) {
             accessorName = "is" + accessorName.substring(3);
             if (importNodes.containsKey(accessorName)) {
-                ImportNode importNode = importNodes.get(accessorName);
-                expression = findStaticPropertyAccessorByFullName(importNode.getType(), importNode.getFieldName());
-                if (expression != null) return expression;
-                expression = findStaticPropertyAccessor(importNode.getType(), getPropNameForAccessor(importNode.getFieldName()));
+                expression = findStaticProperty(importNodes, accessorName);
                 if (expression != null) return expression;
             }
         }
@@ -435,6 +450,18 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer
{
         return null;
     }
 
+    private Expression findStaticProperty(Map<String, ImportNode> importNodes, String
accessorName) {
+        Expression result = null;
+        ImportNode importNode = importNodes.get(accessorName);
+        ClassNode importClass = importNode.getType();
+        String importMember = importNode.getFieldName();
+        result = findStaticPropertyAccessorByFullName(importClass, importMember);
+        if (result == null) {
+            result = findStaticPropertyAccessor(importClass, getPropNameForAccessor(importMember));
+        }
+        return result;
+    }
+
     private Expression findStaticMethodImportFromModule(Expression method, Expression args)
{
         ModuleNode module = currentClass.getModule();
         if (module == null || !(method instanceof ConstantExpression)) return null;
@@ -460,15 +487,17 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer
{
         // look for one of these:
         //   import static SomeClass.someProp [as otherName]
         // when resolving getProp() or setProp()
-        if (validPropName(name)) {
+        if (isValidAccessorName(name)) {
             String propName = getPropNameForAccessor(name);
             if (importNodes.containsKey(propName)) {
                 ImportNode importNode = importNodes.get(propName);
-                expression = findStaticMethod(importNode.getType(), prefix(name) + capitalize(importNode.getFieldName()),
args);
+                ClassNode importClass = importNode.getType();
+                String importMember = importNode.getFieldName();
+                expression = findStaticMethod(importClass, prefix(name) + capitalize(importMember),
args);
                 if (expression != null) return expression;
-                expression = findStaticPropertyAccessorGivenArgs(importNode.getType(), importNode.getFieldName(),
args);
+                expression = findStaticPropertyAccessorGivenArgs(importClass, importMember,
args);
                 if (expression != null) {
-                    return new StaticMethodCallExpression(importNode.getType(), prefix(name)
+ capitalize(importNode.getFieldName()), args);
+                    return new StaticMethodCallExpression(importClass, prefix(name) + capitalize(importMember),
args);
                 }
             }
         }
@@ -497,17 +526,6 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer
{
         return name.startsWith("is") ? "is" : name.substring(0, 3);
     }
 
-    private static String getPropNameForAccessor(String fieldName) {
-        int prefixLength = fieldName.startsWith("is") ? 2 : 3;
-        if (fieldName.length() < prefixLength + 1) return fieldName;
-        if (!validPropName(fieldName)) return fieldName;
-        return String.valueOf(fieldName.charAt(prefixLength)).toLowerCase() + fieldName.substring(prefixLength
+ 1);
-    }
-
-    private static boolean validPropName(String propName) {
-        return propName.startsWith("get") || propName.startsWith("is") || propName.startsWith("set");
-    }
-
     private String getAccessorName(String name) {
         return (inLeftExpression ? "set" : "get") + capitalize(name);
     }
@@ -533,21 +551,6 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer
{
         return accessor;
     }
 
-    private static boolean hasStaticProperty(ClassNode cNode, String propName) {
-        return getStaticProperty(cNode, propName) != null;
-    }
-
-    private static PropertyNode getStaticProperty(ClassNode cNode, String propName) {
-        ClassNode classNode = cNode;
-        while (classNode != null) {
-            for (PropertyNode pn : classNode.getProperties()) {
-                if (pn.getName().equals(propName) && pn.isStatic()) return pn;
-            }
-            classNode = classNode.getSuperClass();
-        }
-        return null;
-    }
-
     private Expression findStaticPropertyAccessorByFullName(ClassNode staticImportType, String
accessorMethodName) {
         // anything will do as we only check size == 1
         ArgumentListExpression dummyArgs = new ArgumentListExpression();

http://git-wip-us.apache.org/repos/asf/groovy/blob/b0a75519/src/main/org/codehaus/groovy/control/StaticVerifier.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/control/StaticVerifier.java b/src/main/org/codehaus/groovy/control/StaticVerifier.java
index 7c70bd4..166f1b5 100644
--- a/src/main/org/codehaus/groovy/control/StaticVerifier.java
+++ b/src/main/org/codehaus/groovy/control/StaticVerifier.java
@@ -38,6 +38,8 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 
+import static org.codehaus.groovy.ast.tools.ClassNodeUtils.isInnerClass;
+
 /**
  * Verifier to check non-static access in static contexts
  */
@@ -123,6 +125,16 @@ public class StaticVerifier extends ClassCodeVisitorSupport {
 
     @Override
     public void visitMethodCallExpression(MethodCallExpression mce) {
+        if (inSpecialConstructorCall && !isInnerClass(currentMethod.getDeclaringClass()))
{
+            Expression objectExpression = mce.getObjectExpression();
+            if (objectExpression instanceof VariableExpression) {
+                VariableExpression ve = (VariableExpression) objectExpression;
+                if (ve.isThisExpression()) {
+                    addError("Can't access instance method '" + mce.getMethodAsString() +
"' before the class is constructed", mce);
+                    return;
+                }
+            }
+        }
         super.visitMethodCallExpression(mce);
     }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/b0a75519/src/test/groovy/bugs/ConstructorThisCallBug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/ConstructorThisCallBug.groovy b/src/test/groovy/bugs/ConstructorThisCallBug.groovy
index 4f5c88b..d9873ec 100644
--- a/src/test/groovy/bugs/ConstructorThisCallBug.groovy
+++ b/src/test/groovy/bugs/ConstructorThisCallBug.groovy
@@ -19,6 +19,74 @@
 package groovy.bugs
 
 class ConstructorThisCallBug extends GroovyTestCase {
+    // GROOVY-7014
+    void testThisCallingInstanceMethod() {
+        def msg = shouldFail '''
+            class Base {
+                String getData() { return "ABCD" }
+                Base() { this(getData()) }
+                Base(String arg) {}
+            }
+        '''
+        assert msg.contains("Can't access instance method 'getData' before the class is constructed")
+    }
+
+    void testThisCallingStaticMethod() {
+        assertScript '''
+            class Base {
+                private String b
+                static String getData() { return "ABCD" }
+                Base() { this(getData()) }
+                Base(String b) { this.b = b }
+                String toString() { b }
+            }
+            assert new Base().toString() == 'ABCD'
+        '''
+    }
+
+    void testInnerClassSuperCallingInstanceMethod() {
+        assertScript '''
+            class Parent {
+                String str
+                Parent(String s) { str = s }
+            }
+            class Outer {
+                static String a
+
+                private class Inner extends Parent {
+                   Inner() { super(myA()) }
+                }
+
+                String test() { new Inner().str }
+                String myA() { a }
+            }
+            def o = new Outer()
+            Outer.a = 'ok'
+            assert o.test() == 'ok'
+        '''
+    }
+
+    void testInnerClassSuperCallingStaticProperty() {
+        assertScript '''
+            class Parent {
+                String str
+                Parent(String s) { str = s }
+            }
+            class Outer {
+                static String a
+
+                private class Inner extends Parent {
+                   Inner() { super(getA()) }
+                }
+
+                String test() { new Inner().str }
+            }
+            def o = new Outer()
+            Outer.a = 'ok'
+            assert o.test() == 'ok'
+        '''
+    }
+
     // GROOVY-994
     void testCallA() {
         assert new ConstructorCallA("foo").toString() == 'foo'


Mime
View raw message