Skip to content

Commit 05b764f

Browse files
tomasz-tylenda-sonarsourceerwan-serandour
tomasz-tylenda-sonarsource
and
erwan-serandour
authored
SONARJAVA-5598 S3063: StringBuilder data should be used (#5185)
Co-authored-by: erwan-serandour <erwan.serandour@sonarsource.com>
1 parent c032cef commit 05b764f

7 files changed

Lines changed: 422 additions & 0 deletions

File tree

‎its/autoscan/src/test/resources/autoscan/diffs/diff_S3063.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"ruleKey": "S3063",
3+
"hasTruePositives": true,
4+
"falseNegatives": 0,
5+
"falsePositives": 0
6+
}

‎java-checks-test-sources/default/src/main/java/checks/unused/UnusedStringBuilderCheckSample.java‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
package checks.unused;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
import java.util.function.Supplier;
6+
7+
public class UnusedStringBuilderCheckSample {
8+
9+
public void usedTerminalMethod() {
10+
StringBuilder sb = new StringBuilder();
11+
sb.append("Hello");
12+
sb.append("!");
13+
System.out.println(sb.toString());
14+
}
15+
16+
public void usedPassedAsArgument() {
17+
StringBuilder sb = new StringBuilder();
18+
sb.append("Hello!");
19+
System.out.println(sb);
20+
}
21+
22+
public String usedButInitializedSeparately() {
23+
// FN, because it is not implemented.
24+
StringBuilder sb;
25+
sb = new StringBuilder();
26+
sb.append("Hello");
27+
sb.append("!");
28+
return sb.toString();
29+
}
30+
31+
public void unused() {
32+
StringBuilder sb = new StringBuilder(); // Noncompliant {{Consume or remove this unused StringBuilder}}
33+
// ^^
34+
sb.append("Hello");
35+
sb.append("!");
36+
System.out.println("Hello!");
37+
}
38+
39+
public void unusedNoOperationsAtAll() {
40+
StringBuilder sb = new StringBuilder(); // Noncompliant {{Consume or remove this unused StringBuilder}}
41+
// ^^
42+
}
43+
44+
public String usedTerminalSubstring() {
45+
StringBuilder sb = new StringBuilder();
46+
sb.append("Hello!");
47+
return sb.substring(2);
48+
}
49+
50+
public StringBuilder usedReturned() {
51+
StringBuilder sb = new StringBuilder();
52+
sb.append("returned");
53+
return sb;
54+
}
55+
56+
public StringBuilder usedChainedReturned() {
57+
StringBuilder sb = new StringBuilder();
58+
return sb.append("returned");
59+
}
60+
61+
public void usedChainedTwicePrinted() {
62+
StringBuilder sb = new StringBuilder();
63+
System.out.println(sb.append("one").append("two"));
64+
}
65+
66+
public String usedChainedTwiceReturned() {
67+
StringBuilder sb = new StringBuilder();
68+
return sb.append("one").append("two").toString();
69+
}
70+
71+
private void passedAsArg(StringBuilder fromOutside) {
72+
// Considered used, because the caller can invoke a terminal operation.
73+
fromOutside.append("good");
74+
}
75+
76+
private void retrievedWithoutCallingTheConstructor(Supplier<StringBuilder> supplier) {
77+
// Considered used, because the caller can invoke a terminal operation.
78+
StringBuilder sb = supplier.get();
79+
sb.append("good");
80+
}
81+
82+
private void assignedLhs(Supplier<StringBuilder> supplier) {
83+
// Considered used, because it is assigned.
84+
StringBuilder sb = new StringBuilder();
85+
sb = supplier.get();
86+
sb.append("assignment");
87+
}
88+
89+
private void assignedRhs(Supplier<StringBuilder> supplier) {
90+
// FP, because sb escapes analysis, but we do not handle it.
91+
StringBuilder sb = new StringBuilder(); // Noncompliant {{Consume or remove this unused StringBuilder}}
92+
// ^^
93+
sb.append("assignment");
94+
StringBuilder sb2 = sb;
95+
System.out.println(sb2);
96+
}
97+
98+
private String unusedConstructorWithArgument() {
99+
StringBuilder sb = new StringBuilder("Hello"); // Noncompliant {{Consume or remove this unused StringBuilder}}
100+
// ^^
101+
sb.append("!");
102+
return "Hello!";
103+
}
104+
105+
public void usedStringBufferTerminalMethod() {
106+
StringBuffer stringBuffer = new StringBuffer();
107+
stringBuffer.append("Hello");
108+
stringBuffer.append("!");
109+
System.out.println(stringBuffer.toString());
110+
}
111+
112+
public void unusedStringBuffer() {
113+
StringBuffer stringBuffer = new StringBuffer(); // Noncompliant {{Consume or remove this unused StringBuffer}}
114+
// ^^^^^^^^^^^^
115+
stringBuffer.append("Hello");
116+
stringBuffer.append("!");
117+
System.out.println("Hello!");
118+
}
119+
120+
121+
private void unrelated() {
122+
List<Integer> list = new ArrayList<>();
123+
list.add(5);
124+
}
125+
126+
String usedManyChainedCalls() {
127+
StringBuilder sb = new StringBuilder();
128+
return sb.append("a").append("b").toString();
129+
}
130+
131+
String unusedManyChainedCalls() {
132+
StringBuilder sb = new StringBuilder(); // Noncompliant {{Consume or remove this unused StringBuilder}}
133+
// ^^
134+
sb.append("a").append("b");
135+
return "ab";
136+
}
137+
138+
String unusedManyChainedCallsAssign() {
139+
StringBuilder sb = new StringBuilder(); // Noncompliant {{Consume or remove this unused StringBuilder}}
140+
// ^^
141+
StringBuilder sb2 = sb.append("a").append("b");
142+
return "ab";
143+
}
144+
145+
static class UnusedField1 {
146+
// FN, because it is not implemented (initialization in the constructor).
147+
StringBuilder stringBuilder;
148+
149+
UnusedField1() {
150+
this.stringBuilder = new StringBuilder();
151+
}
152+
153+
void appendHello() {
154+
stringBuilder.append("Hello");
155+
}
156+
}
157+
158+
static class UnusedFieldPrivate {
159+
private StringBuilder stringBuilder = new StringBuilder(); // Noncompliant
160+
// ^^^^^^^^^^^^^
161+
162+
void appendHello() {
163+
stringBuilder.append("Hello");
164+
}
165+
}
166+
167+
static class UnusedFieldProtected {
168+
// It can be used in subclass.
169+
protected StringBuilder stringBuilder = new StringBuilder();
170+
171+
void appendHello() {
172+
stringBuilder.append("Hello");
173+
}
174+
}
175+
176+
static class UsedFieldPrivate {
177+
private StringBuilder stringBuilder = new StringBuilder(); // Compliant
178+
179+
void appendHello() {
180+
stringBuilder.append("Hello");
181+
}
182+
183+
void print() {
184+
System.out.println(stringBuilder);
185+
}
186+
}
187+
}

