From a1a700fc6eb8793d7bfa08a31a4529eb2e49b84c Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Sun, 29 Jan 2023 14:57:58 +0100 Subject: [PATCH] Implement source location matching for weave messages in XML tests WIP (work in progress). Closes #218. Signed-off-by: Alexander Kriegisch --- .../java/org/aspectj/bridge/WeaveMessage.java | 41 ++++++++----- .../lookup/EclipseSourceLocation.java | 8 ++- .../org/aspectj/tools/ajc/AjcTestCase.java | 52 ++++++++++++----- .../java/org/aspectj/testing/CompileSpec.java | 2 +- .../aspectj/testing/ExpectedMessageSpec.java | 32 +++++++++- tests/ajcTestSuite.dtd | 4 ++ .../aspectj/weaver/bcel/BcelTypeMunger.java | 58 +++++++++++++------ .../org/aspectj/weaver/bcel/BcelWorld.java | 32 +++++++--- 8 files changed, 166 insertions(+), 63 deletions(-) diff --git a/bridge/src/main/java/org/aspectj/bridge/WeaveMessage.java b/bridge/src/main/java/org/aspectj/bridge/WeaveMessage.java index 454a3e6be..2a0aec435 100644 --- a/bridge/src/main/java/org/aspectj/bridge/WeaveMessage.java +++ b/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 { diff --git a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceLocation.java b/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceLocation.java index cbd21e585..274bf1e90 100644 --- a/org.aspectj.ajdt.core/src/main/java/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceLocation.java +++ b/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; diff --git a/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java b/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java index bc934815c..0a8f0fcfc 100644 --- a/org.aspectj.ajdt.core/src/test/java/org/aspectj/tools/ajc/AjcTestCase.java +++ b/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 { *

*/ 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 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 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; diff --git a/testing/src/test/java/org/aspectj/testing/CompileSpec.java b/testing/src/test/java/org/aspectj/testing/CompileSpec.java index 889183303..b018102d9 100644 --- a/testing/src/test/java/org/aspectj/testing/CompileSpec.java +++ b/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()); } diff --git a/testing/src/test/java/org/aspectj/testing/ExpectedMessageSpec.java b/testing/src/test/java/org/aspectj/testing/ExpectedMessageSpec.java index 23903beb7..5d438a677 100644 --- a/testing/src/test/java/org/aspectj/testing/ExpectedMessageSpec.java +++ b/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. */ diff --git a/tests/ajcTestSuite.dtd b/tests/ajcTestSuite.dtd index 6d6712fb9..4645542e6 100644 --- a/tests/ajcTestSuite.dtd +++ b/tests/ajcTestSuite.dtd @@ -60,9 +60,13 @@ + + + + diff --git a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelTypeMunger.java b/weaver/src/main/java/org/aspectj/weaver/bcel/BcelTypeMunger.java index 1d33f417e..9e5b4268c 100644 --- a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelTypeMunger.java +++ b/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() + ) + ); } } diff --git a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/main/java/org/aspectj/weaver/bcel/BcelWorld.java index 745bd02b2..408b49870 100644 --- a/weaver/src/main/java/org/aspectj/weaver/bcel/BcelWorld.java +++ b/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); -- 2.39.5