[aspectj-dev] RE: interesting bug and fix

[Jim.Hugunin@xxxxxxxx]

Adrian Colyer wrote:
> I found a problem in testing AJDT with a declaration of the form:
> 
>       pointcut inSetMethod() :
>             withincode(* FigureElement+.set*(..));
> 
>       pointcut insideConstructor() :
>             withincode(FigureElement+.new(..));
> 
>       declare warning : privateFieldUpdate() &&
>        !(inSetMethod() || insideConstructor()) :
>        "Only update private fields through setters";
> 
> where the insideConstructor pointcut was matching against *every*
> joinpoint
> in the types FigureElement+. Here's what I did to fix it, which would
> benefit from a sanity check!

Have you added a case to the core test suite for this?  You should add it to the tests module and feel free to ask for help if the documentation there isn't sufficiently clear.

> First off, I found that AspectDeclaration.buildInterTypeandPerClause(..)
> was calling DeclareDeclaration.build(..) which resolved the pointcut, and
> then addShadowMunger concretized the pointcut. This is in the first pass.
> When we come round again, and called resolve in the second pass, an
> assertion was failing in Pointcut.resolve because the pointcut was not
> symbolic. I changed the test in PointcutDesignator.finishResolveTypes(..)
> to return true if the pointcut was resolved OR concrete.
> 
> Then... I had to change Signature.matches to insert a test that the
> ResolvedMember kind was the same as the SignaturePattern kind - the
> absence
> of this test caused a pattern of Kind CONSTRUCTOR to match against every
> resolved member in its type specification.,
> 
> (took me a lot longer to do than to write down!!).
> 
> Prior to this fix, and having previously fixed the context for declare
> warning/error messages, all of the expected messages in
> tests/new/declare/DeclareWarning.java  (tests/ajcTestsFailing.xml) were
> being received (see bug 31724), but a couple extra unexpected ones were
> also being generated, After the fix, I'm down two expected messages: lines
> 5 and 74 - one of these I think is caused by the initializer reporting at
> line 0 instead of 5, the other I'm not sure. Still, it's at least closer
> to
> the right solution than when I started!

I'll look at this in the next couple of days.  The change to SignaturePattern sounds exactly right, but the change to PoincutDesignator sounds dangerous and needs to be looked at more closely -- and Erik or I should probably be forced to document this lifecycle more carefully.

> There are still 5 outstanding bugs at P2 or higher on bugzilla (including
> the above).

These bugs shouldn't prevent a 1.1b5 release.  There are many bugs that have been fixed so the current quality should be strictly higher than 1.1b4.  The only thing these P1/P2 bugs are preventing is a candidate1 release.

-Jim