From: jhugunin Date: Mon, 23 Dec 2002 22:17:50 +0000 (+0000) Subject: better errors for bad named pointcut declarations and references X-Git-Tag: V_1_1_b5~220 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=025e908673ceaa62c49c1570fa0264f483755029;p=aspectj.git better errors for bad named pointcut declarations and references handling concrete aspect correctly for cflow fields --- diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java index 40d3448cc..6939d8580 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseObjectType.java @@ -128,31 +128,34 @@ public class EclipseObjectType extends ResolvedTypeX.Name { //XXX make sure this is applied to classes and interfaces public void checkPointcutDeclarations() { ResolvedMember[] pointcuts = getDeclaredPointcuts(); + boolean sawError = false; for (int i=0, len=pointcuts.length; i < len; i++) { if (pointcuts[i].isAbstract()) { if (!this.isAspect()) { - MessageUtil.error( - "abstract pointcut only allowed in aspect" + pointcuts[i].getName(), - pointcuts[i].getSourceLocation()); + eclipseWorld().showMessage(IMessage.ERROR, + "abstract pointcut only allowed in aspect" + pointcuts[i].getName(), + pointcuts[i].getSourceLocation(), null); + sawError = true; } else if (!this.isAbstract()) { - MessageUtil.error( - "abstract pointcut in concrete aspect" + pointcuts[i].getName(), - pointcuts[i].getSourceLocation()); + eclipseWorld().showMessage(IMessage.ERROR, + "abstract pointcut in concrete aspect" + pointcuts[i], + pointcuts[i].getSourceLocation(), null); + sawError = true; } } for (int j=i+1; j < len; j++) { if (pointcuts[i].getName().equals(pointcuts[j].getName())) { - eclipseWorld().getMessageHandler().handleMessage( - MessageUtil.error( - "duplicate pointcut name: " + pointcuts[j].getName(), - pointcuts[j].getSourceLocation())); + eclipseWorld().showMessage(IMessage.ERROR, + "duplicate pointcut name: " + pointcuts[j].getName(), + pointcuts[i].getSourceLocation(), pointcuts[j].getSourceLocation()); + sawError = true; } } } //now check all inherited pointcuts to be sure that they're handled reasonably - if (!isAspect()) return; + if (sawError || !isAspect()) return; 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 49fef3dc6..1035d2932 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 @@ -144,7 +144,8 @@ public class EclipseScope implements IScope { String packageName = new String(CharOperation.concatWith(cuScope.currentPackageName, '.')); - if (packageName.length() > 1) { + //System.err.println("package: " + packageName); + if (packageName.length() > 0) { importedPrefixesList.add(packageName + "."); } diff --git a/weaver/src/org/aspectj/weaver/Advice.java b/weaver/src/org/aspectj/weaver/Advice.java index 4c5dcb47b..361c4a93f 100644 --- a/weaver/src/org/aspectj/weaver/Advice.java +++ b/weaver/src/org/aspectj/weaver/Advice.java @@ -15,6 +15,7 @@ package org.aspectj.weaver; import java.util.*; +import org.aspectj.bridge.*; import org.aspectj.bridge.MessageUtil; import org.aspectj.weaver.patterns.*; @@ -85,6 +86,16 @@ public abstract class Advice extends ShadowMunger { public boolean match(Shadow shadow, World world) { if (super.match(shadow, world)) { + if (shadow.getKind() == Shadow.ExceptionHandler) { + if (kind.isAfter() || kind == AdviceKind.Around) { + world.showMessage(IMessage.WARNING, + "Only before advice is supported on handler join points (compiler limitation)", + getSourceLocation(), shadow.getSourceLocation()); + return false; + } + } + + if (hasExtraParameter() && kind == AdviceKind.AfterReturning) { return getExtraParameterType().isConvertableFrom(shadow.getReturnType(), world); } else if (kind == AdviceKind.PerTargetEntry) { @@ -93,12 +104,9 @@ public abstract class Advice extends ShadowMunger { return shadow.hasThis(); } else if (kind == AdviceKind.Around) { if (shadow.getKind() == Shadow.PreInitialization) { - world.getMessageHandler().handleMessage( - MessageUtil.error("Around on pre-initialization not supported in 1.1", - getSourceLocation())); - world.getMessageHandler().handleMessage( - MessageUtil.error("Around on pre-initialization not supported in 1.1", - shadow.getSourceLocation())); + world.showMessage(IMessage.WARNING, + "around on pre-initialization not supported (compiler limitation)", + getSourceLocation(), shadow.getSourceLocation()); return false; } else { if (!getSignature().getReturnType().isConvertableFrom(shadow.getReturnType(), world)) { diff --git a/weaver/src/org/aspectj/weaver/Checker.java b/weaver/src/org/aspectj/weaver/Checker.java index eec7d05af..4732a8671 100644 --- a/weaver/src/org/aspectj/weaver/Checker.java +++ b/weaver/src/org/aspectj/weaver/Checker.java @@ -30,7 +30,7 @@ public class Checker extends ShadowMunger { } public ShadowMunger concretize(ResolvedTypeX fromType, World world, PerClause clause) { - pointcut = pointcut.concretize(fromType, 0); + pointcut = pointcut.concretize(fromType, 0, this); return this; } diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java index ab60e6897..22e73efb7 100644 --- a/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java +++ b/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java @@ -66,7 +66,7 @@ public class CrosscuttingMembers { } private void addShadowMunger(ShadowMunger m) { - //if (inAspect.isAbstract()) return; // we don't do mungers for abstract aspects + if (inAspect.isAbstract()) return; // we don't do mungers for abstract aspects addConcreteShadowMunger(m.concretize(inAspect, world, perClause)); } diff --git a/weaver/src/org/aspectj/weaver/IntMap.java b/weaver/src/org/aspectj/weaver/IntMap.java index 712a64637..7a12bf0f6 100644 --- a/weaver/src/org/aspectj/weaver/IntMap.java +++ b/weaver/src/org/aspectj/weaver/IntMap.java @@ -18,12 +18,13 @@ import java.util.*; public class IntMap { public static final IntMap EMPTY = new IntMap(0) { public boolean directlyInAdvice() { return true; } - public Advice getEnclosingAdvice() { return null; } //XXX possible + public ShadowMunger getEnclosingAdvice() { return null; } //XXX possible }; // XXX begin hack to avoid a signature refactoring in Pointcut - private Advice enclosingAdvice; + private ResolvedTypeX concreteAspect; + private ShadowMunger enclosingAdvice; private List/*ResolvedPointcutDefinition*/ enclosingDefinition = new ArrayList(); public void pushEnclosingDefinition(ResolvedPointcutDefinition def) { @@ -44,21 +45,32 @@ public class IntMap { return enclosingDefinition.isEmpty(); } - public Advice getEnclosingAdvice() { + public ShadowMunger getEnclosingAdvice() { return enclosingAdvice; } - public void setEnclosingAdvice(Advice advice) { + public void setEnclosingAdvice(ShadowMunger advice) { this.enclosingAdvice = advice; } public Member getAdviceSignature() { - return getEnclosingAdvice().signature; + if (enclosingAdvice instanceof Advice) return ((Advice)enclosingAdvice).signature; + else return null; + } + + + public ResolvedTypeX getConcreteAspect() { + return concreteAspect; + } + + public void setConcreteAspect(ResolvedTypeX concreteAspect) { + this.concreteAspect = concreteAspect; } public void copyContext(IntMap bindings) { this.enclosingAdvice = bindings.enclosingAdvice; this.enclosingDefinition = bindings.enclosingDefinition; + this.concreteAspect = bindings.concreteAspect; } // XXX end hack to avoid a signature refactoring in Pointcut @@ -131,4 +143,5 @@ public class IntMap { } + } diff --git a/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java b/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java index 8c38d6a99..5dfc055d8 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java +++ b/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java @@ -62,14 +62,16 @@ public class ResolvedPointcutDefinition extends ResolvedMember { public String toString() { StringBuffer buf = new StringBuffer(); buf.append("poincut "); + buf.append(getDeclaringType().getName()); + buf.append("."); buf.append(getName()); buf.append("("); for (int i=0; i < getParameterTypes().length; i++) { if (i > 0) buf.append(", "); buf.append(getParameterTypes()[i].toString()); } - buf.append("): "); - buf.append(pointcut); + buf.append(")"); + //buf.append(pointcut); return buf.toString(); } @@ -87,5 +89,4 @@ public class ResolvedPointcutDefinition extends ResolvedMember { new ResolvedPointcutDefinition(TypeX.OBJECT, 0, "missing", TypeX.NONE, Pointcut.makeMatchesNothing(Pointcut.RESOLVED)); - } diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java index 2fa0da533..a57bbd58d 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java +++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java @@ -1019,8 +1019,6 @@ public abstract class ResolvedTypeX extends TypeX { public List getExposedPointcuts() { List ret = new ArrayList(); if (getSuperclass() != null) ret.addAll(getSuperclass().getExposedPointcuts()); - - for (Iterator i = Arrays.asList(getDeclaredInterfaces()).iterator(); i.hasNext(); ) { ResolvedTypeX t = (ResolvedTypeX)i.next(); @@ -1032,7 +1030,7 @@ public abstract class ResolvedTypeX extends TypeX { if (inherited.isAbstract()) { if (!this.isAbstract()) { getWorld().showMessage(IMessage.ERROR, - "inherited abstract pointcut " + inherited + + "inherited abstract pointcut " + inherited.getSignature() + " is not made concrete in " + this.getName(), inherited.getSourceLocation(), this.getSourceLocation()); } diff --git a/weaver/src/org/aspectj/weaver/ShadowMunger.java b/weaver/src/org/aspectj/weaver/ShadowMunger.java index 4cce6743b..61e49286b 100644 --- a/weaver/src/org/aspectj/weaver/ShadowMunger.java +++ b/weaver/src/org/aspectj/weaver/ShadowMunger.java @@ -84,4 +84,8 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH + public Pointcut getPointcut() { + return pointcut; + } + } diff --git a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java index 72348d45b..39566938d 100644 --- a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java @@ -118,7 +118,7 @@ public class CflowPointcut extends Pointcut { Pointcut concreteEntry; - CrosscuttingMembers xcut = inAspect.crosscuttingMembers; + CrosscuttingMembers xcut = bindings.getConcreteAspect().crosscuttingMembers; Collection previousCflowEntries = xcut.getCflowEntries(); entryBindings.pushEnclosingDefinition(CFLOW_MARKER); @@ -140,10 +140,10 @@ public class CflowPointcut extends Pointcut { // add field and initializer to inAspect //XXX and then that info above needs to be mapped down here to help with //XXX getting the exposed state right - inAspect.crosscuttingMembers.addConcreteShadowMunger( + bindings.getConcreteAspect().crosscuttingMembers.addConcreteShadowMunger( Advice.makeCflowEntry(world, concreteEntry, isBelow, cflowField, freeVars.length, innerCflowEntries)); - inAspect.crosscuttingMembers.addTypeMunger( + bindings.getConcreteAspect().crosscuttingMembers.addTypeMunger( world.makeCflowStackFieldAdder(cflowField)); diff --git a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java index 051a2b24d..8ef64b0a0 100644 --- a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java @@ -125,8 +125,8 @@ public class IfPointcut extends Pointcut { IfPointcut ret = new IfPointcut(testMethod, extraParameterFlags); partiallyConcretized = ret; if (bindings.directlyInAdvice()) { - Advice advice = bindings.getEnclosingAdvice(); - ret.baseArgsCount = advice.getBaseParameterCount(); + ShadowMunger advice = bindings.getEnclosingAdvice(); + ret.baseArgsCount = ((Advice)advice).getBaseParameterCount(); ret.residueSource = advice.getPointcut().concretize(inAspect, ret.baseArgsCount, advice); } else { ResolvedPointcutDefinition def = bindings.peekEnclosingDefinitition(); diff --git a/weaver/src/org/aspectj/weaver/patterns/PerCflow.java b/weaver/src/org/aspectj/weaver/patterns/PerCflow.java index 35614041a..37610252e 100644 --- a/weaver/src/org/aspectj/weaver/patterns/PerCflow.java +++ b/weaver/src/org/aspectj/weaver/patterns/PerCflow.java @@ -65,7 +65,7 @@ public class PerCflow extends PerClause { CrosscuttingMembers xcut = inAspect.crosscuttingMembers; Collection previousCflowEntries = xcut.getCflowEntries(); - Pointcut concreteEntry = entry.concretize(inAspect, IntMap.EMPTY); + Pointcut concreteEntry = entry.concretize(inAspect, 0, null); //IntMap.EMPTY); List innerCflowEntries = new ArrayList(xcut.getCflowEntries()); innerCflowEntries.removeAll(previousCflowEntries); diff --git a/weaver/src/org/aspectj/weaver/patterns/PerObject.java b/weaver/src/org/aspectj/weaver/patterns/PerObject.java index de73e7066..167e7002c 100644 --- a/weaver/src/org/aspectj/weaver/patterns/PerObject.java +++ b/weaver/src/org/aspectj/weaver/patterns/PerObject.java @@ -69,7 +69,7 @@ public class PerObject extends PerClause { World world = inAspect.getWorld(); - Pointcut concreteEntry = entry.concretize(inAspect, IntMap.EMPTY); + Pointcut concreteEntry = entry.concretize(inAspect, 0, null); //concreteEntry = new AndPointcut(this, concreteEntry); //concreteEntry.state = Pointcut.CONCRETE; inAspect.crosscuttingMembers.addConcreteShadowMunger( diff --git a/weaver/src/org/aspectj/weaver/patterns/Pointcut.java b/weaver/src/org/aspectj/weaver/patterns/Pointcut.java index 99f7f6bc8..d4f861126 100644 --- a/weaver/src/org/aspectj/weaver/patterns/Pointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/Pointcut.java @@ -104,10 +104,11 @@ public abstract class Pointcut extends PatternNode { //XXX this is the signature we're moving to - public Pointcut concretize(ResolvedTypeX inAspect, int arity, Advice advice) { + public Pointcut concretize(ResolvedTypeX inAspect, int arity, ShadowMunger advice) { //if (state == CONCRETE) return this; //??? IntMap map = IntMap.idMap(arity); map.setEnclosingAdvice(advice); + map.setConcreteAspect(inAspect); return concretize(inAspect, map); } diff --git a/weaver/src/org/aspectj/weaver/patterns/ReferencePointcut.java b/weaver/src/org/aspectj/weaver/patterns/ReferencePointcut.java index 52977f2b7..85c6a3573 100644 --- a/weaver/src/org/aspectj/weaver/patterns/ReferencePointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/ReferencePointcut.java @@ -228,6 +228,14 @@ public class ReferencePointcut extends Pointcut { ); return Pointcut.makeMatchesNothing(Pointcut.CONCRETE); } + + if (pointcutDec.isAbstract()) { + Thread.currentThread().dumpStack(); + searchStart.getWorld().showMessage(IMessage.ERROR, + pointcutDec + " is abstract", + getSourceLocation(), bindings.getEnclosingAdvice().getSourceLocation()); + return Pointcut.makeMatchesNothing(Pointcut.CONCRETE); + } //System.err.println("start: " + searchStart); ResolvedTypeX[] parameterTypes = searchStart.getWorld().resolve(pointcutDec.getParameterTypes());