This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 7bc29825bc GROOVY-11829: check param types of standard methods and
keep best fit
7bc29825bc is described below
commit 7bc29825bcc5548e678353cdb2443d19144e6a66
Author: Eric Milles <[email protected]>
AuthorDate: Thu Jan 1 14:59:42 2026 -0600
GROOVY-11829: check param types of standard methods and keep best fit
---
src/main/java/groovy/lang/MetaClassImpl.java | 115 ++++++++++-----------
.../codehaus/groovy/runtime/MetaClassHelper.java | 5 -
src/test/groovy/bugs/Groovy11829.groovy | 78 ++++++++++++++
.../{Groovy3871Bug.groovy => Groovy3871.groovy} | 28 +++--
src/test/groovy/bugs/Groovy8065.groovy | 8 +-
src/test/groovy/groovy/IfPropertyTest.groovy | 12 +--
6 files changed, 157 insertions(+), 89 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java
b/src/main/java/groovy/lang/MetaClassImpl.java
index 98a25a1c9c..c2e1d5138e 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2768,7 +2768,7 @@ public class MetaClassImpl implements MetaClass,
MutableMetaClass {
//----------------------------------------------------------------------
// generic set method
//----------------------------------------------------------------------
- // check for a generic get method provided through a category
+ // check for a generic set method provided through a category
if (method == null && !useSuper && !isStatic &&
GroovyCategorySupport.hasCategoryInCurrentThread()) {
method = getCategoryMethodSetter(theClass, "set", true);
if (method != null) arguments = new Object[]{name, newValue};
@@ -2994,14 +2994,13 @@ public class MetaClassImpl implements MetaClass,
MutableMetaClass {
//--------------------------------------------------------------------------
/**
- * adds a MetaMethod to this class. WARNING: this method will not
+ * Adds a MetaMethod to this class. WARNING: this method will not
* do the necessary steps for multimethod logic and using this
* method doesn't mean, that a method added here is replacing another
* method from a parent class completely. These steps are usually done
* by initialize, which means if you need these steps, you have to add
* the method before running initialize the first time.
*
- * @param method the MetaMethod
* @see #initialize()
*/
@Override
@@ -3020,69 +3019,75 @@ public class MetaClassImpl implements MetaClass,
MutableMetaClass {
}
/**
- * Checks if the metaMethod is a method from the GroovyObject interface
such as setProperty, getProperty and invokeMethod
+ * Checks if the metaMethod is getProperty, setProperty, or invokeMethod.
*
- * @param metaMethod The metaMethod instance
* @see GroovyObject
*/
- protected final void checkIfGroovyObjectMethod(MetaMethod metaMethod) {
+ protected final void checkIfGroovyObjectMethod(final MetaMethod
metaMethod) {
if (metaMethod instanceof ClosureMetaMethod
|| metaMethod instanceof NewInstanceMetaMethod
|| metaMethod instanceof MixinInstanceMetaMethod) {
- if (isGetPropertyMethod(metaMethod)) {
- getPropertyMethod = metaMethod;
- } else if (isInvokeMethod(metaMethod)) {
- invokeMethodMethod = metaMethod;
- } else if (isSetPropertyMethod(metaMethod)) {
- setPropertyMethod = metaMethod;
+ switch (metaMethod.getName()) {
+ case GET_PROPERTY_METHOD:
+ if (checkMatch(metaMethod, getPropertyMethod,
GETTER_MISSING_ARGS) && metaMethod.getReturnType() != Void.TYPE) {
+ getPropertyMethod = metaMethod;
+ }
+ break;
+ case SET_PROPERTY_METHOD:
+ if (checkMatch(metaMethod, setPropertyMethod,
SETTER_MISSING_ARGS)) {
+ setPropertyMethod = metaMethod;
+ }
+ break;
+ case INVOKE_METHOD_METHOD:
+ if (checkMatch(metaMethod, invokeMethodMethod,
METHOD_MISSING_ARGS)) {
+ invokeMethodMethod = metaMethod;
+ }
+ break;
}
}
}
- private static boolean isSetPropertyMethod(MetaMethod metaMethod) {
- return SET_PROPERTY_METHOD.equals(metaMethod.getName()) &&
metaMethod.getParameterTypes().length == 2;
- }
-
- private static boolean isGetPropertyMethod(MetaMethod metaMethod) {
- return GET_PROPERTY_METHOD.equals(metaMethod.getName());
- }
-
- private static boolean isInvokeMethod(MetaMethod metaMethod) {
- return INVOKE_METHOD_METHOD.equals(metaMethod.getName()) &&
metaMethod.getParameterTypes().length == 2;
- }
-
- private void checkIfStdMethod(MetaMethod method) {
- checkIfGroovyObjectMethod(method);
-
- if (isGenericGetMethod(method) && genericGetMethod == null) {
- genericGetMethod = method;
- } else if (MetaClassHelper.isGenericSetMethod(method) &&
genericSetMethod == null) {
- genericSetMethod = method;
- }
- if (method.getName().equals(PROPERTY_MISSING)) {
- CachedClass[] parameterTypes = method.getParameterTypes();
- if (parameterTypes.length == 1) {
- propertyMissingGet = method;
+ private void checkIfStdMethod(final MetaMethod metaMethod) {
+ switch (metaMethod.getName()) {
+ case GET_PROPERTY_METHOD:
+ case SET_PROPERTY_METHOD:
+ case INVOKE_METHOD_METHOD:
+ checkIfGroovyObjectMethod(metaMethod);
+ break;
+ case "get":
+ if (checkMatch(metaMethod, genericGetMethod, GETTER_MISSING_ARGS)
&& metaMethod.getReturnType() != Void.TYPE) {
+ genericGetMethod = metaMethod;
}
- }
- if (propertyMissingSet == null &&
method.getName().equals(PROPERTY_MISSING)) {
- CachedClass[] parameterTypes = method.getParameterTypes();
- if (parameterTypes.length == 2) {
- propertyMissingSet = method;
+ break;
+ case "set":
+ if (checkMatch(metaMethod, genericSetMethod, SETTER_MISSING_ARGS))
{
+ genericSetMethod = metaMethod;
}
- }
- if (method.getName().equals(METHOD_MISSING)) {
- CachedClass[] parameterTypes = method.getParameterTypes();
- if (parameterTypes.length == 2
- && parameterTypes[0].getTheClass() == String.class
- && parameterTypes[1].getTheClass() == Object.class) {
- methodMissing = method;
+ break;
+ case METHOD_MISSING:
+ if (checkMatch(metaMethod, methodMissing, METHOD_MISSING_ARGS)) {
+ methodMissing = metaMethod;
+ }
+ break;
+ case PROPERTY_MISSING:
+ if (checkMatch(metaMethod, propertyMissingSet,
SETTER_MISSING_ARGS)) {
+ propertyMissingSet = metaMethod;
+ } else if (checkMatch(metaMethod, propertyMissingGet,
GETTER_MISSING_ARGS) && metaMethod.getReturnType() != Void.TYPE) {
+ propertyMissingGet = metaMethod;
+ }
+ break;
+ default:
+ if (theCachedClass.isNumber) {
+
NumberMathModificationInfo.instance.checkIfStdMethod(metaMethod);
}
}
+ }
- if (theCachedClass.isNumber) {
- NumberMathModificationInfo.instance.checkIfStdMethod(method);
- }
+ private static boolean checkMatch(final MetaMethod newMethod, final
MetaMethod oldMethod, final Class<?>[] arguments) {
+ return newMethod.isValidExactMethod(arguments) && (oldMethod == null
+ // GROOVY-11829: new method may provide closer match to arguments
+ || MetaClassHelper.calculateParameterDistance(arguments, newMethod)
+ <= MetaClassHelper.calculateParameterDistance(arguments,
oldMethod));
}
protected boolean isInitialized() {
@@ -3298,14 +3303,6 @@ public class MetaClassImpl implements MetaClass,
MutableMetaClass {
return matchesDistance;
}
- private static boolean isGenericGetMethod(MetaMethod method) {
- if ("get".equals(method.getName())) {
- CachedClass[] parameterTypes = method.getParameterTypes();
- return parameterTypes.length == 1 &&
parameterTypes[0].getTheClass() == String.class;
- }
- return false;
- }
-
/**
* Complete the initialisation process. After this method
* is called no methods should be added to the metaclass.
diff --git a/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
b/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
index 5aba7fc81f..acd9ca8969 100644
--- a/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
@@ -809,11 +809,6 @@ public class MetaClassHelper {
|| classToTransformFrom == Byte.class;
}
- public static boolean isGenericSetMethod(MetaMethod method) {
- return ("set".equals(method.getName()))
- && method.getParameterTypes().length == 2;
- }
-
protected static boolean isSuperclass(Class clazz, Class superclass) {
while (clazz != null) {
if (clazz == superclass) return true;
diff --git a/src/test/groovy/bugs/Groovy11829.groovy
b/src/test/groovy/bugs/Groovy11829.groovy
new file mode 100644
index 0000000000..25b7590167
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy11829.groovy
@@ -0,0 +1,78 @@
+/*
+ * 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 bugs
+
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy11829 {
+
+ @Test
+ void testCustomGetter() {
+ assertScript '''
+ class C {
+ def get(String name) {
+ if (name == 'x') return 'yz'
+ }
+ }
+
+ def c = new C()
+ assert c.x == 'yz'
+ assert c.y == null
+ '''
+ }
+
+ @Test
+ void testCustomSetter() {
+ assertScript '''import static groovy.test.GroovyAssert.shouldFail
+ class C {
+ def set(String name, Object value) {
+ if (name != 'x') throw PropertyMissingException(name, C)
+ }
+ }
+
+ def c = new C()
+ c.x = 'xyz'
+ shouldFail {
+ c.y = null
+ }
+ '''
+ }
+
+ @Test
+ void testCustomSetter2() {
+ assertScript '''import static groovy.test.GroovyAssert.shouldFail
+ class C {
+ def set(Object name, Object value) {
+ throw AssertionError()
+ }
+ def set(String name, Object value) {
+ if (name != 'x') throw PropertyMissingException(name, C)
+ }
+ }
+
+ def c = new C()
+ c.x = 'xyz'
+ shouldFail {
+ c.y = null
+ }
+ '''
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy3871Bug.groovy
b/src/test/groovy/bugs/Groovy3871.groovy
similarity index 76%
rename from src/test/groovy/bugs/Groovy3871Bug.groovy
rename to src/test/groovy/bugs/Groovy3871.groovy
index 413edc23f9..473b1958f8 100644
--- a/src/test/groovy/bugs/Groovy3871Bug.groovy
+++ b/src/test/groovy/bugs/Groovy3871.groovy
@@ -18,40 +18,39 @@
*/
package bugs
-import groovy.test.GroovyTestCase
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
-/**
- * Fix for https://issues.apache.org/jira/browse/GROOVY-3871
- */
-class Groovy3871Bug extends GroovyTestCase {
+final class Groovy3871 {
- protected void setUp() {
- super.setUp()
- G3871Base.metaClass = null
- G3871Child.metaClass = null
+ @BeforeEach
+ void setUp() {
+ tearDown()
}
- protected void tearDown() {
+ @AfterEach
+ void tearDown() {
G3871Base.metaClass = null
G3871Child.metaClass = null
- super.tearDown();
}
+ @Test
void testPropertyMissingInheritanceIssue() {
// defining a propertyMissing on the base class
G3871Base.metaClass.propertyMissing = { String name -> name }
def baseInstance = new G3871Base()
- assert baseInstance.someProp == "someProp"
+ assert baseInstance.someProp == 'someProp'
// the child class inherits the propertyMissing
def childInstance = new G3871Child()
- assert childInstance.otherProp == "otherProp"
+ assert childInstance.otherProp == 'otherProp'
// when a propertyMissing is registered for the child
// it should be used over the inherited one
G3871Child.metaClass.propertyMissing = { String name -> name.reverse()
}
def otherChildInstance = new G3871Child()
- assert otherChildInstance.otherProp == "porPrehto"
+ assert otherChildInstance.otherProp == 'porPrehto'
}
}
@@ -60,4 +59,3 @@ class G3871Base { }
/** a dummy child class */
class G3871Child extends G3871Base { }
-
diff --git a/src/test/groovy/bugs/Groovy8065.groovy
b/src/test/groovy/bugs/Groovy8065.groovy
index e69cae3870..b44a978804 100644
--- a/src/test/groovy/bugs/Groovy8065.groovy
+++ b/src/test/groovy/bugs/Groovy8065.groovy
@@ -18,19 +18,21 @@
*/
package bugs
-import org.junit.Test
+import org.junit.jupiter.api.Test
import static groovy.test.GroovyAssert.assertScript
final class Groovy8065 {
+
@Test
void testMapWithCustomSetDuringAsTypeCast() {
assertScript '''
class MapWithSet extends LinkedHashMap {
- void set(String k, String v) {
- put(k.toLowerCase(), v.toUpperCase())
+ void set(String k, Object v) {
+ put(k.toLowerCase(), v?.toString()?.toUpperCase())
}
}
+
def m = [Foo: 'Bar'] as MapWithSet
assert m == [foo: 'BAR']
'''
diff --git a/src/test/groovy/groovy/IfPropertyTest.groovy
b/src/test/groovy/groovy/IfPropertyTest.groovy
index 3172f59ef7..262dc6e139 100644
--- a/src/test/groovy/groovy/IfPropertyTest.groovy
+++ b/src/test/groovy/groovy/IfPropertyTest.groovy
@@ -18,31 +18,30 @@
*/
package groovy
-import groovy.test.GroovyTestCase
+import org.junit.jupiter.api.Test
-class IfPropertyTest extends GroovyTestCase {
+final class IfPropertyTest {
def dummy
// This is because normal classes are not extensible, but scripts are
extensible by default.
Object get(String key) {
- return dummy
+ dummy
}
void set(Object key, Object value) {
dummy = value
}
+ @Test
void testIfNullPropertySet() {
if (cheese == null) {
cheese = 1
}
- if (cheese != 1) {
- fail("Didn't change cheese")
- }
assert cheese == 1
}
+ @Test
void testIfNullPropertySetRecheck() {
if (cheese == null) {
cheese = 1
@@ -52,5 +51,4 @@ class IfPropertyTest extends GroovyTestCase {
}
assert cheese == 2
}
-
}
