Bug 173725 - Incorrect "cannot be null" after assert
Summary: Incorrect "cannot be null" after assert
Status: VERIFIED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M6   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 319510 322209 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-02-09 21:29 EST by Walter Harley CLA
Modified: 2010-08-16 06:48 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Walter Harley CLA 2007-02-09 21:29:32 EST
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.
Comment 1 Maxime Daniel CLA 2007-02-12 06:04:50 EST
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. 
Comment 2 Walter Harley CLA 2007-02-12 11:59:15 EST
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.
Comment 3 Maxime Daniel CLA 2007-02-19 03:06:35 EST
The only quick fix offered so far is 'Rename in file', which is unlikely to cause much grief, hence I postpone any action.
Comment 4 Denis Roy CLA 2009-08-30 02:22:03 EDT
As of now 'LATER' and 'REMIND' resolutions are no longer supported.
Please reopen this bug if it is still valid for you.
Comment 5 Olivier Thomann CLA 2009-12-18 14:11:55 EST
Reopen.
Comment 6 Olivier Thomann CLA 2009-12-18 14:13:48 EST
This seems to be a consequence of bug 127244.
Comment 7 Ayushman Jain CLA 2010-02-23 00:20:33 EST
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?
Comment 8 Olivier Thomann CLA 2010-02-24 11:33:37 EST
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.
Comment 9 Olivier Thomann CLA 2010-03-08 13:37:37 EST
Verified for 3.6M6 using I20100307-2000.
Comment 10 Ayushman Jain CLA 2010-08-10 05:55:17 EDT
*** Bug 322209 has been marked as a duplicate of this bug. ***
Comment 11 Ayushman Jain CLA 2010-08-16 06:48:16 EDT
*** Bug 319510 has been marked as a duplicate of this bug. ***