Bug 474020 - [ASTRewrite] Nested modifications are suppressed by macros

Added further check to isStatementWithMixedLocation return false if all
macro expansions are further enclosed in child statements.

Change-Id: Ida6994adbbf76674c37a4c1c214aeea79b4963f2
Signed-off-by: Thomas Corbat <tcorbat@hsr.ch>

3 files changed, 292 insertions, 7 deletions

diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ReplaceTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ReplaceTests.java
index 41f15835075..4a04c4f5a2e 100644
--- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ReplaceTests.java
+++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/ReplaceTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2014 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2008, 2015 Institute for Software, HSR Hochschule fuer Technik  
  * Rapperswil, University of applied sciences and others
  * All rights reserved. This program and the accompanying materials 
  * are made available under the terms of the Eclipse Public License v1.0 
@@ -734,4 +734,249 @@ public class ReplaceTests extends ChangeGeneratorTest {
 			}
 		});
 	}
+
+	//#define ONE 1
+	//void foo() {
+	//	if (true) {
+	//		int one = ONE;
+	//		int three = 2;
+	//	}
+	//}
+
+	//#define ONE 1
+	//void foo() {
+	//	if (true) {
+	//		int one = ONE;
+	//		int three = 2;
+	//	}
+	//	if (true) {
+	//		int one = ONE;
+	//		int two = 2;
+	//	}
+	//}
+	public void testNestedReplacementInIfStatementWithMacroInSibling_474020() throws Exception {
+		compareResult(new ASTVisitor() {
+			private ASTModification parentModification;
+
+			{
+				shouldVisitNames = true;
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTIfStatement) {
+					parentModification = addModification(null, ModificationKind.APPEND_CHILD, statement.getParent(), statement);
+				}
+				return super.visit(statement);
+			}
+
+			@Override
+			public int visit(IASTName name) {
+				if (name.toString().equals("three")) {
+					IASTName newName = factory.newName("two");
+					addModification(parentModification, REPLACE, name, newName);
+					
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+		});
+	}
+
+	//#define TRUE true
+	//void foo() {
+	//	if (TRUE) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//}
+
+	//#define TRUE true
+	//void foo() {
+	//	if (TRUE) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//	if (TRUE) {
+	//		int one = 1;
+	//		int two = 2;
+	//	}
+	//}
+	public void testNestedReplacementInIfStatementWithMacroInCondition_474020() throws Exception {
+		compareResult(new ASTVisitor() {
+			private ASTModification parentModification;
+
+			{
+				shouldVisitNames = true;
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTIfStatement) {
+					parentModification = addModification(null, ModificationKind.APPEND_CHILD, statement.getParent(), statement);
+				}
+				return super.visit(statement);
+			}
+
+			@Override
+			public int visit(IASTName name) {
+				if (name.toString().equals("three")) {
+					IASTName newName = factory.newName("two");
+					addModification(parentModification, REPLACE, name, newName);
+					
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+		});
+	}
+
+	//#define ONE 1
+	//void foo() {
+	//	if (ONE == 1) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//}
+
+	//#define ONE 1
+	//void foo() {
+	//	if (ONE == 1) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//	if (ONE == 1) {
+	//		int one = 1;
+	//		int two = 2;
+	//	}
+	//}
+	public void testNestedReplacementInIfStatementWithMacroAsFirstPartOfCondition_474020() throws Exception {
+		compareResult(new ASTVisitor() {
+			private ASTModification parentModification;
+
+			{
+				shouldVisitNames = true;
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTIfStatement) {
+					parentModification = addModification(null, ModificationKind.APPEND_CHILD, statement.getParent(), statement);
+				}
+				return super.visit(statement);
+			}
+
+			@Override
+			public int visit(IASTName name) {
+				if (name.toString().equals("three")) {
+					IASTName newName = factory.newName("two");
+					addModification(parentModification, REPLACE, name, newName);
+					
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+		});
+	}
+
+	//#define ONE 1
+	//void foo() {
+	//	if (0 + ONE == 1) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//}
+
+	//#define ONE 1
+	//void foo() {
+	//	if (0 + ONE == 1) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//	if (0 + ONE == 1) {
+	//		int one = 1;
+	//		int two = 2;
+	//	}
+	//}
+	public void testNestedReplacementInIfStatementWithMacroAsInnerPartOfCondition_474020() throws Exception {
+		compareResult(new ASTVisitor() {
+			private ASTModification parentModification;
+
+			{
+				shouldVisitNames = true;
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTIfStatement) {
+					parentModification = addModification(null, ModificationKind.APPEND_CHILD, statement.getParent(), statement);
+				}
+				return super.visit(statement);
+			}
+
+			@Override
+			public int visit(IASTName name) {
+				if (name.toString().equals("three")) {
+					IASTName newName = factory.newName("two");
+					addModification(parentModification, REPLACE, name, newName);
+					
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+		});
+	}
+
+	//#define TRUE true
+	//void foo() {
+	//	if (!TRUE) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//}
+
+	//#define TRUE true
+	//void foo() {
+	//	if (!TRUE) {
+	//		int one = 1;
+	//		int three = 2;
+	//	}
+	//	if (!TRUE) {
+	//		int one = 1;
+	//		int two = 2;
+	//	}
+	//}
+	public void testNestedReplacementInIfStatementWithMacroAsLastPartOfCondition_474020() throws Exception {
+		compareResult(new ASTVisitor() {
+			private ASTModification parentModification;
+
+			{
+				shouldVisitNames = true;
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTIfStatement) {
+					parentModification = addModification(null, ModificationKind.APPEND_CHILD, statement.getParent(), statement);
+				}
+				return super.visit(statement);
+			}
+
+			@Override
+			public int visit(IASTName name) {
+				if (name.toString().equals("three")) {
+					IASTName newName = factory.newName("two");
+					addModification(parentModification, REPLACE, name, newName);
+					
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+		});
+	}
 }
diff --git a/core/org.eclipse.cdt.core.tests/resources/rewrite/ASTWriterStatementTestSource.awts b/core/org.eclipse.cdt.core.tests/resources/rewrite/ASTWriterStatementTestSource.awts
index 66b9dbe1a01..603eaf61fe1 100644
--- a/core/org.eclipse.cdt.core.tests/resources/rewrite/ASTWriterStatementTestSource.awts
+++ b/core/org.eclipse.cdt.core.tests/resources/rewrite/ASTWriterStatementTestSource.awts
@@ -312,16 +312,14 @@ int foo()
 int foo()
 {
     int i = 1;
-    if (i == ZWO)
-    {
+    if (i == ZWO){
     }
 }
 //=
 int foo()
 {
     int i = 1;
-    if (i == ZWO)
-    {
+    if (i == ZWO){
     }
 }
 
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/MacroExpansionHandler.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/MacroExpansionHandler.java
index 49315c0d272..53307313a15 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/MacroExpansionHandler.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/astwriter/MacroExpansionHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2015 Institute for Software, HSR Hochschule fuer Technik
  * Rapperswil, University of applied sciences and others
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -59,13 +59,55 @@ public class MacroExpansionHandler {
 		if (nodeLocations != null && nodeLocations.length > 1) {
 			for (IASTNodeLocation loc : nodeLocations) {
 				if (loc instanceof IASTMacroExpansionLocation) {
-					return true;
+					if (!hasChildEnclosingMacroLocation(node, (IASTMacroExpansionLocation)loc)) {
+						return true;
+					}
 				}
 			}
 		}
 		return false;
 	}
 
+	private boolean hasChildEnclosingMacroLocation(IASTNode node, IASTMacroExpansionLocation loc) {
+		IASTNode[] children = node.getChildren();
+		for (IASTNode child : children) {
+			if (childEnclosesMacroLocation(child, loc)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	private boolean childEnclosesMacroLocation(IASTNode child, IASTMacroExpansionLocation loc) {
+		IASTNodeLocation[] childLocations = child.getNodeLocations();
+		if (childLocations.length > 0 && hasMacroExpansionLocation(child, loc)) {
+			if (childLocations[0] instanceof IASTMacroExpansionLocation) {
+				IASTMacroExpansionLocation childMacroExpansionLocation = (IASTMacroExpansionLocation) childLocations[0];
+				return macroContainsOnlyPartsOfChild(loc, childMacroExpansionLocation);
+			} else if (childLocations[childLocations.length - 1] instanceof IASTMacroExpansionLocation) {
+				IASTMacroExpansionLocation childMacroExpansionLocation = (IASTMacroExpansionLocation) childLocations[childLocations.length - 1];
+				return macroContainsOnlyPartsOfChild(loc, childMacroExpansionLocation);
+			}
+			return true;
+		}
+		return false;
+	}
+
+	private boolean macroContainsOnlyPartsOfChild(IASTMacroExpansionLocation macroLocation, IASTMacroExpansionLocation childMacroLocation) {
+		return 	childMacroLocation.getExpansion().getMacroDefinition().equals(macroLocation.getExpansion().getMacroDefinition()) 
+				&& childMacroLocation.getNodeOffset() == macroLocation.getNodeOffset()
+				&& childMacroLocation.getNodeLength() == macroLocation.getNodeLength();
+	}
+
+	private boolean hasMacroExpansionLocation(IASTNode child, IASTMacroExpansionLocation macroLocation) {
+		for (IASTNodeLocation childLocation : child.getNodeLocations()) {
+			if (childLocation instanceof IASTMacroExpansionLocation) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	protected boolean macroExpansionAlreadyPrinted(IASTNode node) {
 		IASTNodeLocation[] locs = node.getNodeLocations();
 		if (locs.length ==1) {