From d259ae16392c4adec513cc3cdd38ae52d9504206 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 18 Oct 2023 09:39:21 -0500 Subject: [PATCH] GROOVY-11196: `tap`/`with`/`identity` propagates `null` not `NullObject` 4_0_X backport --- .../groovy/runtime/DefaultGroovyMethods.java | 45 ++- .../codehaus/groovy/runtime/NullObject.java | 161 +++++----- .../groovy/runtime/NullObjectTest.groovy | 288 +++++++++++++----- 3 files changed, 314 insertions(+), 180 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java index 397f9a673fd..67ae614fdd5 100644 --- a/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java +++ b/src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java @@ -259,14 +259,14 @@ public static boolean is(Object self, Object other) { * @see #with(Object, Closure) * @since 1.0 */ - public static T identity( + public static T identity( @DelegatesTo.Target("self") U self, @DelegatesTo(value=DelegatesTo.Target.class, target="self", strategy=Closure.DELEGATE_FIRST) @ClosureParams(FirstParam.class) - Closure closure) { - return DefaultGroovyMethods.with(self, closure); + Closure closure) { + return with(self, closure); } /** @@ -304,7 +304,7 @@ public static T identity( * @since 1.5.0 */ @SuppressWarnings("unchecked") - public static T with( + public static T with( @DelegatesTo.Target("self") U self, @DelegatesTo(value=DelegatesTo.Target.class, target="self", @@ -352,20 +352,22 @@ public static T with( * @see #tap(Object, Closure) * @since 2.5.0 */ - public static T with( + public static T with( @DelegatesTo.Target("self") U self, boolean returning, @DelegatesTo(value=DelegatesTo.Target.class, target="self", strategy=Closure.DELEGATE_FIRST) @ClosureParams(FirstParam.class) - Closure closure) { - @SuppressWarnings("unchecked") - final Closure clonedClosure = (Closure) closure.clone(); - clonedClosure.setResolveStrategy(Closure.DELEGATE_FIRST); - clonedClosure.setDelegate(self); - V result = clonedClosure.call(self); - return returning ? self : result; + Closure closure) { // TODO: T -> V + if (self == NullObject.getNullObject()) { + self = null; // GROOVY-4526, et al. + } + @SuppressWarnings("unchecked") Closure mutableClosure = (Closure) closure.clone(); + mutableClosure.setResolveStrategy(Closure.DELEGATE_FIRST); + mutableClosure.setDelegate(self); + V rv = mutableClosure.call(self); + return returning ? self : rv; } /** @@ -398,7 +400,7 @@ public static T with( * @since 2.5.0 */ @SuppressWarnings("unchecked") - public static U tap( + public static U tap( @DelegatesTo.Target("self") U self, @DelegatesTo(value=DelegatesTo.Target.class, target="self", @@ -18252,16 +18254,13 @@ public static MetaClass getMetaClass(GroovyObject obj) { * @since 1.6.0 */ public static void setMetaClass(Class self, MetaClass metaClass) { - final MetaClassRegistry metaClassRegistry = GroovySystem.getMetaClassRegistry(); - if (metaClass == null) - metaClassRegistry.removeMetaClass(self); - else { - if (metaClass instanceof HandleMetaClass) { - metaClassRegistry.setMetaClass(self, ((HandleMetaClass)metaClass).getAdaptee()); - } else { - metaClassRegistry.setMetaClass(self, metaClass); - } - if (self==NullObject.class) { + if (metaClass == null) { + GroovySystem.getMetaClassRegistry().removeMetaClass(self); + } else { + MetaClass mc = metaClass instanceof HandleMetaClass + ? ((HandleMetaClass)metaClass).getAdaptee() : metaClass; + GroovySystem.getMetaClassRegistry().setMetaClass(self,mc); + if (NullObject.class.equals(self)) { // GROOVY-3803 NullObject.getNullObject().setMetaClass(metaClass); } } diff --git a/src/main/java/org/codehaus/groovy/runtime/NullObject.java b/src/main/java/org/codehaus/groovy/runtime/NullObject.java index 30343852b12..3391412bdf7 100644 --- a/src/main/java/org/codehaus/groovy/runtime/NullObject.java +++ b/src/main/java/org/codehaus/groovy/runtime/NullObject.java @@ -25,29 +25,31 @@ import java.util.Iterator; public class NullObject extends GroovyObjectSupport { + private static final NullObject INSTANCE = new NullObject(); /** - * private constructor + * Returns the NullObject reference. + * + * @return the null object */ + public static NullObject getNullObject() { + return INSTANCE; + } + private NullObject() { if (INSTANCE != null) { throw new RuntimeException("Can't instantiate NullObject. Use NullObject.getNullObject()"); } } - /** - * get the NullObject reference - * - * @return the null object - */ - public static NullObject getNullObject() { - return INSTANCE; - } + //-------------------------------------------------------------------------- /** - * Since this is implemented as a singleton, we should avoid the - * use of the clone method + * Since this is implemented as a singleton, avoid the use of the clone method. + * + * @return never + * @throws NullPointerException */ @Override public Object clone() { @@ -55,132 +57,129 @@ public Object clone() { } /** - * Tries to get a property on null, which will always fail + * null is only equal to null. * - * @param property - the property to get - * @return a NPE + * @param o the reference object with which to compare + * @return true if this object is the same as the to argument */ @Override - public Object getProperty(String property) { - throw new NullPointerException("Cannot get property '" + property + "' on null object"); + public boolean equals(final Object o) { + return o == null; } /** - * Allows the closure to be called for NullObject - * - * @param closure the closure to call on the object - * @return result of calling the closure + * @return never + * @throws NullPointerException */ - public T with( Closure closure ) { - return DefaultGroovyMethods.with( null, closure ) ; + @Override + public int hashCode() { + throw new NullPointerException("Cannot invoke method hashCode() on null object"); + } + + @Override + public String toString() { + return "null"; } /** - * Tries to set a property on null, which will always fail + * Tries to get a property on null, which fails. * - * @param property - the property to set - * @param newValue - the new value of the property + * @return never + * @throws NullPointerException */ @Override - public void setProperty(String property, Object newValue) { - throw new NullPointerException("Cannot set property '" + property + "' on null object"); + public Object getProperty(final String name) { + throw new NullPointerException("Cannot get property '" + name + "' on null object"); } /** - * Tries to invoke a method on null, which will always fail + * Tries to set a property on null, which fails. * - * @param name the name of the method to invoke - * @param args - arguments to the method - * @return a NPE + * @throws NullPointerException */ @Override - public Object invokeMethod(String name, Object args) { - throw new NullPointerException("Cannot invoke method " + name + "() on null object"); + public void setProperty(final String name, final Object value) { + throw new NullPointerException("Cannot set property '" + name + "' on null object"); } /** - * null is only equal to null + * Tries to invoke a method on null, which falis. * - * @param to - the reference object with which to compare - * @return - true if this object is the same as the to argument + * @return never + * @throws NullPointerException */ @Override - public boolean equals(Object to) { - return to == null; + public Object invokeMethod(final String name, final Object arguments) { + throw new NullPointerException("Cannot invoke method " + name + "() on null object"); } + //-------------------------------------------------------------------------- + /** - * iterator() method to be able to iterate on null. - * Note: this part is from Invoker + * A null object coerces to false. * - * @return an iterator for an empty list + * @return false */ - public Iterator iterator() { - return Collections.EMPTY_LIST.iterator(); + public boolean asBoolean() { + return false; } /** - * Allows to add a String to null. - * The result is concatenated String of the result of calling - * toString() on this object and the String in the parameter. + * Type conversion method for null. * - * @param s - the String to concatenate - * @return the concatenated string + * @return null */ - public Object plus(String s) { - return getMetaClass().invokeMethod(this, "toString", new Object[]{}) + s; + public Object asType(final Class c) { + return null; } /** - * Fallback for null+null. - * The result is always a NPE. The plus(String) version will catch - * the case of adding a non-null String to null. + * Tests for equal references. * - * @param o - the Object - * @return nothing + * @return true if object is null */ - public Object plus(Object o) { - throw new NullPointerException("Cannot execute null+" + o); + public boolean is(final Object o) { + return equals(o); } /** - * The method "is" is used to test for equal references. - * This method will return true only if the given parameter - * is null + * Provides ability to iterate on null. * - * @param other - the object to test - * @return true if other is null + * @return an empty iterator */ - public boolean is(Object other) { - return other == null; + public Iterator iterator() { + return Collections.emptyIterator(); } /** - * Type conversion method for null. + * Fallback for {@code null+null}. The {@link plus(String)} variant catches + * the case of adding a non-null String to null. * - * @param c - the class to convert to - * @return always null + * @return never + * @throws NullPointerException */ - public Object asType(Class c) { - return null; + public Object plus(final Object o) { + throw new NullPointerException("Cannot execute null+" + o); } /** - * A null object always coerces to false. + * Allows to add a String to null. + * The result is concatenated String of the result of calling + * toString() on this object and the String in the parameter. * - * @return false + * @return the concatenated string */ - public boolean asBoolean() { - return false; - } - - @Override - public String toString() { - return "null"; + public Object plus(final String s) { + return getMetaClass().invokeMethod(this, "toString", new Object[0]) + s; } - @Override - public int hashCode() { - throw new NullPointerException("Cannot invoke method hashCode() on null object"); + /** + * Allows the closure to be called for NullObject. + * + * @param closure the closure to call on the object + * @return result of calling the closure + */ + public T with(final Closure closure) { + return DefaultGroovyMethods.with(null, closure); // GROOVY-4526, GROOVY-4985 } } diff --git a/src/test/org/codehaus/groovy/runtime/NullObjectTest.groovy b/src/test/org/codehaus/groovy/runtime/NullObjectTest.groovy index bdee23985dc..61681eb4684 100644 --- a/src/test/org/codehaus/groovy/runtime/NullObjectTest.groovy +++ b/src/test/org/codehaus/groovy/runtime/NullObjectTest.groovy @@ -18,129 +18,265 @@ */ package org.codehaus.groovy.runtime -import groovy.test.GroovyTestCase +import groovy.test.NotYetImplemented +import groovy.transform.CompileStatic +import org.junit.Test -class NullObjectTest extends GroovyTestCase { - void testCallingMethod() { - def foo = null - shouldFail(NullPointerException) { - println foo.bar +import static groovy.test.GroovyAssert.shouldFail + +final class NullObjectTest { + + @Test + void testInit1() { + shouldFail(RuntimeException) { + new NullObject() + } + } + + @Test + void testInit2() { + shouldFail(RuntimeException) { + NullObject.newInstance() } } - void testtoStringMethod() { - def foo = null - assert foo.toString() == "null" + @Test + void testInit3() { + shouldFail(NoSuchMethodException) { + NullObject.getConstructor() + } } - void testEquals() { - def a = [1] - assert a[3] == a[4] - assert a[2].equals(a[4]) + @Test + void testClone() { + def nil = null + shouldFail(NullPointerException) { + nil.clone() + } } - void testAsExpression() { - assert null as String == null + @Test + void testEquals() { + def nil = null + assert nil == nil + assert nil == null + assert null == nil + assert null == null + assert !nil.equals(0) + assert !nil.equals('') + assert nil.equals(null) } - void testIs(){ - assert null.is(null) + @Test + void testHashCode() { + def nil = null + shouldFail(NullPointerException) { + nil.hashCode() + } } - void testCategory() { - def n = null + @Test + void testToString() { + def nil = null + assert nil.toString() == 'null' + } - assert "a $n b" == "a null b" - assert n.toString() == "null" - assert n + " is a null value" == "null is a null value" - assert "this is a null value " + null == "this is a null value null" + // GROOVY-4487 + @NotYetImplemented @Test + void testGetClass() { + def nil = null + assert nil.class === nil.getClass() + assert nil.class.simpleName == 'NullObject' + assert nil.getClass().getSimpleName() == 'NullObject' + } - use (MyCategory) { - assert "a $n b" == "a b" - assert n.toString() == "" - assert n + " is a null value" == " is a null value" - assert "this is a null value " + null == "this is a null value " - } + @Test + void testGetProperty() { + def nil = null + shouldFail(NullPointerException) { + nil.foo } + } - void testClone() { - def foo = null + @Test + void testSetProperty() { + def nil = null shouldFail(NullPointerException) { - foo.clone() + nil.foo = 'bar' } } - void testInstantiation1() { - shouldFail(RuntimeException) { - new NullObject() + @Test + void testInvokeMethod() { + def nil = null + shouldFail(NullPointerException) { + nil.foo() } } - void testInstantiation2() { - shouldFail(RuntimeException) { - NullObject.newInstance() + // + + @Test + void testAsBool() { + def nil = null + assert !nil + assert !nil.asBoolean() + } + + @Test + void testAsType1() { + def nil = null + assert (nil as Number) === null + assert (nil as String) === null + } + + @Test + void testAsType2() { + def nil = null + assert nil.asType(String) === null + } + + // GROOVY-7861 + @NotYetImplemented @CompileStatic @Test + void testAsType3() { + def nil = null + assert nil.asType(String) === null + } + + @Test + void testIs() { + def nil = null + assert nil.is(null) + assert !nil.is(' ') + } + + @Test + void testIterator() { + def nil = null + assert !nil.iterator().hasNext() + } + + @Test + void testPlus1() { + def err = shouldFail(NullPointerException) { + null+null } + assert err.message == 'Cannot execute null+null' } - void testInstantiation3() { - shouldFail(NoSuchMethodException) { - NullObject.getConstructor() + @Test + void testPlus2() { + def err = shouldFail(NullPointerException) { + null+1 } + assert err.message == 'Cannot execute null+1' } - void testEMC() { - def oldMC = null.getMetaClass() - NullObject.metaClass.hello = { -> "Greeting from null" } - assert null.hello() == "Greeting from null" - null.setMetaClass(oldMC) + @Test + void testPlus3() { + assert (null + '!') == 'null!' } - void testNullPlusNull() { - String message = shouldFail(NullPointerException) { - null+null + // GROOVY-11196 + @Test + void testTap() { + def nil = null + nil = nil.tap { + assert it === null + assert it !instanceof NullObject } - assert message == "Cannot execute null+null" + assert nil === null + assert nil !instanceof NullObject } - void testNullPlusNumber() { - String message = shouldFail(NullPointerException) { - null+1 - } - assert message == "Cannot execute null+1" + // GROOVY-4526, GROOVY-4985 + @Test + void testWith() { + def nil = null + def ret = nil.with { it -> + assert it === null + assert it !instanceof NullObject + } + assert ret === null + assert ret !instanceof NullObject + + ret = nil.with { + assert it === null + assert it !instanceof NullObject + return 2 + } + assert ret == 2 } - void testNullWith() { - def map = [ a:1, b:2 ] - map.c.with { c -> - assert c == null + //-------------------------------------------------------------------------- + + static class MyCategory { + public static String toString(NullObject obj) { + return '' + } + + static boolean isNull(value) { + value == null + } + + static boolean isNull2(value) { + null == value } - def a = null.with { - assert !(it instanceof NullObject) - 2 + } + + @Test + void testCategory1() { + def nil = null + + assert "a $nil b" == 'a null b' + assert nil.toString() == 'null' + assert nil + ' is a null value' == 'null is a null value' + assert 'this is a null value ' + null == 'this is a null value null' + + use (MyCategory) { + assert "a $nil b" == 'a b' + assert nil.toString() == '' + assert nil + ' is a null value' == ' is a null value' + assert 'this is a null value ' + null == 'this is a null value ' } - assert a == 2 } - void testEqualsInCategory() { - def val = null + @Test + void testCategory2() { + def nil = null use (MyCategory) { - assert val.isNull() - assert val.isNull2() + assert nil.isNull() + assert nil.isNull2() } } -} -class MyCategory { - public static String toString(NullObject obj) { - return "" + // GROOVY-3803 + @Test + void testMetaClass1() { + def oldMC = NullObject.getMetaClass() + try { + NullObject.metaClass.hello = { -> 'Greeting from null' } + assert null.hello() == 'Greeting from null' + } finally { + NullObject.setMetaClass(oldMC) + } } - static boolean isNull(value) { - value == null + // GROOVY-8875 + @Test + void testMetaClass2() { + def oldMC = NullObject.getMetaClass() + try { + NullObject.metaClass.toString = { -> 'bar' } + assert ('foo' + null) == 'foobar' + } finally { + NullObject.setMetaClass(oldMC) + } } - static boolean isNull2(value) { - null == value + @NotYetImplemented @Test + void testMetaClass3() { + assert null.metaClass == null.getMetaClass() + assert null.metaClass.adaptee === null.getMetaClass().getAdaptee() } } -