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/21 21:22:00 UTC
[groovy] branch GROOVY-9382 updated (c96db21 -> 9da8d99)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9382
in repository https://gitbox.apache.org/repos/asf/groovy.git.
omit c96db21 GROOVY-9382, GROOVY-10133: prefer isName() over getName() for boolean
new 9da8d99 GROOVY-9382, GROOVY-10133: prefer isName() over getName() for boolean
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (c96db21)
\
N -- N -- N refs/heads/GROOVY-9382 (9da8d99)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/codehaus/groovy/classgen/VariableScopeVisitor.java | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[groovy] 01/01: GROOVY-9382,
GROOVY-10133: prefer isName() over getName() for boolean
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9382
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 9da8d9999e95c8e6974d52ac68225eab5fb069a6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jun 14 14:12:19 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 | 73 +++---
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 | 22 +-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 44 ++--
.../transform/stc/StaticTypeCheckingVisitor.java | 10 +-
src/test/groovy/CategoryTest.groovy | 15 +-
src/test/groovy/bugs/Groovy10133.groovy | 275 +++++++++++++++++++++
.../{Groovy4206Bug.groovy => Groovy4206.groovy} | 35 ++-
.../groovy/xml/GpathSyntaxTestSupport.groovy | 2 +-
11 files changed, 401 insertions(+), 106 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 2046b3e..b72b174 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2291,37 +2291,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;
- }
- Class returnType = element.getReturnType();
- if (isGetter &&
- !(returnType == Void.class || returnType == Void.TYPE) &&
- (!booleanGetter || returnType == Boolean.class || returnType == Boolean.TYPE) &&
- parameterCount == 0) {
- ret = element;
- }
- }
- if (methodOrList instanceof FastArray) {
+ MetaMethod method = (MetaMethod) methodOrList;
+ int parameterCount = method.getParameterTypes().length;
+ Class<?> returnType = method.getReturnType();
+ if (!isGetter && parameterCount == 1
+ //&& returnType == Void.TYPE
+ ) {
+ ret = method;
+ }
+ if (isGetter && parameterCount == 0 && (booleanGetter ? returnType == Boolean.TYPE
+ : returnType != Void.TYPE && returnType != Void.class)) {
+ ret = method;
+ }
+ } 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);
}
}
@@ -2554,17 +2550,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;
@@ -2576,8 +2570,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);
}
}
@@ -2619,13 +2612,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 b2761b0..fa09745 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -98,6 +98,10 @@ import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
import static org.codehaus.groovy.ast.AnnotationNode.METHOD_TARGET;
+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.tools.GeneralUtils.binX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX;
@@ -114,11 +118,6 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec;
import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod;
-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;
/**
* Verifies the AST node and adds any default AST code before bytecode generation occurs.
@@ -686,7 +685,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();
@@ -696,8 +695,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);
@@ -705,9 +703,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);
}
}
@@ -720,7 +718,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/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/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
}