Bug 284862 - NPE in EclipseResolvedMember.getEclipseAnnotation (when duplicate entries on classpath?)
Summary: NPE in EclipseResolvedMember.getEclipseAnnotation (when duplicate entries on ...
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 minor (vote)
Target Milestone: 1.6.6   Edit
Assignee: AJDT-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-28 09:48 EDT by Simone Gianni CLA
Modified: 2009-08-10 14:11 EDT (History)
2 users (show)

See Also:


Attachments
A failing AJDT project (3.36 KB, application/octet-stream)
2009-08-07 11:33 EDT, Simone Gianni CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simone Gianni CLA 2009-07-28 09:48:21 EDT
Build ID: 20090619-0625

Steps To Reproduce:
I don't have clear steps to reproduce this, but it happens to me quite often doing what follows.

1. Create a Maven project
2. Add the source folder of one of the maven dependencies as a linked folder.
3. Configure the linked folder to have a different output folder.
4. Play a bit with that folder (for example, move it up or down the list of the java build path) 

There is no need for it to be a Maven project, I suppose it is the same with any other binary dependency.

Occasionaly, AJDT will throw :
java.lang.NullPointerException
	at org.aspectj.ajdt.internal.compiler.lookup.EclipseResolvedMember.getEclipseAnnotations(EclipseResolvedMember.java:206)
	at org.aspectj.ajdt.internal.compiler.lookup.EclipseResolvedMember.getAnnotationTypes(EclipseResolvedMember.java:156)
	at org.aspectj.ajdt.internal.compiler.lookup.EclipseResolvedMember.hasAnnotation(EclipseResolvedMember.java:74)
	at org.aspectj.weaver.JoinPointSignature.hasAnnotation(JoinPointSignature.java:83)


More information:
I suppose, but I might be wrong, that AJDT/AspectJ pick the wrong class. For example, maybe it searches for the binary (unresolved) version of it while it needed the source one.

This would also explain why moving the source folder up or down the list helps causing/solving this problem, it could be that the resolver finds the right class (source version or binary one) depending on the order they are presented.

In fact, debugging at the exception point, it seems like everything is fine, except that the method TypeDeclaration.declarationOf((MethodBinding) realBinding) return null, cause in fact the methods[] inside it have no binding (are all null) so no matching method is found.

"hard cleaning" (i.e. rm -fR *) the output folders resolves the problem, probably cause it forces AJDT to rebuild everything and re-resolving all the bindings.
Comment 1 Andrew Clement CLA 2009-07-28 11:10:19 EDT
AspectJ bug
Comment 2 Andrew Clement CLA 2009-07-28 11:16:48 EDT
Being unable to recreate this will make it tricky to progress - I'll have a play with the instructions from Simone when I have time.
Comment 3 Simone Gianni CLA 2009-08-06 23:38:38 EDT
I've tried to investigate this a bit more in depth.

I arrived to identify which aspects are causing the problem, but still cannot completely reproduce it in a simple project.

I have an interface declaring a method. This method has an annotation. Then I have an aspect (A) that ITD a default implementation of that method, and an entity class implementing that interface and not declaring that method, cause the default implementation is fine.

Then I have an aspect (B) with an hasmethod pointcut, searching for all entity classes not defining a method with a specific annotation. 

While searching matches for B, it encounters the entity class in the first package, and the NPE happens.

I don't actually understand how/where it gets confused.

(It is not a problem regarding having both the linked source folder and the jar, cause the jar is not added to the aspectpath nor the inpath, so it is not taking part in the matching game. Removing the jars causes the same error)

Comment 4 Simone Gianni CLA 2009-08-07 11:21:24 EDT
I managed to reproduce it in a plain AJDT project. It builds correctly with ajc, but geas NPE inside Eclipse.

It is related to :
- Presence of an annotation on an interface method
- ITD implementing that method
- The type pattern trying to match a method with an annotation
... Apparently only hasmethod/hasfield triggers this, but I haven't covered the entire possible pointcut expressions.

Seems like it skips the ITD in the main class and jumps to the interface, while still using a handler from the main class, causing the resolved handler to be null despite the fact that there is a "matching" handler in the interface.

Since "hasmembers" pointcuts were having problems with ITDs, probably they have been fixed in main AspectJ but not on source-based Eclipse support? Just wandering.
Comment 5 Simone Gianni CLA 2009-08-07 11:33:30 EDT
Created attachment 143780 [details]
A failing AJDT project

This is a jarred source AJDT project that actually causes the compiler to fail.
Comment 6 Andrew Clement CLA 2009-08-07 12:54:13 EDT
nice bit of investigating there!

Usually when something fails inside AJDT but not outside it can be related to the order in which files are passed to the compiler.  'ajc *.java *.aj' will do one ordering whilst AJDT is likely to do another.  Although some sorting is done to ensure the aspects are first, the rest of the types can be in any order and due to pipelining some may be through the weaver whilst others haven't got to the weaver yet.  If they haven't been through the weaver they are eclipse entities, once woven they become bcel entities.  Whenever I see a bug that mentions an eclipse entity like this, I wonder if it is due to ordering/pipelining.  We may find that changing the order passed to ajc will surface the problem on the command line too.

Comment 7 Andrew Clement CLA 2009-08-07 13:02:04 EDT
failed straightaway on the command line for me (windows 7) with ajc -1.5 *.aj *.java
Comment 8 Andrew Clement CLA 2009-08-07 13:15:06 EDT
Is that project a complete thing?  I'm trying to understand if you expect it to build clean or not?  It seems to be missing InterfaceToAdd?
Comment 9 Andrew Clement CLA 2009-08-07 13:16:04 EDT
It also defines SearchAnnotation but then refers to it as SearchAnnotations in the pointcut?
Comment 10 Andrew Clement CLA 2009-08-07 13:17:28 EDT
Each time I fix an error, I see another one! (Of course, all these are hidden by the original exception that I'm fixing):


	warning at declare parents : ((@Entity *) && !hasmethod(@SearchAnnotation public * get*()) && !hasfield(@SearchAnnotation * *)) implements InterfaceToAdd;
                                                                                             ^^^^^^^^^^^^^^^^^^^
C:\temp\ajcSandbox\aspectj16_2\ajcTest3199.tmp\AspectToMatch.aj:5:0::0 does not match because annotation @SearchAnnotation has @Target{ElementType.METHOD} [Xlint:unmatchedTargetKind]
Comment 11 Simone Gianni CLA 2009-08-08 09:08:14 EDT
Hi Andy, thanks for taking the time to investigate.

That is an "extraction" of a much larger set of experiments I did to reproduce the bug. 

I moved stuff from the old big project full of "tests" to the new small one, then renamed a few classes to make it clear, then zipped the new small one and attached it.

Pretty sure it's my fault, I missed something during the copy/paste/rename cycle.

You see why using a compiler friendly technology is better :D
 
InterfaceToAdd can be any, also empty, interface. In my real world application it contains the missing methods, but for the sake of the test it could be simply a marker interface.

!hasfield(.... I was just trying to test if also hasfield was a problem, you can safely remove it and still see the bug, or keep it to check also hasfield.

Sorry for taking up your time with these stupid mistakes.





Comment 12 Andrew Clement CLA 2009-08-08 18:53:17 EDT
no worries Simone.  Even with those things fixed, I do see something odd as the code where the NPE occurs ought to be able to discover the binding representing the method, but it cannot.
Comment 13 Andrew Clement CLA 2009-08-10 14:11:51 EDT
Right.  I think I understand everything that is going on here.

I didn't particularly just want to put a guard in for null as that really shouldn't occur unless there are other errors in the source code.

The problem is the combination of hasMember and the ITD onto the interface from an aspect.

hasMember causes some early processing of AnnotationMethodInterface - this builds EclipseResolvedMember representations for the methods in the interface.  *Then* we process ITDs, the ITD onto the AnnotationMethodInterface removes the declared binding for getSomething() and records a new ITD method binding in the memberfinder for AnnotationMethodInterface.  Unfortunately the EclipseResolvedMember for getSomething() still exists and is invalid.  This means when we ask it for annotations later, it can't find the method declaration corresponding to the binding it knows about, and we NPE.  The solution is for the resolved member to also have a look at ITDs in case the situation described here has happened.  With a change in to look at ITDs, it works fine.

A better solution would be repairing the damaged EclipseResolvedMember, but that is for another day.

A similar problem may exist for fields.

Thanks Simone, for coming up with a testcase that enabled me to get my brain round the problem.