eclipse-jdt / eclipse-jdt/eclipse.jdt.core
Potential null access after assertion defined in application code
- Dominant language
- Java
- Stars
- 237
- Forks
- 195
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 47
Description
For the following snippet, a potential null access warning is issued at line 9:
```
public class TestAssertNotNull {
public void test(boolean b) {
Object o = null;
if (b) {
o = new Object();
}
//org.junit.jupiter.api.Assertions.assertNotNull(o);
assertNotNull(o);
System.out.println(o.toString()); // warning is issued here
}
public static void assertNotNull(Object o) {
org.junit.jupiter.api.Assertions.assertNotNull(o);
}
}
```
When the following preference is set:
```
org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning
```
Commenting in `Assertions.assertNotNull(o)` at line 7 results in no warning. This is due to code in: `org.eclipse.jdt.internal.compiler.ast.MessageSend.detectAssertionUtility(int)`
That code lists some well-known libraries and their well-known assertions. After such calls (directly in the method body), its assumed that there can be no null pointer dereference.
In our case, we recently moved to JUnit 5. However, we didn't undergo the move of assertions - we don't have time yet to move the assertion message to the last parameter of the method (as opposed to the fail message being the first parameter, in JUnit 4). So we have defined wrappers methods that delegate to JUnit 5, where the signatures of the wrappers match the old parameter order. This however means we see potential null access warnings in our code, where there were none before.
What can we do here?
1. @iloveeclipse suggests assuming any `assertNotNull()` method does what the name says, regardless of where the method is coming from.
2. We can define a system property that allows more classes to be used by `detectAssertionUtility()`, so that a "custom" library that is not "well-known" can still benefit from the method.
3. Try to be somewhat smarter in `detectAssertionUtility()`, check the body of a called `assertNotNull()` method - one stack line deeper would be enough for simple wrappers around known assertions.
4. Do nothing, developers in our situation have 2 options, disable the warning or move lots of assertions to JUnit 5.
Contributor guide
Assessment
This issue has not been assessed yet.