Improve NullNode.equals()

[dsbecker]

Add support for logical equivalence to NullNode (like other node types have) and also equivalence with java null.

[cowtowncoder]

First of all, no, I do not think null should be considered equal to NullNode.
Second: what is the problem being solved with equal()s check -- NullNode is a singleton, and identity check should be fine?

[dsbecker]

There were two issues I was trying to solve:

1. Yes, it's a singleton when serializing and deserializing, etc. But users can create their own object trees, and there's nothing to stop them from doing new NullNode(). It seems like new NullNode().equals(new NullNode()) should be true but it isn't.
2. I have a POJO with a mix of field types, one of which is typed as JsonNode because my input json is a variant type. In code the field gets defaulted to Java null, but when it comes in through deserializing it's NullNode. This breaks my object equals checks. There are multiple ways to address this in my code, which is what I'm doing for now. But, while they clearly don't have strict reference equality ( NullNode.getInstance() != null ), they do seem (to me) to be logically equivalent in the same way that two different TextNode, NumberNode or BooleanNodes can be.

[dsbecker]

RE: #2, On the other hand you can't say myTextNode.equals("foobar") (though that could be handy too) so I could see it both ways, but still feel like null is a special case.

[cowtowncoder]

Ok. I have bit mixed feelings on users being able to create NullNodes, but since that is possible (as per my changing constructor), yes, I concur it should do that.

But I don't think null should be allowed as equal.
Instead, method

    public boolean equals(Comparator<JsonNode> comparator, JsonNode other) { ... }

should be used for comparisons, and that will allow for custom handling, and was explicitly added to allow use cases like coercions between numeric types and so on.
I think this would be more natural place for null coercion as well.

I will go ahead and change NullNode equality like suggested.

[dsbecker]

When using lists and maps, the contains() implementation uses the object.equals() call. That's the specific case it would have been handy for. But, since there are other solutions (using @JsonInclude(NON_NULL) for example) that work, I defer to your judgement since you're much closer to the project. Thanks for listening!

[dsbecker]

Close at your leisure. Or would you prefer I update the PR?

[cowtowncoder]

Ok, so actually creating custom NullNodes would have been difficult before:

1. NullNode has (accidentally) left as final (I changed that now for 2.10), but no sub-classing possible before
2. Constructor is protected

It is possible to create an instance using reflection (or doing split package) but... not sure anyone has actually used it. Also still don't see why someone would do it; perhaps it is necessary by some mock framework or...

[cowtowncoder]

@dsbecker I think I can do that, small enough change. Thank you for asking!

[cowtowncoder]

Ah, equals() used by JDK methods. Fair point.

I hope that ability to create replacements, now, will allow solving that issue.

[dsbecker]

Ah, I overlooked the combination of final with protected constructor. My bad.

[cowtowncoder]

@dsbecker I did too, it looks like. I don't think I meant to leave final in there. :)