diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StringBuilderConstantParameters.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StringBuilderConstantParameters.java index c55649a75..b6eb74a35 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StringBuilderConstantParameters.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/StringBuilderConstantParameters.java @@ -75,6 +75,12 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState if (!result.isPresent()) { return Description.NO_MATCH; } + // Avoid rewriting code that removes comments. + if (ASTHelpers.containsComments(tree, state)) { + return buildDescription(tree) + .setMessage(MESSAGE) + .build(); + } List arguments = result.get(); Stream prefixStream = arguments.stream().findFirst() .map(ASTHelpers::getType) diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StringBuilderConstantParametersTests.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StringBuilderConstantParametersTests.java index ba44e4988..2b5896160 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StringBuilderConstantParametersTests.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/StringBuilderConstantParametersTests.java @@ -195,6 +195,49 @@ public void shouldWarnOnNoParams() { ).doTest(); } + @Test + public void shouldWarnWhenCommentsArePresent() { + compilationHelper.addSourceLines( + "Test.java", + "class Test {", + " String f() {", + " return new StringBuilder()", + " .append(\"foo\") // comment", + " .append(\"bar\")", + " // BUG: Diagnostic contains: StringBuilder with a constant number of parameters", + " .toString();", + " }", + "}" + ).doTest(); + } + + @Test + public void doesNotRemoveComments() { + BugCheckerRefactoringTestHelper.newInstance(new StringBuilderConstantParameters(), getClass()) + .addInputLines( + "Test.java", + "class Test {", + " String f() {", + // Fails validation, but the tool prefers not to remove existing comments + " return new StringBuilder()", + " .append(\"foo\") // comment", + " .append(\"bar\")", + " .toString();", + " }", + "}") + .addOutputLines( + "Test.java", + "class Test {", + " String f() {", + " return new StringBuilder()", + " .append(\"foo\") // comment", + " .append(\"bar\")", + " .toString();", + " }", + "}") + .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); + } + @Test public void shouldWarnOnNoParams_fix() { BugCheckerRefactoringTestHelper.newInstance(new StringBuilderConstantParameters(), getClass()) diff --git a/changelog/@unreleased/pr-877.v2.yml b/changelog/@unreleased/pr-877.v2.yml new file mode 100644 index 000000000..60231d31e --- /dev/null +++ b/changelog/@unreleased/pr-877.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: StringBuilderConstantParameters suggested fix doesn't remove comments + links: + - https://github.com/palantir/gradle-baseline/pull/877