Bug 131921

Summary: NPE caugth in DefaultBindingResolver.resolveName(Name)
Product: [Eclipse Project] JDT Reporter: Martin Aeschlimann <martinae>
Component: CoreAssignee: Olivier Thomann <Olivier_Thomann>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3    
Version: 3.2   
Target Milestone: 3.2 M6   
Hardware: PC   
OS: Windows XP   
Whiteboard:

Description Martin Aeschlimann CLA 2006-03-15 08:47:35 EST
20060315

When debugging, I normally catch NPE by default.

DefaultBindingResolver.resolveName(Name) line: 1061 any many other places catch RuntimeExceptions 

try {
  if (internalScope == null) {
    binding = this.scope.getTypeOrPackage(....);
  } else {
    ...
  }
} catch (RuntimeException e) {
  // see https://bugs.eclipse.org/bugs/show_bug.cgi?id=53357
}
In my example I got plenty of NPE cought as 'this.scope' was null.
However, comment of bug 53357 only mentions AbortCompilation

IMO this is very risky practice as this hides many potential problems, e.g. ArrayOutOfBoundExceptions, IllegalArgumentExeptions, Assertions

In the given scenario, it also would be more performant to add a simple if (scope != null) at the beginning.

My setup:
- connect to :pserver:anonymous@dev.eclipse.org:/cvsroot/webtools
- checkout org.eclipse.wst.validation
- open EMFWorkbenchContextFactory
Comment 1 Olivier Thomann CLA 2006-03-20 14:35:05 EST
this.scope should not be null.
I can change it to catch only AbortCompilation, but I would prefer to get a reproducable test case first. I tried the steps that you specified and I could not get a NPE.
Comment 2 Olivier Thomann CLA 2006-03-20 15:20:55 EST
In fact, this.scope can be null in case of errors. I still would like to get a reproducable test case.
I released a version that is checking for null and only catch AbortCompilation.
Let me know if this is fixing your issue since I cannot reproduce it.
Comment 3 Martin Aeschlimann CLA 2006-03-20 17:01:51 EST
My scenarion was as described, and I don't know what else caused the problems.
Unfortunatly , I currently don't have time to reproduce it. But I think it is good that you added the null checks.
Comment 4 Olivier Thomann CLA 2006-03-21 11:19:48 EST
Closing as FIXED.
If you get a chance to verify it before M6 is released, it would be really nice.
Comment 5 Jerome Lanneluc CLA 2006-03-28 08:41:19 EST
Verified for 3.2 M6 using build I20060328-0010