From 21a7118692ff7daa35edf7a14f106c642ff60d93 Mon Sep 17 00:00:00 2001 From: Thomas Corbat Date: Fri, 25 Jul 2014 17:05:27 +0200 Subject: [PATCH] Bug 440447 - ASTRewrite Loses Nested Modifications Modified ChangeGeneratorWriterVisitor to cope with nested modifications as described in the ticket. Also added some related change generator tests to the corresponding tests suites. Refined change to cope with sibling changes too. Change-Id: Ia04f3d01e50375828588e7fa65e2ec3782c58e79 Signed-off-by: Thomas Corbat Reviewed-on: https://git.eclipse.org/r/30525 Tested-by: Hudson CI Reviewed-by: Sergey Prigogin --- .../replace/CtorChainInitializerTest.java | 13 ++- .../MultilineWhitespaceHandlingTest.java | 18 +++- .../replace/NestedReplaceTest.java | 88 +++++++++++++++++++ .../replace/ReplaceForLoopBodyTest.java | 28 +++++- .../replace/ReplaceTestSuite.java | 27 +++--- .../replace/WhitespaceHandlingTest.java | 28 +++++- .../ChangeGeneratorWriterVisitor.java | 23 +++-- 7 files changed, 194 insertions(+), 31 deletions(-) create mode 100644 core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/NestedReplaceTest.java diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/CtorChainInitializerTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/CtorChainInitializerTest.java index 983445ab9d0..cbf113b416a 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/CtorChainInitializerTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/CtorChainInitializerTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2010 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 @@ -33,8 +33,15 @@ public class CtorChainInitializerTest extends ChangeGeneratorTest { @Override protected void setUp() throws Exception { - source = "TestClass::TestClass(int a):beta(b){\n}\n\n"; //$NON-NLS-1$ - expectedSource = "TestClass::TestClass(int a): alpha(a){\n}\n\n"; //$NON-NLS-1$ + source = + "TestClass::TestClass(int a):beta(b){\n" + + "}\n" + + "\n"; + expectedSource = + "TestClass::TestClass(int a) :\n" + + " alpha(a) {\n" + + "}\n" + + "\n"; super.setUp(); } diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/MultilineWhitespaceHandlingTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/MultilineWhitespaceHandlingTest.java index 89c12346a64..acd7ab47307 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/MultilineWhitespaceHandlingTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/MultilineWhitespaceHandlingTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2011, 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 @@ -24,13 +24,23 @@ import org.eclipse.cdt.internal.core.dom.rewrite.ASTModificationStore; public class MultilineWhitespaceHandlingTest extends ChangeGeneratorTest { public MultilineWhitespaceHandlingTest(){ - super("Whitespace Handling in Replace"); //$NON-NLS-1$ + super("Multiline Whitespace Handling in Replace"); //$NON-NLS-1$ } @Override protected void setUp() throws Exception { - source = "void foo(){\r\n\r\n for(int i = 0; i < 10; i++){\r\n\r\n }\r\n\r\n"; //$NON-NLS-1$ - expectedSource = "void foo(){\r\n\r\n for(int i = 0; i < 10; i++){ int i;\r\n int j;\r\n\r\n }\r\n\r\n"; //$NON-NLS-1$ + source = + "void foo() {\n" + + "\n" + + " for(int i = 0; i < 10; i++){\n" + + "\n" + + " }\n" + + "\n"; + expectedSource = + "void foo() {\n" + + " for (int i = 0; i < 10; i++) {\n" + + " }\n" + + "}"; super.setUp(); } diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/NestedReplaceTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/NestedReplaceTest.java new file mode 100644 index 00000000000..a2b4a191e7a --- /dev/null +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/NestedReplaceTest.java @@ -0,0 +1,88 @@ +/******************************************************************************* + * 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.replace; + +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.IASTExpression; +import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement; +import org.eclipse.cdt.core.dom.ast.IASTName; +import org.eclipse.cdt.core.dom.ast.IASTNullStatement; +import org.eclipse.cdt.core.dom.ast.IASTStatement; +import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression; +import org.eclipse.cdt.core.dom.ast.INodeFactory; +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 NestedReplaceTest extends ChangeGeneratorTest { + + public NestedReplaceTest() { + super("Nested Replace"); //$NON-NLS-1$ + } + + @Override + protected void setUp() throws Exception { + source = + "void foo(int x) {\n" + + " x += 1;\n" + + "}"; + expectedSource = + "void foo(int x) {\n" + + " x++;\n" + + "}"; + super.setUp(); + } + + public static Test suite() { + return new NestedReplaceTest(); + } + + @Override + protected ASTVisitor createModificator(final ASTModificationStore modStore) { + return new ASTVisitor() { + { + shouldVisitStatements = true; + } + + @Override + public int visit(IASTStatement statement) { + if (statement instanceof IASTCompoundStatement) { + IASTCompoundStatement compoundStatement = (IASTCompoundStatement) statement; + INodeFactory factory = statement.getTranslationUnit().getASTNodeFactory(); + + IASTCompoundStatement newCompoundStatement = factory.newCompoundStatement(); + IASTNullStatement dummyStatement = factory.newNullStatement(); + newCompoundStatement.addStatement(dummyStatement); + ASTModification compoundReplacement = new ASTModification(ModificationKind.REPLACE, compoundStatement, newCompoundStatement, null); + modStore.storeModification(null, compoundReplacement); + + IASTName emptyName = factory.newName(); + IASTExpression idExpression = factory.newIdExpression(emptyName); + IASTExpression incrementExpression = factory.newUnaryExpression(IASTUnaryExpression.op_postFixIncr, idExpression); + IASTExpressionStatement newStatement = factory.newExpressionStatement(incrementExpression); + IASTStatement replacedStatement = compoundStatement.getStatements()[0]; + ASTModification statementModification = new ASTModification(ModificationKind.REPLACE, dummyStatement, newStatement, null); + modStore.storeModification(compoundReplacement, statementModification); + + IASTName xName = factory.newName("x".toCharArray()); + ASTModification nameModification = new ASTModification(ModificationKind.REPLACE, emptyName, xName, null); + modStore.storeModification(statementModification, nameModification); + } + return PROCESS_ABORT; + } + }; + } +} diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceForLoopBodyTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceForLoopBodyTest.java index 04dd81f7b6e..546041cecdf 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceForLoopBodyTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceForLoopBodyTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2011, 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 @@ -30,8 +30,30 @@ public class ReplaceForLoopBodyTest extends ChangeGeneratorTest { @Override protected void setUp() throws Exception { - source = "void foo(){\r\n\r\n for(int i = 0; i < 10; i++){\r\n\r\n }\r\n\r\n for(int j = 0; j < 10; j++){\r\n\r\n }\r\n\r\n}"; //$NON-NLS-1$ - expectedSource = "void foo(){\r\n\r\n for(;;);\r\n\r\n for(int j = 0; j < 10; j++){\r\n\r\n }\r\n\r\n}"; //$NON-NLS-1$ + source = + "void foo() {\n" + + "\n" + + " for(int i = 0; i < 10; i++){\n" + + "\n" + + " }\n" + + "\n" + + " for(int j = 0; j < 10; j++){\n" + + "\n" + + " }\n" + + "\n" + + "}"; + expectedSource = + "void foo() {\n" + + "\n" + + " for (;;)\n" + + " ;\n" + + "\n" + + "\n" + + " for(int j = 0; j < 10; j++){\n" + + "\n" + + " }\n" + + "\n" + + "}"; forReplaced = false; super.setUp(); } diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceTestSuite.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceTestSuite.java index ebabb020021..970876bf935 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceTestSuite.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/ReplaceTestSuite.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 @@ -22,20 +22,25 @@ public class ReplaceTestSuite { public static Test suite() throws Exception { TestSuite suite = new TestSuite("ReplaceTestSuite"); - suite.addTest(NameTest.suite()); - suite.addTest(MoveTest.suite()); - suite.addTest(MoveRenameTest.suite()); - suite.addTest(SameNameTest.suite()); - suite.addTest(IdenticalTest.suite()); - suite.addTest(PointerInParameterTest.suite()); - suite.addTest(ExceptionTest.suite()); suite.addTest(ArrayModifierTest.suite()); - suite.addTest(InitializerTest.suite()); - suite.addTest(ExpressionTest.suite()); suite.addTest(ArraySizeExpressionTest.suite()); + suite.addTest(CtorChainInitializerTest.suite()); + suite.addTest(ExceptionTest.suite()); + suite.addTest(ExpressionTest.suite()); + suite.addTest(IdenticalTest.suite()); + suite.addTest(InitializerTest.suite()); + suite.addTest(MoveRenameTest.suite()); + suite.addTest(MoveTest.suite()); + suite.addTest(MultilineWhitespaceHandlingTest.suite()); + suite.addTest(NameTest.suite()); + suite.addTest(NestedReplaceTest.suite()); suite.addTest(NewInitializerExpressionTest.suite()); - suite.addTest(StatementTest.suite()); + suite.addTest(PointerInParameterTest.suite()); + suite.addTest(ReplaceForLoopBodyTest.suite()); suite.addTest(ReplaceInsertStatementTest.suite()); + suite.addTest(SameNameTest.suite()); + suite.addTest(StatementTest.suite()); + suite.addTest(WhitespaceHandlingTest.suite()); return suite; } } diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/WhitespaceHandlingTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/WhitespaceHandlingTest.java index 52703d639ed..f678a150a2b 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/WhitespaceHandlingTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/rewrite/changegenerator/replace/WhitespaceHandlingTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2011, 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 @@ -36,8 +36,30 @@ public class WhitespaceHandlingTest extends ChangeGeneratorTest { @Override protected void setUp() throws Exception { - source = "void foo(){\r\n\r\n for(int i = 0; i < 10; i++){\r\n\r\n }\r\n\r\n for(int j = 0; j < 10; j++){\r\n\r\n }\r\n\r\n}"; //$NON-NLS-1$ - expectedSource = "void foo(){\r\n\r\n for(int i = 0; i < 10; i++);\r\n\r\n for(int j = 0; j < 10; j++){\r\n\r\n }\r\n\r\n}"; //$NON-NLS-1$ + source = + "void foo() {\n" + + "\n" + + " for(int i = 0; i < 10; i++){\n" + + "\n" + + " }\n" + + "\n" + + " for(int j = 0; j < 10; j++){\n" + + "\n" + + " }\n" + + "\n" + + "}"; + expectedSource = + "void foo() {\n" + + "\n" + + " for (int i = 0; i < 10; i++)\n" + + " ;\n" + + "\n" + + "\n" + + " for(int j = 0; j < 10; j++){\n" + + "\n" + + " }\n" + + "\n" + + "}"; forReplaced = false; super.setUp(); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java index 7fd8a078166..ff14bb832c5 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/rewrite/changegenerator/ChangeGeneratorWriterVisitor.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 @@ -9,6 +9,7 @@ * Contributors: * Institute for Software - initial API and implementation * Markus Schorn (Wind River Systems) + * Thomas Corbat (IFS) *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.rewrite.changegenerator; @@ -363,16 +364,13 @@ public class ChangeGeneratorWriterVisitor extends ASTWriterVisitor { return PROCESS_SKIP; } } - // Check all insert before and append modifications for the current node. // If necessary put it onto the stack. for (IASTNode currentModifiedNode : stack.getModifiedNodes()) { for (ASTModification currentMod : stack.getModificationsForNode(currentModifiedNode)) { - if (currentMod.getNewNode() == node) { - if (currentMod.getKind() != ModificationKind.REPLACE) { - stack.pushScope(currentModifiedNode); - return PROCESS_CONTINUE; - } + if (currentMod.getNewNode() == node && currentMod.getKind() != ModificationKind.REPLACE) { + stack.pushScope(currentModifiedNode); + return PROCESS_CONTINUE; } } } @@ -390,6 +388,17 @@ public class ChangeGeneratorWriterVisitor extends ASTWriterVisitor { } } + // Check replace modifications for the current node. Is required as nodes could have been replaced + // externally, e.g. in ASTModificationHelper. + for (IASTNode currentModifiedNode : stack.getModifiedNodes()) { + for (ASTModification currentMod : stack.getModificationsForNode(currentModifiedNode)) { + if (currentMod.getNewNode() == node && currentMod.getKind() == ModificationKind.REPLACE) { + stack.pushScope(currentModifiedNode); + return PROCESS_CONTINUE; + } + } + } + return PROCESS_CONTINUE; }