From 4d6668df0ce64175c209333705f457ee47376ef0 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Wed, 27 Nov 2019 19:03:07 -0800 Subject: [PATCH] Fix 550494 --- .../weaver/MethodDelegateTypeMunger.java | 19 ++-- .../java/org/aspectj/testing/OutputSpec.java | 39 ++++--- tests/bugs195/550494/Application.java | 15 +++ tests/bugs195/550494/BaseClass.java | 19 ++++ tests/bugs195/550494/Code.java | 19 ++++ .../DataSourceConnectionAspectAnno.java | 20 ++++ .../DataSourceConnectionAspectCode.java | 19 ++++ tests/bugs195/550494/SubClass.java | 5 + .../systemtest/ajc195/Ajc195Tests.java | 8 ++ .../org/aspectj/systemtest/ajc195/ajc195.xml | 30 ++++++ .../aspectj/weaver/bcel/AtAjAttributes.java | 100 ++++++++---------- 11 files changed, 210 insertions(+), 83 deletions(-) create mode 100644 tests/bugs195/550494/Application.java create mode 100644 tests/bugs195/550494/BaseClass.java create mode 100644 tests/bugs195/550494/Code.java create mode 100644 tests/bugs195/550494/DataSourceConnectionAspectAnno.java create mode 100644 tests/bugs195/550494/DataSourceConnectionAspectCode.java create mode 100644 tests/bugs195/550494/SubClass.java diff --git a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/MethodDelegateTypeMunger.java b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/MethodDelegateTypeMunger.java index 79eab7d4d..bf8a76529 100644 --- a/org.aspectj.matcher/src/main/java/org/aspectj/weaver/MethodDelegateTypeMunger.java +++ b/org.aspectj.matcher/src/main/java/org/aspectj/weaver/MethodDelegateTypeMunger.java @@ -6,7 +6,7 @@ * which accompanies this distribution and is available at * http://eclipse.org/legal/epl-v10.html * - * Contributors: + * Contributors: * Alexandre Vasseur initial implementation * ******************************************************************/ @@ -20,7 +20,7 @@ import org.aspectj.weaver.patterns.TypePattern; * Type munger for annotation style ITD declare parents. with an interface AND an implementation. Given the aspect that has a field * public static Interface fieldI = ... // impl. we will weave in the Interface' methods and delegate to the aspect public static * field fieldI - * + * * Note: this munger DOES NOT handles the interface addition to the target classes - a regular Parent kinded munger must be added in * coordination. */ @@ -51,7 +51,7 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { /** * Construct a new type munger for @AspectJ ITD - * + * * @param signature * @param aspect * @param implClassName @@ -131,7 +131,7 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { kind.write(s); signature.write(s); aspect.write(s); - s.writeUTF(implClassName); + s.writeUTF(implClassName==null?"":implClassName); typePattern.write(s); fieldType.write(s); s.writeUTF(factoryMethodName); @@ -144,6 +144,9 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { ResolvedMemberImpl signature = ResolvedMemberImpl.readResolvedMember(s, context); UnresolvedType aspect = UnresolvedType.read(s); String implClassName = s.readUTF(); + if (implClassName.equals("")) { + implClassName = null; + } TypePattern tp = TypePattern.read(s, context); MethodDelegateTypeMunger typeMunger = new MethodDelegateTypeMunger(signature, aspect, implClassName, tp); UnresolvedType fieldType = null; @@ -164,7 +167,7 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { /** * Match based on given type pattern, only classes can be matched - * + * * @param matchType * @param aspectType * @return true if match @@ -180,7 +183,7 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { /** * Needed for reweavable - * + * * @return true */ public boolean changesPublicSignature() { @@ -198,7 +201,7 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { /** * Construct a new type munger for @AspectJ ITD - * + * * @param field * @param aspect * @param typePattern @@ -241,7 +244,7 @@ public class MethodDelegateTypeMunger extends ResolvedTypeMunger { /** * Match based on given type pattern, only classes can be matched - * + * * @param matchType * @param aspectType * @return true if match diff --git a/testing/src/test/java/org/aspectj/testing/OutputSpec.java b/testing/src/test/java/org/aspectj/testing/OutputSpec.java index 9eab46098..9a6fd5fef 100644 --- a/testing/src/test/java/org/aspectj/testing/OutputSpec.java +++ b/testing/src/test/java/org/aspectj/testing/OutputSpec.java @@ -1,25 +1,25 @@ /* ******************************************************************* * Copyright (c) 2005 IBM Corporation - * 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://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * Adrian Colyer, + * 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://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Adrian Colyer, * ******************************************************************/ package org.aspectj.testing; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; -import java.util.ArrayList; import java.util.StringTokenizer; import org.aspectj.tools.ajc.AjcTestCase; import org.aspectj.util.LangUtil; public class OutputSpec { - + private List expectedOutputLines = new ArrayList(); public void addLine(OutputLine line) { @@ -27,7 +27,7 @@ public class OutputSpec { expectedOutputLines.add(line.getText()); } } - + /** * For a test output line that has specified a vm version, check if it matches the vm we are running on. * vm might be "1.2,1.3,1.4,1.5" or simply "9" or it may be a version with a '+' suffix indicating that @@ -51,7 +51,7 @@ public class OutputSpec { public void matchAgainst(String output) { matchAgainst(output, "yes"); } - + public void matchAgainst(String output, String ordered) { if (ordered != null && ordered.equals("no")) { unorderedMatchAgainst(output); @@ -76,21 +76,21 @@ public class OutputSpec { createFailureMessage(output, lineNo, strTok.countTokens()); } } - + public void unorderedMatchAgainst(String output) { List outputFound = getOutputFound(output); if(outputFound.size() != expectedOutputLines.size()) { createFailureMessage(output, -1, outputFound.size()); return; - } + } List expected = new ArrayList(); expected.addAll(expectedOutputLines); List found = new ArrayList(); found.addAll(outputFound); for (Iterator iterator = outputFound.iterator(); iterator.hasNext();) { - String lineFound = (String) iterator.next(); + String lineFound = iterator.next(); for (Iterator iterator2 = expectedOutputLines.iterator(); iterator2.hasNext();) { - String lineExpected = (String) iterator2.next(); + String lineExpected = iterator2.next(); if (lineFound.indexOf(lineExpected)!= -1) { found.remove(lineFound); expected.remove(lineExpected); @@ -102,12 +102,11 @@ public class OutputSpec { createFailureMessage(output,-2,outputFound.size()); } } - + private void createFailureMessage(String output, int lineNo, int sizeFound) { StringBuffer failMessage = new StringBuffer(); failMessage.append("\n expecting output:\n"); for (String line: expectedOutputLines) { - failMessage.append(line); failMessage.append("\n"); } failMessage.append(" but found output:\n"); @@ -119,9 +118,9 @@ public class OutputSpec { failMessage.append("First difference is on line " + lineNo); } failMessage.append("\n"); - AjcTestCase.fail(failMessage.toString()); + AjcTestCase.fail(failMessage.toString()); } - + private List getOutputFound(String output) { List found = new ArrayList(); StringTokenizer strTok = new StringTokenizer(output,"\n"); diff --git a/tests/bugs195/550494/Application.java b/tests/bugs195/550494/Application.java new file mode 100644 index 000000000..f539edb17 --- /dev/null +++ b/tests/bugs195/550494/Application.java @@ -0,0 +1,15 @@ +package foo; + +import java.sql.SQLException; + +public class Application { + public static void main(String[] args) throws SQLException { + System.out.println("Aspect should not kick in without ITD, but should with ITD"); + new BaseClass().getConnection(); + new BaseClass().getConnection("user", "pw"); + + System.out.println("Aspect should kick in"); + new SubClass().getConnection(); + new SubClass().getConnection("user", "pw"); + } +} diff --git a/tests/bugs195/550494/BaseClass.java b/tests/bugs195/550494/BaseClass.java new file mode 100644 index 000000000..f3087988a --- /dev/null +++ b/tests/bugs195/550494/BaseClass.java @@ -0,0 +1,19 @@ +package foo; + +import java.io.PrintWriter; +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.util.logging.Logger; + +public class BaseClass { + public PrintWriter getLogWriter() throws SQLException { return null; } + public void setLogWriter(PrintWriter out) throws SQLException {} + public void setLoginTimeout(int seconds) throws SQLException {} + public int getLoginTimeout() throws SQLException { return 0; } + public Logger getParentLogger() throws SQLFeatureNotSupportedException { return null; } + public T unwrap(Class iface) throws SQLException { return null; } + public boolean isWrapperFor(Class iface) throws SQLException { return false; } + public Connection getConnection() throws SQLException { return null; } + public Connection getConnection(String username, String password) throws SQLException { return null; } +} diff --git a/tests/bugs195/550494/Code.java b/tests/bugs195/550494/Code.java new file mode 100644 index 000000000..c7d05ff6a --- /dev/null +++ b/tests/bugs195/550494/Code.java @@ -0,0 +1,19 @@ +public class Code { + public static void main(String[] argv) { + } + + static aspect X { + before(): execution(* Code.main(..)) { + System.out.println( +""" +This +is +on +multiple +lines +""" +); + } + } + +} diff --git a/tests/bugs195/550494/DataSourceConnectionAspectAnno.java b/tests/bugs195/550494/DataSourceConnectionAspectAnno.java new file mode 100644 index 000000000..d78b48192 --- /dev/null +++ b/tests/bugs195/550494/DataSourceConnectionAspectAnno.java @@ -0,0 +1,20 @@ +package foo; + +import javax.sql.DataSource; + +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.annotation.DeclareParents; + +@Aspect +public class DataSourceConnectionAspectAnno { + // This statement crashes the AspectJ compiler + @DeclareParents("foo.BaseClass") + private DataSource dataSource; + + @Before("execution(public java.sql.Connection javax.sql.DataSource+.getConnection(..))") + public void myAdvice(JoinPoint thisJoinPoint) { + System.out.println(thisJoinPoint); + } +} diff --git a/tests/bugs195/550494/DataSourceConnectionAspectCode.java b/tests/bugs195/550494/DataSourceConnectionAspectCode.java new file mode 100644 index 000000000..aedc585b1 --- /dev/null +++ b/tests/bugs195/550494/DataSourceConnectionAspectCode.java @@ -0,0 +1,19 @@ +package foo; + +import javax.sql.DataSource; + +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.annotation.DeclareParents; + +import javax.sql.DataSource; + + +public aspect DataSourceConnectionAspectCode { + declare parents: BaseClass implements DataSource; + + before() : execution(public java.sql.Connection javax.sql.DataSource+.getConnection(..)) { + System.out.println(thisJoinPoint); + } +} diff --git a/tests/bugs195/550494/SubClass.java b/tests/bugs195/550494/SubClass.java new file mode 100644 index 000000000..7a8b4a3f9 --- /dev/null +++ b/tests/bugs195/550494/SubClass.java @@ -0,0 +1,5 @@ +package foo; + +import javax.sql.DataSource; + +public class SubClass extends BaseClass implements DataSource {} diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java b/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java index ef674b404..338ca47e9 100644 --- a/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java +++ b/tests/src/test/java/org/aspectj/systemtest/ajc195/Ajc195Tests.java @@ -16,6 +16,14 @@ import junit.framework.Test; */ public class Ajc195Tests extends XMLBasedAjcTestCase { + public void testAtDecpNPE_code_550494() { + runTest("at decp npe - code"); + } + + public void testAtDecpNPE_anno_550494() { + runTest("at decp npe - anno"); + } + public void testAvoidWeavingSwitchInfrastructure() { runTest("avoid weaving switch infrastructure"); } diff --git a/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml b/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml index 1084faf91..d9d679859 100644 --- a/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml +++ b/tests/src/test/resources/org/aspectj/systemtest/ajc195/ajc195.xml @@ -2,6 +2,36 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/weaver/src/main/java/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/main/java/org/aspectj/weaver/bcel/AtAjAttributes.java index 2da044c11..08a261569 100644 --- a/weaver/src/main/java/org/aspectj/weaver/bcel/AtAjAttributes.java +++ b/weaver/src/main/java/org/aspectj/weaver/bcel/AtAjAttributes.java @@ -85,7 +85,7 @@ import org.aspectj.weaver.patterns.TypePattern; /** * Annotation defined aspect reader. Reads the Java 5 annotations and turns them into AjAttributes - * + * * @author Alexandre Vasseur */ public class AtAjAttributes { @@ -128,7 +128,7 @@ public class AtAjAttributes { /** * A struct when we read @AJ on method - * + * * @author Alexandre Vasseur */ private static class AjAttributeMethodStruct extends AjAttributeStruct { @@ -176,7 +176,7 @@ public class AtAjAttributes { /** * Annotations are RuntimeVisible only. This allow us to not visit RuntimeInvisible ones. - * + * * @param attribute * @return true if runtime visible annotation */ @@ -186,7 +186,7 @@ public class AtAjAttributes { /** * Extract class level annotations and turn them into AjAttributes. - * + * * @param javaClass * @param type * @param context @@ -404,7 +404,7 @@ public class AtAjAttributes { /** * Extract method level annotations and turn them into AjAttributes. - * + * * @param method * @param type * @param context @@ -502,7 +502,7 @@ public class AtAjAttributes { /** * Extract field level annotations and turn them into AjAttributes. - * + * * @param field * @param type * @param context @@ -518,7 +518,7 @@ public class AtAjAttributes { /** * Read @Aspect - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -585,7 +585,7 @@ public class AtAjAttributes { /** * Read a perClause, returns null on failure and issue messages - * + * * @param perClauseString like "pertarget(.....)" * @param struct for which we are parsing the per clause * @return a PerClause instance @@ -638,7 +638,7 @@ public class AtAjAttributes { /** * Read @DeclarePrecedence - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -709,21 +709,12 @@ public class AtAjAttributes { // return false; // } - /** - * Read @DeclareParents - * - * @param runtimeAnnotations - * @param struct - * @return true if found - */ - private static boolean handleDeclareParentsAnnotation(RuntimeAnnos runtimeAnnotations, AjAttributeFieldStruct struct) {// , - // ResolvedPointcutDefinition - // preResolvedPointcut) - // { - AnnotationGen decp = getAnnotation(runtimeAnnotations, AjcMemberMaker.DECLAREPARENTS_ANNOTATION); - if (decp != null) { - NameValuePair decpPatternNVP = getAnnotationElement(decp, VALUE); - String decpPattern = decpPatternNVP.getValue().stringifyValue(); + private static boolean handleDeclareParentsAnnotation(RuntimeAnnos runtimeAnnotations, AjAttributeFieldStruct struct) { + AnnotationGen decpAnno = getAnnotation(runtimeAnnotations, AjcMemberMaker.DECLAREPARENTS_ANNOTATION); + if (decpAnno != null) { + NameValuePair decpPatternNameValuePair = getAnnotationElement(decpAnno, VALUE); + String decpPattern = decpPatternNameValuePair.getValue().stringifyValue(); + System.out.println("decpPatterNVP = "+decpPattern); if (decpPattern != null) { TypePattern typePattern = parseTypePattern(decpPattern, struct); ResolvedType fieldType = UnresolvedType.forSignature(struct.field.getSignature()).resolve( @@ -756,7 +747,7 @@ public class AtAjAttributes { // do we have a defaultImpl=xxx.class (ie implementation) String defaultImplClassName = null; - NameValuePair defaultImplNVP = getAnnotationElement(decp, "defaultImpl"); + NameValuePair defaultImplNVP = getAnnotationElement(decpAnno, "defaultImpl"); if (defaultImplNVP != null) { ClassElementValue defaultImpl = (ClassElementValue) defaultImplNVP.getValue(); defaultImplClassName = UnresolvedType.forSignature(defaultImpl.getClassString()).getName(); @@ -816,9 +807,8 @@ public class AtAjAttributes { } } - - // then iterate on field interface hierarchy (not object) boolean hasAtLeastOneMethod = false; + // then iterate on field interface hierarchy (not object) Iterator methodIterator = fieldType.getMethodsIncludingIntertypeDeclarations(false, true); while (methodIterator.hasNext()) { ResolvedMember method = methodIterator.next(); @@ -894,11 +884,11 @@ public class AtAjAttributes { /** * Process any @DeclareMixin annotation. - * + * * Example Declaration
- * + * * @DeclareMixin("Foo+") public I createImpl(Object o) { return new Impl(o); } - * + * *
* @param runtimeAnnotations * @param struct @@ -938,7 +928,7 @@ public class AtAjAttributes { // supplied as a list in the 'Class[] interfaces' value in the annotation value // supplied as just the interface return value of the annotated method // supplied as just the class return value of the annotated method - NameValuePair interfaceListSpecified = getAnnotationElement(declareMixinAnnotation, "interfaces"); + NameValuePair interfaceListSpecified = getAnnotationElement(declareMixinAnnotation, "interfaces"); List newParents = new ArrayList(1); List newInterfaceTypes = new ArrayList(1); @@ -1036,7 +1026,7 @@ public class AtAjAttributes { /** * Read @Before - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -1089,7 +1079,7 @@ public class AtAjAttributes { /** * Read @After - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -1141,7 +1131,7 @@ public class AtAjAttributes { /** * Read @AfterReturning - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -1231,7 +1221,7 @@ public class AtAjAttributes { /** * Read @AfterThrowing - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -1320,7 +1310,7 @@ public class AtAjAttributes { /** * Read @Around - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -1372,7 +1362,7 @@ public class AtAjAttributes { /** * Read @Pointcut and handle the resolving in a lazy way to deal with pointcut references - * + * * @param runtimeAnnotations * @param struct * @return true if a pointcut was handled @@ -1467,7 +1457,7 @@ public class AtAjAttributes { /** * Read @DeclareError, @DeclareWarning - * + * * @param runtimeAnnotations * @param struct * @return true if found @@ -1521,12 +1511,12 @@ public class AtAjAttributes { * Sets the location for the declare error / warning using the corresponding IProgramElement in the structure model. This will * only fix bug 120356 if compiled with -emacssym, however, it does mean that the cross references view in AJDT will show the * correct information. - * + * * Other possibilities for fix: 1. using the information in ajcDeclareSoft (if this is set correctly) which will fix the problem * if compiled with ajc but not if compiled with javac. 2. creating an AjAttribute called FieldDeclarationLineNumberAttribute * (much like MethodDeclarationLineNumberAttribute) which we can ask for the offset. This will again only fix bug 120356 when * compiled with ajc. - * + * * @param deow * @param struct */ @@ -1547,7 +1537,7 @@ public class AtAjAttributes { /** * Returns a readable representation of a method. Method.toString() is not suitable. - * + * * @param method * @return a readable representation of a method */ @@ -1560,7 +1550,7 @@ public class AtAjAttributes { /** * Build the bindings for a given method (pointcut / advice) - * + * * @param struct * @return null if no debug info is available */ @@ -1637,7 +1627,7 @@ public class AtAjAttributes { /** * Compute the flag for the xxxJoinPoint extra argument - * + * * @param method * @return extra arg flag */ @@ -1652,7 +1642,7 @@ public class AtAjAttributes { /** * Compute the flag for the xxxJoinPoint extra argument - * + * * @param argumentSignatures * @return extra arg flag */ @@ -1674,7 +1664,7 @@ public class AtAjAttributes { /** * Returns the runtime (RV/RIV) annotation of type annotationType or null if no such annotation - * + * * @param rvs * @param annotationType * @return annotation @@ -1691,7 +1681,7 @@ public class AtAjAttributes { /** * Returns the value of a given element of an annotation or null if not found Caution: Does not handles default value. - * + * * @param annotation * @param elementName * @return annotation NVP @@ -1731,7 +1721,7 @@ public class AtAjAttributes { * Extract the method argument names. First we try the debug info attached to the method (the LocalVariableTable) - if we cannot * find that we look to use the argNames value that may have been supplied on the associated annotation. If that fails we just * don't know and return an empty string. - * + * * @param method * @param argNamesFromAnnotation * @param methodStruct @@ -1853,7 +1843,7 @@ public class AtAjAttributes { /** * A method argument, used for sorting by indexOnStack (ie order in signature) - * + * * @author Alexandre Vasseur */ private static class MethodArgument { @@ -1869,7 +1859,7 @@ public class AtAjAttributes { /** * LazyResolvedPointcutDefinition lazyly resolve the pointcut so that we have time to register all pointcut referenced before * pointcut resolution happens - * + * * @author Alexandre Vasseur */ public static class LazyResolvedPointcutDefinition extends ResolvedPointcutDefinition { @@ -1901,7 +1891,7 @@ public class AtAjAttributes { /** * Helper to test empty strings - * + * * @param s * @return true if empty or null */ @@ -1911,7 +1901,7 @@ public class AtAjAttributes { /** * Set the pointcut bindings for which to ignore unbound issues, so that we can implicitly bind xxxJoinPoint for @AJ advices - * + * * @param pointcut * @param bindings */ @@ -1936,7 +1926,7 @@ public class AtAjAttributes { /** * Report an error - * + * * @param message * @param location */ @@ -1954,7 +1944,7 @@ public class AtAjAttributes { /** * Report a warning - * + * * @param message * @param location */ @@ -1966,7 +1956,7 @@ public class AtAjAttributes { /** * Parse the given pointcut, return null on failure and issue an error - * + * * @param pointcutString * @param struct * @param allowIf @@ -2000,7 +1990,7 @@ public class AtAjAttributes { /** * Parse the given type pattern, return null on failure and issue an error - * + * * @param patternString * @param location * @return type pattern -- 2.39.5