This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11775 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f27407ceed2e4ae004e026465b056f11d95c3184 Author: Eric Milles <[email protected]> AuthorDate: Fri Nov 14 13:48:37 2025 -0600 minor refactor --- src/main/java/groovy/lang/ExpandoMetaClass.java | 6 +- .../groovy/reflection/MixinInMetaClass.java | 36 ++- src/test/groovy/groovy/lang/MixinTest.groovy | 279 +++++++++++---------- 3 files changed, 171 insertions(+), 150 deletions(-) diff --git a/src/main/java/groovy/lang/ExpandoMetaClass.java b/src/main/java/groovy/lang/ExpandoMetaClass.java index 51215b27a0..d6e8d9ed0f 100644 --- a/src/main/java/groovy/lang/ExpandoMetaClass.java +++ b/src/main/java/groovy/lang/ExpandoMetaClass.java @@ -1242,7 +1242,7 @@ public class ExpandoMetaClass extends MetaClassImpl implements GroovyObject { * Either the first or second letter must be upperCase for that to be true. */ private static boolean isPropertyName(String name) { - return ((!name.isEmpty()) && Character.isUpperCase(name.charAt(0))) || ((name.length() > 1) && Character.isUpperCase(name.charAt(1))); + return (!name.isEmpty() && Character.isUpperCase(name.charAt(0))) || (name.length() > 1 && Character.isUpperCase(name.charAt(1))); } /** @@ -1479,9 +1479,9 @@ public class ExpandoMetaClass extends MetaClassImpl implements GroovyObject { private static class MixedInAccessor { private final Object object; - private final Set<MixinInMetaClass> mixinClasses; + private final Iterable<MixinInMetaClass> mixinClasses; - private MixedInAccessor(Object object, Set<MixinInMetaClass> mixinClasses) { + private MixedInAccessor(Object object, Iterable<MixinInMetaClass> mixinClasses) { this.object = object; this.mixinClasses = mixinClasses; } diff --git a/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java b/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java index ea2d88c2c9..649fdce11e 100644 --- a/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java +++ b/src/main/java/org/codehaus/groovy/reflection/MixinInMetaClass.java @@ -34,11 +34,14 @@ import org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaMethod; import org.codehaus.groovy.runtime.metaclass.MixinInstanceMetaProperty; import org.codehaus.groovy.runtime.metaclass.NewInstanceMetaMethod; +import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; +import static java.util.Arrays.stream; + public class MixinInMetaClass { private final ExpandoMetaClass emc; @@ -50,21 +53,12 @@ public class MixinInMetaClass { public MixinInMetaClass(final ExpandoMetaClass emc, final CachedClass mixinClass) { this.emc = emc; this.mixinClass = mixinClass; - this.mixinConstructor = findDefaultConstructor(mixinClass); + this.mixinConstructor = stream(mixinClass.getConstructors()).filter(it -> it.isPublic() && it.getParameterTypes().length == 0).findFirst() + .orElseThrow(() -> new GroovyRuntimeException("No default constructor for class " + mixinClass.getName() + "! Can't be mixed in.")); emc.addMixinClass(this); } - private static CachedConstructor findDefaultConstructor(final CachedClass mixinClass) { - for (CachedConstructor ctor : mixinClass.getConstructors()) { - if (ctor.isPublic() && ctor.getParameterTypes().length == 0) { - return ctor; - } - } - - throw new GroovyRuntimeException("No default constructor for class " + mixinClass.getName() + "! Can't be mixed in."); - } - public synchronized Object getMixinInstance(final Object object) { return mixinAssociations.computeIfAbsent(object, (final Object owner) -> { var mixinInstance = mixinConstructor.invoke(MetaClassHelper.EMPTY_ARRAY); @@ -104,21 +98,21 @@ public class MixinInMetaClass { } } - ExpandoMetaClass mc = (ExpandoMetaClass) self; + ExpandoMetaClass emc = (ExpandoMetaClass) self; List<MetaMethod> toRegister = new ArrayList<>(); for (Class<?> categoryClass : categoryClasses) { final CachedClass cachedCategoryClass = ReflectionCache.getCachedClass(categoryClass); - final MixinInMetaClass mixin = new MixinInMetaClass(mc, cachedCategoryClass); + final MixinInMetaClass mixin = new MixinInMetaClass(emc, cachedCategoryClass); final MetaClass metaClass = GroovySystem.getMetaClassRegistry().getMetaClass(categoryClass); for (MetaProperty mp : metaClass.getProperties()) { - if (self.getMetaProperty(mp.getName()) == null) { - mc.registerBeanProperty(mp.getName(), new MixinInstanceMetaProperty(mp, mixin)); + if (emc.getMetaProperty(mp.getName()) == null) { + emc.registerBeanProperty(mp.getName(), new MixinInstanceMetaProperty(mp, mixin)); } } for (MetaProperty mp : cachedCategoryClass.getFields()) { - if (self.getMetaProperty(mp.getName()) == null) { - mc.registerBeanProperty(mp.getName(), new MixinInstanceMetaProperty(mp, mixin)); + if (emc.getMetaProperty(mp.getName()) == null) { + emc.registerBeanProperty(mp.getName(), new MixinInstanceMetaProperty(mp, mixin)); } } for (MetaMethod method : metaClass.getMethods()) { @@ -135,7 +129,7 @@ public class MixinInMetaClass { if (method instanceof CachedMethod cachedMethod) staticMethod(self, toRegister, cachedMethod); } else if (method.getDeclaringClass().getTheClass() != Object.class || "toString".equals(method.getName())) { - //if (self.pickMethod(method.getName(), method.getNativeParameterTypes()) == null) { + //if (emc.pickMethod(method.getName(), method.getNativeParameterTypes()) == null) { toRegister.add(new MixinInstanceMetaMethod(method, mixin)); //} } @@ -144,14 +138,14 @@ public class MixinInMetaClass { for (MetaMethod mm : toRegister) { if (mm.getDeclaringClass().isAssignableFrom(selfClass)) { - mc.registerInstanceMethod(mm); + emc.registerInstanceMethod(mm); } else { - mc.registerSubclassInstanceMethod(mm); + emc.registerSubclassInstanceMethod(mm); } } } - private static boolean hasAnnotation(final CachedMethod method, final Class<Internal> annotationClass) { + private static boolean hasAnnotation(final CachedMethod method, final Class<? extends Annotation> annotationClass) { return method.getAnnotation(annotationClass) != null; } diff --git a/src/test/groovy/groovy/lang/MixinTest.groovy b/src/test/groovy/groovy/lang/MixinTest.groovy index 3694da2252..fa24f5ce9a 100644 --- a/src/test/groovy/groovy/lang/MixinTest.groovy +++ b/src/test/groovy/groovy/lang/MixinTest.groovy @@ -18,24 +18,33 @@ */ package groovy.lang -import groovy.test.GroovyTestCase +import groovy.transform.CompileStatic +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test import java.util.concurrent.locks.ReentrantLock -import org.codehaus.groovy.reflection.ClassInfo -class MixinTest extends GroovyTestCase { +import static groovy.test.GroovyAssert.assertScript +import static org.junit.jupiter.api.Assertions.assertEquals +import static org.junit.jupiter.api.Assertions.assertTrue - @groovy.transform.CompileStatic - protected void setUp() { - ClassInfo.clearModifiedExpandos() +final class MixinTest { + + @BeforeEach + @CompileStatic + void setUp() { + org.codehaus.groovy.reflection.ClassInfo.clearModifiedExpandos() } - protected void tearDown() { + @AfterEach + void tearDown() { ArrayList.metaClass = null - List.metaClass = null + List.metaClass = null ObjToTest.metaClass = null } + @Test void testOneClass() { List.mixin ListExt ArrayList.mixin ArrayListExt @@ -45,6 +54,7 @@ class MixinTest extends GroovyTestCase { assertEquals 1, [0, 1].swap().unswap()[1] } + @Test void testWithList() { ArrayList.mixin ArrayListExt, ListExt assertEquals 1, [0, 1].swap()[0] @@ -53,6 +63,7 @@ class MixinTest extends GroovyTestCase { assertEquals 1, [0, 1].swap().unswap()[1] } + @Test void testCombined() { ArrayList.mixin Combined assertEquals 1, [0, 1].swap()[0] @@ -61,6 +72,7 @@ class MixinTest extends GroovyTestCase { assertEquals 1, [0, 1].swap().unswap()[1] } + @Test void testWithEmc() { ArrayList.metaClass.unswap = { [delegate[1], delegate[0]] @@ -72,25 +84,28 @@ class MixinTest extends GroovyTestCase { assertEquals 1, [0, 1].swap().unswap()[1] } + @Test void testGroovyObject() { def obj = new ObjToTest() - assertEquals "original", obj.value + assertEquals 'original', obj.value obj.metaClass.mixin ObjToTestCategory - assertEquals "changed by category", obj.value - assertEquals "original", new ObjToTest().value + assertEquals 'changed by category', obj.value + assertEquals 'original', new ObjToTest().value } + @Test void testGroovyObjectWithEmc() { ObjToTest.metaClass.getValue = {-> - "emc changed" + 'emc changed' } ObjToTest obj = new ObjToTest() - assertEquals "emc changed", obj.getValue() + assertEquals 'emc changed', obj.getValue() obj.metaClass.mixin ObjToTestCategory - assertEquals "changed by category", obj.value - assertEquals "emc changed", new ObjToTest().value + assertEquals 'changed by category', obj.value + assertEquals 'emc changed', new ObjToTest().value } + @Test void testFlatten() { Object.metaClass.mixin DeepFlattenToCategory assertEquals([8, 9, 3, 2, 1, 4], [[8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo() as List) @@ -110,11 +125,11 @@ class MixinTest extends GroovyTestCase { return set } Object.metaClass.flattenTo(ArrayList) {Set set -> - set << "oops" - return Collection.metaClass.invokeMethod(delegate, "flattenTo", set) + set << 'oops' + return Collection.metaClass.invokeMethod(delegate, 'flattenTo', set) } ArrayList.metaClass = null - assertEquals(["oops", -2, -3, 8, 9, 3, 2, 1, 4], [x, [8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo() as List) + assertEquals(['oops', -2, -3, 8, 9, 3, 2, 1, 4], [x, [8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo() as List) ArrayList.metaClass = null Object.metaClass { @@ -125,15 +140,15 @@ class MixinTest extends GroovyTestCase { } flattenTo(ArrayList) {Set set -> - set << "oopsssss" - return Collection.metaClass.invokeMethod(delegate, "flattenTo", set) + set << 'oopsssss' + return Collection.metaClass.invokeMethod(delegate, 'flattenTo', set) } asList {-> delegate as List } } - assertEquals(["oopsssss", -2, -3, 8, 9, 3, 2, 1, 4], [x, [8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo().asList()) + assertEquals(['oopsssss', -2, -3, 8, 9, 3, 2, 1, 4], [x, [8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo().asList()) ArrayList.metaClass = null Object.metaClass { @@ -145,8 +160,8 @@ class MixinTest extends GroovyTestCase { } flattenTo {Set set -> - set << "ssoops" - return Collection.metaClass.invokeMethod(delegate, "flattenTo", set) + set << 'ssoops' + return Collection.metaClass.invokeMethod(delegate, 'flattenTo', set) } } @@ -154,14 +169,15 @@ class MixinTest extends GroovyTestCase { delegate as List } } - assertEquals(["ssoops", -2, -3, 8, 9, 3, 2, 1, 4], [x, [8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo().asList()) + assertEquals(['ssoops', -2, -3, 8, 9, 3, 2, 1, 4], [x, [8, 9] as Object[], [3, 2, [2: 1, 3: 4]], [2, 3]].flattenTo().asList()) Object.metaClass = null } + @Test void testMixingLockable() { Object.metaClass.mixin ReentrantLock - def name = "abcdef" + def name = 'abcdef' name.lock() try { assertTrue name.isLocked() @@ -172,6 +188,7 @@ class MixinTest extends GroovyTestCase { Object.metaClass = null } + @Test void testConcurrentQueue() { ReentrantLock.metaClass { withLock {Closure operation -> @@ -196,6 +213,7 @@ class MixinTest extends GroovyTestCase { ReentrantLock.metaClass = null } + @Test void testDynamicConcurrentQueue() { ReentrantLock.metaClass { withLock {Closure operation -> @@ -264,6 +282,7 @@ class MixinTest extends GroovyTestCase { ReentrantLock.metaClass = null } + @Test void testNoDupCollection() { def list = new Object() list.metaClass { @@ -287,6 +306,7 @@ class MixinTest extends GroovyTestCase { assertEquals 3, list[2] } + @Test void testList() { def u = [] u.metaClass { @@ -311,8 +331,8 @@ class MixinTest extends GroovyTestCase { assertEquals 2, ((Set) u).size() } + @Test void testWPM() { - new WPM_B().foo() WPM_C.metaClass { mixin WPM_B } @@ -322,6 +342,7 @@ class MixinTest extends GroovyTestCase { c.foobar() } + @Test void testStackOverflow() { Overflow_B.metaClass { mixin Overflow_A @@ -338,15 +359,17 @@ class MixinTest extends GroovyTestCase { b.foo() } + // GROOVY-3474 + @Test void testStackOverflowErrorWithMixinsAndClosure() { - assertScript """ + assertScript ''' class Groovy3474A { int counter = 1 protected final foo() { bar { counter } } - private final String bar(Closure code) { - return "Bar " + code() + private final String bar(Closure code) { + return "Bar " + code() } } @@ -357,11 +380,11 @@ class MixinTest extends GroovyTestCase { def c = new Groovy3474C() assert c.foo() == 'Bar 1' println "testStackOverflowErrorWithMixinsAndClosure() Done" - """ + ''' } void testMixinWithVarargs() { - assertScript """ + assertScript ''' class Dsl { static novarargs(java.util.List s) { "novarargs" + s.size() } static plainVarargs(Object... s) { "plainVarargs" + s.size() } @@ -371,143 +394,147 @@ class MixinTest extends GroovyTestCase { assert novarargs(["a", "b"]) == "novarargs2" assert plainVarargs("a", "b", 35) == "plainVarargs3" assert mixedVarargs(3, "a", "b", "c", "d") == "mixedVarargs4" - """ + ''' } -} -class ArrayListExt { - static def swap(ArrayList self) { - [self[1], self[0]] - } -} + //-------------------------------------------------------------------------- -class ListExt { - static def unswap(List self) { - [self[1], self[0]] - } -} - -class Combined { - static def swap(ArrayList self) { - [self[1], self[0]] + static class ArrayListExt { + static def swap(ArrayList self) { + [self[1], self[0]] + } } - static def unswap(List self) { - [self[1], self[0]] + static class ListExt { + static def unswap(List self) { + [self[1], self[0]] + } } -} -class ObjToTest { - def getValue() { - "original" - } -} + static class Combined { + static def swap(ArrayList self) { + [self[1], self[0]] + } -class ObjToTestCategory { - static getValue(ObjToTest self) { - "changed by category" + static def unswap(List self) { + [self[1], self[0]] + } } -} -class DeepFlattenToCategory { - static Set flattenTo(element) { - LinkedHashSet set = new LinkedHashSet() - element.flattenTo(set) - return set + static class ObjToTest { + def getValue() { + 'original' + } } - // Object - put to result set - static void flattenTo(element, Set addTo) { - addTo << element + static class ObjToTestCategory { + static getValue(ObjToTest self) { + 'changed by category' + } } - // Collection - flatten each element - static void flattenTo(Collection elements, Set addTo) { - elements.each {element -> - element.flattenTo(addTo) + static class DeepFlattenToCategory { + static Set flattenTo(element) { + LinkedHashSet set = new LinkedHashSet() + element.flattenTo(set) + return set } - } - // Map - flatten each value - static void flattenTo(Map elements, Set addTo) { - elements.values().flattenTo(addTo) - } + // Object - put to result set + static void flattenTo(element, Set addTo) { + addTo << element + } - // Array - flatten each element - static void flattenTo(Object[] elements, Set addTo) { - elements.each {element -> - element.flattenTo(addTo) + // Collection - flatten each element + static void flattenTo(Collection elements, Set addTo) { + elements.each {element -> + element.flattenTo(addTo) + } } - } -} -class NoFlattenArrayListCategory { - // Object - put to result set + // Map - flatten each value + static void flattenTo(Map elements, Set addTo) { + elements.values().flattenTo(addTo) + } - static void flattenTo(ArrayList element, Set addTo) { - addTo << element + // Array - flatten each element + static void flattenTo(Object[] elements, Set addTo) { + elements.each {element -> + element.flattenTo(addTo) + } + } } -} -class ConcurrentQueue { - static { - ConcurrentQueue.metaClass.mixin LinkedList, ReentrantLock - } + static class NoFlattenArrayListCategory { + // Object - put to result set - def get() { - withLock { - removeFirst() + static void flattenTo(ArrayList element, Set addTo) { + addTo << element } } - void put(def obj) { - withLock { - addLast(obj) + static class ConcurrentQueue { + static { + ConcurrentQueue.metaClass.mixin LinkedList, ReentrantLock } - } -} -class NoDuplicateCollection { - void put(def obj) { - def clone = find { - it == obj + def get() { + withLock { + removeFirst() + } } - if (!clone) - add obj + void put(def obj) { + withLock { + addLast(obj) + } + } } -} -class WPM_A { + static class NoDuplicateCollection { + void put(def obj) { + def clone = find { + it == obj + } - final foo() { - bar() + if (!clone) + add obj + } } - private final String bar() { return "Bar" } -} + static class WPM_A { + final foo() { + bar() + } -class WPM_B extends WPM_A { - def foobar() { - super.foo() + private final String bar() { + return 'Bar' + } } -} -class WPM_C {} + static class WPM_B extends WPM_A { + def foobar() { + super.foo() + } + } -class Overflow_A { - public void foo() { - println 'Original foo ' + receive('') + static class WPM_C { } - protected Object receive() { - return "Message" + static class Overflow_A { + public void foo() { + println 'Original foo ' + receive('') + } + + protected Object receive() { + return 'Message' + } + + protected Object receive(param) { + receive() + param + } } - protected Object receive(Object param) { - receive() + param + static class Overflow_B { } } - -class Overflow_B {} -
