From 4c8531ff0f76b30aa76e5915bea21fef9e1f662d Mon Sep 17 00:00:00 2001 From: I-Al-Istannen Date: Sun, 17 Nov 2024 23:25:14 +0100 Subject: [PATCH] fix: Improve comment handling in lambda parameters and local variables (#6076) Print inline comments on the same line as their element Do not crash for comments inside lambda parameters Attribute comments to local variable default expressions Previously comments like `int a = /* foo */ 5` were attributed to the CtLocalVariable, which did not print them (as they appear within and not before/after). --- .../reflect/visitor/ElementPrinterHelper.java | 5 +++- .../compiler/jdt/JDTCommentBuilder.java | 22 +++++++++++++-- .../java/spoon/test/comment/CommentTest.java | 28 +++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java index 6348b86d05d..6884895ee2f 100644 --- a/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java +++ b/src/main/java/spoon/reflect/visitor/ElementPrinterHelper.java @@ -454,7 +454,10 @@ public List getComments(CtElement element, CommentOffset offset) { final int line = element.getPosition().getLine(); final int sourceEnd = element.getPosition().getSourceEnd(); final int sourceStart = element.getPosition().getSourceStart(); - if (offset == CommentOffset.BEFORE && (comment.getPosition().getLine() < line || (sourceStart <= comment.getPosition().getSourceStart() && sourceEnd > comment.getPosition().getSourceEnd()))) { + boolean commentStartsInLineBefore = comment.getPosition().getLine() < line; + boolean commentStartsInsideUs = sourceStart <= comment.getPosition().getSourceStart() && sourceEnd > comment.getPosition().getSourceEnd(); + boolean commentStartsBeforeUs = sourceStart >= comment.getPosition().getSourceStart() && sourceEnd > comment.getPosition().getSourceEnd(); + if (offset == CommentOffset.BEFORE && (commentStartsInLineBefore || commentStartsInsideUs || commentStartsBeforeUs)) { commentsToPrint.add(comment); } else if (offset == CommentOffset.AFTER && (comment.getPosition().getSourceStart() > sourceEnd || comment.getPosition().getSourceEnd() == sourceEnd)) { commentsToPrint.add(comment); diff --git a/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java b/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java index f331a2a1b3f..bba7eb17ecb 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java @@ -22,9 +22,11 @@ import spoon.reflect.code.CtCatch; import spoon.reflect.code.CtComment; import spoon.reflect.code.CtConditional; +import spoon.reflect.code.CtExpression; import spoon.reflect.code.CtIf; import spoon.reflect.code.CtLambda; import spoon.reflect.code.CtLiteral; +import spoon.reflect.code.CtLocalVariable; import spoon.reflect.code.CtNewArray; import spoon.reflect.code.CtStatement; import spoon.reflect.code.CtStatementList; @@ -350,7 +352,21 @@ public void visitCtImport(CtImport ctImport) { @Override public void scanCtVariable(CtVariable e) { - e.addComment(comment); + if (e instanceof CtLocalVariable lv && lv.getDefaultExpression() != null) { + CtExpression defaultExpression = lv.getDefaultExpression(); + int variableStart = e.getPosition().getSourceStart(); + int commentStart = comment.getPosition().getSourceStart(); + int defaultExprStart = defaultExpression.getPosition().getSourceStart(); + // Handle `int a = /* foobar */ foo();` by attaching the comment to `foo()` + // In those cases the comment and `foo()` start at the same position + if (commentStart > variableStart && commentStart >= defaultExprStart) { + defaultExpression.addComment(comment); + } else { + e.addComment(comment); + } + } else { + e.addComment(comment); + } } private void visitSwitch(CtAbstractSwitch e) { @@ -456,7 +472,9 @@ public void visitCtAnonymousExecutable(CtAnonymousExecutable e) { @Override public void visitCtLambda(CtLambda e) { - if (e.getExpression() != null) { + if (e.getExpression() != null && e.getParameters().isEmpty()) { + e.getExpression().addComment(comment); + } else if (e.getExpression() != null && !e.getParameters().isEmpty()) { CtParameter lastParameter = e.getParameters().get(e.getParameters().size() - 1); if (comment.getPosition().getSourceStart() > lastParameter.getPosition().getSourceEnd()) { e.getExpression().addComment(comment); diff --git a/src/test/java/spoon/test/comment/CommentTest.java b/src/test/java/spoon/test/comment/CommentTest.java index 64bd5e37dd8..f43f85f65b5 100644 --- a/src/test/java/spoon/test/comment/CommentTest.java +++ b/src/test/java/spoon/test/comment/CommentTest.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.condition.JRE; import org.junit.jupiter.api.extension.ExtendWith; import spoon.Launcher; +import spoon.LauncherTest; import spoon.SpoonException; import spoon.reflect.CtModel; import spoon.reflect.code.CtArrayAccess; @@ -110,6 +111,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static spoon.testing.assertions.SpoonAssertions.assertThat; public class CommentTest { @@ -1325,4 +1327,30 @@ public void testTypeParameterComments(CtModel model) { assertEquals("comment 3", typeParameters.get(1).getComments().get(1).getContent()); assertEquals("comment 4", typeParameters.get(1).getComments().get(2).getContent()); } + + @Test + @GitHubIssue(issueNumber = 6069, fixed = true) + @ExtendWith(LineSeparatorExtension.class) + public void testCommentInLambda() { + // contract: Comments inside lambdas should not crash or disappear. + CtClass ctClass = Launcher.parseClass(""" + class Foo { + public void bar() { + Runnable r = () -> { + /* hello */ System.exit(1); + }; + Runnable b = () -> /* hello2 */ System.exit(1); + Runnable c = /* hello3 */ () -> System.exit(1); + java.util.function.IntFunction d = ( /* hello4 */ int a ) -> null; + int e = /* hello5 */ 5; + } + } + """); + assertThat(ctClass).asString().contains("/* hello */"); + // Wrong output, there should not be a newline here. Codify it for now in the test case + assertThat(ctClass).asString().containsPattern("/\\* hello2 \\*/\n\\s+System"); + assertThat(ctClass).asString().contains("/* hello3 */"); + assertThat(ctClass).asString().contains("/* hello4 */"); + assertThat(ctClass).asString().contains("/* hello5 */"); + } }