assertThat(...).as(...).isNotNull() is not recognized

[vladykin]

NullAway recognizes the assertThat(...).isNotNull() (#301), but fails to recognize if the description is added. Please refer to the example below. NullAway version is 0.10.18.

package com.example;

import static org.assertj.core.api.Assertions.assertThat;

import jakarta.annotation.Nullable;
import org.junit.jupiter.api.Test;

public class NullAwayTest {

  @Test
  void assertNotNull() {
    String s = getString();
    assertThat(s).isNotNull();
    assertThat(s.length()).isEqualTo(0); // NullAway doesn't complain, this is correct
  }

  @Test
  void assertNotNullAs() {
    String s = getString();
    assertThat(s).as("text").isNotNull();
    assertThat(s.length()).isEqualTo(0); // NullAway complains, this is wrong
  }

  @Test
  void assertNotNullDescribedAs() {
    String s = getString();
    assertThat(s).describedAs("text").isNotNull();
    assertThat(s.length()).isEqualTo(0); // NullAway complains, this is wrong
  }

  static @Nullable String getString() {
    return null;
  }
}

[msridhar]

Thanks for the report. The relevant code in NullAway is here:

NullAway/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java

Lines 59 to 75 in 5fbee1f

	// Look for statements of the form: assertThat(A).isNotNull() or
	// assertThat(A).isInstanceOf(Foo.class)
	// A will not be NULL after this statement.
	if (methodNameUtil.isMethodIsNotNull(callee) || methodNameUtil.isMethodIsInstanceOf(callee)) {
	Node receiver = node.getTarget().getReceiver();
	if (receiver instanceof MethodInvocationNode) {
	MethodInvocationNode receiver_method = (MethodInvocationNode) receiver;
	Symbol.MethodSymbol receiver_symbol = ASTHelpers.getSymbol(receiver_method.getTree());
	if (methodNameUtil.isMethodAssertThat(receiver_symbol)) {
	Node arg = receiver_method.getArgument(0);
	AccessPath ap = AccessPath.getAccessPathForNode(arg, state, apContext);
	if (ap != null) {
	bothUpdates.set(ap, NONNULL);
	}
	}
	}
	}

The logic is explicitly looking for an AST corresponding to assertThat(A).isNotNull(). Instead, we'd have to generalize this logic to recurse when appropriate to look for a more deeply-nested assertThat call.

@vladykin is it the case that any chain of method calls starting with assertThat(s) and ending in isNotNull() should make s @NonNull? Or is it only for specific intermediate method calls like as() or describedAs()?

We would welcome a PR for this one.

[vladykin]

is it the case that any chain of method calls starting with assertThat(s) and ending in isNotNull() should make s @NonNull? Or is it only for specific intermediate method calls like as() or describedAs()?

AssertJ API is very rich, with a lot of things possible in the call chain between assertThat(s) and isNotNull(). I can't confidently say that any such call chain should make s @NonNull.

as() and describedAs() is the thing I noticed when enabling NullAway in our codebase. Supporting just this case would address our needs.