Community
Participate
Working Groups
Hi I was attempting to write an aspect that checks for null arguments passed to any constructors defined in our project, the problem i was having is that calls to constructors of nested classes have a false parameter appended to their parameter list, with type equal to that of the nested class, which evaluates to null. This is incorrect according to the Java language specification What follows is a test case with output, probably the easiest thing would be to copy and past this and test it out for yourself. Yours Piers First the aspect that i have created |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| package com.historicalengineering.wwii.aspect.test; import java.lang.IllegalArgumentException; public aspect EnforceContractAspect { pointcut allMethodCallsWithParameters() : call(* com.historicalengineering..*(*,..)); pointcut allConstructorCallsWithParameters() : call(com.historicalengineering..new(*,..)) || call(private com.historicalengineering..new(*,..)) || call(protected com.historicalengineering..new(*,..)) || call(public com.historicalengineering..new(*,..)); /* * Select all calls to any methods or constructors defined within * com.historicalengineering taking one or more parameters */ pointcut allCallsWithParameters() : allConstructorCallsWithParameters() || allMethodCallsWithParameters(); before() : allCallsWithParameters() { Object[] args = thisJoinPoint.getArgs(); for(int i = 0; i < args.length; i++) { if(args[i] == null) { throw new IllegalArgumentException( thisJoinPointStaticPart.getSignature() +" passed null parameter at position "+(i+1) +" of "+args.length); } } } } |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| Second the test class |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| package com.historicalengineering.wwii.aspect.test; public class A { public static void main(String[] args) { //A a = new A(); //InnerStatic i = new InnerStatic(); //a.foo(); //a.bar(); } protected void foo() { InnerStatic inner = new InnerStatic(); } protected void bar() { Inner inner = new Inner(); } static class InnerStatic { private InnerStatic() { } } class Inner { private Inner() { } } } |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| Third some output |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| Output when uncommenting the line: A a = new A(); -None because this constructor really doesn't take any parameters. Output when uncommenting the line: InnerStatic i = new InnerStatic(); Exception in thread "main" java.lang.IllegalArgumentException: com.historicalengineering.wwii.aspect.test.A.InnerStatic(A.InnerStatic) passed null parameter at position 1 of 1 at com.historicalengineering.wwii.aspect.test.EnforceContractAspect.ajc$before$com_historicalengineering_wwii_aspect_test_EnforceContractAspect$1$f789a125(EnforceContractAspect.aj:45) at com.historicalengineering.wwii.aspect.test.A.main(A.java:9) - As you can see from the trace the inner static constructor was passed an "A.InnerStatic" type which has evaluated to null. Output when uncommenting the lines: A a = new A(); & a.foo(); Exception in thread "main" java.lang.IllegalArgumentException: com.historicalengineering.wwii.aspect.test.A.InnerStatic(A.InnerStatic) passed null parameter at position 1 of 1 at com.historicalengineering.wwii.aspect.test.EnforceContractAspect.ajc$before$com_historicalengineering_wwii_aspect_test_EnforceContractAspect$1$f789a125(EnforceContractAspect.aj:40) at com.historicalengineering.wwii.aspect.test.A.foo(A.java:17) at com.historicalengineering.wwii.aspect.test.A.main(A.java:10) - Again the null "A.InnerStatic" type has been passed to the constructor. Output when uncommenting the lines: A a = new A(); & a.bar(); Exception in thread "main" java.lang.IllegalArgumentException: com.historicalengineering.wwii.aspect.test.A.Inner(A, A.Inner) passed null parameter at position 2 of 2 at com.historicalengineering.wwii.aspect.test.EnforceContractAspect.ajc$before$com_historicalengineering_wwii_aspect_test_EnforceContractAspect$1$f789a125(EnforceContractAspect.aj:40) at com.historicalengineering.wwii.aspect.test.A.bar(A.java:22) at com.historicalengineering.wwii.aspect.test.A.main(A.java:11) -In the case of the non static nested class it is passed 2 hidden parameters the enclosing class "A" which does not evaluate to null and again the infuriating "A.Inner" class which is null.
Thanks for posting. This is an AspectJ bug.
The generation of the extra constructors and the passing of extra parameters is nothing to do with AspectJ - it simply makes them visible because it exposes what is in the bytecode. > This is incorrect according to the Java language specification I can't find the section that covers this - which is it? If I compile your standalone java class with either javac or the JDT compiler, I get the various extra constructors and parameters passed around. AspectJ is merely exposing those. Now whether AspectJ should expose them is another matter but currently it is behaving as designed by exposing what is in the bytecode. Depending on whether I use javac or JDT, I do see different results but the .class files resulting will execute in the same way. Here is a simple variant: public class A { static class InnerStatic { private InnerStatic() { } } } > javac A.java > javap -private A$InnerStatic class A$InnerStatic extends java.lang.Object{ private A$InnerStatic(); } all is as expected. Now if we change it: public class A { protected void foo() { InnerStatic inner = new InnerStatic(); } static class InnerStatic { private InnerStatic() { } } } There is now a call to the constructor for the inner class from a non-static context in the outer class. > javac A.java > javap -private A$InnerStatic class A$InnerStatic extends java.lang.Object{ private A$InnerStatic(); A$InnerStatic(A$1); } Or after using the JDT compiler: > javap -private A$InnerStatic class A$InnerStatic extends java.lang.Object{ private A$InnerStatic(); A$InnerStatic(A$InnerStatic); } Now the original constructor was declared private, so it has to perform a little trick to get around visibility rules since they are effectively two different classes. The trick is to generate another constructor, one that it can see - it needs a parameter since otherwise it would have the same signature as the existing private constructor. The value passed as the parameter to this constructor doesn't really matter (so you see null) and the implementation is simply a this() to the private constructor. The various visibilities in this program are what is causing the various extra constructors and synthetic arguments to be passed around. > In the case of the non static nested class it is passed 2 hidden parameters > the enclosing class "A" which does not evaluate to null and again the > infuriating "A.Inner" class which is null. This is the same issue. In the non-static class a variation on the private ctor needs creating that can be called from the outer class. Hence the extra parameter. It gets more fun too. Suppose there already was a private ctor in that type which took an instance of A$InnerStatic (I'm considering the JDT compiler case here). Well the JDT compiler adds a third constructor: class A$InnerStatic extends java.lang.Object{ private A$InnerStatic(); private A$InnerStatic(A$InnerStatic); A$InnerStatic(A$InnerStatic, A$InnerStatic); } In fact it will keep adding parameters until there is no clash with an existing constructor. I'm not sure what I would change AspectJ to do. This is similar to a problem due to annotation matching that we have where the synthetic parameters flying around damage the argument order (causing us to consider an annotation as on the wrong parameter). Due to earlier compilation with javac (in the case of annotation style) or even just due to binary weaving, we cannot stick anything extra in the bytecode to tag these peculiarities earlier on - so if we are to do anything we have to determine the situation based on the bytecode. This is very tricky, there aren't many markers around. The generated extra ctor is marked synthetic, but the extraneous parameters are not individually marked. We can recognize a pattern but the javac and jdt compilers do different things...
not going to get to this in the short term
