diff options
author | jhugunin <jhugunin> | 2003-03-11 23:46:51 +0000 |
---|---|---|
committer | jhugunin <jhugunin> | 2003-03-11 23:46:51 +0000 |
commit | fd1560a8a1c91d1b65c738d4e9163d93700d1f00 (patch) | |
tree | 79e8faae79285850656b6b64632ed5900568db03 | |
parent | d6b8b38cd0a707741788f8d4fae3850b213f50a4 (diff) | |
download | aspectj-fd1560a8a1c91d1b65c738d4e9163d93700d1f00.tar.gz aspectj-fd1560a8a1c91d1b65c738d4e9163d93700d1f00.zip |
fixing Bug 31724
declare warning/error emitted without context
and generally providing better error context information
-rw-r--r-- | org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseShadow.java | 3 | ||||
-rw-r--r-- | org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java | 13 | ||||
-rw-r--r-- | org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/EclipseSourceContext.java | 10 | ||||
-rw-r--r-- | tests/ajcTests.xml | 29 | ||||
-rw-r--r-- | tests/ajcTestsFailing.xml | 22 | ||||
-rw-r--r-- | tests/jimTests.xml | 22 | ||||
-rw-r--r-- | tests/new/declare/DeclareWarningEmpty.java | 16 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/ISourceContext.java | 1 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/Position.java | 32 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/ResolvedTypeX.java | 40 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/Shadow.java | 3 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java | 13 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/bcel/BcelShadow.java | 32 | ||||
-rw-r--r-- | weaver/src/org/aspectj/weaver/bcel/BcelSourceContext.java | 40 | ||||
-rw-r--r-- | weaver/testsrc/org/aspectj/weaver/TestShadow.java | 3 |
15 files changed, 196 insertions, 83 deletions
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseShadow.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseShadow.java index 01ed6dcc2..2e0630f6c 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseShadow.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseShadow.java @@ -18,6 +18,7 @@ import org.aspectj.ajdt.internal.compiler.ast.InterTypeConstructorDeclaration; import org.aspectj.ajdt.internal.compiler.ast.InterTypeDeclaration; import org.aspectj.ajdt.internal.compiler.ast.InterTypeFieldDeclaration; import org.aspectj.ajdt.internal.compiler.ast.InterTypeMethodDeclaration; +import org.aspectj.bridge.ISourceLocation; import org.aspectj.bridge.SourceLocation; import org.aspectj.weaver.Member; import org.aspectj.weaver.Shadow; @@ -65,7 +66,7 @@ public class EclipseShadow extends Shadow { return world.fromBinding(enclosingMethod.binding.declaringClass); } - public SourceLocation getSourceLocation() { + public ISourceLocation getSourceLocation() { //XXX need to fill this in ASAP return null; } diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java index dbb985ab0..abee60abf 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java @@ -16,6 +16,7 @@ package org.aspectj.ajdt.internal.compiler.lookup; import java.util.*; import org.aspectj.ajdt.internal.compiler.ast.*; +import org.aspectj.ajdt.internal.core.builder.EclipseSourceContext; import org.aspectj.bridge.IMessage; import org.aspectj.bridge.ISourceLocation; import org.aspectj.weaver.*; @@ -54,6 +55,10 @@ public class EclipseSourceType extends ResolvedTypeX.ConcreteName { this.factory = factory; this.binding = binding; this.declaration = declaration; + + resolvedTypeX.setSourceContext(new EclipseSourceContext(declaration.compilationResult)); + resolvedTypeX.setStartPos(declaration.sourceStart); + resolvedTypeX.setEndPos(declaration.sourceEnd); } @@ -195,10 +200,10 @@ public class EclipseSourceType extends ResolvedTypeX.ConcreteName { // return crosscuttingMembers; // } - public ISourceLocation getSourceLocation() { - TypeDeclaration dec = binding.scope.referenceContext; - return new EclipseSourceLocation(dec.compilationResult, dec.sourceStart, dec.sourceEnd); - } +// public ISourceLocation getSourceLocation() { +// TypeDeclaration dec = binding.scope.referenceContext; +// return new EclipseSourceLocation(dec.compilationResult, dec.sourceStart, dec.sourceEnd); +// } public boolean isInterface() { return binding.isInterface(); diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/EclipseSourceContext.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/EclipseSourceContext.java index f54ebc1a1..fd107e77a 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/EclipseSourceContext.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/EclipseSourceContext.java @@ -13,8 +13,11 @@ package org.aspectj.ajdt.internal.core.builder; +import java.io.File; + import org.aspectj.ajdt.internal.compiler.lookup.EclipseSourceLocation; import org.aspectj.bridge.ISourceLocation; +import org.aspectj.bridge.SourceLocation; import org.aspectj.weaver.IHasPosition; import org.aspectj.weaver.ISourceContext; import org.eclipse.jdt.internal.compiler.CompilationResult; @@ -29,9 +32,16 @@ public class EclipseSourceContext implements ISourceContext { this.result = result; } + private File getSourceFile() { + return new File(new String(result.fileName)); + } public ISourceLocation makeSourceLocation(IHasPosition position) { return new EclipseSourceLocation(result, position.getStart(), position.getEnd()); } + public ISourceLocation makeSourceLocation(int line) { + return new SourceLocation(getSourceFile(), line); + } + } diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml index 321afe3d9..a2e0b3c6e 100644 --- a/tests/ajcTests.xml +++ b/tests/ajcTests.xml @@ -5630,4 +5630,33 @@ <message kind="error" line="25"/> </compile> </ajc-test> + + <ajc-test dir="new/declare" pr="31724" + title="omnibus declare warning context with no initializer/constructor"> + <compile files="DeclareWarningEmpty.java"> + <message kind="warning" line="3"/> + </compile> + </ajc-test> + + + <ajc-test dir="new/declare" pr="31724" + title="omnibus declare warning context"> + <compile files="DeclareWarning.java"> + <message kind="warning" line="5"/> + <message kind="warning" line="12"/> + <message kind="warning" line="13"/> + <message kind="warning" line="14"/> + <message kind="warning" line="15"/> + <message kind="warning" line="16"/> + <message kind="warning" line="17"/> + <message kind="warning" line="18"/> + <message kind="warning" line="21"/> + <message kind="warning" line="22"/> + <message kind="warning" line="23"/> + <message kind="warning" line="33"/> + <message kind="warning" line="36"/> + <message kind="warning" line="39"/> + <message kind="warning" line="74"/> + </compile> + </ajc-test> </suite> diff --git a/tests/ajcTestsFailing.xml b/tests/ajcTestsFailing.xml index 051196a60..b7fadc146 100644 --- a/tests/ajcTestsFailing.xml +++ b/tests/ajcTestsFailing.xml @@ -30,28 +30,6 @@ </compile> </ajc-test> - <ajc-test dir="new/declare" pr="31724" - title="omnibus declare warning context" - comment="XXX untested: no source context shown"> - <compile files="DeclareWarning.java"> - <message kind="warning" line="5"/> - <message kind="warning" line="12"/> - <message kind="warning" line="13"/> - <message kind="warning" line="14"/> - <message kind="warning" line="15"/> - <message kind="warning" line="16"/> - <message kind="warning" line="17"/> - <message kind="warning" line="18"/> - <message kind="warning" line="21"/> - <message kind="warning" line="22"/> - <message kind="warning" line="23"/> - <message kind="warning" line="33"/> - <message kind="warning" line="36"/> - <message kind="warning" line="39"/> - <message kind="warning" line="74"/> - </compile> - </ajc-test> - <ajc-test dir="errors" keywords="error" title="class extending abstract aspect"> <compile files="ClassExtendingAbstractAspectCE.java"> diff --git a/tests/jimTests.xml b/tests/jimTests.xml index 17db88e59..53a3a9ef1 100644 --- a/tests/jimTests.xml +++ b/tests/jimTests.xml @@ -1,26 +1,6 @@ <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"> <suite> - <ajc-test dir="new/declare" pr="31724" - title="omnibus declare warning context" - comment="XXX untested: no source context shown"> - <compile files="DeclareWarning.java"> - <message kind="warning" line="5"/> - <message kind="warning" line="12"/> - <message kind="warning" line="13"/> - <message kind="warning" line="14"/> - <message kind="warning" line="15"/> - <message kind="warning" line="16"/> - <message kind="warning" line="17"/> - <message kind="warning" line="18"/> - <message kind="warning" line="21"/> - <message kind="warning" line="22"/> - <message kind="warning" line="23"/> - <message kind="warning" line="33"/> - <message kind="warning" line="36"/> - <message kind="warning" line="39"/> - <message kind="warning" line="74"/> - </compile> - </ajc-test> + <!-- diff --git a/tests/new/declare/DeclareWarningEmpty.java b/tests/new/declare/DeclareWarningEmpty.java new file mode 100644 index 000000000..b310022b4 --- /dev/null +++ b/tests/new/declare/DeclareWarningEmpty.java @@ -0,0 +1,16 @@ + +/** @testcase PR#31724 omnibus declare-warning test using default initializers/constructors*/ +public class DeclareWarningEmpty { // CE 3 + + + + + +} + +aspect A { + declare warning: staticinitialization(DeclareWarningEmpty) + : "staticinitialization(DeclareWarningEmpty)"; + declare warning: initialization(DeclareWarningEmpty.new(..)) + : "initialization(DeclareWarningEmpty)"; +} diff --git a/weaver/src/org/aspectj/weaver/ISourceContext.java b/weaver/src/org/aspectj/weaver/ISourceContext.java index 8b591a479..16cb3c8bc 100644 --- a/weaver/src/org/aspectj/weaver/ISourceContext.java +++ b/weaver/src/org/aspectj/weaver/ISourceContext.java @@ -17,4 +17,5 @@ import org.aspectj.bridge.ISourceLocation; public interface ISourceContext { public ISourceLocation makeSourceLocation(IHasPosition position); + public ISourceLocation makeSourceLocation(int line); } diff --git a/weaver/src/org/aspectj/weaver/Position.java b/weaver/src/org/aspectj/weaver/Position.java new file mode 100644 index 000000000..5e5c5c5a9 --- /dev/null +++ b/weaver/src/org/aspectj/weaver/Position.java @@ -0,0 +1,32 @@ +/* ******************************************************************* + * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC). + * All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Common Public License v1.0 + * which accompanies this distribution and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * PARC initial implementation + * ******************************************************************/ + + +package org.aspectj.weaver; + +public class Position implements IHasPosition { + private int start, end; + + public Position(int start, int end) { + this.start = start; + this.end = end; + } + + public int getEnd() { + return end; + } + + public int getStart() { + return start; + } + +} diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java index 6249a9b21..9de2348ea 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java +++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java @@ -499,6 +499,9 @@ public abstract class ResolvedTypeX extends TypeX { public static class Name extends ResolvedTypeX { private ConcreteName delegate = null; + private ISourceContext sourceContext = null; + private int startPos = 0; + private int endPos = 0; //??? should set delegate before any use public Name(String signature, World world) { @@ -556,10 +559,13 @@ public abstract class ResolvedTypeX extends TypeX { } public ISourceContext getSourceContext() { - return delegate.getSourceContext(); + return sourceContext; } - public ISourceLocation getSourceLocation() { return delegate.getSourceLocation(); } + public ISourceLocation getSourceLocation() { + if (sourceContext == null) return null; + return sourceContext.makeSourceLocation(new Position(startPos, endPos)); + } public boolean isExposedToWeaver() { return delegate.isExposedToWeaver(); //??? where does this belong @@ -608,10 +614,30 @@ public abstract class ResolvedTypeX extends TypeX { public void setDelegate(ConcreteName delegate) { this.delegate = delegate; } + public int getEndPos() { + return endPos; + } + + public int getStartPos() { + return startPos; + } + + public void setEndPos(int endPos) { + this.endPos = endPos; + } + + public void setSourceContext(ISourceContext sourceContext) { + this.sourceContext = sourceContext; + } + + public void setStartPos(int startPos) { + this.startPos = startPos; + } + } public static abstract class ConcreteName { - protected ISourceContext sourceContext; + //protected ISourceContext sourceContext; protected boolean exposedToWeaver; ResolvedTypeX.Name resolvedTypeX; @@ -648,13 +674,13 @@ public abstract class ResolvedTypeX extends TypeX { public abstract ResolvedTypeX getSuperclass(); - public abstract ISourceLocation getSourceLocation(); +// public abstract ISourceLocation getSourceLocation(); public abstract boolean isWovenBy(ResolvedTypeX aspectType); - public ISourceContext getSourceContext() { - return sourceContext; - } +// public ISourceContext getSourceContext() { +// return sourceContext; +// } public boolean isExposedToWeaver() { return exposedToWeaver; diff --git a/weaver/src/org/aspectj/weaver/Shadow.java b/weaver/src/org/aspectj/weaver/Shadow.java index dd36e0eda..9a3f79082 100644 --- a/weaver/src/org/aspectj/weaver/Shadow.java +++ b/weaver/src/org/aspectj/weaver/Shadow.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import org.aspectj.bridge.*; import org.aspectj.bridge.MessageUtil; import org.aspectj.bridge.SourceLocation; import org.aspectj.lang.JoinPoint; @@ -315,7 +316,7 @@ public abstract class Shadow { return null; //XXX } - public abstract SourceLocation getSourceLocation(); + public abstract ISourceLocation getSourceLocation(); // ---- utility diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java index 05d7248b1..5cd46b71d 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java @@ -25,6 +25,7 @@ import org.apache.bcel.classfile.Field; import org.apache.bcel.classfile.JavaClass; import org.apache.bcel.classfile.Method; import org.aspectj.bridge.ISourceLocation; +import org.aspectj.weaver.*; import org.aspectj.weaver.AjAttribute; import org.aspectj.weaver.BCException; import org.aspectj.weaver.ResolvedMember; @@ -76,7 +77,9 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { super(resolvedTypeX, exposedToWeaver); this.javaClass = javaClass; - sourceContext = new BcelSourceContext(this); + if (resolvedTypeX.getSourceContext() == null) { + resolvedTypeX.setSourceContext(new BcelSourceContext(this)); + } // this should only ever be java.lang.Object which is // the only class in Java-1.4 with no superclasses @@ -155,7 +158,7 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { List pointcuts = new ArrayList(); typeMungers = new ArrayList(); declares = new ArrayList(); - List l = BcelAttributes.readAjAttributes(javaClass.getAttributes(), getSourceContext()); + List l = BcelAttributes.readAjAttributes(javaClass.getAttributes(), getResolvedTypeX().getSourceContext()); for (Iterator iter = l.iterator(); iter.hasNext();) { AjAttribute a = (AjAttribute) iter.next(); //System.err.println("unpacking: " + this + " and " + a); @@ -172,7 +175,9 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { } else if (a instanceof AjAttribute.PrivilegedAttribute) { privilegedAccess = ((AjAttribute.PrivilegedAttribute)a).getAccessedMembers(); } else if (a instanceof AjAttribute.SourceContextAttribute) { - ((BcelSourceContext)sourceContext).addAttributeInfo((AjAttribute.SourceContextAttribute)a); + if (getResolvedTypeX().getSourceContext() instanceof BcelSourceContext) { + ((BcelSourceContext)getResolvedTypeX().getSourceContext()).addAttributeInfo((AjAttribute.SourceContextAttribute)a); + } } else { throw new BCException("bad attribute " + a); } @@ -257,7 +262,7 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { } public ISourceLocation getSourceLocation() { - return null; //FIXME, we can do much better than this + return getResolvedTypeX().getSourceContext().makeSourceLocation(0); //FIXME, we can do better than this } } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java index 9a9efd798..f0b0742d8 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java @@ -42,6 +42,7 @@ import org.apache.bcel.generic.ReturnInstruction; import org.apache.bcel.generic.SWAP; import org.apache.bcel.generic.TargetLostException; import org.apache.bcel.generic.Type; +import org.aspectj.bridge.ISourceLocation; import org.aspectj.bridge.SourceLocation; import org.aspectj.weaver.AdviceKind; import org.aspectj.weaver.AjcMemberMaker; @@ -294,7 +295,13 @@ public class BcelShadow extends Shadow { } public int getSourceLine() { - if (range == null) return 0; + if (range == null) { + if (getEnclosingMethod().hasBody()) { + return Utility.getSourceLine(getEnclosingMethod().getBody().getStart()); + } else { + return 0; + } + } int ret = Utility.getSourceLine(range.getStart()); if (ret < 0) return 0; return ret; @@ -1867,24 +1874,15 @@ public class BcelShadow extends Shadow { return getEnclosingClass().getFactory(); } - public SourceLocation getSourceLocation() { - // AMC - a temporary "fudge" to give as much information as possible about the identity of the - // source file this source location points to. - String internalClassName = getEnclosingClass().getInternalClassName(); - String fileName = getEnclosingClass().getFileName(); - String extension = fileName.substring( fileName.lastIndexOf("."), fileName.length()); - String filePrefix = fileName.substring( 0, fileName.lastIndexOf(".")); - // internal class name is e.g. figures/Point, we don't know whether the file was - // .aj or .java so we put it together with the file extension of the enclosing class - // BUT... sometimes internalClassName is a different class (an aspect), so we only use it if it - // matches the file name. - String mostAccurateFileNameGuess; - if ( internalClassName.endsWith(filePrefix)) { - mostAccurateFileNameGuess = internalClassName + extension; + public ISourceLocation getSourceLocation() { + int sourceLine = getSourceLine(); + if (sourceLine == 0) { +// Thread.currentThread().dumpStack(); +// System.err.println(this + ": " + range); + return getEnclosingClass().getType().getSourceLocation(); } else { - mostAccurateFileNameGuess = fileName; + return getEnclosingClass().getType().getSourceContext().makeSourceLocation(sourceLine); } - return new SourceLocation(new File(mostAccurateFileNameGuess), getSourceLine()); } public Shadow getEnclosingShadow() { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelSourceContext.java b/weaver/src/org/aspectj/weaver/bcel/BcelSourceContext.java index 157a20740..72a7939e1 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelSourceContext.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelSourceContext.java @@ -31,22 +31,52 @@ public class BcelSourceContext implements ISourceContext { this.inObject = inObject; } - - public ISourceLocation makeSourceLocation(IHasPosition position) { + private File getSourceFile() { + //XXX make this work better borrowing code from below String fileName = sourceFileName; if (fileName == null) inObject.getJavaClass().getFileName(); if (fileName == null) fileName = inObject.getResolvedTypeX().getName() + ".class"; + return new File(fileName); + } + + /* + // AMC - a temporary "fudge" to give as much information as possible about the identity of the + // source file this source location points to. + String internalClassName = getEnclosingClass().getInternalClassName(); + String fileName = getEnclosingClass().getFileName(); + String extension = fileName.substring( fileName.lastIndexOf("."), fileName.length()); + String filePrefix = fileName.substring( 0, fileName.lastIndexOf(".")); + // internal class name is e.g. figures/Point, we don't know whether the file was + // .aj or .java so we put it together with the file extension of the enclosing class + // BUT... sometimes internalClassName is a different class (an aspect), so we only use it if it + // matches the file name. + String mostAccurateFileNameGuess; + if ( internalClassName.endsWith(filePrefix)) { + mostAccurateFileNameGuess = internalClassName + extension; + } else { + mostAccurateFileNameGuess = fileName; + } + return new SourceLocation(new File(mostAccurateFileNameGuess), getSourceLine()); + */ + + + + public ISourceLocation makeSourceLocation(IHasPosition position) { + + if (lineBreaks != null) { int line = Arrays.binarySearch(lineBreaks, position.getStart()); if (line < 0) line = -line; - return new SourceLocation(new File(fileName), line); //??? have more info + return new SourceLocation(getSourceFile(), line); //??? have more info } else { - return new SourceLocation(new File(fileName), 0); + return new SourceLocation(getSourceFile(), 0); } } - + public ISourceLocation makeSourceLocation(int line) { + return new SourceLocation(getSourceFile(), line); + } public void addAttributeInfo(SourceContextAttribute sourceContextAttribute) { this.sourceFileName = sourceContextAttribute.getSourceFileName(); diff --git a/weaver/testsrc/org/aspectj/weaver/TestShadow.java b/weaver/testsrc/org/aspectj/weaver/TestShadow.java index 0a91cf1e3..534eca0a5 100644 --- a/weaver/testsrc/org/aspectj/weaver/TestShadow.java +++ b/weaver/testsrc/org/aspectj/weaver/TestShadow.java @@ -14,6 +14,7 @@ package org.aspectj.weaver; import org.aspectj.weaver.ast.Var; +import org.aspectj.bridge.ISourceLocation; import org.aspectj.bridge.SourceLocation; public class TestShadow extends Shadow { @@ -62,7 +63,7 @@ public class TestShadow extends Shadow { throw new RuntimeException("unimplemented"); } - public SourceLocation getSourceLocation() { + public ISourceLocation getSourceLocation() { throw new RuntimeException("unimplemented"); } |