Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug related to "insertBefore()" and "getSignature()" #450

Merged
merged 3 commits into from
Dec 9, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,13 @@ void insert(CtBlock<?> block, CtStatementList statementsToBeInserted) {

@Override
void insertFromFirstStatement(CtBlock<?> block, CtStatement target, CtStatementList statementsToBeInserted) {
int indexOfTargetElement = block.getStatements().indexOf(target);
int indexOfTargetElement = -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops? indentation with tabs? (this is the cause of travis' failure)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I just tried to use the formatter included with the spoon' repository ( SpoonFormatterPrefs.xml ), however when I apply it to the project, 458 files are modified. Do you really use this formatter ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope: #393

for (int i = 0; i < block.getStatements().size(); i++) {
if (block.getStatements().get(i) == target) {
indexOfTargetElement = i;
break;
}
}
for (CtStatement s : statementsToBeInserted) {
s.setParent(block);
block.getStatements().add(indexOfTargetElement++, s);
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/spoon/support/visitor/SignaturePrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ public void visitCtBreak(CtBreak breakStatement) {
public <E> void visitCtCase(CtCase<E> caseStatement) {
write("case (");
scan(caseStatement.getCaseExpression());
for (CtStatement statement : caseStatement.getStatements()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to scan(caseStatement.getStatements())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no scan method for List

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to |scan(caseStatement.getStatements())|

|it's not equivalent, the scan does not work, we trued it first :-)|

scan(statement);
}
write(")");
}

Expand Down
17 changes: 17 additions & 0 deletions src/test/java/spoon/test/processing/ProcessingTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
import org.junit.Test;
import spoon.Launcher;
import spoon.SpoonException;
import spoon.reflect.code.CtSwitch;
import spoon.reflect.declaration.CtClass;
import spoon.reflect.declaration.CtConstructor;
import spoon.reflect.declaration.CtMethod;
import spoon.reflect.visitor.filter.TypeFilter;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.fail;
import static spoon.test.TestUtils.build;

Expand Down Expand Up @@ -41,6 +44,20 @@ public void testInsertBegin() throws Exception {
+ constructor.getSimpleName(), "int i = 0;",
constructor.getBody().getStatement(1).toString());
}

CtConstructor<?> constructor = type.getConstructor(type.getFactory().Type().INTEGER_PRIMITIVE);
String myBeforeStatementAsString = "int before";
for (CtSwitch<?> ctSwitch : constructor.getElements(new TypeFilter<CtSwitch<?>>(CtSwitch.class))) {
ctSwitch.insertBefore(type.getFactory().Code()
.createCodeSnippetStatement(myBeforeStatementAsString));
}
assertEquals("insert has not been done at the right position", myBeforeStatementAsString, constructor.getBody().getStatement(3).toString());
assertEquals("insert has not been done at the right position", myBeforeStatementAsString, constructor.getBody().getStatement(5).toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constructor.getBody().getStatement(5) doesn't seem to be a switch at this position. Why there is an insertion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not the switch, it is the statement I add in the ForLoop above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a copy paste the constructor here:

    public SampleForInsertBefore(int j) {
        this(j, 0);
        new Thread() {
        };
        switch (j) {
      default:
        break;
    }
    switch (j) {
      default:
        break;
    }
    switch (j) {
      default: {
        break;
      }
    }

We can see that the position 5 is a break.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the statement 5 is not a break, because getStatements only returns the statements which have the method block as parent.

assertEquals("insert has not been done at the right position", myBeforeStatementAsString, constructor.getBody().getStatement(7).toString());

assertFalse("switch should not be the same", constructor.getBody().getStatement(6).equals(constructor.getBody().getStatement(8)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion was red before your fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

assertFalse("switch should not be the same", constructor.getBody().getStatement(6).toString().equals(constructor.getBody().getStatement(8).toString()));

}

@Test
Expand Down
13 changes: 13 additions & 0 deletions src/test/java/spoon/test/processing/SampleForInsertBefore.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ public SampleForInsertBefore(int j) {
this(j, 0);
new Thread() {
};
switch (j) {
default:
break;
}
switch (j) {
default:
break;
}
switch (j) {
default: {
break;
}
}
}

public SampleForInsertBefore(int j, int k) {
Expand Down