Bug 226357 - [dom] NPE in MethodBinding.getParameterAnnotations() if some, but not all parameters are annotated
Summary: [dom] NPE in MethodBinding.getParameterAnnotations() if some, but not all par...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4.1   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-09 16:00 EDT by Kevin CLA
Modified: 2008-08-28 13:04 EDT (History)
5 users (show)

See Also:


Attachments
Fix for NPE. (930 bytes, patch)
2008-04-21 20:57 EDT, Kevin Doyle CLA
no flags Details | Diff
Regression test to verify that the fix works properly (3.13 KB, patch)
2008-06-24 10:56 EDT, Frederic Fusier CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin CLA 2008-04-09 16:00:48 EDT
Build ID: I20080330-1350

Steps To Reproduce:
1. Create a method with some, but not all parameters annotated.
2. Create a call site to that method in a separate compilation unit (it's unknown whether this problem occurs in the same comp unit).
3. Request an AST for the comp unit containing the call site with bindings.  Get the IMethodBinding for the called method.  Calling getParameterAnnotations() for *any* parameter results in an NPE:

java.lang.NullPointerException
	at org.eclipse.jdt.core.dom.MethodBinding.getParameterAnnotations(MethodBinding.java:149)

The NPE occurs in 3.4M5 and 3.4M6.  This is different from bugs #214002 and #214647 as the NPE is thrown in a different location and the mentioned bugs were fixed for M5.

More information:
Stepping through with the debugger indicates that 

org.eclipse.jdt.internal.compiler.lookup.MethodBinding.getParameterAnnotations()

returns null entries in the returned array for parameters without annotations.  The NPE is thrown when such a null entry is dereferenced (paramBindingAnnotations.length), which happens no matter for which parameter annotations are requested.

Here are two test classes that exhibit the problem when requesting parameter annotation bindings for the method binding of the call to safeDiv(5, 0).

public class ParameterSubsetAnnotated {
	
	public @interface NonZero { }
	
	public static int safeDiv(int a, @NonZero int b) {
		return a / b;
	}

}

public class ParameterSubsetClient {
	
	public void client() {
		System.out.println(ParameterSubsetAnnotated.safeDiv(5, 0));
	}

}
Comment 1 Olivier Thomann CLA 2008-04-09 17:05:37 EDT
I could not reproduce using the same build.
On line 149:
org.eclipse.jdt.internal.compiler.lookup.AnnotationBinding[] paramBindingAnnotations = bindingAnnotations[i];
, I always get an empty array, not null.

Comment 2 Walter Harley CLA 2008-04-09 20:09:53 EDT
Kevin, since Olivier can't repro, could you also attach the code that you're using to request the AST?  Maybe there is some difference in how you guys are doing that.
Comment 3 Kevin CLA 2008-04-10 10:22:53 EDT
Here's the code I use for parsing the ICompilationUnit of the class containing the call site.  

// compUnit is ICompilationUnit of ParameterSubsetClient
ASTParser parser = ASTParser.newParser(AST.JLS3);
parser.setResolveBindings(true);
parser.setSource(compUnit);  
return parser.createAST(null);

I don't know if this is relevant, but I never parse the comp unit containing the annotated method.

Here's the code that accesses parameter annotation bindings on the call site.

// node is MethodInvocation for call to ParameterSubsetAnnotated.safeDiv(5, 0)
// IMethodBinding binding = node.resolveMethodBinding();
try {
    for(int i = 0; i < paramCount; i++) {
        IAnnotationBinding[] annos = binding.getParameterAnnotations(i);
        // ...
    }
}
catch(NullPointerException e) {
    if(log.isLoggable(Level.WARNING))
        log.log(Level.WARNING, "Bug in JDT (Eclipse 3.4M5) triggered in " + binding + ".  Not all annotations on parameters might be available.", e);
}

The problem ends up being that in the following loop in compiler.lookup.MethodBinding.getParameterAnnotations() (line 526), argument.annotations is null for the un-annotated parameter, which in turn never initializes the corresponding element in the result array.

for (int i = 0; i < length; i++) {
    Argument argument = methodDecl.arguments[i];
    if (argument.annotations != null) {
        ASTNode.resolveAnnotations(methodDecl.scope, argument.annotations, argument.binding);
        allParameterAnnotations[i] = argument.binding.getAnnotations();
    }
}

Please let me know if this helps, or how I can dig deeper to figure out what's going on.
Comment 4 Kevin Doyle CLA 2008-04-21 20:57:37 EDT
Created attachment 96946 [details]
Fix for NPE.

I was also able to reproduce.  Attached patch fixes the problem.
Comment 5 Kevin CLA 2008-04-22 13:55:45 EDT
Oh great, thanks so much, Kevin.  I was beginning to think I was seeing ghosts.
Comment 6 Kevin CLA 2008-06-12 18:42:08 EDT
This bug is still present in Eclipse 3.4 M7.  Looking at org.eclipse.jdt.internal.compiler.lookup.MethodBinding, it seems that Kevin Doyle's fix (which I expect to resolve the bug) from April 21 hasn't made it in there.  Is this bug still being worked on?
Comment 7 Walter Harley CLA 2008-06-13 03:10:20 EDT
Adding Philippe to cc list of this bug in the interest of timeliness.

I too thought this bug was supposed to have been fixed.  If it is not in there now, though, it is probably too late for 3.4 and we will have to wait for 3.4.1.  Frederic, can you clarify status of this bug?
Comment 8 Frederic Fusier CLA 2008-06-13 04:13:14 EDT
(In reply to comment #7)
> Adding Philippe to cc list of this bug in the interest of timeliness.
> 
> I too thought this bug was supposed to have been fixed.  If it is not in there
> now, though, it is probably too late for 3.4 and we will have to wait for
> 3.4.1.  Frederic, can you clarify status of this bug?
> 
Sorry about that but I was very busy with some other stuff since M6 and missed to have a look on this :-(

So, I'll put it in for 3.4.1...
Comment 9 Frederic Fusier CLA 2008-06-24 10:56:50 EDT
Created attachment 105712 [details]
Regression test to verify that the fix works properly

Test testBug226357 added in org.eclipse.jdt.core.tests.dom.ASTConverterBugsTest
Comment 10 Frederic Fusier CLA 2008-06-24 11:00:46 EDT
Released for 3.5M1
Comment 11 Jerome Lanneluc CLA 2008-06-25 02:20:00 EDT
+1 for backport to 3.4.1
Comment 12 Frederic Fusier CLA 2008-06-25 04:58:56 EDT
Released for 3.4.1
Comment 13 Olivier Thomann CLA 2008-08-06 12:21:46 EDT
Verified for 3.5M1 using I20080805-1307
Comment 14 Olivier Thomann CLA 2008-08-06 13:14:15 EDT
Reopen to close as RESOLVED/FIXED. Will be closed as VERIFIED during 3.4.1
verification pass.
Comment 15 Olivier Thomann CLA 2008-08-06 13:14:29 EDT
Fixed.
Comment 16 Olivier Thomann CLA 2008-08-28 13:04:18 EDT
Verified for 3.4.1 using M20080827-2000