From c57e9c7b63669925587390db9f250325cdc24e8f Mon Sep 17 00:00:00 2001 From: aclement Date: Wed, 18 Feb 2009 22:21:52 +0000 Subject: [PATCH] 265360: test and fix for annotation style references from string pointcuts --- .../ValidateAtAspectJAnnotationsVisitor.java | 73 ++-- .../compiler/lookup/EclipseScope.java | 332 +++++++++--------- 2 files changed, 222 insertions(+), 183 deletions(-) diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java index 8f1f624c1..711ec4538 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/ValidateAtAspectJAnnotationsVisitor.java @@ -68,18 +68,20 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { private static final char[] aspectSig = "Lorg/aspectj/lang/annotation/Aspect;".toCharArray(); private static final char[] declareParentsSig = "Lorg/aspectj/lang/annotation/DeclareParents;".toCharArray(); private static final char[] adviceNameSig = "Lorg/aspectj/lang/annotation/AdviceName;".toCharArray(); - // private static final char[] orgAspectJLangAnnotation = "org/aspectj/lang/annotation/".toCharArray(); + // private static final char[] orgAspectJLangAnnotation = + // "org/aspectj/lang/annotation/".toCharArray(); private static final char[] voidType = "void".toCharArray(); private static final char[] booleanType = "boolean".toCharArray(); private static final char[] joinPoint = "Lorg/aspectj/lang/JoinPoint;".toCharArray(); private static final char[] joinPointStaticPart = "Lorg/aspectj/lang/JoinPoint$StaticPart;".toCharArray(); private static final char[] joinPointEnclosingStaticPart = "Lorg/aspectj/lang/JoinPoint$EnclosingStaticPart;".toCharArray(); private static final char[] proceedingJoinPoint = "Lorg/aspectj/lang/ProceedingJoinPoint;".toCharArray(); - // private static final char[][] adviceSigs = new char[][] { beforeAdviceSig, afterAdviceSig, afterReturningAdviceSig, + // private static final char[][] adviceSigs = new char[][] { + // beforeAdviceSig, afterAdviceSig, afterReturningAdviceSig, // afterThrowingAdviceSig, aroundAdviceSig }; - private CompilationUnitDeclaration unit; - private Stack typeStack = new Stack(); + private final CompilationUnitDeclaration unit; + private final Stack typeStack = new Stack(); private AspectJAnnotations ajAnnotations; public ValidateAtAspectJAnnotationsVisitor(CompilationUnitDeclaration unit) { @@ -143,7 +145,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { } } } else { - // check that aspect doesn't have @Aspect annotation, we've already added on ourselves. + // check that aspect doesn't have @Aspect annotation, we've already + // added on ourselves. if (ajAnnotations.hasMultipleAspectAnnotations) { typeDecl.scope.problemReporter().signalError(typeDecl.sourceStart, typeDecl.sourceEnd, "aspects cannot have @Aspect annotation"); @@ -169,7 +172,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { CompilationAndWeavingContext.VALIDATING_AT_ASPECTJ_ANNOTATIONS, methodDeclaration.selector); ajAnnotations = new AspectJAnnotations(methodDeclaration.annotations); if (!methodDeclaration.getClass().equals(AjMethodDeclaration.class)) { - // simply test for innapropriate use of annotations on code-style members + // simply test for innapropriate use of annotations on code-style + // members if (methodDeclaration instanceof PointcutDeclaration) { if (ajAnnotations.hasMultiplePointcutAnnotations || ajAnnotations.hasAdviceAnnotation || ajAnnotations.hasAspectAnnotation || ajAnnotations.hasAdviceNameAnnotation) { @@ -221,7 +225,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { } /** - * aspect must be public nested aspect must be static cannot extend a concrete aspect pointcut in perclause must be good. + * aspect must be public nested aspect must be static cannot extend a + * concrete aspect pointcut in perclause must be good. */ private void validateAspectDeclaration(TypeDeclaration typeDecl) { if (typeStack.size() > 1) { @@ -243,7 +248,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { // FIXME AV - do we really want that // if (!Modifier.isPublic(typeDecl.modifiers)) { - // typeDecl.scope.problemReporter().signalError(typeDecl.sourceStart,typeDecl.sourceEnd,"@Aspect class must be public"); + // typeDecl.scope.problemReporter().signalError(typeDecl.sourceStart, + // typeDecl.sourceEnd,"@Aspect class must be public"); // } TypeReference parentRef = typeDecl.superclass; @@ -251,7 +257,9 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { TypeBinding parentBinding = parentRef.resolvedType; if (parentBinding instanceof SourceTypeBinding) { SourceTypeBinding parentSTB = (SourceTypeBinding) parentBinding; - if (parentSTB.scope != null) { // scope is null if its a binarytypebinding (in AJ world, thats a subclass of + if (parentSTB.scope != null) { // scope is null if its a + // binarytypebinding (in AJ + // world, thats a subclass of // SourceTypeBinding) TypeDeclaration parentDecl = parentSTB.scope.referenceContext; if (isAspect(parentDecl) && !Modifier.isAbstract(parentDecl.modifiers)) { @@ -266,7 +274,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { int[] pcLoc = new int[2]; String perClause = getStringLiteralFor("value", aspectAnnotation, pcLoc); - // AspectDeclaration aspectDecl = new AspectDeclaration(typeDecl.compilationResult); + // AspectDeclaration aspectDecl = new + // AspectDeclaration(typeDecl.compilationResult); try { if (perClause != null && !perClause.equals("")) { @@ -284,9 +293,10 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { } /** - * 1) Advice must be public 2) Advice must have a void return type if not around advice 3) Advice must not have any other @AspectJ - * annotations 4) After throwing advice must declare the thrown formal 5) After returning advice must declare the returning - * formal 6) Advice must not be static + * 1) Advice must be public 2) Advice must have a void return type if not + * around advice 3) Advice must not have any other @AspectJ annotations 4) + * After throwing advice must declare the thrown formal 5) After returning + * advice must declare the returning formal 6) Advice must not be static */ private void validateAdvice(MethodDeclaration methodDeclaration) { @@ -382,7 +392,8 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { FormalBinding[] bindings = buildFormalAdviceBindingsFrom(methodDeclaration); pc.resolve(new EclipseScope(bindings, methodDeclaration.scope)); EclipseFactory factory = EclipseFactory.fromScopeLookupEnvironment(methodDeclaration.scope); - // now create a ResolvedPointcutDefinition,make an attribute out of it, and add it to the method + // now create a ResolvedPointcutDefinition,make an attribute out of + // it, and add it to the method UnresolvedType[] paramTypes = new UnresolvedType[bindings.length]; for (int i = 0; i < paramTypes.length; i++) paramTypes[i] = bindings[i].getType(); @@ -528,20 +539,31 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { pcDecl.setGenerateSyntheticPointcutMethod(); TypeDeclaration onType = (TypeDeclaration) typeStack.peek(); pcDecl.postParse(onType); - // EclipseFactory factory = EclipseFactory.fromScopeLookupEnvironment(methodDeclaration.scope); - // int argsLength = methodDeclaration.arguments == null ? 0 : methodDeclaration.arguments.length; + // EclipseFactory factory = + // EclipseFactory.fromScopeLookupEnvironment + // (methodDeclaration.scope); + // int argsLength = methodDeclaration.arguments == null ? 0 : + // methodDeclaration.arguments.length; FormalBinding[] bindings = buildFormalAdviceBindingsFrom(methodDeclaration); // FormalBinding[] bindings = new FormalBinding[argsLength]; // for (int i = 0, len = bindings.length; i < len; i++) { // Argument arg = methodDeclaration.arguments[i]; // String name = new String(arg.name); - // UnresolvedType type = factory.fromBinding(methodDeclaration.binding.parameters[i]); - // bindings[i] = new FormalBinding(type, name, i, arg.sourceStart, arg.sourceEnd, "unknown"); + // UnresolvedType type = + // factory.fromBinding(methodDeclaration.binding.parameters[i]); + // bindings[i] = new FormalBinding(type, name, i, arg.sourceStart, + // arg.sourceEnd, "unknown"); // } swap(onType, methodDeclaration, pcDecl); if (pc != null) { // has an expression - pc.resolve(new EclipseScope(bindings, methodDeclaration.scope)); + EclipseScope eScope = new EclipseScope(bindings, methodDeclaration.scope); + char[] packageName = null; + if (typeDecl.binding != null && typeDecl.binding.getPackage() != null) { + packageName = typeDecl.binding.getPackage().readableName(); + } + eScope.setLimitedImports(packageName); + pc.resolve(eScope); HasIfPCDVisitor ifFinder = new HasIfPCDVisitor(); pc.traverse(ifFinder, null); containsIfPcd = ifFinder.containsIfPcd; @@ -579,10 +601,17 @@ public class ValidateAtAspectJAnnotationsVisitor extends ASTVisitor { } if (pcDecl.pointcutDesignator == null) { - if (Modifier.isAbstract(methodDeclaration.modifiers) || noValueSupplied // this is a matches nothing pointcut - // those 2 checks makes sense for aop.xml concretization but NOT for regular abstraction of pointcut + if (Modifier.isAbstract(methodDeclaration.modifiers) || noValueSupplied // this + // is + // a + // matches + // nothing + // pointcut + // those 2 checks makes sense for aop.xml concretization but NOT for + // regular abstraction of pointcut // && returnsVoid - // && (methodDeclaration.arguments == null || methodDeclaration.arguments.length == 0)) { + // && (methodDeclaration.arguments == null || + // methodDeclaration.arguments.length == 0)) { ) { // fine } else { diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseScope.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseScope.java index 6290b7d3e..af1ad6705 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseScope.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseScope.java @@ -10,7 +10,6 @@ * PARC initial implementation * ******************************************************************/ - package org.aspectj.ajdt.internal.compiler.lookup; import java.util.ArrayList; @@ -20,194 +19,194 @@ import org.aspectj.bridge.IMessage; import org.aspectj.bridge.IMessageHandler; import org.aspectj.bridge.ISourceLocation; import org.aspectj.bridge.Message; -import org.aspectj.weaver.IHasPosition; -import org.aspectj.weaver.ResolvedType; -import org.aspectj.weaver.UnresolvedType; -import org.aspectj.weaver.World; -import org.aspectj.weaver.patterns.*; +import org.aspectj.org.eclipse.jdt.core.compiler.CharOperation; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ClassScope; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ImportBinding; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.Scope; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.TypeBinding; -import org.aspectj.org.eclipse.jdt.core.compiler.CharOperation; +import org.aspectj.weaver.IHasPosition; +import org.aspectj.weaver.ResolvedType; +import org.aspectj.weaver.UnresolvedType; +import org.aspectj.weaver.World; +import org.aspectj.weaver.patterns.FormalBinding; +import org.aspectj.weaver.patterns.IScope; +import org.aspectj.weaver.patterns.WildTypePattern; /** - * Adaptor from org.eclipse.jdt.internal.compiler.lookup.Scope to org.aspectj.weaver.IScope + * Adaptor from org.eclipse.jdt.internal.compiler.lookup.Scope to + * org.aspectj.weaver.IScope * * @author Jim Hugunin */ public class EclipseScope implements IScope { - private Scope scope; - private EclipseFactory world; - private ResolvedType enclosingType; - private FormalBinding[] bindings; - + private static final char[] javaLang = "java.lang".toCharArray(); + private static final String[] JL = new String[] { "java.lang." }; + private static final String[] NONE = new String[0]; + private final Scope scope; + private final EclipseFactory world; + private final ResolvedType enclosingType; + private final FormalBinding[] bindings; + private String[] importedPrefixes = null; private String[] importedNames = null; - public EclipseScope(FormalBinding[] bindings, Scope scope) { this.bindings = bindings; - + this.scope = scope; - this.world = EclipseFactory.fromScopeLookupEnvironment(scope); - - this.enclosingType = world.fromEclipse(scope.enclosingSourceType()); + world = EclipseFactory.fromScopeLookupEnvironment(scope); + + enclosingType = world.fromEclipse(scope.enclosingSourceType()); } - - + public UnresolvedType lookupType(String name, IHasPosition location) { - char[][] splitName = WildTypePattern.splitNames(name,true); - TypeBinding b = scope.getType(splitName,splitName.length); - //FIXME ??? need reasonable error handling... + char[][] splitName = WildTypePattern.splitNames(name, true); + TypeBinding b = scope.getType(splitName, splitName.length); + // FIXME ??? need reasonable error handling... if (!b.isValidBinding()) { return ResolvedType.MISSING; } - - //System.err.println("binding: " + b); - // Binding(tokens, bits & RestrictiveFlagMASK, this) - return world.fromBinding(b); - - /* - computeImports(); - -// System.out.println("lookup: " + name + " in " + -// Arrays.asList(importedPrefixes)); - - ResolvedType ret = null; - String dotName = "." + name; - for (int i=0; i 0) { importedPrefixesList.add(packageName + "."); } - ImportBinding[] imports = cuScope.imports; for (int i = 0; i < imports.length; i++) { ImportBinding importBinding = imports[i]; - String importName = - new String(CharOperation.concatWith(importBinding.compoundName, '.')); - - //XXX wrong behavior for java.util.Map.* + String importName = new String(CharOperation.concatWith(importBinding.compoundName, '.')); + + // XXX wrong behavior for java.util.Map.* if (importBinding.onDemand) { importedPrefixesList.add(importName + "."); } else { importedNamesList.add(importName); } } - + TypeBinding[] topTypes = cuScope.topLevelTypes; for (int i = 0; i < topTypes.length; i++) { importedNamesList.add(world.fromBinding(topTypes[i]).getName()); } - - importedNames = - (String[])importedNamesList.toArray(new String[importedNamesList.size()]); - - importedPrefixes = - (String[])importedPrefixesList.toArray(new String[importedPrefixesList.size()]); + + importedNames = (String[]) importedNamesList.toArray(new String[importedNamesList.size()]); + + importedPrefixes = (String[]) importedPrefixesList.toArray(new String[importedPrefixesList.size()]); } - private void addClassAndParentsToPrefixes( - ReferenceBinding binding, - List importedPrefixesList) - { - if (binding == null) return; - importedPrefixesList.add(world.fromBinding(binding).getName()+"$"); - + private void addClassAndParentsToPrefixes(ReferenceBinding binding, List importedPrefixesList) { + if (binding == null) + return; + importedPrefixesList.add(world.fromBinding(binding).getName() + "$"); + addClassAndParentsToPrefixes(binding.superclass(), importedPrefixesList); ReferenceBinding[] superinterfaces = binding.superInterfaces(); if (superinterfaces != null) { @@ -216,7 +215,6 @@ public class EclipseScope implements IScope { } } } - public String[] getImportedNames() { computeImports(); @@ -225,57 +223,48 @@ public class EclipseScope implements IScope { public String[] getImportedPrefixes() { computeImports(); - //System.err.println("prefixes: " + Arrays.asList(importedPrefixes)); + // System.err.println("prefixes: " + Arrays.asList(importedPrefixes)); return importedPrefixes; } - - //XXX add good errors when would bind to extra parameters - public FormalBinding lookupFormal(String name) { - for (int i = 0, len = bindings.length; i < len; i++) { - if (bindings[i].getName().equals(name)) return bindings[i]; - } - return null; - } - + // XXX add good errors when would bind to extra parameters + public FormalBinding lookupFormal(String name) { + for (int i = 0, len = bindings.length; i < len; i++) { + if (bindings[i].getName().equals(name)) + return bindings[i]; + } + return null; + } public FormalBinding getFormal(int i) { return bindings[i]; } - public int getFormalCount() { - return bindings.length; - } + public int getFormalCount() { + return bindings.length; + } public ISourceLocation makeSourceLocation(IHasPosition location) { - return new EclipseSourceLocation(scope.problemReporter().referenceContext.compilationResult(), - location.getStart(), location.getEnd()); + return new EclipseSourceLocation(scope.problemReporter().referenceContext.compilationResult(), location.getStart(), + location.getEnd()); } public IMessageHandler getMessageHandler() { return world.getWorld().getMessageHandler(); } - - public void message( - IMessage.Kind kind, - IHasPosition location1, - IHasPosition location2, - String message) { - message(kind, location1, message); - message(kind, location2, message); + public void message(IMessage.Kind kind, IHasPosition location1, IHasPosition location2, String message) { + message(kind, location1, message); + message(kind, location2, message); } - public void message( - IMessage.Kind kind, - IHasPosition location, - String message) { - //System.out.println("message: " + message + " loc: " + makeSourceLocation(location)); - getMessageHandler() - .handleMessage(new Message(message, kind, null, makeSourceLocation(location))); + public void message(IMessage.Kind kind, IHasPosition location, String message) { + // System.out.println("message: " + message + " loc: " + + // makeSourceLocation(location)); + getMessageHandler().handleMessage(new Message(message, kind, null, makeSourceLocation(location))); } - + public void message(IMessage aMessage) { getMessageHandler().handleMessage(aMessage); } @@ -288,4 +277,25 @@ public class EclipseScope implements IScope { return enclosingType; } + private boolean referenceFromAnnotationStylePointcut = false; + private char[] validPackage; + + /** + * Mark this scope as only allowing limited support for imports. This is to + * ensure that references in annotation style pointcuts are accidentally + * resolved against import statements. They won't be if javac is used (and + * the resulting .class file will contain 'bad pointcuts') so this method + * enables it to also be policed when compiling with ajc. + * + * @param validPackage unqualified references can be resolved if the type is + * in the same package as the type containing the pointcut + * declaration. + */ + public void setLimitedImports(char[] validPackage) { + referenceFromAnnotationStylePointcut = true; + this.validPackage = validPackage; + importedPrefixes = JL; // Consider only java.lang. as an import + importedNames = NONE; // No imported names + } + } -- 2.39.5