null and .with{}

Had a look into this, and I can get the examples to work given the following changes:

• In org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation.compareEqual( left, right ), add a test for the left object being null and the right being NullObject and vice-versa.

diff --git a/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java b/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
index 5ac61d1..9df1187 100644
--- a/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
+++ b/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
@@ -587,6 +587,9 @@ public class DefaultTypeTransformation {
 
     public static boolean compareEqual(Object left, Object right) {
         if (left == right) return true;
+        if( ( left  instanceof NullObject && right == null ) ||
+            ( right instanceof NullObject && left  == null ) )
+            return true ;
         if (left == null || right == null) return false;
         if (left instanceof Comparable) {
             return compareToWithEqualityCheck(left, right, true) == 0;

• In org.codehaus.groovy.runtime.NullObject add a check for instanceof NullObject to equals and is

  diff --git a/src/main/org/codehaus/groovy/runtime/NullObject.java b/src/main/org/codehaus/groovy/runtime/NullObject.java
  index ef5106c..a5eca7a 100644
  --- a/src/main/org/codehaus/groovy/runtime/NullObject.java
  +++ b/src/main/org/codehaus/groovy/runtime/NullObject.java
  @@ -84,7 +84,7 @@ public class NullObject extends GroovyObjectSupport {
        * @return - true if this object is the same as the to argument
        */
       public boolean equals(Object to) {
  -        return to == null;
  +        return to == null || to instanceof NullObject ;
       }
   
       /**
  @@ -118,7 +118,7 @@ public class NullObject extends GroovyObjectSupport {
        * @return true if other is null
        */
       public boolean is(Object other) {
  -        return other == null;
  +        return other == null || other instanceof NullObject ;
       }
   
       /**

And I also added the three tests described in this question and first comment to the test class groovy.lang.WithMethodTest

I have attached the patch...

Hope it's ok...

I realise changing anything this low level could cause issues and may be a breaking change...

I am sure the patch will fix the issues pointed out here, but nothing else. Somehow NullObject is leaking when it should not, and we should try prevnting the leak. equals should never be called with NullObject actually. In fact I would think here "with" is at fault instead. But probably there is no complete solution for this issue atm

I agree that there is probably some underlying problem causing this leak...

I will have a look into the code for with and see if I can find where the null is leaking out, rather than this fix which does feel like papering over a crack

Oddly, it's actually the null inside the closure which isn't getting converted to a NullObject for some reason, as this passes:

null.with { 
  assert it == org.codehaus.groovy.runtime.NullObject.nullObject
}

edit

Seems to be the case inside any closure?

def f = {
  println null == org.codehaus.groovy.runtime.NullObject.nullObject
}
f()

prints false... But I would expect true.

I think I probably have completely the wrong end of the stick with this bug...