Bug 367203 - [compiler][null] detect assigning null to nonnull argument
Summary: [compiler][null] detect assigning null to nonnull argument
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.8   Edit
Hardware: Other Linux
: P3 normal (vote)
Target Milestone: 3.8 M5   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-20 10:52 EST by Stephan Herrmann CLA
Modified: 2012-01-23 03:23 EST (History)
2 users (show)

See Also:

Attachments
tests & fix (4.59 KB, patch)
2011-12-20 13:03 EST, Stephan Herrmann CLA 		no flags 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 Stephan Herrmann CLA 2011-12-20 10:52:54 EST 
From bug 367154 comment 1:
> Is this case already captured in a bug?
> 
> @NonNullByDefault
> public class ShowNPE2 {     
>      public Object foo(Object o1, final boolean b) {
>          o1 = null;   // MISSING NPE warning
>         System.out.println(o1.toString());   
>         return null;  // Correct warning
> 
>     }
> }

This test reveals an obvious omission inside 
AbstractMethodDeclaration.addParameternonNullAnnotation
which can be easily fixed: update the tagBits of argument.binding.

I should also add a test for the inverse:
arg is nonnull from default, passed to a method requiring nonnull.
Expecting this to fail currently, too.
Comment 1 Stephan Herrmann CLA 2011-12-20 13:03:25 EST 
Created attachment 208637 [details]
tests & fix

(In reply to comment #0)
> This test reveals an obvious omission inside 
> AbstractMethodDeclaration.addParameternonNullAnnotation
> which can be easily fixed: update the tagBits of argument.binding.

The patch does this at the call site in MethodBinding.fillInDefaultNonNullness
 
> I should also add a test for the inverse:
> arg is nonnull from default, passed to a method requiring nonnull.
> Expecting this to fail currently, too.

Test has been added, my prediction was wrong, though: this case was
already handled via AbstractMethodDeclaration.analyseArguments()
(which uses MethodBinding.parameterNonNullness instead of tagBits).

Aggressive tests are currently running.
Comment 2 Ayushman Jain CLA 2011-12-20 14:35:48 EST 
(In reply to comment #1)
> > I should also add a test for the inverse:
> > arg is nonnull from default, passed to a method requiring nonnull.
> > Expecting this to fail currently, too.
> 
> Test has been added, my prediction was wrong, though: this case was
> already handled via AbstractMethodDeclaration.analyseArguments()
> (which uses MethodBinding.parameterNonNullness instead of tagBits).

Yes, we'd already tested this particular case.
Comment 3 Stephan Herrmann CLA 2011-12-20 17:03:59 EST 
Released for 3.8 M5 via commit 3a3faf3d0aa82cf2ed2b8375e651ef0b429b1314
Comment 4 Srikanth Sankaran CLA 2012-01-23 03:23:38 EST 
Verified for 3.8 M5 using build id: I20120122-2000