Skip to content

Commit

Permalink
fix wrong code and tests which makes invalid AST
Browse files Browse the repository at this point in the history
  • Loading branch information
pvojtechovsky committed May 27, 2018
1 parent 465aa18 commit 5fcf05a
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/spoon/support/reflect/code/CtCaseImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,11 @@ public <T extends CtStatementList> T insertEnd(CtStatement statement) {

@Override
public <T extends CtStatementList> T insertEnd(CtStatementList statements) {
for (CtStatement s : statements.getStatements()) {
List<CtStatement> 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;
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/spoon/support/reflect/code/CtStatementImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -196,6 +199,9 @@ <T extends CtElement> List<T> insertFromLastStatement(List<T> 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;
}
},
Expand All @@ -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);
}

Expand All @@ -222,6 +231,9 @@ <T extends CtElement> List<T> insertFromLastStatement(List<T> 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;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ public <T extends CtStatementList> T insertEnd(CtStatement statement) {

@Override
public <T extends CtStatementList> T insertEnd(CtStatementList statements) {
for (CtStatement s : statements.getStatements()) {
List<CtStatement> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ public <R> void visitCtBlock(CtBlock<R> 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
Expand Down
2 changes: 2 additions & 0 deletions src/test/java/spoon/generating/CtBiScannerGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ public void process() {
private CtClass<Object> createBiScanner() {
final CtPackage aPackage = getFactory().Package().getOrCreate(TARGET_BISCANNER_PACKAGE);
final CtClass<Object> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public void process(CtType<?> element) {
private CtClass<Object> createReplacementVisitor() {
final CtPackage aPackage = getFactory().Package().getOrCreate(TARGET_REPLACE_PACKAGE);
final CtClass<Object> 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<CtTypeReference> references = target.getElements(new TypeFilter<CtTypeReference>(CtTypeReference.class) {
Expand Down
4 changes: 1 addition & 3 deletions src/test/java/spoon/test/signature/SignatureTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
57 changes: 53 additions & 4 deletions src/test/java/spoon/testing/CtPackageAssertTest.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,42 @@
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;

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<CtType<?>> 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<CtType<?>> 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)
Expand All @@ -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());
}
}

0 comments on commit 5fcf05a

Please sign in to comment.