You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2016/08/17 04:41:28 UTC
groovy git commit: GROOVY-7014: A Java compile time error results in
a runtime exception in Groovy (closes #381)
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 <pa...@asert.com.au>
Authored: Wed Aug 10 17:49:17 2016 +1000
Committer: paulk <pa...@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'