Bug 295260 - Wrong warnings on Java.Compiler.Errors/Warnings "Redundant null check"
Summary: Wrong warnings on Java.Compiler.Errors/Warnings "Redundant null check"
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.6 M4   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-16 10:12 EST by Dietmar Hechensteiner CLA
Modified: 2009-12-08 02:56 EST (History)
2 users (show)

See Also:
Olivier_Thomann: review+

Attachments
Contains test case (1.05 KB, text/plain)
2009-11-16 10:15 EST, Dietmar Hechensteiner CLA 		no flags Details
proposed fix v0.5 + regression test (5.64 KB, patch)
2009-11-19 08:38 EST, Ayushman Jain CLA 		Olivier_Thomann: iplog+
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dietmar Hechensteiner CLA 2009-11-16 10:12:51 EST 
User-Agent:       Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; WOW64; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; InfoPath.2)
Build Identifier: M20090917-0800

See attached test cases

Reproducible: Always

Steps to Reproduce:
1. Compile test case. You can see 3 false warnings if the compiler setting "Redundant null check" is on.
Comment 1 Dietmar Hechensteiner CLA 2009-11-16 10:15:44 EST 
Created attachment 152308 [details]
Contains test case
Comment 2 Ayushman Jain CLA 2009-11-17 07:33:21 EST 
(In reply to comment #1)
> Created an attachment (id=152308) [details]
> Contains test case

In your test case, the redundant null check warning on the if statement after the assert statement is perfectly ok. If "args" is null, the assert statement should throw an AssertException. The if statement is only reached in case "args" is not null, hence the check is indeed redundant! Please correct me if i'm wrong.
Comment 3 Dietmar Hechensteiner CLA 2009-11-18 02:32:05 EST 
(In reply to comment #2)
This is true only if assertions are enabled, but I have to guarantee
a deterministic behavior of the method test2 also in the case of disabled
assertions (In productive environments they are mostly disabled).
Asserts can prove the presence of a specific error condition, but they
can never furnish proof for the absence of specific errors.
But considering that this question is already discussed lengthy in
Bug 234549, Bug 198044, Bug 173725 please ignore test2.
Comment 4 Ayushman Jain CLA 2009-11-19 08:38:19 EST 
Created attachment 152575 [details]
proposed fix v0.5 + regression test

Regarding the wrong warning in test1 above, fix for unchecked exceptions was done in bug 130359, but for checked exceptions we were still discarding the null info from inside the try loop. Because of this the wrong warning was raised. This fix includes the null info of the try block.
Comment 5 Olivier Thomann CLA 2009-11-19 09:47:37 EST 
I prefer the behavior with the patch.
+1.
Comment 6 Olivier Thomann CLA 2009-11-19 10:56:36 EST 
Released for 3.6M4.
Regression tests added in:
org.eclipse.jdt.core.tests.compiler.regression.NullReferenceTest#test0535_try_finally
org.eclipse.jdt.core.tests.compiler.regression.NullReferenceTest#test0567_try_catch_checked_exception
org.eclipse.jdt.core.tests.compiler.regression.NullReferenceTest#test0568_try_catch_checked_exception
Comment 7 Srikanth Sankaran CLA 2009-12-08 02:56:53 EST 
Verified for 3.6M4 using Build id: I20091207-1800