Invalid refactor from isEqualTo(@Nullable Map) to containsExactlyInAnyOrderElementsOf

[ash211]

What happened?

I maintain a repo that has a helper method like this:

    private static void assertAttributesEqual(
            @Nullable Map<String, Map<String, String>> expected, @Nullable Object actual) {
        Map<String, Map<String, String>> actualAttrs = (Map<String, Map<String, String>>) actual;
        assertThat(actualAttrs).isEqualTo(expected);
    }

When assertj-automation comes around it attempts to reword that last statement to this instead:

        assertThat(actualAttrs).containsExactlyInAnyOrderEntriesOf(expected);

However this fails at test runtime with this exception:

java.lang.NullPointerException: Cannot invoke "java.util.Map.entrySet()" because "map" is null
	at org.assertj.core.api.AbstractMapAssert.toEntries(AbstractMapAssert.java:695)
	at org.assertj.core.api.AbstractMapAssert.containsExactlyInAnyOrderEntriesOf(AbstractMapAssert.java:690)
	at com.mypackage.MyClass.assertAttributesEqual(MyClass.java:38)

The problem is that if the expected parameter is null (as is declared via the @Nullable annotation) then the

What did you want to happen?

For expected values that have been annotated @Nullable, this refactor should be skipped. Or changed to produce a valid refactor that still passes.

See https://github.com/palantir/assertj-automation/pull/631/files for desired behavior (no change)

[carterkozak]

I disagree -- when values are expected to be null, we should assertThat(value).isNull(); assertj-automation attempts to make asserts more precise, which is difficult when the original assertion is imprecise, sometimes it will cause tests to fail until they're updated with greater precision.

[ash211]

Fair enough. I ended up rewriting the above assertThat to this:

        // if expected is null then check for null directly. containsExactlyInAnyOrderEntriesOf() fails when given a
        // null expected value, and assert-automation refactors continuously try to use that method, so we need to
        // branch here to avoid a null pointer exception.
        // See https://github.com/palantir/assertj-automation/issues/630
        if (expected == null) {
            assertThat(actual).isNull();
        } else {
            assertThat(actualAttrs).containsExactlyInAnyOrderEntriesOf(expected);
        }