diff options
10 files changed, 207 insertions, 33 deletions
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java index 4d0e4cae4..ffc6333db 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java @@ -23,6 +23,7 @@ import org.aspectj.ajdt.internal.compiler.ast.PointcutDeclaration; //import org.aspectj.asm.internal.Relationship; import org.aspectj.bridge.IMessage; import org.aspectj.weaver.*; +import org.aspectj.weaver.bcel.LazyClassGen; import org.aspectj.weaver.patterns.*; import org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration; import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration; @@ -359,9 +360,26 @@ public class AjLookupEnvironment extends LookupEnvironment { newI[n] = parentBinding; } sourceType.superInterfaces = newI; + warnOnAddedInterface(factory.fromEclipse(sourceType),parent); } } + + public void warnOnAddedInterface (ResolvedTypeX type, ResolvedTypeX parent) { + World world = factory.getWorld(); + ResolvedTypeX serializable = world.resolve(TypeX.SERIALIZABLE); + if (serializable.isAssignableFrom(type) + && !serializable.isAssignableFrom(parent) + && !LazyClassGen.hasSerialVersionUIDField(type)) { + world.getLint().needsSerialVersionUIDField.signal( + new String[] { + type.getName().toString(), + "added interface " + parent.getName().toString() + }, + null, + null); + } + } diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/BcweaverJarMaker.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/BcweaverJarMaker.java index aa2b13276..25e621ca1 100644 --- a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/BcweaverJarMaker.java +++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/BcweaverJarMaker.java @@ -165,6 +165,18 @@ public class BcweaverJarMaker { args.add("../tests/new/options11/injar/*.java"); CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS); + + args = new ArrayList(); + + args.add("-classpath"); + args.add("../lib/test/aspectjrt.jar;../lib/test/testing-client.jar" + + File.pathSeparator + System.getProperty("aspectjrt.path")); + args.add("-outjar"); + args.add("../tests/bugs/serialVersionUID/injar.jar"); + args.add("../tests/bugs/serialVersionUID/Test.java"); + args.add("../tests/bugs/serialVersionUID/Util.java"); + + CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS); } diff --git a/weaver/src/org/aspectj/weaver/Lint.java b/weaver/src/org/aspectj/weaver/Lint.java index c9fc271cd..b1be8a276 100644 --- a/weaver/src/org/aspectj/weaver/Lint.java +++ b/weaver/src/org/aspectj/weaver/Lint.java @@ -47,12 +47,18 @@ public class Lint { public final Kind shadowNotInStructure = new Kind("shadowNotInStructure", "the shadow for this join point is not exposed in the structure model: {0}"); - public final Kind unmatchedSuperTypeInCall = - new Kind("unmatchedSuperTypeInCall", "does not match because declaring type is {0}, if match desired use target({1})"); - + public final Kind unmatchedSuperTypeInCall = + new Kind("unmatchedSuperTypeInCall", "does not match because declaring type is {0}, if match desired use target({1})"); + public final Kind canNotImplementLazyTjp = new Kind("canNotImplementLazyTjp", "can not implement lazyTjp on this joinpoint {0} because around advice is used"); + public final Kind needsSerialVersionUIDField = + new Kind("needsSerialVersionUIDField", "serialVersionUID of type {0} needs to be set because of {1}"); + + public final Kind serialVersionUIDBroken = + new Kind("brokeSerialVersionCompatibility", "serialVersionUID of type {0} is broken because of added field {1}"); + public Lint(World world) { this.world = world; } diff --git a/weaver/src/org/aspectj/weaver/XlintDefault.properties b/weaver/src/org/aspectj/weaver/XlintDefault.properties index 7ddba894d..91ce5d469 100644 --- a/weaver/src/org/aspectj/weaver/XlintDefault.properties +++ b/weaver/src/org/aspectj/weaver/XlintDefault.properties @@ -9,4 +9,7 @@ shadowNotInStructure = ignore unmatchedSuperTypeInCall = warning -canNotImplementLazyTjp = warning
\ No newline at end of file +canNotImplementLazyTjp = warning + +needsSerialVersionUIDField = ignore +brokeSerialVersionCompatibility = ignore
\ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java index 65b90ef2e..ddc6392b7 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java @@ -13,15 +13,29 @@ package org.aspectj.weaver.bcel; -import java.util.*; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import org.apache.bcel.generic.*; -import org.aspectj.weaver.*; +import org.apache.bcel.generic.InstructionFactory; +import org.apache.bcel.generic.InstructionHandle; +import org.apache.bcel.generic.InstructionList; +import org.aspectj.weaver.Advice; +import org.aspectj.weaver.AdviceKind; +import org.aspectj.weaver.AjAttribute; +import org.aspectj.weaver.BCException; +import org.aspectj.weaver.ISourceContext; +import org.aspectj.weaver.Member; +import org.aspectj.weaver.ResolvedMember; +import org.aspectj.weaver.ResolvedTypeX; +import org.aspectj.weaver.Shadow; +import org.aspectj.weaver.TypeX; +import org.aspectj.weaver.World; import org.aspectj.weaver.ast.Literal; import org.aspectj.weaver.ast.Test; -import org.aspectj.weaver.patterns.*; +import org.aspectj.weaver.patterns.ExactTypePattern; +import org.aspectj.weaver.patterns.ExposedState; +import org.aspectj.weaver.patterns.Pointcut; /** * Advice implemented for bcel. @@ -89,14 +103,17 @@ public class BcelAdvice extends Advice { // make sure thisJoinPoint parameters are initialized if ((getExtraParameterFlags() & ThisJoinPointStaticPart) != 0) { ((BcelShadow)shadow).getThisJoinPointStaticPartVar(); + ((BcelShadow)shadow).getEnclosingClass().warnOnAddedStaticInitializer(shadow,getSourceLocation()); } if ((getExtraParameterFlags() & ThisJoinPoint) != 0) { - ((BcelShadow)shadow).requireThisJoinPoint(pointcutTest != Literal.TRUE && getKind() != AdviceKind.Around); + ((BcelShadow)shadow).requireThisJoinPoint(pointcutTest != Literal.TRUE && getKind() != AdviceKind.Around); + ((BcelShadow)shadow).getEnclosingClass().warnOnAddedStaticInitializer(shadow,getSourceLocation()); } if ((getExtraParameterFlags() & ThisEnclosingJoinPointStaticPart) != 0) { ((BcelShadow)shadow).getThisEnclosingJoinPointStaticPartVar(); + ((BcelShadow)shadow).getEnclosingClass().warnOnAddedStaticInitializer(shadow,getSourceLocation()); } } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java index 0de033913..0a1c8235e 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java @@ -40,7 +40,7 @@ public class BcelCflowStackFieldAdder extends BcelTypeMunger { BcelWorld.makeBcelType(cflowStackField.getReturnType()), cflowStackField.getName(), gen.getConstantPoolGen()).getField(); - gen.addField(f); + gen.addField(f,getSourceLocation()); LazyMethodGen clinit = gen.getAjcPreClinit(); //StaticInitializer(); InstructionList setup = new InstructionList(); diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index e0997573a..1c3b93be0 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -1115,6 +1115,9 @@ class BcelClassWeaver implements IClassWeaver { if (munger.match(shadow, world)) { shadow.addMunger(munger); isMatched = true; + if (shadow.getKind() == Shadow.StaticInitialization) { + clazz.warnOnAddedStaticInitializer(shadow,munger.getSourceLocation()); + } } } if (isMatched) shadowAccumulator.add(shadow); diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java index c4ce575bb..fc5c0c1a8 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java @@ -995,6 +995,7 @@ public class BcelShadow extends Shadow { world.resolve(TypeX.forName("org.aspectj.lang.JoinPoint$StaticPart")), getEnclosingClass().getClassName(), field.getName()); +// getEnclosingClass().warnOnAddedStaticInitializer(this,munger.getSourceLocation()); } return thisJoinPointStaticPartVar; } @@ -1434,7 +1435,9 @@ public class BcelShadow extends Shadow { extractMethod( NameMangler.aroundCallbackMethodName( getSignature(), - getEnclosingClass())); + getEnclosingClass()), + Modifier.PRIVATE, + munger); // now extract the advice into its own method @@ -1498,7 +1501,7 @@ public class BcelShadow extends Shadow { LazyMethodGen localAdviceMethod = new LazyMethodGen( - Modifier.FINAL | Modifier.STATIC, + Modifier.PRIVATE | Modifier.FINAL | Modifier.STATIC, adviceMethod.getReturnType(), adviceMethodName, parameterTypes, @@ -1694,7 +1697,9 @@ public class BcelShadow extends Shadow { extractMethod( NameMangler.aroundCallbackMethodName( getSignature(), - getEnclosingClass())); + getEnclosingClass()), + 0, + munger); BcelVar[] adviceVars = munger.getExposedStateAsBcelVars(); @@ -1979,10 +1984,10 @@ public class BcelShadow extends Shadow { // ---- extraction methods - public LazyMethodGen extractMethod(String newMethodName) { + public LazyMethodGen extractMethod(String newMethodName, int visibilityModifier, ShadowMunger munger) { LazyMethodGen.assertGoodBody(range.getBody(), newMethodName); if (!getKind().allowsExtraction()) throw new BCException(); - LazyMethodGen freshMethod = createMethodGen(newMethodName); + LazyMethodGen freshMethod = createMethodGen(newMethodName,visibilityModifier); // System.err.println("******"); // System.err.println("ABOUT TO EXTRACT METHOD for" + this); @@ -2000,7 +2005,7 @@ public class BcelShadow extends Shadow { freshMethod, getSuperConstructorParameterTypes()); } - getEnclosingClass().addMethodGen(freshMethod); + getEnclosingClass().addMethodGen(freshMethod,munger.getSourceLocation()); return freshMethod; } @@ -2101,9 +2106,9 @@ public class BcelShadow extends Shadow { * If it's argsOnFrame, it shares this and target. * ??? rewrite this to do less array munging, please */ - private LazyMethodGen createMethodGen(String newMethodName) { + private LazyMethodGen createMethodGen(String newMethodName, int visibilityModifier) { Type[] parameterTypes = BcelWorld.makeBcelTypes(getArgTypes()); - int modifiers = Modifier.FINAL; + int modifiers = Modifier.FINAL | visibilityModifier; // XXX some bug // if (! isExpressionKind() && getSignature().isStrict(world)) { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java index c3fdb2f6a..2fef3a0c1 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java @@ -87,7 +87,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { if (newParent.isClass()) { //gen.setSuperClass(newParent); } else { - gen.addInterface(newParent); + gen.addInterface(newParent,getSourceLocation()); } return true; } @@ -158,7 +158,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { il.append(InstructionFactory.createReturn(BcelWorld.makeBcelType(field.getType()))); mg.getBody().insert(il); - gen.addMethodGen(mg); + gen.addMethodGen(mg,getSignature().getSourceLocation()); } private void addFieldSetter( @@ -188,7 +188,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { il.append(InstructionFactory.createReturn(Type.VOID)); mg.getBody().insert(il); - gen.addMethodGen(mg); + gen.addMethodGen(mg,getSignature().getSourceLocation()); } private void addMethodDispatch( @@ -260,7 +260,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { FieldGen fg = makeFieldGen(gen, AjcMemberMaker.perObjectField(gen.getType(), aspectType)); - gen.addField(fg.getField()); + gen.addField(fg.getField(),getSourceLocation()); Type fieldType = BcelWorld.makeBcelType(aspectType); @@ -301,7 +301,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { gen.addMethodGen(mg1); - gen.addInterface(munger.getInterfaceType()); + gen.addInterface(munger.getInterfaceType(),getSourceLocation()); return true; } else { @@ -357,6 +357,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { // XXX make sure to check that we set exceptions properly on this guy. weaver.addLazyMethodGen(mg); + weaver.getLazyClassGen().warnOnAddedMethod(mg.getMethod(),getSignature().getSourceLocation()); addNeededSuperCallMethods(weaver, onType, munger.getSuperMethodsCalled()); @@ -593,7 +594,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { weaver.addInitializer(this); FieldGen fg = makeFieldGen(gen, AjcMemberMaker.interFieldClassField(field, aspectType)); - gen.addField(fg.getField()); + gen.addField(fg.getField(),getSourceLocation()); } return true; } else if (onInterface && gen.getType().isTopmostImplementor(onType)) { @@ -605,7 +606,7 @@ public class BcelTypeMunger extends ConcreteTypeMunger { FieldGen fg = makeFieldGen(gen, AjcMemberMaker.interFieldInterfaceField(field, onType, aspectType)); - gen.addField(fg.getField()); + gen.addField(fg.getField(),getSourceLocation()); //this uses a shadow munger to add init method to constructors //weaver.getShadowMungers().add(makeInitCallShadowMunger(initMethod)); diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java index c00320171..67b00ebf8 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java @@ -48,14 +48,18 @@ import org.apache.bcel.generic.PUSH; import org.apache.bcel.generic.RETURN; import org.apache.bcel.generic.Type; import org.aspectj.bridge.IMessage; +import org.aspectj.bridge.ISourceLocation; import org.aspectj.util.CollectionUtil; import org.aspectj.weaver.AjAttribute; import org.aspectj.weaver.BCException; import org.aspectj.weaver.Member; import org.aspectj.weaver.NameMangler; +import org.aspectj.weaver.ResolvedMember; import org.aspectj.weaver.ResolvedTypeX; +import org.aspectj.weaver.Shadow; import org.aspectj.weaver.TypeX; import org.aspectj.weaver.WeaverStateInfo; +import org.aspectj.weaver.World; public final class LazyClassGen { @@ -203,6 +207,10 @@ public final class LazyClassGen { } private InstructionFactory fact; + + private boolean isSerializable = false; + private boolean hasSerialVersionUIDField = false; + private boolean hasClinit = false; // ---- public LazyClassGen( @@ -224,12 +232,49 @@ public final class LazyClassGen { fact = new InstructionFactory(myGen, constantPoolGen); this.myType = myType; + /* Does this class support serialization */ + if (TypeX.SERIALIZABLE.isAssignableFrom(getType(),getType().getWorld())) { + isSerializable = true; + +// ResolvedMember[] fields = getType().getDeclaredFields(); +// for (int i = 0; i < fields.length; i++) { +// ResolvedMember field = fields[i]; +// if (field.getName().equals("serialVersionUID") +// && field.isStatic() && field.getType().equals(ResolvedTypeX.LONG)) { +// hasSerialVersionUIDField = true; +// } +// } + hasSerialVersionUIDField = hasSerialVersionUIDField(getType()); + + ResolvedMember[] methods = getType().getDeclaredMethods(); + for (int i = 0; i < methods.length; i++) { + ResolvedMember method = methods[i]; + if (method.getName().equals("<clinit>")) { + hasClinit = true; + } + } + } + Method[] methods = myGen.getMethods(); for (int i = 0; i < methods.length; i++) { addMethodGen(new LazyMethodGen(methods[i], this)); } } + public static boolean hasSerialVersionUIDField (ResolvedTypeX type) { + + ResolvedMember[] fields = type.getDeclaredFields(); + for (int i = 0; i < fields.length; i++) { + ResolvedMember field = fields[i]; + if (field.getName().equals("serialVersionUID") + && field.isStatic() && field.getType().equals(ResolvedTypeX.LONG)) { + return true; + } + } + + return false; + } + // public void addAttribute(Attribute i) { // myGen.addAttribute(i); // } @@ -271,11 +316,59 @@ public final class LazyClassGen { } - public void addMethodGen(LazyMethodGen gen) { - //assert gen.getClassName() == super.getClassName(); - methodGens.add(gen); - if (highestLineNumber < gen.highestLineNumber) highestLineNumber = gen.highestLineNumber; - } + public void addMethodGen(LazyMethodGen gen) { + //assert gen.getClassName() == super.getClassName(); + methodGens.add(gen); + if (highestLineNumber < gen.highestLineNumber) highestLineNumber = gen.highestLineNumber; + } + + public void addMethodGen(LazyMethodGen gen, ISourceLocation sourceLocation) { + addMethodGen(gen); + if (!gen.getMethod().isPrivate()) { + warnOnAddedMethod(gen.getMethod(),sourceLocation); + } + } + + public void errorOnAddedField (Field field, ISourceLocation sourceLocation) { + if (isSerializable && !hasSerialVersionUIDField) { + getWorld().getLint().serialVersionUIDBroken.signal( + new String[] { + myType.getResolvedTypeX().getName().toString(), + field.getName() + }, + sourceLocation, + null); + } + } + + public void warnOnAddedInterface (String name, ISourceLocation sourceLocation) { + warnOnModifiedSerialVersionUID(sourceLocation,"added interface " + name); + } + + public void warnOnAddedMethod (Method method, ISourceLocation sourceLocation) { + warnOnModifiedSerialVersionUID(sourceLocation,"added non-private method " + method.getName()); + } + + public void warnOnAddedStaticInitializer (Shadow shadow, ISourceLocation sourceLocation) { + if (!hasClinit) { + warnOnModifiedSerialVersionUID(sourceLocation,"added static initializer"); + } + } + + public void warnOnModifiedSerialVersionUID (ISourceLocation sourceLocation, String reason) { + if (isSerializable && !hasSerialVersionUIDField) + getWorld().getLint().needsSerialVersionUIDField.signal( + new String[] { + myType.getResolvedTypeX().getName().toString(), + reason + }, + sourceLocation, + null); + } + + public World getWorld () { + return myType.getResolvedTypeX().getWorld(); + } public List getMethodGens() { return methodGens; //???Collections.unmodifiableList(methodGens); @@ -336,8 +429,9 @@ public final class LazyClassGen { classGens.add(newClass); } - public void addInterface(TypeX typeX) { + public void addInterface(TypeX typeX, ISourceLocation sourceLocation) { myGen.addInterface(typeX.getName()); + warnOnAddedInterface(typeX.getName(),sourceLocation); } public void setSuperClass(TypeX typeX) { @@ -551,7 +645,14 @@ public final class LazyClassGen { Field ret = (Field)tjpFields.get(shadow); if (ret != null) return ret; - ret = new FieldGen(Modifier.STATIC | Modifier.PUBLIC | Modifier.FINAL, + int modifiers = Modifier.STATIC | Modifier.FINAL; + if (getType().isInterface()) { + modifiers |= Modifier.PUBLIC; + } + else { + modifiers |= Modifier.PRIVATE; + } + ret = new FieldGen(modifiers, staticTjpType, "ajc$tjp_" + tjpFields.size(), getConstantPoolGen()).getField(); @@ -661,9 +762,17 @@ public final class LazyClassGen { return myGen.getFileName(); } - public void addField(Field field) { + private void addField(Field field) { myGen.addField(field); } + + public void addField(Field field, ISourceLocation sourceLocation) { + addField(field); + if (!(field.isPrivate() + && (field.isStatic() || field.isTransient()))) { + errorOnAddedField(field,sourceLocation); + } + } public String getClassName() { return myGen.getClassName(); |