Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}