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:11 UTC
[groovy] 01/01: 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 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
+ '''
+ }
}