Bugzilla – Full Text Bug Listing
| Summary: | Incorrect "cannot be null" after assert | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Walter Harley <eclipse> |
| Component: | Core | Assignee: | Ayushman Jain <amj87.iitr> |
| Status: | VERIFIED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | apeeters, dominik.stadler, Olivier_Thomann |
| Version: | 3.3 | ||
| Target Milestone: | 3.6 M6 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
This is the intended behavior (see NullReferenceTest#953).
My initial reaction was:
I would place assertions and compiler warnings in the same realm here: tools that help the programmer to improve her code. Assertions have two extra characteristics: a) they document the source code, independently of the compiler used to compile the code (whereas warnings vary with the compiler/code validator at hand), b) they have the capability to enforce contracts at runtime, the use of that capability being decided by the deployer (not the programmer).
For assertions to be useful, we must consider that they tell us something about the code. I would then consider that the warning is right, in that it will encourage the programmer to reconsider her code: if the assert is satisfied, the test is useless.
I understand that you fear that the test may be simply suppressed and runtime cases be overlooked. But this remains a programmer decision, and she still can rewrite the code in a manner that provides abnormal termination when assertions are active and a more innocuous behavior when assertions are not active (there has been much raging debate outside in the past fifteen years about the merits and limits of contracts in programming, hence I will simply avoid it and attempt to suggest a reconciling view):
int x(String foo) {
if (foo != null)
return foo.length();
else {
assert false;
return 0;
}
}
When discussing with Philippe, he raised the following point: if we get a quick fix at some point in time that fixes the situation, people could well suppress the check more or less 'inadvertantly'. Besides, we may prefer to follow the flow analysis specification, hence admit that the assert could well not be executed at all.
The quick fix wins my vote, I think. We will loosen the certainty of the variable status after the assertion.
Thanks for looking into it. The argument that "it will encourage the programmer to reconsider her code: if the assert is satisfied, the test is useless" is a good one. The only quick fix offered so far is 'Rename in file', which is unlikely to cause much grief, hence I postpone any action. As of now 'LATER' and 'REMIND' resolutions are no longer supported. Please reopen this bug if it is still valid for you. Reopen. This seems to be a consequence of bug 127244. In light of what we discussed for bug 250056, I think it is pretty clear that this is the intended behaviour. So this bug should be closed as INVALID. What do you think Olivier? Now we report a redundant null check for the test case in comment 0. Even if asserts are not enabled by default at runtime, the condition is expected to be true. Closing as WONTFIX. We plan to keep a redundant null check warning. Verified for 3.6M6 using I20100307-2000. *** Bug 322209 has been marked as a duplicate of this bug. *** *** Bug 319510 has been marked as a duplicate of this bug. *** |
The following code (with compiler set to Java 5 compliance) triggers the "The variable foo cannot be null" warning: int x(String foo) { assert foo != null; if (foo != null) // INCORRECT WARNING return foo.length(); else return 0; } Since the assert is not necessarily present at runtime, this warning is incorrect.