[Java] False positive for null dereference
- Dominant language
- OCaml
- Stars
- 15.7k
- Forks
- 2.1k
- Avg merge
- 19h 36m
- Merged PRs (30d)
- 13
Description
## Information
- [x] The version of infer from `infer --version`: 1.1.0
- [x] Your operating system and version: using https://github.com/facebook/infer/blob/main/docker/1.1.0/Dockerfile
- [x] Which command you ran, for example `infer run --fail-on-issue -- mvn compile`.
- [/] The full output in a paste, for instance a [gist](https://gist.github.com/).
- [ ] If possible, a minimal example to reproduce your problem (for instance, some code where
infer reports incorrectly, together with the way you run infer to reproduce the incorrect
report).
## Logs
Unfortunately, I am unable to create a smaller reproducer so far (and the code where infer finds this is not shareable), but the logs are pretty clear:
```java
source.java:789: error: Null Dereference
object `inProgress` last assigned on line 788 could be null and is dereferenced by call to `updateInProgressStatus(...)` at line 789.
787. final boolean inProgress =
788. task.getStatus() != null && task.getStatus().equals(Task.Status.IN_PROGRESS);
789. > updateInProgressStatus(connection, task, inProgress);
790. }
```
So, infer thinks that `inProgress` can be null which is not possible for 2 reasons:
- code compilation was fine
- it's a primitive type `boolean`
- it is the result of a `a && b` which cannot return null (it's a Java language comparison, always returning a `boolean`)
- `updateInProgressStatus` takes `(A, B, boolean)` as input parameter types (and so not a `Boolean` but a `boolean`)
So this is fairly weird.
I think that Infer should know better that a `&&` comparison must return a `boolean`, but also that `boolean` types (or any primitive type, cannot be null, by definition). It's a good invariant to check for I think.
## Test 1: rewrite the code with an if
If I try to change the code a bit, then infer is happy, even though it's the exact same behaviour:
```java
final boolean inProgress;
if (task.getStatus() != null && task.getStatus().equals(Task.Status.IN_PROGRESS)) {
inProgress = true;
} else {
inProgress = false;
}
updateInProgressStatus(connection, task, inProgress);
```
Infer still complains about it:
```
source.java:793: error: Null Dereference
#14 410.1 object `inProgress` last assigned on line 791 could be null and is dereferenced by call to `updateInProgressStatus(...)` at line 793.
#14 410.1 791. inProgress = false;
#14 410.1 792. }
#14 410.1 793. > updateInProgressStatus(connection, task, inProgress);
#14 410.1 794. }
#14 410.1 795.
```
## Test 2: Test 1 but with inverted if condition
I was curious to see if inverting the if condition would help infer think it's correct:
```java
final boolean inProgress;
if (task.getStatus() == null || !task.getStatus().equals(Task.Status.IN_PROGRESS)) {
inProgress = false;
} else {
inProgress = true;
}
updateInProgressStatus(connection, task, inProgress);
```
But infer still thinks that this isn't correct:
```
object `inProgress` last assigned on line 789 could be null and is dereferenced by call to `updateInProgressStatus(...)` at line 793.
#15 422.4 791. inProgress = true;
#15 422.4 792. }
#15 422.4 793. > updateInProgressStatus(connection, task, inProgress);
#15 422.4 794. }
```
## Test 3: yet another form
This time by assigning the variable first, and overriding it instead:
```java
boolean inProgress = false;
if (task.getStatus() != null && task.getStatus().equals(Task.Status.IN_PROGRESS)) {
inProgress = true;
}
updateInProgressStatus(connection, task, inProgress);
```
But, still the same:
```
#15 423.7 source.java:791: error: Null Dereference
#15 423.7 object `inProgress` last assigned on line 787 could be null and is dereferenced by call to `updateInProgressStatus(...)` at line 791.
#15 423.7 789. inProgress = true;
#15 423.7 790. }
#15 423.7 791. > updateInProgressStatus(connection, task, inProgress);
#15 423.7 792. }
```
## Test 4
This time, I remove the if assignment (but then it's no longer strictly equivalent):
```java
boolean inProgress = false;
updateInProgressStatus(connection, task, inProgress);
```
Still, the same:
```java
#15 433.3 source.java:788: error: Null Dereference
#15 433.3 object `inProgress` last assigned on line 787 could be null and is dereferenced by call to `updateInProgressStatus(...)` at line 788.
#15 433.3 787. boolean inProgress = false;
#15 433.3 788. > updateInProgressStatus(connection, task, inProgress);
#15 433.3 789. }
#15 433.3 790.
```
## Test 5
I move that inProgress variable set inside updateInProgressStatus method (first line in the method), and now infer is happy (finally).
Contributor guide
Assessment
This issue has not been assessed yet.