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 2021/06/27 16:57:59 UTC
[groovy] branch master updated: GROOVY-9382,
GROOVY-10133: prefer isName() over getName() for boolean
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new ba9e78b GROOVY-9382, GROOVY-10133: prefer isName() over getName() for boolean
ba9e78b is described below
commit ba9e78b95eb025a098181f328f424a1a05b30ce2
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Jun 27 11:57:49 2021 -0500
GROOVY-9382, GROOVY-10133: prefer isName() over getName() for boolean
- java.lang.Boolean does not support "name" to "isName()" translation
---
src/main/java/groovy/lang/MetaClassImpl.java | 69 +++---
src/main/java/groovy/lang/MetaProperty.java | 13 +-
.../java/org/codehaus/groovy/ast/ClassNode.java | 8 +-
.../groovy/classgen/VariableScopeVisitor.java | 10 +-
.../org/codehaus/groovy/classgen/Verifier.java | 14 +-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 44 ++--
.../groovy/control/StaticImportVisitor.java | 129 +++++-----
.../transform/stc/StaticTypeCheckingVisitor.java | 10 +-
src/test/groovy/CategoryTest.groovy | 15 +-
src/test/groovy/StaticImportTest.groovy | 29 +++
src/test/groovy/bugs/Groovy10133.groovy | 275 +++++++++++++++++++++
.../{Groovy4206Bug.groovy => Groovy4206.groovy} | 35 ++-
.../groovy/xml/GpathSyntaxTestSupport.groovy | 2 +-
13 files changed, 494 insertions(+), 159 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 23f201b..ac30f78 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2441,37 +2441,33 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
Object ret = null;
if (methodOrList instanceof MetaMethod) {
- MetaMethod element = (MetaMethod) methodOrList;
- int parameterCount = element.getParameterTypes().length;
- if (!isGetter &&
- //(element.getReturnType() == Void.class || element.getReturnType() == Void.TYPE) &&
- parameterCount == 1) {
- ret = element;
+ MetaMethod method = (MetaMethod) methodOrList;
+ int parameterCount = method.getParameterTypes().length;
+ Class<?> returnType = method.getReturnType();
+ if (!isGetter && parameterCount == 1
+ //&& returnType == Void.TYPE
+ ) {
+ ret = method;
}
- Class returnType = element.getReturnType();
- if (isGetter &&
- !(returnType == Void.class || returnType == Void.TYPE) &&
- (!booleanGetter || returnType == Boolean.class || returnType == Boolean.TYPE) &&
- parameterCount == 0) {
- ret = element;
+ if (isGetter && parameterCount == 0 && (booleanGetter ? returnType == Boolean.TYPE
+ : returnType != Void.TYPE && returnType != Void.class)) {
+ ret = method;
}
- }
- if (methodOrList instanceof FastArray) {
+ } else if (methodOrList instanceof FastArray) {
FastArray methods = (FastArray) methodOrList;
- final int len = methods.size();
+ final int n = methods.size();
final Object[] data = methods.getArray();
- for (int i = 0; i != len; ++i) {
+ for (int i = 0; i < n; i += 1) {
MetaMethod element = (MetaMethod) data[i];
int parameterCount = element.getParameterTypes().length;
- if (!isGetter &&
- //(element.getReturnType() == Void.class || element.getReturnType() == Void.TYPE) &&
- parameterCount == 1) {
+ Class<?> returnType = element.getReturnType();
+ if (!isGetter && parameterCount == 1
+ //&& returnType == Void.TYPE
+ ) {
ret = addElementToList(ret, element);
}
- Class returnType = element.getReturnType();
- if (isGetter &&
- !(returnType == Void.class || returnType == Void.TYPE) &&
- parameterCount == 0) {
+ if (isGetter && parameterCount == 0 && (booleanGetter ? returnType == Boolean.TYPE
+ : returnType != Void.TYPE && returnType != Void.class)) {
ret = addElementToList(ret, element);
}
}
@@ -2704,17 +2700,15 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
private void applyStrayPropertyMethods(LinkedList<CachedClass> superClasses, Map<CachedClass, LinkedHashMap<String, MetaProperty>> classPropertyIndex, boolean isThis) {
// now look for any stray getters that may be used to define a property
- for (CachedClass klass : superClasses) {
- MetaMethodIndex.Header header = metaMethodIndex.getHeader(klass.getTheClass());
- Map<String, MetaProperty> propertyIndex = classPropertyIndex.computeIfAbsent(klass, k -> new LinkedHashMap<>());
+ for (CachedClass superClass : superClasses) {
+ MetaMethodIndex.Header header = metaMethodIndex.getHeader(superClass.getTheClass());
+ Map<String, MetaProperty> propertyIndex = classPropertyIndex.computeIfAbsent(superClass, sc -> new LinkedHashMap<>());
for (MetaMethodIndex.Entry e = header.head; e != null; e = e.nextClassEntry) {
String methodName = e.name;
- // name too short?
- final int methodNameLength = methodName.length();
- if (methodNameLength < 3 ||
- (methodNameLength < 4 && !methodName.startsWith("is"))) continue;
- // possible getter/setter?
+ int methodNameLength = methodName.length();
boolean isBooleanGetter = methodName.startsWith("is");
+ if (methodNameLength < (isBooleanGetter ? 3 : 4)) continue;
+
boolean isGetter = methodName.startsWith("get") || isBooleanGetter;
boolean isSetter = methodName.startsWith("set");
if (!isGetter && !isSetter) continue;
@@ -2726,8 +2720,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
if (propertyMethods instanceof MetaMethod) {
createMetaBeanProperty(propertyIndex, propName, isGetter, (MetaMethod) propertyMethods);
} else {
- LinkedList<MetaMethod> methods = (LinkedList<MetaMethod>) propertyMethods;
- for (MetaMethod m : methods) {
+ for (MetaMethod m : (Iterable<MetaMethod>) propertyMethods) {
createMetaBeanProperty(propertyIndex, propName, isGetter, m);
}
}
@@ -2769,13 +2762,19 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
} else if (mp instanceof MultipleSetterProperty) {
MultipleSetterProperty msp = (MultipleSetterProperty) mp;
if (isGetter) {
- msp.setGetter(propertyMethod);
+ // GROOVY-10133: do not replace "isPropName()" with "getPropName()" or ...
+ if (msp.getGetter() == null || !msp.getGetter().getName().startsWith("is")) {
+ msp.setGetter(propertyMethod);
+ }
}
return msp;
} else if (mp instanceof MetaBeanProperty) {
MetaBeanProperty mbp = (MetaBeanProperty) mp;
if (isGetter) {
- mbp.setGetter(propertyMethod);
+ // GROOVY-10133: do not replace "isPropName()" with "getPropName()" or ...
+ if (mbp.getGetter() == null || !mbp.getGetter().getName().startsWith("is")) {
+ mbp.setGetter(propertyMethod);
+ }
return mbp;
} else if (mbp.getSetter() == null || mbp.getSetter() == propertyMethod) {
mbp.setSetter(propertyMethod);
diff --git a/src/main/java/groovy/lang/MetaProperty.java b/src/main/java/groovy/lang/MetaProperty.java
index c52eee6..757b788 100644
--- a/src/main/java/groovy/lang/MetaProperty.java
+++ b/src/main/java/groovy/lang/MetaProperty.java
@@ -27,9 +27,10 @@ import static org.apache.groovy.util.BeanUtils.capitalize;
*/
public abstract class MetaProperty {
+ public static final String PROPERTY_SET_PREFIX = "set";
+
protected final String name;
protected Class type;
- public static final String PROPERTY_SET_PREFIX = "set";
/**
* Constructor that sets the property name and type (class)
@@ -47,7 +48,7 @@ public abstract class MetaProperty {
/**
* Sets the property on the given object to the new value
- *
+ *
* @param object on which to set the property
* @param newValue the new value of the property
* @throws RuntimeException if the property could not be set
@@ -69,7 +70,7 @@ public abstract class MetaProperty {
public Class getType() {
return type;
}
-
+
/**
* Returns the access modifier.
* @return Modifier.PUBLIC
@@ -84,8 +85,8 @@ public abstract class MetaProperty {
* @return The name of the property. The name is "get" + the capitalized propertyName
* or, in the case of boolean values, "is" + the capitalized propertyName
*/
- public static String getGetterName(String propertyName, Class type) {
- String prefix = type == boolean.class || type == Boolean.class ? "is" : "get";
+ public static String getGetterName(final String propertyName, final Class type) {
+ String prefix = type == Boolean.TYPE ? "is" : "get";
return prefix + capitalize(propertyName);
}
@@ -94,7 +95,7 @@ public abstract class MetaProperty {
*
* @return The name of the property. The name is "set"+ the capitalized propertyName.
*/
- public static String getSetterName(String propertyName) {
+ public static String getSetterName(final String propertyName) {
return PROPERTY_SET_PREFIX + capitalize(propertyName);
}
}
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassNode.java b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
index be59f51..51fc266 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
@@ -51,8 +51,8 @@ import java.util.stream.Collectors;
import static java.util.Arrays.stream;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid;
-import static org.codehaus.groovy.ast.ClassHelper.isWrapperBoolean;
import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
import static org.objectweb.asm.Opcodes.ACC_ANNOTATION;
import static org.objectweb.asm.Opcodes.ACC_ENUM;
@@ -1132,10 +1132,8 @@ public class ClassNode extends AnnotatedNode {
MethodNode getterMethod = null;
boolean booleanReturnOnly = getterName.startsWith("is");
for (MethodNode method : getDeclaredMethods(getterName)) {
- if (getterName.equals(method.getName())
- && !isPrimitiveVoid(method.getReturnType())
- && method.getParameters().length == 0
- && (!booleanReturnOnly || isWrapperBoolean(ClassHelper.getWrapper(method.getReturnType())))) {
+ if (method.getName().equals(getterName) && method.getParameters().length == 0
+ && (booleanReturnOnly ? isPrimitiveBoolean(method.getReturnType()) : !method.isVoidMethod())) {
// GROOVY-7363: There can be multiple matches for a getter returning a generic parameter type, due to
// the generation of a bridge method. The real getter is really the non-bridge, non-synthetic one as it
// has the most specific and exact return type of the two. Picking the bridge method results in loss of
diff --git a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
index 8fba09c..7067b78 100644
--- a/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/VariableScopeVisitor.java
@@ -58,7 +58,7 @@ import java.util.function.BiConsumer;
import static java.lang.reflect.Modifier.isFinal;
import static java.lang.reflect.Modifier.isStatic;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
-import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
/**
* Initializes the variable scopes for an AST.
@@ -170,7 +170,7 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
private Variable findClassMember(final ClassNode node, final String name) {
final boolean abstractType = node.isAbstract();
- for (ClassNode cn = node; cn != null && !isObjectType(cn); cn = cn.getSuperClass()) {
+ for (ClassNode cn = node; cn != null && !ClassHelper.isObjectType(cn); cn = cn.getSuperClass()) {
for (FieldNode fn : cn.getFields()) {
if (name.equals(fn.getName())) return fn;
}
@@ -181,12 +181,18 @@ public class VariableScopeVisitor extends ClassCodeVisitorSupport {
for (MethodNode mn : cn.getMethods()) {
if ((abstractType || !mn.isAbstract()) && name.equals(getPropertyName(mn))) {
+ // check for super property before returning a pseudo-property
+ for (PropertyNode pn : getAllProperties(cn.getSuperClass())) {
+ if (name.equals(pn.getName())) return pn;
+ }
+
FieldNode fn = new FieldNode(name, mn.getModifiers() & 0xF, ClassHelper.OBJECT_TYPE, cn, null);
fn.setHasNoRealSourcePosition(true);
fn.setDeclaringClass(cn);
fn.setSynthetic(true);
PropertyNode pn = new PropertyNode(fn, fn.getModifiers(), null, null);
+ pn.putNodeMetaData("access.method", mn);
pn.setDeclaringClass(cn);
return pn;
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index ec068b4..a6bd1bd 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -104,7 +104,6 @@ import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveDouble;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveLong;
-import static org.codehaus.groovy.ast.ClassHelper.isWrapperBoolean;
import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX;
@@ -715,7 +714,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
String getterName = node.getGetterName();
if (getterName == null) {
- getterName = "get" + capitalize(name); // we handle "is" below
+ getterName = "get" + capitalize(name);
}
String setterName = node.getSetterNameOrDefault();
@@ -725,8 +724,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
if (getterBlock == null) {
MethodNode getter = classNode.getGetterMethod(getterName, !node.isStatic());
if (getter == null && isPrimitiveBoolean(node.getType())) {
- String secondGetterName = "is" + capitalize(name);
- getter = classNode.getGetterMethod(secondGetterName);
+ getter = classNode.getGetterMethod("is" + capitalize(name));
}
if (!node.isPrivate() && methodNeedsReplacement(getter)) {
getterBlock = createGetterBlock(node, field);
@@ -734,9 +732,9 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
}
Statement setterBlock = node.getSetterBlock();
if (setterBlock == null) {
- // 2nd arg false below: though not usual, allow setter with non-void return type
- MethodNode setter = classNode.getSetterMethod(setterName, false);
- if (!node.isPrivate() && !isFinal(accessorModifiers) && methodNeedsReplacement(setter)) {
+ MethodNode setter = classNode.getSetterMethod(setterName,
+ false); // atypical: allow setter with non-void return type
+ if ((accessorModifiers & (ACC_FINAL | ACC_PRIVATE)) == 0 && methodNeedsReplacement(setter)) {
setterBlock = createSetterBlock(node, field);
}
}
@@ -749,7 +747,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
if (getterBlock != null) {
visitGetter(node, field, getterBlock, getterModifiers, getterName);
- if (node.getGetterName() == null && getterName.startsWith("get") && (isPrimitiveBoolean(node.getType()) || isWrapperBoolean(node.getType()))) {
+ if (node.getGetterName() == null && getterName.startsWith("get") && isPrimitiveBoolean(node.getType())) {
String altGetterName = "is" + capitalize(name);
MethodNode altGetter = classNode.getGetterMethod(altGetterName, !node.isStatic());
if (methodNeedsReplacement(altGetter)) {
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 639b5be..3958dee 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
@@ -58,7 +58,6 @@ import java.util.Map;
import static org.apache.groovy.ast.tools.ClassNodeUtils.getField;
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
import static org.apache.groovy.util.BeanUtils.capitalize;
-import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.CLASS_Type;
import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.GROOVY_OBJECT_TYPE;
@@ -71,8 +70,15 @@ import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.getUnwrapper;
import static org.codehaus.groovy.ast.ClassHelper.getWrapper;
import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.isBigDecimalType;
+import static org.codehaus.groovy.ast.ClassHelper.isBigIntegerType;
+import static org.codehaus.groovy.ast.ClassHelper.isClassType;
import static org.codehaus.groovy.ast.ClassHelper.isGeneratedFunction;
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
+import static org.codehaus.groovy.ast.ClassHelper.isStringType;
+import static org.codehaus.groovy.ast.ClassHelper.isWrapperInteger;
+import static org.codehaus.groovy.ast.ClassHelper.isWrapperLong;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
@@ -84,13 +90,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
-import static org.codehaus.groovy.ast.ClassHelper.isBigDecimalType;
-import static org.codehaus.groovy.ast.ClassHelper.isBigIntegerType;
-import static org.codehaus.groovy.ast.ClassHelper.isClassType;
-import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveBoolean;
-import static org.codehaus.groovy.ast.ClassHelper.isStringType;
-import static org.codehaus.groovy.ast.ClassHelper.isWrapperInteger;
-import static org.codehaus.groovy.ast.ClassHelper.isWrapperLong;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.chooseBestMethod;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDGMMethodsByNameAndArguments;
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf;
@@ -200,13 +199,14 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
if (makeGetPrivateFieldWithBridgeMethod(receiver, receiverType, propertyName, safe, implicitThis)) return;
// GROOVY-5580: it is still possible that we're calling a superinterface property
+ String isserName = "is" + capitalize(propertyName);
String getterName = "get" + capitalize(propertyName);
- String altGetterName = "is" + capitalize(propertyName);
if (receiverType.isInterface()) {
MethodNode getterMethod = null;
for (ClassNode anInterface : receiverType.getAllInterfaces()) {
- getterMethod = anInterface.getGetterMethod(getterName);
- if (getterMethod == null) getterMethod = anInterface.getGetterMethod(altGetterName);
+ getterMethod = anInterface.getGetterMethod(isserName);
+ if (getterMethod == null)
+ getterMethod = anInterface.getGetterMethod(getterName);
if (getterMethod != null) break;
}
// GROOVY-5585
@@ -225,12 +225,9 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
}
// GROOVY-5568: we would be facing a DGM call, but instead of foo.getText(), have foo.text
- List<MethodNode> methods = findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), receiverType, getterName, ClassNode.EMPTY_ARRAY);
- for (MethodNode dgm : findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), receiverType, altGetterName, ClassNode.EMPTY_ARRAY)) {
- if (Boolean_TYPE.equals(getWrapper(dgm.getReturnType()))) {
- methods.add(dgm);
- }
- }
+ List<MethodNode> methods = findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), receiverType, isserName, ClassNode.EMPTY_ARRAY);
+ methods.removeIf(dgm -> !isPrimitiveBoolean(dgm.getReturnType()));
+ findDGMMethodsByNameAndArguments(controller.getSourceUnit().getClassLoader(), receiverType, getterName, ClassNode.EMPTY_ARRAY, methods);
if (!methods.isEmpty()) {
List<MethodNode> methodNodes = chooseBestMethod(receiverType, methods, ClassNode.EMPTY_ARRAY);
if (methodNodes.size() == 1) {
@@ -466,11 +463,11 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
}
private boolean makeGetPropertyWithGetter(final Expression receiver, final ClassNode receiverType, final String propertyName, final boolean safe, final boolean implicitThis) {
- // does a getter exist?
- String getterName = "get" + capitalize(propertyName);
+ // check for an accessor method
+ String getterName = "is" + capitalize(propertyName);
MethodNode getterNode = receiverType.getGetterMethod(getterName);
if (getterNode == null) {
- getterName = "is" + capitalize(propertyName);
+ getterName = "get" + capitalize(propertyName);
getterNode = receiverType.getGetterMethod(getterName);
}
if (getterNode != null && receiver instanceof ClassExpression && !isClassType(receiverType) && !getterNode.isStatic()) {
@@ -482,11 +479,8 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
// generated by the compiler yet (generated by the Verifier)
PropertyNode propertyNode = receiverType.getProperty(propertyName);
if (getterNode == null && propertyNode != null) {
- // it is possible to use a getter
- String prefix = "get";
- if (isPrimitiveBoolean(propertyNode.getOriginType())) {
- prefix = "is";
- }
+ // it is possible to use an accessor method
+ String prefix = isPrimitiveBoolean(propertyNode.getOriginType()) ? "is" : "get";
getterName = prefix + capitalize(propertyName);
getterNode = new MethodNode(
getterName,
diff --git a/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java b/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java
index d7777ab..5a31bc1 100644
--- a/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/StaticImportVisitor.java
@@ -21,6 +21,7 @@ package org.codehaus.groovy.control;
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
+import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.DynamicVariable;
import org.codehaus.groovy.ast.FieldNode;
@@ -216,7 +217,7 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
protected Expression transformVariableExpression(VariableExpression ve) {
Variable v = ve.getAccessedVariable();
if (v instanceof DynamicVariable) {
- Expression result = findStaticFieldOrPropAccessorImportFromModule(v.getName());
+ Expression result = findStaticFieldOrPropertyAccessorImportFromModule(v.getName());
if (result != null) {
setSourcePosition(result, ve);
if (inAnnotation) {
@@ -253,7 +254,7 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
return result;
}
if (name != null && !inLeftExpression) { // maybe a closure field
- result = findStaticFieldOrPropAccessorImportFromModule(name);
+ result = findStaticFieldOrPropertyAccessorImportFromModule(name);
if (result != null) {
result = new MethodCallExpression(result, "call", args);
result.setSourcePosition(mce);
@@ -388,61 +389,42 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
return pe;
}
- private Expression findStaticFieldOrPropAccessorImportFromModule(String name) {
- ModuleNode module = currentClass.getModule();
- if (module == null) return null;
- Map<String, ImportNode> importNodes = module.getStaticImports();
- Expression expression;
- String accessorName = getAccessorName(name);
+ //--------------------------------------------------------------------------
+
+ private Expression findStaticFieldOrPropertyAccessorImportFromModule(String name) {
+ ModuleNode module = currentClass.getModule(); if (module == null) return null;
+ Map<String, ImportNode> staticImports = module.getStaticImports();
+
// look for one of these:
- // import static MyClass.setProp [as setOtherProp]
+ // import static MyClass.isProp [as isOtherProp]
// import static MyClass.getProp [as getOtherProp]
- // when resolving prop reference
- if (importNodes.containsKey(accessorName)) {
- expression = findStaticProperty(importNodes, accessorName);
+ // import static MyClass.setProp [as setOtherProp]
+ // when resolving property reference
+ Expression expression;
+ if (!inLeftExpression) {
+ expression = findStaticProperty(staticImports, "is" + capitalize(name));
if (expression != null) return expression;
}
- if (accessorName.startsWith("get")) {
- accessorName = "is" + accessorName.substring(3);
- if (importNodes.containsKey(accessorName)) {
- expression = findStaticProperty(importNodes, accessorName);
- if (expression != null) return expression;
- }
- }
+ expression = findStaticProperty(staticImports, getAccessorName(name));
+ if (expression != null) return expression;
// look for one of these:
// import static MyClass.prop [as otherProp]
- // when resolving prop or field reference
- if (importNodes.containsKey(name)) {
- ImportNode importNode = importNodes.get(name);
- expression = findStaticPropertyAccessor(importNode.getType(), importNode.getFieldName());
- if (expression != null) return expression;
- expression = findStaticField(importNode.getType(), importNode.getFieldName());
+ // when resolving property or field reference
+ if (staticImports.containsKey(name)) { ImportNode importNode = staticImports.get(name);
+ expression = findStaticPropertyOrField(importNode.getType(), importNode.getFieldName());
if (expression != null) return expression;
}
+
// look for one of these:
// import static MyClass.*
- // when resolving prop or field reference
+ // when resolving property or field reference
for (ImportNode importNode : module.getStaticStarImports().values()) {
- ClassNode node = importNode.getType();
- expression = findStaticPropertyAccessor(node, name);
- if (expression != null) return expression;
- expression = findStaticField(node, name);
+ expression = findStaticPropertyOrField(importNode.getType(), name);
if (expression != null) return expression;
}
- 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;
+ return null;
}
private Expression findStaticMethodImportFromModule(Expression method, Expression args) {
@@ -513,41 +495,70 @@ public class StaticImportVisitor extends ClassCodeExpressionTransformer {
return inLeftExpression ? getSetterName(name) : getGetterName(name);
}
+ private Expression findStaticPropertyAccessorByFullName(ClassNode staticImportType, String accessorName) {
+ Expression argumentList = inLeftExpression ? new ArgumentListExpression(EmptyExpression.INSTANCE) : ArgumentListExpression.EMPTY_ARGUMENTS;
+ Expression accessorExpr = findStaticMethod(staticImportType, accessorName, argumentList);
+ if (accessorExpr != null && accessorName.startsWith("is")) { // GROOVY-9382, GROOVY-10133
+ MethodNode method = staticImportType.getMethod(accessorName, Parameter.EMPTY_ARRAY);
+ if (method == null || !ClassHelper.isPrimitiveBoolean(method.getReturnType())) {
+ accessorExpr = null;
+ }
+ }
+ return accessorExpr;
+ }
+
private Expression findStaticPropertyAccessorGivenArgs(ClassNode staticImportType, String propName, Expression args) {
- // TODO validate args?
- return findStaticPropertyAccessor(staticImportType, propName);
+ return findStaticPropertyAccessor(staticImportType, propName); // TODO: validate args?
}
private Expression findStaticPropertyAccessor(ClassNode staticImportType, String propName) {
String accessorName = getAccessorName(propName);
- Expression accessor = findStaticPropertyAccessorByFullName(staticImportType, accessorName);
- if (accessor == null && accessorName.startsWith("get")) {
+ Expression accessor = null;
+ if (!inLeftExpression) {
accessor = findStaticPropertyAccessorByFullName(staticImportType, "is" + accessorName.substring(3));
}
+ if (accessor == null) {
+ accessor = findStaticPropertyAccessorByFullName(staticImportType, accessorName);
+ }
if (accessor == null && hasStaticProperty(staticImportType, propName)) {
- // args will be replaced
if (inLeftExpression)
- accessor = newStaticMethodCallX(staticImportType, accessorName, ArgumentListExpression.EMPTY_ARGUMENTS);
+ accessor = newStaticMethodCallX(staticImportType, accessorName, ArgumentListExpression.EMPTY_ARGUMENTS); // <-- will be replaced
else
accessor = newStaticPropertyX(staticImportType, propName);
}
return accessor;
}
- private Expression findStaticPropertyAccessorByFullName(ClassNode staticImportType, String accessorMethodName) {
- // anything will do as we only check size == 1
- ArgumentListExpression dummyArgs = new ArgumentListExpression();
- dummyArgs.addExpression(EmptyExpression.INSTANCE);
- return findStaticMethod(staticImportType, accessorMethodName, (inLeftExpression ? dummyArgs : ArgumentListExpression.EMPTY_ARGUMENTS));
+ private Expression findStaticPropertyOrField(ClassNode staticImportType, String variableName) {
+ Expression expression = findStaticPropertyAccessor(staticImportType, variableName);
+ if (expression == null) {
+ if (staticImportType.isPrimaryClassNode() || staticImportType.isResolved()) {
+ FieldNode field = getField(staticImportType, variableName);
+ if (field != null && field.isStatic())
+ expression = newStaticPropertyX(staticImportType, variableName);
+ }
+ }
+ return expression;
}
- private static Expression findStaticField(ClassNode staticImportType, String fieldName) {
- if (staticImportType.isPrimaryClassNode() || staticImportType.isResolved()) {
- FieldNode field = getField(staticImportType, fieldName);
- if (field != null && field.isStatic())
- return newStaticPropertyX(staticImportType, fieldName);
+ private Expression findStaticProperty(Map<String, ImportNode> staticImports, String accessorName) {
+ Expression expression = null;
+ ImportNode importNode = staticImports.get(accessorName);
+ if (importNode != null) { ClassNode importType = importNode.getType();
+ expression = findStaticPropertyAccessorByFullName(importType, importNode.getFieldName());
+ if (expression == null) { // perhaps the property accessor will be generated
+ String propertyName = getPropNameForAccessor(importNode.getFieldName());
+ if (hasStaticProperty(importType, propertyName)) {
+ if (inLeftExpression) {
+ expression = newStaticMethodCallX(importType, importNode.getFieldName(),
+ ArgumentListExpression.EMPTY_ARGUMENTS); // <-- will be replaced
+ } else {
+ expression = newStaticPropertyX(importType, propertyName);
+ }
+ }
+ }
}
- return null;
+ return expression;
}
private static Expression findStaticMethod(ClassNode staticImportType, String methodName, Expression args) {
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 cf64542..5324936 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -145,7 +145,6 @@ import static org.apache.groovy.util.BeanUtils.decapitalize;
import static org.codehaus.groovy.ast.ClassHelper.AUTOCLOSEABLE_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.BigInteger_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Byte_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.CLASS_Type;
import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
@@ -215,6 +214,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName;
import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
@@ -1562,11 +1562,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
- MethodNode getter = findGetter(current, "get" + capName, pexp.isImplicitThis());
+ MethodNode getter = findGetter(current, "is" + capName, pexp.isImplicitThis());
getter = allowStaticAccessToMember(getter, staticOnly);
- if (getter == null) getter = findGetter(current, "is" + capName, pexp.isImplicitThis());
+ if (getter == null) getter = findGetter(current, getGetterName(propertyName), pexp.isImplicitThis());
getter = allowStaticAccessToMember(getter, staticOnly);
- List<MethodNode> setters = findSetters(current, getSetterName(propertyName), false);
+ List<MethodNode> setters = findSetters(current, getSetterName(propertyName), /*enforce void:*/false);
setters = allowStaticAccessToMember(setters, staticOnly);
// need to visit even if we only look for setters for compatibility
@@ -1632,7 +1632,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
for (ClassNode dgmReceiver : isPrimitiveType(receiverType) ? new ClassNode[]{receiverType, getWrapper(receiverType)} : new ClassNode[]{receiverType}) {
List<MethodNode> methods = findDGMMethodsByNameAndArguments(getSourceUnit().getClassLoader(), dgmReceiver, "get" + capName, ClassNode.EMPTY_ARRAY);
for (MethodNode method : findDGMMethodsByNameAndArguments(getSourceUnit().getClassLoader(), dgmReceiver, "is" + capName, ClassNode.EMPTY_ARRAY)) {
- if (Boolean_TYPE.equals(getWrapper(method.getReturnType()))) methods.add(method);
+ if (isPrimitiveBoolean(method.getReturnType())) methods.add(method);
}
if (isUsingGenericsOrIsArrayUsingGenerics(dgmReceiver)) {
methods.removeIf(method -> // GROOVY-10075: "List<Integer>" vs "List<String>"
diff --git a/src/test/groovy/CategoryTest.groovy b/src/test/groovy/CategoryTest.groovy
index 743a5e9..a4d7ba5 100644
--- a/src/test/groovy/CategoryTest.groovy
+++ b/src/test/groovy/CategoryTest.groovy
@@ -68,9 +68,22 @@ final class CategoryTest extends GroovyTestCase {
}
}
- // GROOVY-5245
+ // GROOVY-10133
void testCategoryDefinedProperties2() {
assertScript '''
+ class Cat {
+ static boolean isAbc(self) { true }
+ static boolean getAbc(self) { false }
+ }
+ use (Cat) {
+ assert abc // should select "isAbc()"
+ }
+ '''
+ }
+
+ // GROOVY-5245
+ void testCategoryDefinedProperties3() {
+ assertScript '''
class Isser {
boolean isWorking() { true }
}
diff --git a/src/test/groovy/StaticImportTest.groovy b/src/test/groovy/StaticImportTest.groovy
index d44b616..4ddf9b1 100644
--- a/src/test/groovy/StaticImportTest.groovy
+++ b/src/test/groovy/StaticImportTest.groovy
@@ -230,6 +230,35 @@ final class StaticImportTest extends groovy.test.GroovyTestCase {
}
}
+ // GROOVY-9382, GROOVY-10133
+ void testStaticImportPropertyWithChoices() {
+ assertScript '''
+ import static Foo.isX
+ import static Foo.getX
+ class Foo {
+ static boolean isX() { true }
+ static boolean getX() { false }
+ }
+ assert x
+ '''
+
+ def err = shouldFail '''
+ import static Foo.isX
+ class Foo { static isX() {} }
+
+ x
+ '''
+ assert err =~ /No such property: x for class/
+
+ err = shouldFail '''
+ import static Foo.isX as isY
+ class Foo { static isX() {} }
+
+ y
+ '''
+ assert err =~ /No such property: y for class/
+ }
+
void testConstructorArgsAliasing() {
// not recommended style to use statics in constructors but supported
assertScript '''
diff --git a/src/test/groovy/bugs/Groovy10133.groovy b/src/test/groovy/bugs/Groovy10133.groovy
new file mode 100644
index 0000000..b474b4e
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy10133.groovy
@@ -0,0 +1,275 @@
+/*
+ * 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
+
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy10133 {
+
+ @Test
+ void testGetterVersusIsser1() {
+ assertScript '''
+ class C {
+ boolean isX() { true }
+ boolean getX() { false }
+
+ void test1() {
+ assert x
+ assert this.x
+ }
+ }
+
+ new C().test1()
+ assert new C().x
+
+ class D extends C {
+ void test2() {
+ assert x
+ assert this.x
+ assert !super.x // GROOVY-6097
+ }
+ }
+
+ new D().test1()
+ new D().test2()
+ assert new D().x
+ '''
+ }
+
+ @Test
+ void testGetterVersusIsser2() {
+ assertScript '''
+ class C {
+ boolean x = false
+ boolean getX() { true }
+ // no isX() if getX() declared
+
+ void test1() {
+ // direct
+ assert !x
+ assert !this.x
+ }
+ }
+
+ new C().test1()
+ assert new C().x
+
+ class D extends C {
+ void test2() {
+ assert x
+ assert this.x
+ assert super.x
+ }
+ }
+
+ new D().test1()
+ new D().test2()
+ assert new D().x
+ '''
+ }
+
+ @Test
+ void testGetterVersusIsser3() {
+ assertScript '''
+ class C {
+ boolean x = false
+ boolean isX() { true }
+ // no getX() if isX() declared
+
+ void test1() {
+ // direct
+ assert !x
+ assert !this.x
+ }
+ }
+
+ new C().test1()
+ assert new C().x
+
+ class D extends C {
+ void test2() {
+ assert x
+ assert this.x
+ try {
+ assert super.x // GROOVY-6097
+ assert false : 'remove catch'
+ } catch (MissingMethodException mme) {
+ }
+ }
+ }
+
+ new D().test1()
+ new D().test2()
+ assert new D().x
+ '''
+ }
+
+ @Test
+ void testGetterVersusIsser4() {
+ assertScript '''
+ class C {
+ boolean x = true
+ }
+
+ class D extends C {
+ boolean getX() { false }
+ // TODO: warning for no "isX" override
+
+ void test() {
+ assert x
+ assert this.x
+ assert super.x
+ }
+ }
+
+ new D().test()
+ assert new D().x
+ '''
+ }
+
+ @Test
+ void testGetterVersusIsser5() {
+ assertScript '''
+ class C {
+ boolean x = false
+ }
+
+ class D extends C {
+ boolean isX() { true }
+ // TODO: warning for no "getX" override
+
+ void test() {
+ assert x
+ assert this.x
+ assert !super.x
+ }
+ }
+
+ new D().test()
+ assert new D().x
+ '''
+ }
+
+ @Test
+ void testGetterVersusIsser6() {
+ assertScript '''
+ class C {
+ boolean isX() { true }
+ boolean getX() { false }
+
+ @groovy.transform.CompileStatic
+ void test1() {
+ assert x
+ assert this.x
+ }
+ }
+
+ class D extends C {
+ @groovy.transform.CompileStatic
+ void test2() {
+ assert x
+ assert this.x
+ assert !super.x // GROOVY-6097
+ }
+ }
+
+ @groovy.transform.CompileStatic
+ void test() {
+ new C().test1()
+ assert new C().x
+
+ new D().test1()
+ new D().test2()
+ assert new D().x
+ }
+ test()
+ '''
+ }
+
+ @Test
+ void testGetterVersusIsser7() {
+ assertScript '''
+ class C {
+ Boolean isX() { Boolean.FALSE }
+ Boolean getX() { Boolean.TRUE }
+
+ void test1() {
+ assert x
+ assert this.x
+ }
+ }
+
+ new C().test1()
+ assert new C().x
+
+ class D extends C {
+ void test2() {
+ assert x
+ assert this.x
+ assert super.x
+ }
+ }
+
+ new D().test1()
+ new D().test2()
+ assert new D().x
+ '''
+ }
+
+ @Test // GROOVY-9382
+ void testGetterVersusIsser8() {
+ shouldFail MissingPropertyException, '''
+ class C {
+ Boolean isX() { null }
+
+ void test() {
+ x
+ }
+ }
+ new C().test()
+ '''
+
+ def err = shouldFail '''
+ class C {
+ Boolean isX() { null }
+
+ @groovy.transform.TypeChecked
+ void test() {
+ x
+ }
+ }
+ '''
+ assert err =~ /The variable \[x\] is undeclared/
+
+ err = shouldFail '''
+ class C {
+ Boolean isX() { null }
+
+ @groovy.transform.TypeChecked
+ void test() {
+ new C().x
+ }
+ }
+ '''
+ assert err =~ /No such property: x for class: C/
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy4206Bug.groovy b/src/test/groovy/bugs/Groovy4206.groovy
similarity index 62%
rename from src/test/groovy/bugs/Groovy4206Bug.groovy
rename to src/test/groovy/bugs/Groovy4206.groovy
index 011c118..b0ded82 100644
--- a/src/test/groovy/bugs/Groovy4206Bug.groovy
+++ b/src/test/groovy/bugs/Groovy4206.groovy
@@ -18,19 +18,30 @@
*/
package groovy.bugs
-import groovy.test.GroovyTestCase
+import org.junit.Test
-class Groovy4206Bug extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy4206 {
+
+ @Test
void testIsNamesForBooleanProps() {
- assert Bar4206.isValid()
- assert Bar4206.valid
- assert '1.1E2'.isBigDecimal()
- assert '1.1E2'.bigDecimal
- assert ' '.isAllWhitespace()
- assert ' '.allWhitespace
- }
-}
+ assertScript '''
+ class C {
+ static boolean isValid() { true }
+ }
-class Bar4206 {
- static Boolean isValid() { true }
+ assert C.isValid()
+ assert C.valid
+
+ assert !3.14f.isNaN()
+ assert !3.14f.NaN
+
+ assert '1.1E2'.isBigDecimal()
+ assert '1.1E2'.bigDecimal
+
+ assert ' '.isAllWhitespace()
+ assert ' '.allWhitespace
+ '''
+ }
}
diff --git a/subprojects/groovy-xml/src/test/groovy/groovy/xml/GpathSyntaxTestSupport.groovy b/subprojects/groovy-xml/src/test/groovy/groovy/xml/GpathSyntaxTestSupport.groovy
index f26d20f..19b39f1 100644
--- a/subprojects/groovy-xml/src/test/groovy/groovy/xml/GpathSyntaxTestSupport.groovy
+++ b/subprojects/groovy-xml/src/test/groovy/groovy/xml/GpathSyntaxTestSupport.groovy
@@ -87,7 +87,7 @@ class GpathSyntaxTestSupport {
// additional DOM long-hand syntax
// for illustrative purposes only
assert likes.item(0).nodeName == 'likes'
- assert wallaceLikes.firstChild.nodeValue == 'cheese'
+ assert wallaceLikes.getFirstChild().nodeValue == 'cheese'
if (wallaceLikes.class.name.contains('xerces')) {
assert 'cheese' == wallaceLikes.textContent
}