Bugzilla Bug 33635

   Negation of if pointcut does not work

3 files changed, 74 insertions, 6 deletions

diff --git a/tests/bugs/NotIf.java b/tests/bugs/NotIf.java
new file mode 100644
index 000000000..3220104c7
--- /dev/null
+++ b/tests/bugs/NotIf.java
@@ -0,0 +1,49 @@
+// for Bug#:  33635  
+import org.aspectj.testing.Tester;
+
+
+public class NotIf {
+    public static void main(String[] args) {
+		Tester.checkEqual(Aspect1.ranNone, 0, "shouldn't run");
+		Tester.checkEqual(Aspect1.ranTwo, 2, "should run");
+		Tester.checkEqual(Aspect2.ran, 1, "should run with values");
+    }
+}
+
+aspect Aspect1 {
+	static int ranNone = 0;
+	static int ranTwo = 0;
+	
+	static boolean testTrue() { return true; }
+	
+	static boolean testFalse() { return false; }
+	
+	before(): execution(void main(..)) && !if(testTrue()) {
+		ranNone += 1;
+	}
+	
+	before(): execution(void main(..)) && if(!testTrue()) {
+		ranNone += 1;
+	}
+	before(): execution(void main(..)) && !if(testFalse()) {
+		ranTwo += 1;
+	}
+	
+	before(): execution(void main(..)) && if(!testFalse()) {
+		ranTwo += 1;
+	}
+}
+
+aspect Aspect2 {
+	static int ran = 0;
+	
+	static boolean testValues(int i, String s, Object o) {
+		return false;
+	}
+	
+	before(String[] a): execution(void main(String[])) && 
+				!if(testValues(a.length, a.toString(), a)) && args(a)
+	{
+		ran += 1;
+	}
+}
diff --git a/tests/jimTests.xml b/tests/jimTests.xml
index e7e9b8b97..6306ec843 100644
--- a/tests/jimTests.xml
+++ b/tests/jimTests.xml
@@ -1,6 +1,13 @@
 <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd">
 <suite>
 
+    <ajc-test dir="bugs" pr="33635" 
+    		title="Negation of if pointcut does not work">
+        <compile files="NotIf.java"/>
+        <run class="NotIf"/>
+    </ajc-test>
+    
+    
     <!--
     
 
diff --git a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
index 4c4beb1ab..267e5e00c 100644
--- a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
@@ -29,6 +29,7 @@ import org.aspectj.weaver.ResolvedPointcutDefinition;
 import org.aspectj.weaver.ResolvedTypeX;
 import org.aspectj.weaver.Shadow;
 import org.aspectj.weaver.ShadowMunger;
+import org.aspectj.weaver.ast.*;
 import org.aspectj.weaver.ast.Expr;
 import org.aspectj.weaver.ast.Literal;
 import org.aspectj.weaver.ast.Test;
@@ -84,6 +85,9 @@ public class IfPointcut extends Pointcut {
 		return "if(" + testMethod + ")";
 	}
 
+
+	//??? The implementation of name binding and type checking in if PCDs is very convoluted
+	//    There has to be a better way...
 	private boolean findingResidue = false;
 	public Test findResidue(Shadow shadow, ExposedState state) {
 		if (findingResidue) return Literal.TRUE;
@@ -91,15 +95,21 @@ public class IfPointcut extends Pointcut {
 		try {
 			ExposedState myState = new ExposedState(baseArgsCount);
 			//System.out.println(residueSource);
-			//??? some of these tests are preconditions for the if to run correctly
-			//    this implementation will duplicate those tests, we should be more careful
-			Test preTest = residueSource.findResidue(shadow, myState); // might need this
+			//??? we throw out the test that comes from this walk.  All we want here
+			//    is bindings for the arguments
+			residueSource.findResidue(shadow, myState);
 			
 			//System.out.println(myState);
 			
+			Test ret = Literal.TRUE;
+			
 			List args = new ArrayList();
 	        for (int i=0; i < baseArgsCount; i++) {
-	        	args.add(myState.get(i));
+	        	Var v = myState.get(i);
+	        	args.add(v);
+	        	ret = Test.makeAnd(ret, 
+	        		Test.makeInstanceof(v, 
+	        			testMethod.getParameterTypes()[i].resolve(shadow.getIWorld())));
 	        }
 	
 	        // handle thisJoinPoint parameters
@@ -114,8 +124,10 @@ public class IfPointcut extends Pointcut {
 	        if ((extraParameterFlags & Advice.ThisEnclosingJoinPointStaticPart) != 0) {
 	        	args.add(shadow.getThisEnclosingJoinPointStaticPartVar());
 	        }
-			Test myTest = Test.makeCall(testMethod, (Expr[])args.toArray(new Expr[args.size()]));
-			return Test.makeAnd(preTest, myTest);
+	        
+	        ret = Test.makeAnd(ret, Test.makeCall(testMethod, (Expr[])args.toArray(new Expr[args.size()])));
+
+			return ret; 
 			
 		} finally {
 			findingResidue = false;