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/14 19:27:10 UTC
[groovy] branch GROOVY-9382 created (now 5a17358)
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.
at 5a17358 GROOVY-9382, GROOVY-10133: prefer isName() over getName() for boolean
This branch includes the following new commits:
new 5a17358 GROOVY-9382, GROOVY-10133: prefer isName() over getName() for boolean
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.
[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 5a17358bede35d2714c8174bf1f40431f66dfdff
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 +-
.../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 ++-
9 files changed, 392 insertions(+), 103 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/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..0ed15bd 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) { true }
+ }
+ 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
+ '''
+ }
}