From: aclement Date: Mon, 12 Jun 2006 13:09:51 +0000 (+0000) Subject: test and fix for 145963: faulting in aspects coming in as binary inputs X-Git-Tag: V1_5_2rc1~44 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4dcb8f3d88ed5e464bf31378d9c222fdf791ff4a;p=aspectj.git test and fix for 145963: faulting in aspects coming in as binary inputs --- diff --git a/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java b/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java index 42c60a203..8224c6d9f 100644 --- a/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java +++ b/asm/src/org/aspectj/asm/internal/AspectJElementHierarchy.java @@ -272,8 +272,8 @@ public class AspectJElementHierarchy implements IHierarchy { lastSlash = sourceFilePath.lastIndexOf('/'); } String fileName = sourceFilePath.substring(lastSlash+1); - IProgramElement fileNode = new ProgramElement(fileName, IProgramElement.Kind.FILE_JAVA, null); - fileNode.setSourceLocation(new SourceLocation(new File(sourceFilePath), 1, 1)); + IProgramElement fileNode = new ProgramElement(fileName, IProgramElement.Kind.FILE_JAVA, new SourceLocation(new File(sourceFilePath), 1, 1),0,null,null); + //fileNode.setSourceLocation(); fileNode.addChild(NO_STRUCTURE); return fileNode; } diff --git a/asm/src/org/aspectj/asm/internal/ProgramElement.java b/asm/src/org/aspectj/asm/internal/ProgramElement.java index acddddfb2..af5a16960 100644 --- a/asm/src/org/aspectj/asm/internal/ProgramElement.java +++ b/asm/src/org/aspectj/asm/internal/ProgramElement.java @@ -162,8 +162,10 @@ public class ProgramElement implements IProgramElement { return sourceLocation; } + // not really sure why we have this setter ... how can we be in the situation where we didn't + // know the location when we built the node but we learned it later on? public void setSourceLocation(ISourceLocation sourceLocation) { - //this.sourceLocation = sourceLocation; +// this.sourceLocation = sourceLocation; } public IMessage getMessage() { diff --git a/tests/bugs152/pr145963/A.aj b/tests/bugs152/pr145963/A.aj new file mode 100644 index 000000000..657c79b36 --- /dev/null +++ b/tests/bugs152/pr145963/A.aj @@ -0,0 +1,10 @@ +package pkg; + +public aspect A { + + pointcut p() : execution(* *.*(..)) && !within(pkg.*); + + before() : p() { + } + +} diff --git a/tests/bugs152/pr145963/ClassForAspectPath.java b/tests/bugs152/pr145963/ClassForAspectPath.java new file mode 100644 index 000000000..0494c9c84 --- /dev/null +++ b/tests/bugs152/pr145963/ClassForAspectPath.java @@ -0,0 +1,12 @@ +public class ClassForAspectPath { + + public static void main(String[] args) { + new ClassForAspectPath().method(); + + } + + public void method() { + System.out.println("blah"); + } + +} diff --git a/tests/bugs152/pr145963/Deow.aj b/tests/bugs152/pr145963/Deow.aj new file mode 100644 index 000000000..bde434340 --- /dev/null +++ b/tests/bugs152/pr145963/Deow.aj @@ -0,0 +1,7 @@ +package pkg; + +public aspect Deow { + + declare warning : (get(* System.out) || get(* System.err)) : "There should be no printlns"; + +} diff --git a/tests/bugs152/pr145963/Itd.aj b/tests/bugs152/pr145963/Itd.aj new file mode 100644 index 000000000..f53963c3b --- /dev/null +++ b/tests/bugs152/pr145963/Itd.aj @@ -0,0 +1,7 @@ +package pkg; + +public aspect Itd { + + declare parents : Point extends NewClass; + +} diff --git a/tests/bugs152/pr145963/NewClass.java b/tests/bugs152/pr145963/NewClass.java new file mode 100644 index 000000000..187e321c6 --- /dev/null +++ b/tests/bugs152/pr145963/NewClass.java @@ -0,0 +1,5 @@ +package pkg; + +public class NewClass { + +} diff --git a/tests/bugs152/pr145963/adviceAndDeow.jar b/tests/bugs152/pr145963/adviceAndDeow.jar new file mode 100644 index 000000000..d9ab8c793 Binary files /dev/null and b/tests/bugs152/pr145963/adviceAndDeow.jar differ diff --git a/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java b/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java index 05c311f50..1cf8167f7 100644 --- a/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java +++ b/tests/src/org/aspectj/systemtest/ajc152/AllTestsAspectJ152.java @@ -19,6 +19,7 @@ public class AllTestsAspectJ152 { TestSuite suite = new TestSuite("AspectJ 1.5.2 tests"); //$JUnit-BEGIN$ suite.addTest(Ajc152Tests.suite()); + suite.addTest(CreatingModelForInjarTests.suite()); suite.addTest(SynchronizationTests.suite()); suite.addTest(SynchronizationTransformTests.suite()); //$JUnit-END$ diff --git a/tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java b/tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java new file mode 100644 index 000000000..213dd2bdc --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc152/CreatingModelForInjarTests.java @@ -0,0 +1,134 @@ +/******************************************************************** + * Copyright (c) 2006 Contributors. All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution and is available at + * http://eclipse.org/legal/epl-v10.html + * + * Contributors: IBM Corporation - initial API and implementation + * Helen Hawkins - initial version + *******************************************************************/ +package org.aspectj.systemtest.ajc152; + +import java.io.File; +import java.util.Iterator; +import java.util.List; + +import junit.framework.Test; + +import org.aspectj.asm.AsmManager; +import org.aspectj.asm.IHierarchy; +import org.aspectj.asm.IProgramElement; +import org.aspectj.testing.XMLBasedAjcTestCase; +import org.aspectj.weaver.World; + +public class CreatingModelForInjarTests extends org.aspectj.testing.XMLBasedAjcTestCase { + + public void testAdviceAndNamedPCD() { + runTest("advice and deow"); + + // expect: + // - pkg {package} + // - A.aj (binary) {java source file} + // - import declarations {import reference} + // - A {aspect} + // - p {pointcut} + // - before {advice} + + IProgramElement pkgNode = getPkgNode(); + IProgramElement srcFile = checkChild(pkgNode,IProgramElement.Kind.FILE_JAVA,"A.aj (binary)",1); + checkChild(srcFile,IProgramElement.Kind.IMPORT_REFERENCE,"import declarations",-1); + IProgramElement aspectNode = checkChild(srcFile,IProgramElement.Kind.ASPECT,"A",-1); + checkChild(aspectNode,IProgramElement.Kind.POINTCUT,"p",5); + checkChild(aspectNode,IProgramElement.Kind.ADVICE,"before",7); + } + + public void testDeclareWarning() { + runTest("advice and deow"); + + // expect: + // - pkg {package} + // - Deow.aj (binary) {java source file} + // - import declarations {import reference} + // - Deow {aspect} + // - declare warning {declare warning} + + IHierarchy top = AsmManager.getDefault().getHierarchy(); + IProgramElement dwNode = top.findElementForLabel(top.getRoot(), + IProgramElement.Kind.DECLARE_WARNING, + "declare warning: \"There should be n..\""); + assertNotNull("Couldn't find 'declare warning: \"There should be n..\"' " + + "element in the tree",dwNode); + assertEquals("expected 'declare warning: \"There should be n..\"'" + + " to be on line 5 but was on " + dwNode.getSourceLocation().getLine(), + 5, dwNode.getSourceLocation().getLine()); + } + + public void testNumberOfPackageNodes() { + runTest("advice and deow"); + // check that the 'pkg' package node has not been added twice + IProgramElement root = AsmManager.getDefault().getHierarchy().getRoot(); + List l = root.getChildren(); + int numberOfPkgs = 0; + for (Iterator iter = l.iterator(); iter.hasNext();) { + IProgramElement element = (IProgramElement) iter.next(); + if (element.getKind().equals(IProgramElement.Kind.PACKAGE) + && element.getName().equals("pkg")) { + numberOfPkgs++; + } + } + assertEquals("expected one package called 'pkg' but found " + numberOfPkgs,1,numberOfPkgs); + } + + private IProgramElement getPkgNode() { + IHierarchy top = AsmManager.getDefault().getHierarchy(); + IProgramElement pkgNode = top.findElementForLabel(top.getRoot(), + IProgramElement.Kind.PACKAGE,"pkg"); + assertNotNull("Couldn't find 'pkg' element in the tree",pkgNode); + return pkgNode; + } + + private IProgramElement checkChild(IProgramElement parent, + IProgramElement.Kind childKind, + String childName, + int childLineNumbr) { + List children = parent.getChildren(); + boolean foundChild = false; + for (Iterator iter = children.iterator(); iter.hasNext();) { + IProgramElement element = (IProgramElement) iter.next(); + if (element.getKind().equals(childKind) + && element.getName().equals(childName) ) { + foundChild = true; + if (childLineNumbr != -1) { + assertEquals("expected " + childKind.toString() + childName + + " to be on line " + childLineNumbr + " but was on " + + element.getSourceLocation().getLine(), + childLineNumbr, element.getSourceLocation().getLine()); + } + return element; + } + } + assertTrue("expected " + parent.getName() + " to have child " + childName + + " but it did not", foundChild); + return null; + } + + protected void setUp() throws Exception { + super.setUp(); + World.createInjarHierarchy = true; + } + + protected void tearDown() throws Exception { + super.tearDown(); + World.createInjarHierarchy = false; + } + + // /////////////////////////////////////// + public static Test suite() { + return XMLBasedAjcTestCase.loadSuite(CreatingModelForInjarTests.class); + } + + protected File getSpecFile() { + return new File("../tests/src/org/aspectj/systemtest/ajc152/injar.xml"); + } +} diff --git a/tests/src/org/aspectj/systemtest/ajc152/injar.xml b/tests/src/org/aspectj/systemtest/ajc152/injar.xml new file mode 100644 index 000000000..955069a3e --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc152/injar.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/weaver/src/org/aspectj/weaver/Advice.java b/weaver/src/org/aspectj/weaver/Advice.java index 959ce3a18..737e0d087 100644 --- a/weaver/src/org/aspectj/weaver/Advice.java +++ b/weaver/src/org/aspectj/weaver/Advice.java @@ -428,5 +428,11 @@ public abstract class Advice extends ShadowMunger { public ResolvedType getConcreteAspect() { return concreteAspect; } + + public ResolvedType getResolvedDeclaringAspect() { + // The aspect which declares this piece of advice + // is 'concreteAspect' since 'declaringType' is null + return concreteAspect; + } } diff --git a/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java b/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java index 68398df13..7ea351930 100644 --- a/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java +++ b/weaver/src/org/aspectj/weaver/AsmRelationshipProvider.java @@ -61,7 +61,11 @@ public class AsmRelationshipProvider { checker.getSourceLocation().getLine(), checker.getSourceLocation().getColumn(), checker.getSourceLocation().getOffset()); - + + if (World.createInjarHierarchy) { + checker.createHierarchy(); + } + String targetHandle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier( shadow.getSourceLocation().getSourceFile(), shadow.getSourceLocation().getLine(), diff --git a/weaver/src/org/aspectj/weaver/Checker.java b/weaver/src/org/aspectj/weaver/Checker.java index 6700b8b36..329716106 100644 --- a/weaver/src/org/aspectj/weaver/Checker.java +++ b/weaver/src/org/aspectj/weaver/Checker.java @@ -13,11 +13,17 @@ package org.aspectj.weaver; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; import org.aspectj.asm.IRelationship; -import org.aspectj.bridge.*; -import org.aspectj.weaver.patterns.*; +import org.aspectj.bridge.IMessage; +import org.aspectj.bridge.ISourceLocation; +import org.aspectj.bridge.Message; +import org.aspectj.weaver.patterns.DeclareErrorOrWarning; +import org.aspectj.weaver.patterns.PerClause; +import org.aspectj.weaver.patterns.Pointcut; public class Checker extends ShadowMunger { @@ -127,4 +133,8 @@ public class Checker extends ShadowMunger { return isError; } + public ResolvedType getResolvedDeclaringAspect() { + // The aspect which declares this deow is the declaring type + return getDeclaringType(); + } } diff --git a/weaver/src/org/aspectj/weaver/ShadowMunger.java b/weaver/src/org/aspectj/weaver/ShadowMunger.java index 9646cfc20..6e045e83b 100644 --- a/weaver/src/org/aspectj/weaver/ShadowMunger.java +++ b/weaver/src/org/aspectj/weaver/ShadowMunger.java @@ -13,12 +13,19 @@ package org.aspectj.weaver; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; import java.util.Map; import org.aspectj.asm.AsmManager; +import org.aspectj.asm.IProgramElement; +import org.aspectj.asm.internal.ProgramElement; import org.aspectj.bridge.ISourceLocation; import org.aspectj.util.PartialOrder; +import org.aspectj.weaver.bcel.BcelAdvice; +import org.aspectj.weaver.patterns.DeclareErrorOrWarning; import org.aspectj.weaver.patterns.PerClause; import org.aspectj.weaver.patterns.Pointcut; @@ -91,11 +98,14 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH if (null == handle) { ISourceLocation sl = getSourceLocation(); if (sl != null) { + if (World.createInjarHierarchy) { + createHierarchy(); + } handle = AsmManager.getDefault().getHandleProvider().createHandleIdentifier( sl.getSourceFile(), sl.getLine(), sl.getColumn(), - sl.getOffset()); + sl.getOffset()); } } return handle; @@ -141,4 +151,118 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH * @return true if munger has to check that its exceptions can be throwned based on the shadow */ public abstract boolean mustCheckExceptions(); + + /** + * Returns the ResolvedType corresponding to the aspect in which this + * shadowMunger is declared. This is different for deow's and advice. + */ + public abstract ResolvedType getResolvedDeclaringAspect(); + + public void createHierarchy() { + IProgramElement sourceFileNode = AsmManager.getDefault().getHierarchy().findElementForSourceLine(getSourceLocation()); + if (!sourceFileNode.getKind().equals(IProgramElement.Kind.FILE_JAVA)) { + return; + } + String name = sourceFileNode.getName(); + sourceFileNode.setName(name + " (binary)"); + + ResolvedType aspect = getResolvedDeclaringAspect(); + + // create package ipe if one exists.... + IProgramElement root = AsmManager.getDefault().getHierarchy().getRoot(); + if (aspect.getPackageName() != null) { + // check that there doesn't already exist a node with this name + IProgramElement pkgNode = AsmManager.getDefault().getHierarchy().findElementForLabel( + root,IProgramElement.Kind.PACKAGE,aspect.getPackageName()); + // note packages themselves have no source location + if (pkgNode == null) { + pkgNode = new ProgramElement( + aspect.getPackageName(), + IProgramElement.Kind.PACKAGE, + new ArrayList()); + root.addChild(pkgNode); + } + pkgNode.addChild(sourceFileNode); + } else { + root.addChild(sourceFileNode); + } + + // remove the error child from the 'A.aj' node + if (sourceFileNode instanceof ProgramElement) { + IProgramElement errorNode = (IProgramElement) sourceFileNode.getChildren().get(0); + if (errorNode.getKind().equals(IProgramElement.Kind.ERROR)) { + ((ProgramElement)sourceFileNode).removeChild(errorNode); + } + } + + // add and create empty import declaration ipe + sourceFileNode.addChild(new ProgramElement( + "import declarations", + IProgramElement.Kind.IMPORT_REFERENCE, + null,0,null,null)); + + // add and create aspect ipe + IProgramElement aspectNode = new ProgramElement( + aspect.getSimpleName(), + IProgramElement.Kind.ASPECT, + aspect.getSourceLocation(), + aspect.getModifiers(), + null,null); + sourceFileNode.addChild(aspectNode); + + addChildNodes(aspectNode,aspect.getDeclaredPointcuts()); + + addChildNodes(aspectNode,aspect.getDeclaredAdvice()); + addChildNodes(aspectNode,aspect.getDeclares()); + } + + private void addChildNodes(IProgramElement parent,ResolvedMember[] children) { + for (int i = 0; i < children.length; i++) { + ResolvedMember pcd = children[i]; + if (pcd instanceof ResolvedPointcutDefinition) { + ResolvedPointcutDefinition rpcd = (ResolvedPointcutDefinition)pcd; + parent.addChild(new ProgramElement( + pcd.getName(), + IProgramElement.Kind.POINTCUT, + rpcd.getPointcut().getSourceLocation(), + pcd.getModifiers(), + null, + Collections.EMPTY_LIST)); + } + } + } + + private void addChildNodes(IProgramElement parent, Collection children) { + for (Iterator iter = children.iterator(); iter.hasNext();) { + Object element = (Object) iter.next(); + if (element instanceof DeclareErrorOrWarning) { + DeclareErrorOrWarning decl = (DeclareErrorOrWarning)element; + IProgramElement deowNode = new ProgramElement( + decl.isError() ? "declare error" : "declare warning", + decl.isError() ? IProgramElement.Kind.DECLARE_ERROR : IProgramElement.Kind.DECLARE_WARNING, + getSourceLocation(), + this.getDeclaringType().getModifiers(), + null,null); + deowNode.setDetails("\"" + genDeclareMessage(decl.getMessage()) + "\""); + parent.addChild(deowNode); + } else if (element instanceof BcelAdvice) { + BcelAdvice advice = (BcelAdvice)element; + parent.addChild(new ProgramElement( + advice.kind.getName(), + IProgramElement.Kind.ADVICE, + getSourceLocation(), + advice.signature.getModifiers(),null,Collections.EMPTY_LIST)); + } + } + } + + // taken from AsmElementFormatter + private String genDeclareMessage(String message) { + int length = message.length(); + if (length < 18) { + return message; + } else { + return message.substring(0, 17) + ".."; + } + } } diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java index 5cb41d2b4..108fff263 100644 --- a/weaver/src/org/aspectj/weaver/World.java +++ b/weaver/src/org/aspectj/weaver/World.java @@ -59,7 +59,10 @@ public abstract class World implements Dump.INode { // to be independent of location before we can do that. /** Should we take into account source location when comparing mungers - which may trigger full builds */ public final static boolean compareLocations = true; - + + // see pr145963 + /** Should we create the hierarchy for binary classes and aspects*/ + public static boolean createInjarHierarchy = true; /** Calculator for working out aspect precedence */ private AspectPrecedenceCalculator precedenceCalculator;