fix: attach comment to CtLiteral nodes
#4836
Conversation
|
This doesn't look specific to case expressions or am I missing something? It seems like the comment isn't attached to the CtLiteral, but I didn't really figure out yet how the JDTCommentBuilder works (or is supposed to work). Adding a @Override
public <T> void visitCtLiteral(CtLiteral<T> e) {
e.addComment(comment);
}to this visitor seems to do the trick, however I'm not sure if that's an appropriate fix at all (and it prints the comment in front of the literal, but that might be rather a printer issue - I don't now). |
I observed it only for literals inside
Let's fix the AST in this PR and then we can focus on printing comments. EDIT: I tried your fix. There is a very weird side-effect in this test. I will investigate this later. |
| @@ -415,7 +416,7 @@ class CounterScanner extends CtScanner { | |||
| @Override | |||
| public <T> void visitCtFieldWrite(CtFieldWrite<T> fieldWrite) { | |||
| visited++; | |||
| assertEquals("a", ((CtVariableWrite) fieldWrite.getTarget()).getVariable().getSimpleName()); | |||
| assertEquals("a", ((CtVariableRead<?>) fieldWrite.getTarget()).getVariable().getSimpleName()); | |||
There was a problem hiding this comment.
Probably I had some local changes and that is why it was failing. I have reverted them in the latest commit.
algomaster99
changed the title
CtLiteral nodes
|
@SirYwell Can you have a look again? I added your suggested fix which fixes the AST parsing. |
| String stageStr; | ||
| boolean fullStatus; | ||
| switch (stage) { | ||
| case (1/*org.apache.coyote.Constants.STAGE_PARSE*/): |
There was a problem hiding this comment.
Could you improve the test case and a comment before a case, e.g.,/*org.apache.coyote.Constants.STAGE_PARSE*/1
There was a problem hiding this comment.
I added inline comment to add variety.
This reverts commit 87fed7d.
| @@ -57,6 +59,22 @@ | |||
|
|
|||
| public class AstCheckerTest { | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
You could use the @ModelTest annotation here instead. This way, you don't need the arrange block.
There are a few examples already you can look into if needed.
| @@ -57,6 +59,22 @@ | |||
|
|
|||
| public class AstCheckerTest { | |||
|
|
|||
| @Test | |||
| void ctLiteralsInCtCaseExpressionShouldHaveCommentsAttached() { | |||
There was a problem hiding this comment.
You could use the new ModelTest Annotation here. This removes all of the arrange code.
|
Thanks @algomaster99 |
I was trying to reproduce #4727 and I came across another possible bug in the Spoon model builder. It seems that when comments are added to
CtCaseExpressions, they are not parsed and stored in the model. Is this a bug or does Spoon has some configuration that I need to enable to include comments in the model? Note that I already set one in my knowledge -launcher.getEnvironment().setCommentEnabled(true);.