google / google/error-prone

ParameterMissingNullable should not warn about test code passing null

Open
#3,228 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

Sometimes we want to test that certain APIs throw `NullPointerException` when a parameter is `null` without the possibility to use something like Guava's `NullPointerTester`. Since version 2.14.0, Error Prone's `ParameterMissingNullable` complains about this.

Test code:
```java
import static org.junit.Assert.assertThrows;

import org.junit.Test;

public class MyTest {

interface TestInterface {
void m(Object o);
}

@Test
public void test(TestInterface i) {
assertThrows(NullPointerException.class, () -> i.m(null));
}
}
```

Output:
```
[javac] src/MyTest.java:13: warning: [ParameterMissingNullable] Parameter has handling for null but is not annotated @Nullable
[javac] assertThrows(NullPointerException.class, () -> i.m(null));
[javac] ^
[javac] (see https://errorprone.info/bugpattern/ParameterMissingNullable)
[javac] Did you mean 'void m(@Nullable Object o);'?
```

I would guess that in test code, the risk of passing `null` somewhere is relatively low compared to other code (chances are high that it either works as intended or fails the test). Thus my suggestion would be to disable this check in test code completely, because a more fine granular exclusion of patterns like the above `assertThrows()` will likely not be enough in large code bases (where helper methods are used in tests etc.).

Note that the warning message is also confusing: "Parameter has handling for null" is wrong, it needs to be something like "Parameter is set to null".

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.