Bug 440710 - [ASTRewriter] UnhandledASTModificationException when ...

When a statement has been inserted into a replaced compound statement
createModifiedChildArray is called for the new compound statement. The
insert modification is attached to its sibling node. The implementation
for handling this insert was on the parent (compound statement) though.
I moved this (adapted) functionality to handling of the sibling nodes.
With the way insert modifications are stored the previous implementation
cannot have worked. Furthermore, there is no test code which covers the
deleted code.

Change-Id: I9615787177c12a870df7c9663170f38b2c28accb
Signed-off-by: Thomas Corbat <tcorbat@hsr.ch>
Reviewed-on: https://git.eclipse.org/r/30796
Reviewed-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>
Tested-by: Hudson CI

4 files changed, 177 insertions, 32 deletions

diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertBeforeTestSuite.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertBeforeTestSuite.java
index f22e76c32e0..1896b20a9a0 100644
--- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertBeforeTestSuite.java
+++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertBeforeTestSuite.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2008, 2014 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 
@@ -32,7 +32,9 @@ public class InsertBeforeTestSuite {
 		suite.addTest(AddDeclarationBugTest.suite());
 		suite.addTest(MultilineWhitespaceHandlingTest.suite());
 		suite.addTest(SelfInsertionTest.suite());
-				
+		suite.addTest(InsertStatementTest.suite());
+		suite.addTest(InsertMultipleStatementsTest.suite());
+
 		return suite;
 	}
 }
diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertMultipleStatementsTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertMultipleStatementsTest.java
new file mode 100644
index 00000000000..bb8883125aa
--- /dev/null
+++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertMultipleStatementsTest.java
@@ -0,0 +1,82 @@
+/*******************************************************************************
+ * Copyright (c) 2014 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
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Thomas Corbat (IFS)- initial API and implementation
+ ******************************************************************************/
+package org.eclipse.cdt.core.parser.tests.rewrite.changegenerator.insertbefore;
+
+import junit.framework.Test;
+
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTStatement;
+import org.eclipse.cdt.core.parser.tests.rewrite.changegenerator.ChangeGeneratorTest;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPNodeFactory;
+import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification;
+import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification.ModificationKind;
+import org.eclipse.cdt.internal.core.dom.rewrite.ASTModificationStore;
+import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ContainerNode;
+
+public class InsertMultipleStatementsTest extends ChangeGeneratorTest {
+	public InsertMultipleStatementsTest() {
+		super("InsertMultipleStatementsTest");
+	}
+
+	public static Test suite() {
+		return new InsertMultipleStatementsTest();
+	}
+
+	@Override
+	protected void setUp() throws Exception {
+		source =
+				"void function() {\n" +
+				"	int i;\n" +
+				"	int j;\n" +
+				"}";
+		expectedSource =
+				"void function() {\n" +
+				"	int i;\n" +
+				"	s1;\n" +
+				"	s2;\n" +
+				"	int j;\n" +
+				"}";
+		super.setUp();
+	}
+
+	@Override
+	protected ASTVisitor createModificator(final ASTModificationStore modStore) {
+		return new ASTVisitor() {
+			{
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTCompoundStatement) {
+					ASTModification compoundReplacement = new ASTModification(ModificationKind.REPLACE, statement, statement, null);
+					modStore.storeModification(null, compoundReplacement);
+					IASTNode secondStatement = statement.getChildren()[1];
+					IASTNode firstNewStatement = createStatement("s1");
+					IASTNode secondNewStatement = createStatement("s2");
+					ContainerNode newNodes = new ContainerNode(firstNewStatement, secondNewStatement);
+					ASTModification modification = new ASTModification(ModificationKind.INSERT_BEFORE, secondStatement, newNodes, null);
+					modStore.storeModification(compoundReplacement, modification);
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+
+			private IASTNode createStatement(String name) {
+				CPPNodeFactory factory = CPPNodeFactory.getDefault();
+				return factory.newExpressionStatement(factory.newIdExpression(factory.newName(name.toCharArray())));
+			}
+		};
+	}
+}
diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertStatementTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertStatementTest.java
new file mode 100644
index 00000000000..afeda48acb3
--- /dev/null
+++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/insertbefore/InsertStatementTest.java
@@ -0,0 +1,71 @@
+/*******************************************************************************
+ * Copyright (c) 2014 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
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Thomas Corbat (IFS)- initial API and implementation
+ ******************************************************************************/
+package org.eclipse.cdt.core.parser.tests.rewrite.changegenerator.insertbefore;
+
+import junit.framework.Test;
+
+import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement;
+import org.eclipse.cdt.core.dom.ast.IASTNode;
+import org.eclipse.cdt.core.dom.ast.IASTStatement;
+import org.eclipse.cdt.core.parser.tests.rewrite.changegenerator.ChangeGeneratorTest;
+import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification;
+import org.eclipse.cdt.internal.core.dom.rewrite.ASTModification.ModificationKind;
+import org.eclipse.cdt.internal.core.dom.rewrite.ASTModificationStore;
+
+public class InsertStatementTest extends ChangeGeneratorTest {
+	public InsertStatementTest() {
+		super("InsertStatementTest");
+	}
+
+	public static Test suite() {
+		return new InsertStatementTest();
+	}
+
+	@Override
+	protected void setUp() throws Exception {
+		source =
+				"void function() {\n" +
+				"	int i;\n" +
+				"	int j;\n" +
+				"}";
+		expectedSource =
+				"void function() {\n" +
+				"	int i;\n" +
+				"	int j;\n" +
+				"	int j;\n" +
+				"}";
+		super.setUp();
+	}
+
+	@Override
+	protected ASTVisitor createModificator(final ASTModificationStore modStore) {
+		return new ASTVisitor() {
+			{
+				shouldVisitStatements = true;
+			}
+
+			@Override
+			public int visit(IASTStatement statement) {
+				if (statement instanceof IASTCompoundStatement) {
+					ASTModification compoundReplacement = new ASTModification(ModificationKind.REPLACE, statement, statement, null);
+					modStore.storeModification(null, compoundReplacement);
+					IASTNode secondStatement = statement.getChildren()[1];
+					ASTModification modification = new ASTModification(ModificationKind.INSERT_BEFORE, secondStatement, secondStatement, null);
+					modStore.storeModification(compoundReplacement, modification);
+					return PROCESS_ABORT;
+				}
+				return PROCESS_CONTINUE;
+			}
+		};
+	}
+}
diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java
index 30fd72f613a..77f39e8cebd 100644
--- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java
+++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ASTModificationHelper.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2011 Institute for Software, HSR Hochschule fuer Technik  
+ * Copyright (c) 2008, 2014 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 
@@ -8,6 +8,7 @@
  *  
  * Contributors: 
  *     Institute for Software - initial API and implementation
+ *     Thomas Corbat (IFS)
  *******************************************************************************/
 package org.eclipse.cdt.internal.core.dom.rewrite.changegenerator;
 
@@ -54,39 +55,38 @@ public class ASTModificationHelper {
 					}
 				}
 				break;
