Implement source location matching for weave messages in XML tests

WIP (work in progress).

Closes #218.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>

bridge/src/main/java/org/aspectj/bridge/WeaveMessage.java

@@ -39,14 +39,18 @@ public class WeaveMessage extends Message {
 	public static WeaveMessageKind WEAVEMESSAGE_REMOVES_ANNOTATION = new WeaveMessageKind(6,
 			"'%1' (%2) has had %3 %4 annotation removed by '%5' (%6)");
 
-	private String affectedtypename;
-	private String aspectname;
+	private String affectedTypeName;
+	private String aspectName;
 
 	// private ctor - use the static factory method
-	private WeaveMessage(String message, String affectedtypename, String aspectname) {
-		super(message, IMessage.WEAVEINFO, null, null);
-		this.affectedtypename = affectedtypename;
-		this.aspectname = aspectname;
+	private WeaveMessage(
+		String message,
+		String affectedTypeName, String aspectName,
+		ISourceLocation affectedTypeLocation, ISourceLocation aspectLocation
+	) {
+		super(message, null, IMessage.WEAVEINFO, affectedTypeLocation, null, new ISourceLocation[] { aspectLocation });
+		this.affectedTypeName = affectedTypeName;
+		this.aspectName = aspectName;
 	}
 
 	/**
@@ -63,7 +67,7 @@ public class WeaveMessage extends Message {
 			int n = Character.getNumericValue(str.charAt(pos + 1));
 			str.replace(pos, pos + 2, inserts[n - 1]);
 		}
-		return new WeaveMessage(str.toString(), null, null);
+		return new WeaveMessage(str.toString(), null, null, null, null);
 	}
 
 	/**
@@ -71,33 +75,38 @@ public class WeaveMessage extends Message {
 	 *
 	 * @param kind what kind of message (e.g. declare parents)
 	 * @param inserts inserts for the message (inserts are marked %n in the message)
-	 * @param affectedtypename the type which is being advised/declaredUpon
-	 * @param aspectname the aspect that defined the advice or declares
+	 * @param affectedTypeName the type which is being advised/declaredUpon
+	 * @param aspectName the aspect that defined the advice or declares
+   * @param affectedTypeLocation the source location of the advised/declaredUpon type
+   * @param aspectLocation the source location of the declaring/defining advice/declare
 	 * @return new weaving message
 	 */
-	public static WeaveMessage constructWeavingMessage(WeaveMessageKind kind, String[] inserts, String affectedtypename,
-			String aspectname) {
+	public static WeaveMessage constructWeavingMessage(
+		WeaveMessageKind kind, String[] inserts,
+		String affectedTypeName, String aspectName,
+		ISourceLocation affectedTypeLocation, ISourceLocation aspectLocation
+	) {
 		StringBuilder str = new StringBuilder(kind.getMessage());
 		int pos = -1;
 		while ((pos = new String(str).indexOf("%")) != -1) {
 			int n = Character.getNumericValue(str.charAt(pos + 1));
 			str.replace(pos, pos + 2, inserts[n - 1]);
 		}
-		return new WeaveMessage(str.toString(), affectedtypename, aspectname);
+		return new WeaveMessage(str.toString(), affectedTypeName, aspectName, affectedTypeLocation, aspectLocation);
 	}
 
 	/**
 	 * @return Returns the aspectname.
 	 */
-	public String getAspectname() {
-		return aspectname;
+	public String getAspectName() {
+		return aspectName;
 	}
 
 	/**
 	 * @return Returns the affectedtypename.
 	 */
-	public String getAffectedtypename() {
-		return affectedtypename;
+	public String getAffectedTypeName() {
+		return affectedTypeName;
 	}
 
 	public static class WeaveMessageKind {

org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceLocation.java

@@ -108,8 +108,12 @@ public class EclipseSourceLocation implements ISourceLocation {
 
     public String getContext() {
         if (null == context) {
-            ICompilationUnit compilationUnit = result.compilationUnit;
-            IProblem[] problems = result.problems;
+            ICompilationUnit compilationUnit = null;
+            IProblem[] problems = null;
+            if (result != null) {
+                compilationUnit = result.compilationUnit;
+                problems = result.problems;
+            }
             if ((null == compilationUnit) || (null == problems)
                 || (1 != problems.length)) { // ?? which of n>1 problems?
                 context = NO_CONTEXT;

org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java

@@ -33,6 +33,7 @@ import java.util.stream.Collectors;
 
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
+import org.aspectj.bridge.WeaveMessage;
 import org.aspectj.testing.util.TestUtil;
 import org.aspectj.util.LangUtil;
 
@@ -134,8 +135,10 @@ public abstract class AjcTestCase extends TestCase {
 	 */
 	public static class Message {
 		private int line = -1;
+		private int aspectLine = -1;
 		private String text;
 		private String sourceFileName;
+		private String aspectFileName;
 		private ISourceLocation[] seeAlsos;
 		public boolean careAboutOtherMessages = true;
 
@@ -172,19 +175,28 @@ public abstract class AjcTestCase extends TestCase {
 		 * </p>
 		 */
 		public Message(int line, String srcFile, String text, ISourceLocation[] seeAlso) {
+			this(line, srcFile, -1, null, text, seeAlso);
+		}
+
+		public Message(int line, String srcFile, int aspectLine, String aspectFile, String text, ISourceLocation[] seeAlso) {
 			this.line = line;
 			StringBuilder srcFileName = new StringBuilder();
 			if (srcFile != null) {
 				char[] chars = srcFile.toCharArray();
 				for (char c : chars) {
-					if ((c == '\\') || (c == '/')) {
-						srcFileName.append(separator);
-					} else {
-						srcFileName.append(c);
-					}
+					srcFileName.append((c == '\\' || c == '/') ? separator : c);
 				}
 				this.sourceFileName = srcFileName.toString();
 			}
+			this.aspectLine = aspectLine;
+			StringBuilder aspectFileName = new StringBuilder();
+			if (aspectFile != null) {
+				char[] chars = aspectFile.toCharArray();
+				for (char c : chars) {
+					aspectFileName.append((c == '\\' || c == '/') ? separator : c);
+				}
+				this.aspectFileName = aspectFileName.toString();
+			}
 			this.text = text;
 			if (this.text != null && text.startsWith("*")) {
 				// Don't care what other messages are around
@@ -211,33 +223,41 @@ public abstract class AjcTestCase extends TestCase {
 		 */
 		public boolean matches(IMessage message) {
 			ISourceLocation loc = message.getSourceLocation();
-			if ((loc == null) && ((line != -1) || (sourceFileName != null))) {
+			if ((loc == null) && ((line != -1) || (sourceFileName != null)))
 				return false;
-			}
 			if (line != -1) {
-				if (loc.getLine() != line) {
+				if (loc.getLine() != line)
 					return false;
-				}
 			}
 			if (sourceFileName != null) {
-				if (!loc.getSourceFile().getPath().endsWith(sourceFileName)) {
+				if (!loc.getSourceFile().getPath().endsWith(sourceFileName))
 					return false;
+			}
+			if (message instanceof WeaveMessage) {
+				List<ISourceLocation> extraLocations = message.getExtraSourceLocations();
+				loc = extraLocations.size() > 0 ? extraLocations.get(0) : null;
+				if ((loc == null) && ((aspectLine != -1) || (aspectFileName != null)))
+					return false;
+				if (aspectLine != -1) {
+					if (loc.getLine() != aspectLine)
+						return false;
+				}
+				if (aspectFileName != null) {
+					if (!loc.getSourceFile().getPath().endsWith(aspectFileName))
+						return false;
 				}
 			}
 			if (text != null) {
-				if (!message.getMessage().contains(text)) {
+				if (!message.getMessage().contains(text))
 					return false;
-				}
 			}
 			if (seeAlsos != null) {
 				List<ISourceLocation> extraLocations = message.getExtraSourceLocations();
-				if (extraLocations.size() != seeAlsos.length) {
+				if (extraLocations.size() != seeAlsos.length)
 					return false;
-				}
 				for (ISourceLocation seeAlso : seeAlsos) {
-					if (!hasAMatch(extraLocations, seeAlso)) {
+					if (!hasAMatch(extraLocations, seeAlso))
 						return false;
-					}
 				}
 			}
 			return true;

testing/src/test/java/org/aspectj/testing/CompileSpec.java

@@ -329,7 +329,7 @@ public class CompileSpec implements ITestStep {
 			} else if (kind.equals("abort")) {
 				fails.add(exMsg.toMessage());
 			} else if (kind.equals("weave")) {
-				weaveInfos.add(exMsg.toMessage());
+				weaveInfos.add(exMsg.toWeaveMessage());
 			} else if (kind.equals("usage")) {
 				weaveInfos.add(exMsg.toMessage());
 			}

testing/src/test/java/org/aspectj/testing/ExpectedMessageSpec.java

@@ -23,12 +23,18 @@ public class ExpectedMessageSpec {
 
 	private String kind = "error";
 	private int line = -1;
+	private int aspectLine = -1;
 	private String text;
 	private String file;
+	private String aspectFile;
 	private String details;
 
 	public AjcTestCase.Message toMessage() {
-		return new AjcTestCase.Message(line,file,text,null);
+		return new AjcTestCase.Message(line, file, text, null);
+	}
+
+	public AjcTestCase.Message toWeaveMessage() {
+		return new AjcTestCase.Message(line, file, aspectLine, aspectFile, text, null);
 	}
 
 	/**
@@ -55,6 +61,18 @@ public class ExpectedMessageSpec {
 	public void setFile(String file) {
 		this.file = file;
 	}
+	/**
+	 * @return Returns the aspect file.
+	 */
+	public String getAspectFile() {
+		return aspectFile;
+	}
+	/**
+	 * @param aspectFile The aspect file to set.
+	 */
+	public void setAspectFile(String aspectFile) {
+		this.aspectFile = aspectFile;
+	}
 	/**
 	 * @return Returns the kind.
 	 */
@@ -79,6 +97,18 @@ public class ExpectedMessageSpec {
 	public void setLine(int line) {
 		this.line = line;
 	}
+	/**
+	 * @return Returns the asperct line.
+	 */
+	public int getAspectLine() {
+		return aspectLine;
+	}
+	/**
+	 * @param aspectLine The aspect line to set.
+	 */
+	public void setAspectLine(int aspectLine) {
+		this.aspectLine = aspectLine;
+	}
 	/**
 	 * @return Returns the text.
 	 */

tests/ajcTestSuite.dtd

@@ -60,9 +60,13 @@
 
    <!ELEMENT message (source*)>
    <!ATTLIST message kind (abort | fail | error | warning | info | Xlint | weave) #IMPLIED >
+   <!-- Java source code line; in weaving messages it denotes the weaving target, e.g. to where an advice is applied -->
    <!ATTLIST message line CDATA #IMPLIED >
+   <!-- Aspect source code line; only useful for weaving messages where it denotes the source of what is woven in (e.g. advice) -->
+   <!ATTLIST message aspectLine CDATA #IMPLIED >
    <!ATTLIST message text CDATA #IMPLIED >
    <!ATTLIST message file CDATA #IMPLIED >
+   <!ATTLIST message aspectFile CDATA #IMPLIED >
    <!ATTLIST message details CDATA #IMPLIED >
 
    <!ELEMENT source (#PCDATA)>

weaver/src/main/java/org/aspectj/weaver/bcel/BcelTypeMunger.java

@@ -193,21 +193,36 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 				NewParentTypeMunger parentTM = (NewParentTypeMunger) munger;
 				if (parentTM.isMixin()) {
 					weaver.getWorld()
-							.getMessageHandler()
-							.handleMessage(
-									WeaveMessage.constructWeavingMessage(WeaveMessage.WEAVEMESSAGE_MIXIN, new String[] {
-											parentTM.getNewParent().getName(), fName, weaver.getLazyClassGen().getType().getName(),
-											tName }, weaver.getLazyClassGen().getClassName(), getAspectType().getName()));
-				} else {
+						.getMessageHandler()
+						.handleMessage(
+							WeaveMessage.constructWeavingMessage(
+								WeaveMessage.WEAVEMESSAGE_MIXIN,
+								new String[] {
+									parentTM.getNewParent().getName(), fName,
+									weaver.getLazyClassGen().getType().getName(), tName
+								},
+								weaver.getLazyClassGen().getClassName(), getAspectType().getName(),
+								parentTM.getNewParent().getSourceLocation(), weaver.getLazyClassGen().getType().getSourceLocation()
+							)
+						);
+				}
+				else {
 					if (parentTM.getNewParent().isInterface()) {
 						weaver.getWorld()
-								.getMessageHandler()
-								.handleMessage(
-										WeaveMessage.constructWeavingMessage(WeaveMessage.WEAVEMESSAGE_DECLAREPARENTSIMPLEMENTS,
-												new String[] { weaver.getLazyClassGen().getType().getName(), tName,
-														parentTM.getNewParent().getName(), fName }, weaver.getLazyClassGen()
-														.getClassName(), getAspectType().getName()));
-					} else {
+							.getMessageHandler()
+							.handleMessage(
+								WeaveMessage.constructWeavingMessage(
+									WeaveMessage.WEAVEMESSAGE_DECLAREPARENTSIMPLEMENTS,
+									new String[] {
+										weaver.getLazyClassGen().getType().getName(), tName,
+										parentTM.getNewParent().getName(), fName
+									},
+									weaver.getLazyClassGen().getClassName(), getAspectType().getName(),
+									parentTM.getNewParent().getSourceLocation(), weaver.getLazyClassGen().getType().getSourceLocation()
+								)
+							);
+					}
+					else {
 						weaver.getWorld()
 								.getMessageHandler()
 								.handleMessage(
@@ -232,11 +247,18 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 					fromString = fName;
 				}
 				weaver.getWorld()
-						.getMessageHandler()
-						.handleMessage(
-								WeaveMessage.constructWeavingMessage(WeaveMessage.WEAVEMESSAGE_ITD, new String[] {
-										weaver.getLazyClassGen().getType().getName(), tName, kindString, getAspectType().getName(),
-										fromString }, weaver.getLazyClassGen().getClassName(), getAspectType().getName()));
+					.getMessageHandler()
+					.handleMessage(
+						WeaveMessage.constructWeavingMessage(
+							WeaveMessage.WEAVEMESSAGE_ITD,
+							new String[] {
+								weaver.getLazyClassGen().getType().getName(), tName,
+								kindString, getAspectType().getName(), fromString
+							},
+							weaver.getLazyClassGen().getClassName(), getAspectType().getName(),
+							weaver.getLazyClassGen().getType().getSourceLocation(), getAspectType().getSourceLocation()
+						)
+					);
 			}
 		}
 

weaver/src/main/java/org/aspectj/weaver/bcel/BcelWorld.java

@@ -190,17 +190,31 @@ public class BcelWorld extends World implements Repository {
 		String advisingType = advice.getConcreteAspect().getName();
 		Message msg = null;
 		if (advice.getKind().equals(AdviceKind.Softener)) {
-			msg = WeaveMessage.constructWeavingMessage(WeaveMessage.WEAVEMESSAGE_SOFTENS, new String[] { advisedType,
-					beautifyLocation(shadow.getSourceLocation()), advisingType, beautifyLocation(munger.getSourceLocation()) },
-					advisedType, advisingType);
-		} else {
+			msg = WeaveMessage.constructWeavingMessage(
+				WeaveMessage.WEAVEMESSAGE_SOFTENS,
+				new String[] {
+					advisedType, beautifyLocation(shadow.getSourceLocation()),
+					advisingType, beautifyLocation(munger.getSourceLocation())
+				},
+				advisedType, advisingType,
+				shadow.getSourceLocation(), munger.getSourceLocation()
+			);
+		}
+		else {
 			boolean runtimeTest = advice.hasDynamicTests();
 			String joinPointDescription = shadow.toString();
-			msg = WeaveMessage
-					.constructWeavingMessage(WeaveMessage.WEAVEMESSAGE_ADVISES,
-							new String[] { joinPointDescription, advisedType, beautifyLocation(shadow.getSourceLocation()),
-									description, advisingType, beautifyLocation(munger.getSourceLocation()),
-									(runtimeTest ? " [with runtime test]" : "") }, advisedType, advisingType);
+			msg = WeaveMessage.constructWeavingMessage(
+				WeaveMessage.WEAVEMESSAGE_ADVISES,
+				new String[] {
+					joinPointDescription,
+					advisedType, beautifyLocation(shadow.getSourceLocation()),
+					description,
+					advisingType, beautifyLocation(munger.getSourceLocation()),
+					(runtimeTest ? " [with runtime test]" : "")
+				},
+				advisedType, advisingType,
+				shadow.getSourceLocation(), munger.getSourceLocation()
+			);
 			// Boolean.toString(runtimeTest)});
 		}
 		getMessageHandler().handleMessage(msg);