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.

Sign up for GitHub

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 for a bug concerning multiples comments before a catch block #1074

Merged
merged 2 commits into from
Jan 3, 2017
Merged

Fix for a bug concerning multiples comments before a catch block #1074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7082dcf
Show the bug expressed in #1073
surli Dec 24, 2016
72e4336
Fix a bug concerning comments before catch block. #1073
surli Dec 24, 2016
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
1 change: 1 addition & 0 deletions src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ public <E> void visitCtCase(CtCase<E> caseStatement) {

@Override
public void visitCtCatch(CtCatch catchBlock) {
elementPrinterHelper.writeComment(catchBlock, CommentOffset.BEFORE);
printer.write(" catch (");
CtCatchVariable<? extends Throwable> parameter = catchBlock.getParameter();
if (parameter.getMultiTypes().size() > 0) {
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import spoon.reflect.code.CtBlock;
import spoon.reflect.code.CtBodyHolder;
import spoon.reflect.code.CtCase;
import spoon.reflect.code.CtCatch;
import spoon.reflect.code.CtComment;
import spoon.reflect.code.CtConditional;
import spoon.reflect.code.CtIf;
Expand Down Expand Up @@ -187,6 +188,7 @@ private void insertCommentInAST(final CtComment comment) {
// visitor that inserts the comment in the element
CtInheritanceScanner insertionVisitor = new CtInheritanceScanner() {
private boolean isScanned = false;

@Override
public void scan(CtElement e) {
if (e == null) {
Expand Down Expand Up @@ -373,6 +375,14 @@ public <T> void visitCtNewArray(CtNewArray<T> e) {
public <T> void visitCtParameter(CtParameter<T> e) {
e.addComment(comment);
}

@Override
public void visitCtCatch(CtCatch e) {
if (comment.getPosition().getLine() <= e.getPosition().getLine()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

use sourceStart() instead of getLine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually I used the same getLine which is used in visitCtClass which manage properly multiple inline comments. If I use here sourceStart() one comment is missing when writing something like:

try {}
// one comment
// two comment
catch {}

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is strange because getLine uses sourceStart() to perform the line number

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What I understand is that sourceStart starts at the same position than the first comment, which means the equality is respected and the comment is included (it was the previous implementation). But then the sourceStart of the second comment is greater than this sourcePosition and so it's not considered.
Now with getLine either it considers all are on the same line, either it computes well the different lines using the line separator position, which is the case I think.

e.addComment(comment);
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

call super.visitCtCatch()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This super call is directly implemented in the scan override, do we really need it here? It is missing in all the other overriding visitCtXX methods...

}
};
insertionVisitor.scan(commentParent);
try {
Expand Down
22 changes: 20 additions & 2 deletions src/test/java/spoon/test/comment/CommentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,19 @@
import spoon.reflect.declaration.CtPackage;
import spoon.reflect.declaration.CtParameter;
import spoon.reflect.declaration.ModifierKind;
import spoon.reflect.declaration.ParentNotInitializedException;
import spoon.reflect.factory.Factory;
import spoon.reflect.factory.FactoryImpl;
import spoon.reflect.visitor.AstParentConsistencyChecker;
import spoon.reflect.visitor.filter.AbstractFilter;
import spoon.reflect.visitor.filter.TypeFilter;
import spoon.support.DefaultCoreFactory;
import spoon.support.JavaOutputProcessor;
import spoon.support.StandardEnvironment;
import spoon.support.compiler.jdt.JDTSnippetCompiler;
import spoon.test.comment.testclasses.BlockComment;
import spoon.test.comment.testclasses.Comment1;
import spoon.test.comment.testclasses.Comment2;
import spoon.test.comment.testclasses.InlineComment;

import java.io.FileOutputStream;
Expand Down Expand Up @@ -107,7 +111,7 @@ public void testInLineComment() {

List<CtComment> comments = type.getElements(new TypeFilter<CtComment>(CtComment.class));
// verify that the number of comment present in the AST is correct
assertEquals(57, comments.size());
assertEquals(59, comments.size());

// verify that all comments present in the AST is printed
for (CtComment comment : comments) {
Expand Down Expand Up @@ -225,7 +229,9 @@ public void testInLineComment() {
+ "try {" + newLine
+ " // comment in try" + newLine
+ " i++;" + newLine
+ "} catch (java.lang.Exception e) {" + newLine
+ "}// between" + newLine
+ "// try/catch" + newLine
+ " catch (java.lang.Exception e) {" + newLine
+ " // comment in catch" + newLine
+ "}", ctTry.toString());

Expand Down Expand Up @@ -618,4 +624,16 @@ public boolean matches(CtElement element) {
write(codeElementsDocumentationPage.toString(), new FileOutputStream("doc/code_elements.md"));
}
}

@Test
public void testCommentsInComment1And2() {
Factory f = getSpoonFactory();
CtClass<?> type = (CtClass<?>) f.Type().get(Comment1.class);
List<CtComment> comments = type.getElements(new TypeFilter<CtComment>(CtComment.class));
assertEquals(4, comments.size());

type = (CtClass<?>) f.Type().get(Comment2.class);
comments = type.getElements(new TypeFilter<CtComment>(CtComment.class));
assertEquals(1, comments.size());
}
}
16 changes: 16 additions & 0 deletions src/test/java/spoon/test/comment/testclasses/Comment1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package spoon.test.comment.testclasses;

// comment 1
// comment 2
public class Comment1 {

public void code_1()
{
try { }
// A
// B
catch (Exception ex)
{ }
}

}
8 changes: 8 additions & 0 deletions src/test/java/spoon/test/comment/testclasses/Comment2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package spoon.test.comment.testclasses;

public class Comment2 {

// C
@interface Code_2{}

}
5 changes: 4 additions & 1 deletion src/test/java/spoon/test/comment/testclasses/InlineComment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ public void m1() {
try {
// comment in try
i++;
} catch (Exception e) {
}
// between
// try/catch
catch (Exception e) {
// comment in catch
}
// comment synchronized
Expand Down