From dfdae41d5bb2cce3b9730a86f716ab481bb11759 Mon Sep 17 00:00:00 2001
From: Md Tanvir Alam <tanviralamshuvo@gmail.com>
Date: Wed, 26 Aug 2026 19:04:52 +0000
Subject: [PATCH] Recognize split declaration/assignment in
 UnnecessaryStringBuilder

`StringBuilder x; x = new StringBuilder(...);` is observationally the same
as `StringBuilder x = new StringBuilder(...);`, but the check only handled
the combined declarator form. Teach it to treat a sole initializing
assignment the same way, and ignore that assignment when deciding whether
the local is still required to be a StringBuilder.

Fixes #5828
---
 .../bugpatterns/UnnecessaryStringBuilder.java | 41 +++++++++++++++++++
 .../UnnecessaryStringBuilderTest.java         | 38 +++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilder.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilder.java
index 3726cff3cf2..8437f3ea193 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilder.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilder.java
@@ -38,6 +38,7 @@
 import com.google.errorprone.suppliers.Supplier;
 import com.google.errorprone.util.ASTHelpers;
 import com.google.errorprone.util.TargetType;
+import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.MemberReferenceTree;
@@ -142,9 +143,41 @@ public Description matchNewClass(NewClassTree tree, VisitorState state) {
         return describeMatch(variableTree, fix.build());
       }
     }
+    // Support `StringBuilder x; x = new StringBuilder(...);` the same as the combined form.
+    if (leaf instanceof AssignmentTree assignmentTree
+        && assignmentTree.getVariable() instanceof IdentifierTree) {
+      Symbol assigned = getSymbol(assignmentTree.getVariable());
+      if (assigned != null && assigned.getKind().equals(ElementKind.LOCAL_VARIABLE)) {
+        VariableTree declaration = findVariableDeclaration(assigned, state);
+        if (declaration != null && isRewritableVariable(declaration, state)) {
+          SuggestedFix.Builder fix = SuggestedFix.builder();
+          if (!hasImplicitType(declaration, state)) {
+            fix.replace(declaration.getType(), "String");
+          }
+          fix.replace(assignmentTree.getExpression(), replacement(state, parts));
+          return describeMatch(assignmentTree, fix.build());
+        }
+      }
+    }
     return NO_MATCH;
   }
 
+  /** Finds the {@link VariableTree} that declares {@code symbol} in the current compilation unit. */
+  private static VariableTree findVariableDeclaration(Symbol symbol, VisitorState state) {
+    VariableTree[] found = {null};
+    new TreePathScanner<Void, Void>() {
+      @Override
+      public Void visitVariable(VariableTree tree, Void unused) {
+        if (symbol.equals(getSymbol(tree))) {
+          found[0] = tree;
+          return null;
+        }
+        return super.visitVariable(tree, null);
+      }
+    }.scan(state.getPath().getCompilationUnit(), null);
+    return found[0];
+  }
+
   /**
    * Returns true if the StringBuilder is assigned to a variable, and the type of the variable can
    * safely be refactored to be a String.
@@ -159,6 +192,14 @@ boolean isRewritableVariable(VariableTree variableTree, VisitorState state) {
       @Override
       public Void visitIdentifier(IdentifierTree tree, Void unused) {
         if (sym.equals(getSymbol(tree))) {
+          Tree parent = getCurrentPath().getParentPath().getLeaf();
+          // The variable's own declaration or an initializing assignment does not count as a
+          // use that requires a StringBuilder.
+          if (parent instanceof VariableTree
+              || (parent instanceof AssignmentTree assignmentTree
+                  && assignmentTree.getVariable() == tree)) {
+            return super.visitIdentifier(tree, null);
+          }
           TargetType target = targetType(state.withPath(getCurrentPath()));
           if (isUsedAsStringBuilder(state, target)) {
             ok[0] = false;
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilderTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilderTest.java
index 2b71cae6039..eaec077b65a 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilderTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryStringBuilderTest.java
@@ -225,4 +225,42 @@ void f(String hello) {
             """)
         .doTest();
   }
+
+  @Test
+  public void splitDeclarationAndAssignment() {
+    testHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            class Test {
+              void f() {
+                StringBuilder foo2;
+                // BUG: Diagnostic contains:
+                foo2 = new StringBuilder("x");
+                StringBuilder foo4;
+                // BUG: Diagnostic contains:
+                foo4 = new StringBuilder("x");
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void splitDeclarationAndAssignment_combinedStillFires() {
+    testHelper
+        .addSourceLines(
+            "Test.java",
+            """
+            class Test {
+              void f() {
+                // BUG: Diagnostic contains:
+                StringBuilder foo2 = new StringBuilder("x");
+                // BUG: Diagnostic contains:
+                StringBuilder foo4 = new StringBuilder("x");
+              }
+            }
+            """)
+        .doTest();
+  }
 }