NullAway
NullAway copied to clipboard
Feature request: Recognize common "assertThat(...).isNotNull()" statements
These are common in tests
assertThat(foo).isNotNull();
foo.bar(); // <- will currently error
This sounds doable, but only with a custom handler, since library models are not currently expressive enough to reason across multiple chained calls like that (e.g. isNotNull()
performs the nullness check, but doesn't take the argument to be checked).
I think it should be a simple handler, the question is if it's one that we want enabled by default. Is assertThat(...)
common in general, or common at Uber? We might want to make this handler optional/flag controlled.
Also, another quick question: what's the FQN of assertThat
in here? org.junit.Assert.assertThat
? Or is there an Uber internal version?
I think covering the standard libraries for this is probably a safe start
Junit's is one, there's also AssertJ, Truth, probably some others. Maybe cover the common ones and add a hook for others to provide their own expressions? (not sure how simple that is)
assertThat is pretty common in general I'd say, but running nullaway in tests probably isn't. I could see a good reason to make this off by default and opt-in to enable only in tests.
I'm pretty sure we fixed this one some time back