From 9dbf2250f61137fcbfb62d084b90c370ad8a77eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20Vojt=C4=9Bchovsk=C3=BD?= Date: Sat, 26 May 2018 12:51:47 +0200 Subject: [PATCH] fix wrong code and tests which makes invalid AST --- .../compiler/SnippetCompilationHelper.java | 3 + .../support/reflect/code/CtCaseImpl.java | 6 +- .../support/reflect/code/CtStatementImpl.java | 12 ++++ .../reflect/code/CtStatementListImpl.java | 6 +- .../reflect/eval/VisitorPartialEvaluator.java | 4 +- .../generating/CtBiScannerGenerator.java | 2 + .../ReplacementVisitorGenerator.java | 2 + .../spoon/test/signature/SignatureTest.java | 4 +- .../InsertBlockProcessor.java | 4 +- .../spoon/testing/CtPackageAssertTest.java | 57 +++++++++++++++++-- 10 files changed, 87 insertions(+), 13 deletions(-) diff --git a/src/main/java/spoon/support/compiler/SnippetCompilationHelper.java b/src/main/java/spoon/support/compiler/SnippetCompilationHelper.java index 468af8a3d05..4edec2da07a 100644 --- a/src/main/java/spoon/support/compiler/SnippetCompilationHelper.java +++ b/src/main/java/spoon/support/compiler/SnippetCompilationHelper.java @@ -92,6 +92,9 @@ private static CtStatement internalCompileStatement(CtElement st, CtTypeReferenc // Clean up c.getPackage().removeType(c); + //disconnect element from the parent, so it can be added to another model + ret.delete(); + if (ret instanceof CtClass) { CtClass klass = (CtClass) ret; ret.getFactory().Package().getRootPackage().addType(klass); diff --git a/src/main/java/spoon/support/reflect/code/CtCaseImpl.java b/src/main/java/spoon/support/reflect/code/CtCaseImpl.java index 52da7b5fc12..2269d138623 100644 --- a/src/main/java/spoon/support/reflect/code/CtCaseImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtCaseImpl.java @@ -140,7 +140,11 @@ public T insertEnd(CtStatement statement) { @Override public T insertEnd(CtStatementList statements) { - for (CtStatement s : statements.getStatements()) { + List tobeInserted = new ArrayList<>(statements.getStatements()); + //remove statements from the `statementsToBeInserted` before they are added to spoon model + //note: one element MUST NOT be part of two models. + statements.setStatements(null); + for (CtStatement s : tobeInserted) { insertEnd(s); } return (T) this; diff --git a/src/main/java/spoon/support/reflect/code/CtStatementImpl.java b/src/main/java/spoon/support/reflect/code/CtStatementImpl.java index c30ac1342e4..ee55e23243a 100644 --- a/src/main/java/spoon/support/reflect/code/CtStatementImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtStatementImpl.java @@ -186,6 +186,9 @@ void insertFromFirstStatement(CtBlock block, CtStatement target, CtStatementL for (CtStatement ctStatement : statementsToBeInserted) { copy.add(indexOfTargetElement++, ctStatement); } + //remove statements from the `statementsToBeInserted` before they are added to spoon model + //note: one element MUST NOT be part of two models. + statementsToBeInserted.setStatements(null); block.setStatements(copy); } @@ -196,6 +199,9 @@ List insertFromLastStatement(List statements, CtStat for (int j = statementsToBeInserted.getStatements().size() - 1; j >= 0; j--) { copy.add(indexOfTargetElement, (T) statementsToBeInserted.getStatements().get(j)); } + //remove statements from the `statementsToBeInserted` before they are added to spoon model + //note: one element MUST NOT be part of two models. + statementsToBeInserted.setStatements(null); return copy; } }, @@ -212,6 +218,9 @@ void insertFromFirstStatement(CtBlock block, CtStatement target, CtStatementL for (CtStatement s : statementsToBeInserted) { copy.add(++indexOfTargetElement, s); } + //remove statements from the `statementsToBeInserted` before they are added to spoon model + //note: one element MUST NOT be part of two models. + statementsToBeInserted.setStatements(null); block.setStatements(copy); } @@ -222,6 +231,9 @@ List insertFromLastStatement(List statements, CtStat for (int j = statementsToBeInserted.getStatements().size() - 1; j >= 0; j--) { copy.add(indexOfTargetElement, (T) statementsToBeInserted.getStatements().get(j)); } + //remove statements from the `statementsToBeInserted` before they are added to spoon model + //note: one element MUST NOT be part of two models. + statementsToBeInserted.setStatements(null); return copy; } }; diff --git a/src/main/java/spoon/support/reflect/code/CtStatementListImpl.java b/src/main/java/spoon/support/reflect/code/CtStatementListImpl.java index a5cb0fcc89a..a265a2065a6 100644 --- a/src/main/java/spoon/support/reflect/code/CtStatementListImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtStatementListImpl.java @@ -124,7 +124,11 @@ public T insertEnd(CtStatement statement) { @Override public T insertEnd(CtStatementList statements) { - for (CtStatement s : statements.getStatements()) { + List tobeInserted = new ArrayList<>(statements.getStatements()); + //remove statements from the `statementsToBeInserted` before they are added to spoon model + //note: one element MUST NOT be part of two models. + statements.setStatements(null); + for (CtStatement s : tobeInserted) { insertEnd(s); } return (T) this; diff --git a/src/main/java/spoon/support/reflect/eval/VisitorPartialEvaluator.java b/src/main/java/spoon/support/reflect/eval/VisitorPartialEvaluator.java index d3f2787f718..9b45562042c 100644 --- a/src/main/java/spoon/support/reflect/eval/VisitorPartialEvaluator.java +++ b/src/main/java/spoon/support/reflect/eval/VisitorPartialEvaluator.java @@ -256,8 +256,8 @@ public void visitCtBlock(CtBlock block) { if (res instanceof CtStatement) { b.addStatement((CtStatement) res); } else { - //the context expectes statement. We cannot simplify in this case - b.addStatement(s); + //the context expects statement. We cannot simplify in this case + b.addStatement(s.clone()); } } // do not copy unreachable statements diff --git a/src/test/java/spoon/generating/CtBiScannerGenerator.java b/src/test/java/spoon/generating/CtBiScannerGenerator.java index 2b42fbf6b46..25b9d8c2978 100644 --- a/src/test/java/spoon/generating/CtBiScannerGenerator.java +++ b/src/test/java/spoon/generating/CtBiScannerGenerator.java @@ -104,6 +104,8 @@ public void process() { private CtClass createBiScanner() { final CtPackage aPackage = getFactory().Package().getOrCreate(TARGET_BISCANNER_PACKAGE); final CtClass target = getFactory().Class().get(GENERATING_BISCANNER); + //remove class from the old package so it can be added into new package + target.delete(); target.setSimpleName("CtBiScannerDefault"); target.addModifier(ModifierKind.PUBLIC); aPackage.addType(target); diff --git a/src/test/java/spoon/generating/ReplacementVisitorGenerator.java b/src/test/java/spoon/generating/ReplacementVisitorGenerator.java index 2b638e7aedc..769a9962b71 100644 --- a/src/test/java/spoon/generating/ReplacementVisitorGenerator.java +++ b/src/test/java/spoon/generating/ReplacementVisitorGenerator.java @@ -45,6 +45,8 @@ public void process(CtType element) { private CtClass createReplacementVisitor() { final CtPackage aPackage = getFactory().Package().getOrCreate(TARGET_REPLACE_PACKAGE); final CtClass target = getFactory().Class().get(GENERATING_REPLACE_VISITOR); + //remove type from old package so it can be added into new package + target.delete(); target.addModifier(ModifierKind.PUBLIC); aPackage.addType(target); final List references = target.getElements(new TypeFilter(CtTypeReference.class) { diff --git a/src/test/java/spoon/test/signature/SignatureTest.java b/src/test/java/spoon/test/signature/SignatureTest.java index 82f6f14cc42..b5e670874f1 100644 --- a/src/test/java/spoon/test/signature/SignatureTest.java +++ b/src/test/java/spoon/test/signature/SignatureTest.java @@ -123,9 +123,7 @@ public void testLiteralSignature(){ CtStatement sta2 = (factory).Code().createCodeSnippetStatement("String hello =\"t1\"; System.out.println(hello)") .compile(); - CtStatement sta2bis = ((CtBlock)sta2.getParent()).getStatement(1); - - assertFalse(sta1.equals(sta2bis));// equals depends on deep equality + assertFalse(sta1.equals(sta2));// equals depends on deep equality String parameterWithQuotes = ((CtInvocation)sta1).getArguments().get(0).toString(); assertEquals("\"hello\"",parameterWithQuotes); diff --git a/src/test/java/spoon/test/staticFieldAccess/InsertBlockProcessor.java b/src/test/java/spoon/test/staticFieldAccess/InsertBlockProcessor.java index 02d8fdd51a9..e4f3cb671f0 100644 --- a/src/test/java/spoon/test/staticFieldAccess/InsertBlockProcessor.java +++ b/src/test/java/spoon/test/staticFieldAccess/InsertBlockProcessor.java @@ -18,8 +18,8 @@ public boolean isToBeProcessed(CtMethod candidate) { @Override public void process(CtMethod element) { CtBlock block = new CtBlockImpl(); - - block.getStatements().add(element.getBody()); + // we clone the body so that there is no two elements with the same parent + block.addStatement(element.getBody().clone()); element.setBody(block); } } diff --git a/src/test/java/spoon/testing/CtPackageAssertTest.java b/src/test/java/spoon/testing/CtPackageAssertTest.java index 5e118e69706..ab01a272906 100644 --- a/src/test/java/spoon/testing/CtPackageAssertTest.java +++ b/src/test/java/spoon/testing/CtPackageAssertTest.java @@ -1,12 +1,22 @@ package spoon.testing; import org.junit.Test; + +import spoon.SpoonException; +import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtPackage; +import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.ModifierKind; import spoon.reflect.factory.Factory; +import spoon.reflect.visitor.filter.TypeFilter; import java.io.File; +import java.util.List; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; import static spoon.testing.Assert.assertThat; import static spoon.testing.utils.ModelUtils.build; import static spoon.testing.utils.ModelUtils.createFactory; @@ -14,11 +24,19 @@ public class CtPackageAssertTest { @Test public void testEqualityBetweenTwoCtPackage() throws Exception { + //contract: two packages, one made by test code, second made by compilation from sources are equal final Factory factory = createFactory(); final CtPackage aRootPackage = factory.Package().getOrCreate(""); - aRootPackage.addType(factory.Class().create("spoon.testing.testclasses.Foo").addModifier(ModifierKind.PUBLIC)); - aRootPackage.addType(factory.Class().create("spoon.testing.testclasses.Bar").addModifier(ModifierKind.PUBLIC)); - assertThat(build(new File("./src/test/java/spoon/testing/testclasses/")).Package().getRootPackage()).isEqualTo(aRootPackage); + List> types1 = aRootPackage.filterChildren(new TypeFilter<>(CtClass.class)).list(); + assertEquals(0, types1.size()); + factory.Class().create("spoon.testing.testclasses.Foo").addModifier(ModifierKind.PUBLIC); + factory.Class().create("spoon.testing.testclasses.Bar").addModifier(ModifierKind.PUBLIC); + List> types2 = aRootPackage.filterChildren(new TypeFilter<>(CtClass.class)).list(); + assertEquals(2, types2.size()); + final CtPackage aRootPackage2 = build(new File("./src/test/java/spoon/testing/testclasses/")).Package().getRootPackage(); + assertNotSame(aRootPackage, aRootPackage2); + assertThat(aRootPackage2).isEqualTo(aRootPackage); + } @Test(expected = AssertionError.class) @@ -30,7 +48,38 @@ public void testEqualityBetweenTwoDifferentCtPackage() throws Exception { public void testEqualityBetweenTwoCtPackageWithDifferentTypes() throws Exception { final Factory factory = createFactory(); final CtPackage aRootPackage = factory.Package().getOrCreate(""); - aRootPackage.addType(factory.Class().create("spoon.testing.testclasses.Foo").addModifier(ModifierKind.PUBLIC)); + factory.Class().create("spoon.testing.testclasses.Foo").addModifier(ModifierKind.PUBLIC); assertThat(build(new File("./src/test/java/spoon/testing/testclasses/")).Package().getRootPackage()).isEqualTo(aRootPackage); } + + + @Test + public void testAddTypeToPackage() throws Exception { + final Factory factory = createFactory(); + final CtType type = factory.Core().createClass(); + type.setSimpleName("X"); + //contract: type is created in the root package + assertSame(factory.getModel().getRootPackage(), type.getPackage()); + assertEquals("X", type.getQualifiedName()); + final CtPackage aPackage1= factory.Package().getOrCreate("some.package"); + //contract: type can be moved from root package to any other package + aPackage1.addType(type); + assertEquals("some.package.X", type.getQualifiedName()); + //contract: second add of type into same package does nothing + aPackage1.addType(type); + assertEquals("some.package.X", type.getQualifiedName()); + final CtPackage aPackage2= factory.Package().getOrCreate("another.package"); + try { + //contract: type cannot be moved to another package as long as it belongs to some not root package + aPackage2.addType(type); + fail(); + } catch (SpoonException e) { + //OK + } + assertEquals("some.package.X", type.getQualifiedName()); + //contract: type can be moved to another package after it is removed from current package + type.getPackage().removeType(type); + aPackage2.addType(type); + assertEquals("another.package.X", type.getQualifiedName()); + } }