Bugzilla – Full Text Bug Listing
| Summary: | NPE in HierarchyResolver | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Dani Megert <daniel_megert> | ||||||
| Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | P3 | CC: | jarthana, Olivier_Thomann, pbenedict, satyam.kandula | ||||||
| Version: | 3.8 | Flags: | satyam.kandula:
review+
|
||||||
| Target Milestone: | 3.8 M2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Dani Megert
Satyam, please take a look. If this is a regression over 3.6.x, we might fix it for 3.7.2. Here is a simplified test case
-------
public class Q {
class R{
void setInfo(String abc) {
}
class S {
}
class T {
}
S s = new S() {
T t = new T() {
void myMethod() {
setInfo("a");
}
};
};
}
}
--------
When you try to find the implementation for setInfo, the NPE is thrown. Note that this doesn't seem to affect the functionality as such. The cursor is taken to the appropriate position.
And looking at the code, this must have been introduced by the fix for bug 169678 and I think this might have always been around. On investigation I found that we try to look for member types even though what we have at hand is an anonymous and in the process a null focusType is being set at HierarchyResolver:848. We probably don't need that code to be executed in this case as the original fix (for bug 169678) seemed to target static member types.
Reassigning to Jay since he is already looking at it. Created attachment 202401 [details]
Proposed fix
Looks like all we have to do is just to avoid the NPE. And since the null scenario has already been taken care of in MatchLocator#createHierarchyResolver(), returning at the first occurrence of null would do. The patch also keeps the null value assigned to MatchLocator#focusType.
Satyam, can you please review the patch. Should be a simple one. Patch looks good. Do you plan to have a testcase? (In reply to comment #6) > Patch looks good. Do you plan to have a testcase? Okay, I will update with a test in a bit. Created attachment 202886 [details]
Patch with test
This patch includes a new test.
Satyam, can you please look at the test and tell me how I can avoid the multiple searches to reach the method invocation inside the nested anonymous? Thanks.
Released in HEAD 3.8M2. . I found lines 849-850 to be odd:
>> if (this.focusType == null)
>> return this.focusType;
Wouldn't it be more straightforward to return null?
(In reply to comment #11) > Wouldn't it be more straightforward to return null? Yes, indeed. I have corrected that in the HEAD also shortened the testcase a bit. Verified for 3.8M2 using build I20110912-0800 |