From 66eb977fec1de72e17264f33434f098af758d8c0 Mon Sep 17 00:00:00 2001 From: Nakul Joshi Date: Wed, 5 Jun 2024 12:03:00 -0700 Subject: [PATCH 1/2] Add test --- .../StringBuilderConstantParametersTests.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 d132f1a9b..d22b28faf 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 @@ -311,6 +311,30 @@ public void suggestedFixHandlesAddition() { .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } + @Test + public void suggestedFixHandlesMethodCalledOnBuilt() { + RefactoringValidator.of(StringBuilderConstantParameters.class, getClass()) + .addInputLines( + "Test.java", + "class Test {", + " String f() {", + " return new StringBuilder()", + " .append(\"foo\")", + " .append(\"bar\")", + " .toString()", + " .toLowerCase();", + " }", + "}") + .addOutputLines( + "Test.java", + "class Test {", + " String f() {", + " return (\"foo\" + \"bar\").toLowerCase();", + " }", + "}") + .doTest(); + } + @Test public void negativeDynamicStringBuilder() { compilationHelper From 8ada3c1baf41606774c51a1371e4182e898bba8b Mon Sep 17 00:00:00 2001 From: Nakul Joshi Date: Wed, 5 Jun 2024 14:59:45 -0700 Subject: [PATCH 2/2] Add parens around the replaced node & update tests --- .../StringBuilderConstantParameters.java | 2 ++ .../StringBuilderConstantParametersTests.java | 18 +++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) 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 4a8f62ceb..461cdc656 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 @@ -95,6 +95,8 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState return buildDescription(tree) .setMessage(MESSAGE) .addFix(SuggestedFix.builder() + .prefixWith(tree, "(") + .postfixWith(tree, ")") .replace( tree, Streams.concat( 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 d22b28faf..d0f1a67bf 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 @@ -54,7 +54,7 @@ public void shouldWarnOnConstantNumberOfParams_fix() { " }", "}") .addOutputLines( - "Test.java", "class Test {", " String f() {", " return \"foo\" + 1;", " }", "}") + "Test.java", "class Test {", " String f() {", " return (\"foo\" + 1);", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -86,7 +86,7 @@ public void shouldWarnOnConstantNumberOfParams_stringCtor_fix() { "Test.java", "class Test {", " String f() {", - " return \"ctor\" + \"foo\" + 1;", + " return (\"ctor\" + \"foo\" + 1);", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -120,7 +120,7 @@ public void shouldWarnOnConstantNumberOfParams_charSequenceCtor_fix() { "Test.java", "class Test {", " String f(CharSequence charSequence) {", - " return \"\" + charSequence + \"foo\" + 1;", + " return (\"\" + charSequence + \"foo\" + 1);", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -154,7 +154,7 @@ public void shouldWarnOnConstantNumberOfNonConstantParams_fix() { "Test.java", "class Test {", " String f(long param0, double param1) {", - " return \"\" + param0 + param1;", + " return (\"\" + param0 + param1);", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -174,7 +174,7 @@ public void shouldWarnOnConstantNumberOfNonConstantParams_firstString_fix() { "Test.java", "class Test {", " String f(String param0, double param1) {", - " return param0 + param1;", + " return (param0 + param1);", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -239,7 +239,7 @@ public void shouldWarnOnNoParams_fix() { " return new StringBuilder().toString();", " }", "}") - .addOutputLines("Test.java", "class Test {", " String f() {", " return \"\";", " }", "}") + .addOutputLines("Test.java", "class Test {", " String f() {", " return (\"\");", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); } @@ -257,7 +257,7 @@ public void suggestedFixRetainsCast() { "Test.java", "class Test {", " String f(Object obj) {", - " return (String) obj + 1;", + " return ((String) obj + 1);", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -281,7 +281,7 @@ public void suggestedFixHandlesTernary() { "Test.java", "class Test {", " String f(Object obj) {", - " return \"a\" + (obj == null ? \"nil\" : obj) + \"b\";", + " return (\"a\" + (obj == null ? \"nil\" : obj) + \"b\");", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH); @@ -305,7 +305,7 @@ public void suggestedFixHandlesAddition() { "Test.java", "class Test {", " String f(int param0, int param1) {", - " return \"a\" + (param0 + param1) + \"b\";", + " return (\"a\" + (param0 + param1) + \"b\");", " }", "}") .doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);