Bug 337964 - [DOM] code that would definitely cause NPE if executed
Summary: [DOM] code that would definitely cause NPE if executed
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 M6   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-23 07:15 EST by Stephan Herrmann CLA
Modified: 2011-03-08 09:40 EST (History)
1 user (show)

See Also:

Attachments
Proposed fix (1.97 KB, patch)
2011-02-24 11:03 EST, Olivier Thomann 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-02-23 07:15:04 EST 
While playing with null-annotation support (bug 186342) I found the 
following non-sense code:

DefaultBindingResolver:1530 reads:
  return getTypeBinding(this.scope.createArrayType(binding, arrayType.getDimensions()));

At this point the analysis knows that 'binding' is definitely null.
Inside createArrayType(..) the first action is to dereference that parameter.

If that could would ever get executed it would definitely throw NPE.

This manifested in a conflict that the parameter in question could neither
be type-checked with @Nullable nor with @NonNull.
Comment 1 Olivier Thomann CLA 2011-02-24 10:56:12 EST 
In fact that code can never be run. I'll clean it up.
Comment 2 Olivier Thomann CLA 2011-02-24 11:03:50 EST 
Created attachment 189706 [details]
Proposed fix
Comment 3 Stephan Herrmann CLA 2011-02-24 11:21:21 EST 
(In reply to comment #1)
> In fact that code can never be run.

I had hoped so :)

So the code was trying to protect against inconsistencies between the dom Type
and the compiler TypeBinding (one being an array type while the other's not)?

And by now we're confident that such inconsistency will never occur?
Sounds good to me.
Comment 4 Olivier Thomann CLA 2011-02-24 11:32:25 EST 
(In reply to comment #3)
> So the code was trying to protect against inconsistencies between the dom Type
> and the compiler TypeBinding (one being an array type while the other's not)?
> 
> And by now we're confident that such inconsistency will never occur?
> Sounds good to me.
I believe the code was over paranoiac. At that point, it can only be an array type with an array type binding.
So I'll run all tests for safety and I'll release afterwards.
Comment 5 Olivier Thomann CLA 2011-02-25 13:30:32 EST 
Released for 3.7M6.
Comment 6 Stephan Herrmann CLA 2011-03-08 09:38:59 EST 
(In reply to comment #4)
> I believe the code was over paranoiac. At that point, it can only be an array
> type with an array type binding.

I filed bug 339226 for recovering the underlying tacit knowledge.
Comment 7 Stephan Herrmann CLA 2011-03-08 09:40:20 EST 
Verified for 3.7M6 via code inspection.