‎java-checks/src/main/java/org/sonar/java/checks/unused/UnusedStringBuilderCheck.java‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.unused;
18+
19+
import java.util.List;
20+
import java.util.Optional;
21+
import java.util.Set;
22+
import javax.annotation.Nullable;
23+
import org.sonar.check.Rule;
24+
import org.sonar.java.ast.parser.ArgumentListTreeImpl;
25+
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
26+
import org.sonar.plugins.java.api.semantic.Symbol;
27+
import org.sonar.plugins.java.api.semantic.Type;
28+
import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
29+
import org.sonar.plugins.java.api.tree.ExpressionTree;
30+
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
31+
import org.sonar.plugins.java.api.tree.NewClassTree;
32+
import org.sonar.plugins.java.api.tree.ReturnStatementTree;
33+
import org.sonar.plugins.java.api.tree.Tree;
34+
import org.sonar.plugins.java.api.tree.VariableTree;
35+
36+
/**
37+
* Check that `StringBuilder` and `StringBuffer` instances are consumed by calling `toString()` or in a similar way.
38+
*/
39+
@Rule(key = "S3063")
40+
public class UnusedStringBuilderCheck extends IssuableSubscriptionVisitor {
41+
private static final Set<String> TERMINAL_METHODS = Set.of("toString", "substring");
42+
43+
@Override
44+
public List<Tree.Kind> nodesToVisit() {
45+
return List.of(Tree.Kind.VARIABLE);
46+
}
47+
48+
@Override
49+
public void visitNode(Tree tree) {
50+
if (tree instanceof VariableTree variableTree) {
51+
Symbol symbol = variableTree.symbol();
52+
String typeName = getStringBuilderOrStringBuffer(symbol.type());
53+
54+
// Exclude non-local variables with non-private visibility,
55+
// because they can be changed in a way that is hard to track.
56+
if (typeName != null && isInitializedByConstructor(variableTree.initializer()) &&
57+
isLocalOrPrivate(symbol) &&
58+
symbol.usages().stream().noneMatch(UnusedStringBuilderCheck::isUsedInAssignment) &&
59+
symbol.usages().stream().noneMatch(UnusedStringBuilderCheck::isConsumed)) {
60+
reportIssue(variableTree.simpleName(), "Consume or remove this unused %s".formatted(typeName));
61+
}
62+
}
63+
}
64+
65+
/**
66+
* Returns `"StringBuilder"` or `"StringBuffer"` if the variable is one of these types.
67+
* This is to both check the type and extract the name for use in the issue message.
68+
*/
69+
private static @Nullable String getStringBuilderOrStringBuffer(Type type) {
70+
if (type.is("java.lang.StringBuilder")) {
71+
return "StringBuilder";
72+
} else if (type.is("java.lang.StringBuffer")) {
73+
return "StringBuffer";
74+
}
75+
return null;
76+
}
77+
78+
/**
79+
* Verify that the initializer is a call to a constructor, for example `new StringBuilder()`,
80+
* and not a method call or missing, which happens when the variable is a parameter.
81+
*/
82+
private static boolean isInitializedByConstructor(@Nullable ExpressionTree initializer) {
83+
return initializer instanceof NewClassTree;
84+
}
85+
86+
private static boolean isLocalOrPrivate(Symbol symbol) {
87+
return symbol.isLocalVariable() || symbol.isPrivate();
88+
}
89+
90+
/**
91+
* True if the argument is the LHS of an assignment.
92+
*
93+
* Ideally, we should also check the RHS, to account for variables escaping the analysis.
94+
*/
95+
private static boolean isUsedInAssignment(@Nullable Tree tree) {
96+
return Optional.ofNullable(tree)
97+
.map(Tree::parent)
98+
.filter(AssignmentExpressionTree.class::isInstance)
99+
.isPresent();
100+
}
101+
102+
/**
103+
* True if a tree, representing a variable, is consumed by calling `toString()`, returning it,
104+
* passing it as an argument, etc.
105+
*/
106+
private static boolean isConsumed(Tree tree) {
107+
Tree parent = tree.parent();
108+
if (parent instanceof MemberSelectExpressionTree mset) {
109+
if (TERMINAL_METHODS.contains(mset.identifier().name())) {
110+
return true;
111+
} else {
112+
// Handle chained method calls, for example `return sb.append("text")`.
113+
return Optional.ofNullable(parent.parent())
114+
.filter(UnusedStringBuilderCheck::isConsumed)
115+
.isPresent();
116+
}
117+
} else if (parent instanceof ReturnStatementTree || parent instanceof ArgumentListTreeImpl) {
118+
return true;
119+
}
120+
return false;
121+
}
122+
}

‎java-checks/src/test/java/org/sonar/java/checks/unused/UnusedStringBuilderCheckTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* SonarQube Java
3+
* Copyright (C) 2012-2025 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.java.checks.unused;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.java.checks.verifier.CheckVerifier;
21+
22+
import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
23+
24+
class UnusedStringBuilderCheckTest {
25+
@Test
26+
void test() {
27+
CheckVerifier.newVerifier()
28+
.onFile(mainCodeSourcesPath("checks/unused/UnusedStringBuilderCheckSample.java"))
29+
.withCheck(new UnusedStringBuilderCheck())
30+
.verifyIssues();
31+
}
32+
33+
@Test
34+
void test_without_semantics() {
35+
CheckVerifier.newVerifier()
36+
.onFile(mainCodeSourcesPath("checks/unused/UnusedStringBuilderCheckSample.java"))
37+
.withCheck(new UnusedStringBuilderCheck())
38+
.withoutSemantic()
39+
.verifyIssues();
40+
}
41+
}

0 commit comments

Comments
 (0)