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 issue #2374 Comments of added Nodes are ignored in LexicalPreserv… #2390

Merged
merged 3 commits into from Oct 20, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,38 @@
package com.github.javaparser.printer.lexicalpreservation;

import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Optional;

import org.junit.jupiter.api.Test;

import com.github.javaparser.StaticJavaParser;
import com.github.javaparser.ast.CompilationUnit;
import com.github.javaparser.ast.body.MethodDeclaration;
import com.github.javaparser.ast.stmt.Statement;

public class Issue2374Test {

@Test
public void test() {
String lineComment = "Example comment";
CompilationUnit cu = StaticJavaParser.parse(
"public class Bar {\n" +
" public void foo() {\n" +
" System.out.print(\"Hello\");\n" +
" }\n" +
"}"
);
LexicalPreservingPrinter.setup(cu);
// contruct a statement with a comment
Statement stmt = StaticJavaParser.parseStatement("System.out.println(\"World!\");");
stmt.setLineComment(lineComment);
// add the statement to the ast
Optional<MethodDeclaration> md = cu.findFirst(MethodDeclaration.class);
md.get().getBody().get().addStatement(stmt);
// print the result from LexicalPreservingPrinter
String result = LexicalPreservingPrinter.print(cu);
// verify that the LexicalPreservingPrinter don't forget the comment
assertTrue(result.contains(lineComment));
}
}
@@ -1,16 +1,19 @@
package com.github.javaparser.printer.lexicalpreservation;

import com.github.javaparser.GeneratedJavaParserConstants;
import com.github.javaparser.JavaToken.Kind;
import com.github.javaparser.ast.Modifier;
import com.github.javaparser.ast.Node;
import com.github.javaparser.ast.NodeList;
import com.github.javaparser.ast.expr.StringLiteralExpr;
import com.github.javaparser.ast.observer.ObservableProperty;
import com.github.javaparser.ast.stmt.ExpressionStmt;
import com.github.javaparser.printer.ConcreteSyntaxModel;
import com.github.javaparser.printer.Printable;
import com.github.javaparser.printer.SourcePrinter;
import com.github.javaparser.printer.concretesyntaxmodel.*;
import com.github.javaparser.printer.lexicalpreservation.changes.*;
import com.github.javaparser.utils.Utils;

import java.util.*;

Expand Down Expand Up @@ -146,6 +149,19 @@ private void calculatedSyntaxModelForNode(CsmElement csm, Node node, List<CsmEle
child = csmSingleReference.getProperty().getValueAsSingleReference(node);
}
if (child != null) {
// fix issue #2374
// Add node comment if needed (it's not very elegant but it works)
// We need to be sure that the node is an ExpressionStmt because we can meet
// this class definition
// a line comment <This is my class, with my comment> followed by
// class A {}
// In this case keyworld [class] is considered as a token and [A] is a child element
// So if we don't care that the node is an ExpressionStmt we could try to generate a wrong definition
// like this [class // This is my class, with my comment A {}]
if (node.getComment().isPresent() && node instanceof ExpressionStmt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the defining characteristic of nodes like ExpressionStmt that make them need behaviour like this? Maybe we can make that a property of the node, so we can avoid node specific exceptional logic here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's more specific to lexical printer than expression statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, but do you think there are other nodes likes this? If not, it's okay.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sincerely, I don't know! I don't know enough JavaParser to answer this question. Maybe the author could answer it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just merge it then :-)

elements.add(new CsmChild(node.getComment().get()));
elements.add(new CsmToken(Kind.EOF.getKind(), Utils.EOL));
}
elements.add(new CsmChild(child));
}
} else if (csm instanceof CsmNone) {
Expand Down