-			
 			case INSERT_BEFORE:
-				newNode = parentModification.getNewNode();
-				if (newNode instanceof ContainerNode) {
-					ContainerNode contNode = (ContainerNode) newNode;
-					for (IASTNode node : contNode.getNodes()) {
-						insertNode(clazz, modifiedChildren, parentModification, node);
-					}
-				} else {
-					insertNode(clazz, modifiedChildren, parentModification, newNode);
-				}
-				break;	
-				
 			case REPLACE:
 				break;
 			}
 		}
-		
+
 		for (T currentChild : unmodifiedChildren) {
 			for (ASTModification childModification : modificationsForNode(currentChild)) {
 				try {
-					final T newNode = cast(childModification.getNewNode(), clazz);
+					IASTNode newNode = childModification.getNewNode();
+					final T castedNewNode = cast(newNode, clazz);
 					switch (childModification.getKind()) {
 					case REPLACE:
-						if (newNode != null) {
-							copyComments(newNode, currentChild, commentMap);
-							modifiedChildren.add(
-									modifiedChildren.indexOf(childModification.getTargetNode()),
-									newNode);
+						if (castedNewNode != null) {
+							copyComments(castedNewNode, currentChild, commentMap);
+							modifiedChildren.add(modifiedChildren.indexOf(childModification.getTargetNode()), castedNewNode);
 						}
 						modifiedChildren.remove(childModification.getTargetNode());
 						break;
 					case INSERT_BEFORE:
+						if (castedNewNode != null) {
+							modifiedChildren.add(modifiedChildren.indexOf(childModification.getTargetNode()), castedNewNode);
+						} else if (newNode instanceof ContainerNode) {
+							ContainerNode nodeContainer = (ContainerNode) newNode;
+							for (IASTNode currentNode : nodeContainer.getNodes()) {
+								T tnode = cast(currentNode, clazz);
+								if (tnode != null) {
+									modifiedChildren.add(modifiedChildren.indexOf(childModification.getTargetNode()), tnode);
+								}
+							}
+						}
+						break;
 					case APPEND_CHILD:
 						throw new UnhandledASTModificationException(childModification);
 					}
@@ -126,16 +126,6 @@ public class ASTModificationHelper {
 		freestandingMap.remove(oldNode);
 	}
 
-	private <T> void insertNode(Class<T> clazz, ArrayList<T> modifiedChildren,
-			ASTModification parentModification, IASTNode newNode) {
-		T insertedTNode = cast(newNode, clazz);
-
-		int targetNodeIndex = modifiedChildren.indexOf(parentModification.getTargetNode());
-		if (targetNodeIndex >= 0) {
-			modifiedChildren.add(targetNodeIndex, insertedTNode);
-		}
-	}
-
 	@SuppressWarnings("unchecked")
 	private <T> T[] newArrayInstance(Class<T> clazz, int size) {
 		return (T[]) Array.newInstance(clazz, size);