Skip to content

Commit f4f7017

Browse files
tomasz-tylenda-sonarsourcetomasz-tylenda-sonarsource
authored
SONARJAVA-5599 S3024: Report issues only inside loops (#5204)
1 parent 478223a commit f4f7017

6 files changed

Lines changed: 259 additions & 134 deletions

File tree

‎its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S3024.json‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎its/ruling/src/test/resources/eclipse-jetty/java-S3024.json‎

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎its/ruling/src/test/resources/guava/java-S3024.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
22
"com.google.guava:guava:src/com/google/common/collect/Iterators.java": [
3-
313,
43
315
54
]
65
}

‎its/ruling/src/test/resources/sonar-server/java-S3024.json‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎java-checks-test-sources/default/src/main/java/checks/StringBufferAndBuilderConcatenationCheckSample.java‎

Lines changed: 196 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,99 +1,235 @@
11
package checks;
22

33
import java.util.Iterator;
4+
import java.util.List;
45

56
public class StringBufferAndBuilderConcatenationCheckSample {
6-
public String appendSimple() {
7+
public String appendCorrect() {
8+
StringBuffer sb = new StringBuffer();
9+
for (int i = 0; i < 10; i++) {
10+
sb.append("text1")
11+
.append("text2");
12+
}
13+
return sb.toString();
14+
}
15+
16+
public String noLoop() {
17+
StringBuffer sb = new StringBuffer();
18+
sb.append("text1" + "text2");
19+
return sb.toString();
20+
}
21+
22+
public String nestedLoops() {
23+
StringBuffer sbOutside = new StringBuffer();
24+
StringBuffer sbInside1 = new StringBuffer();
25+
StringBuffer sbInside2 = new StringBuffer();
26+
StringBuffer sbInside21 = new StringBuffer();
27+
28+
sbInside1.append("text1" + "text2");
29+
for (int i = 0; i < 5; i++) {
30+
sbInside1.append("i = " + i); // Noncompliant
31+
for (int j = 0; j < 5; j++) {
32+
sbInside2.append("j = " + j); // Noncompliant
33+
if (i == j) {
34+
sbInside21.append("i eq j = " + i); // Noncompliant
35+
}
36+
}
37+
}
38+
739
return new StringBuilder()
8-
.append("text1")
9-
.append("text2")
40+
.append(sbOutside)
41+
.append(sbInside1)
42+
.append(sbInside2)
43+
.append(sbInside21)
1044
.toString();
1145
}
1246

47+
public String loopWhile() {
48+
StringBuffer sb = new StringBuffer();
49+
int i = 0;
50+
while (i < 10) {
51+
sb.append("text1" + "text2"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfWhile]]
52+
// ^^^^^^^^^^^^^^^^^
53+
// fix@qfWhile {{Call "append" multiple times.}}
54+
// edit@qfWhile [[sc=16;ec=35]] {{("text1").append("text2")}}
55+
i++;
56+
}
57+
return sb.toString();
58+
}
59+
60+
public String loopDoWhile() {
61+
StringBuffer sb = new StringBuffer();
62+
int i = 0;
63+
do {
64+
sb.append("text1" + "text2"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfDoWhile]]
65+
// ^^^^^^^^^^^^^^^^^
66+
// fix@qfDoWhile {{Call "append" multiple times.}}
67+
// edit@qfDoWhile [[sc=16;ec=35]] {{("text1").append("text2")}}
68+
i++;
69+
} while (i < 10);
70+
return sb.toString();
71+
}
72+
73+
public String loopForeach(List<String> items) {
74+
StringBuffer sb = new StringBuffer();
75+
for (String item : items) {
76+
sb.append("<" + item + ">\n"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfForeach]]
77+
// ^^^^^^^^^^^^^^^^^^
78+
// fix@qfForeach {{Call "append" multiple times.}}
79+
// edit@qfForeach [[sc=16;ec=36]] {{("<").append(item).append(">\n")}}
80+
}
81+
return sb.toString();
82+
}
83+
84+
public String functionalForeach(List<String> items) {
85+
// FN: not implemented
86+
StringBuffer sb = new StringBuffer();
87+
items.forEach(item -> sb.append("<" + item + ">\n"));
88+
return sb.toString();
89+
}
90+
91+
public String functionalStreamForeach(List<String> items) {
92+
// FN: not implemented
93+
StringBuffer sb = new StringBuffer();
94+
items
95+
.stream()
96+
.forEach(item -> sb.append("<" + item + ">\n"));
97+
return sb.toString();
98+
}
99+
100+
public String loopForIf() {
101+
StringBuffer sb = new StringBuffer();
102+
for (int i = 0; i < 10; i++) {
103+
if (i % 2 == 0) {
104+
sb.append("number: " + i); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfForIf]]
105+
// ^^^^^^^^^^^^^^
106+
// fix@qfForIf {{Call "append" multiple times.}}
107+
// edit@qfForIf [[sc=18;ec=34]] {{("number: ").append(i)}}
108+
}
109+
}
110+
return sb.toString();
111+
}
112+
13113
public String concatThree() {
14-
return new StringBuilder()
15-
.append("text1" + "text2" + "text3") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf2]]
16-
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
17-
// fix@qf2 {{Call "append" multiple times.}}
18-
// edit@qf2 [[sc=14;ec=43]] {{("text1").append("text2").append("text3")}}
19-
.toString();
114+
StringBuffer sb = new StringBuffer();
115+
for (int i = 0; i < 10; i++) {
116+
sb.append("text1" + "text2" + "text3"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfThree]]
117+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
118+
// fix@qfThree {{Call "append" multiple times.}}
119+
// edit@qfThree [[sc=16;ec=45]] {{("text1").append("text2").append("text3")}}
120+
}
121+
return sb.toString();
20122
}
21123

22-
public String concatIntegerNoncompliant() {
23-
return new StringBuilder()
24-
.append("text" + 1) // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf3]]
25-
// ^^^^^^^^^^
26-
// fix@qf3 {{Call "append" multiple times.}}
27-
// edit@qf3 [[sc=14;ec=26]] {{("text").append(1)}}
28-
.toString();
124+
public String concatIntNoncompliant() {
125+
StringBuffer sb = new StringBuffer();
126+
for (int i = 0; i < 10; i++) {
127+
sb.append("text" + 1); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfInt]]
128+
// ^^^^^^^^^^
129+
// fix@qfInt {{Call "append" multiple times.}}
130+
// edit@qfInt [[sc=16;ec=28]] {{("text").append(1)}}
131+
}
132+
return sb.toString();
29133
}
30134

31-
public String concatIntegerCompliant() {
32-
return new StringBuilder()
33-
.append(1 + 1)
34-
.toString();
135+
public String concatIntCompliant() {
136+
StringBuffer sb = new StringBuffer();
137+
for (int i = 0; i < 10; i++) {
138+
sb.append(1 + 1);
139+
}
140+
return sb.toString();
35141
}
36142

37143
public String complexArgumentList() {
38-
return new StringBuilder()
39-
.append("two " + (1 + 1)) // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf4]]
40-
// ^^^^^^^^^^^^^^^^
41-
// fix@qf4 {{Call "append" multiple times.}}
42-
// edit@qf4 [[sc=14;ec=32]] {{("two ").append(1 + 1)}}
43-
.toString();
144+
StringBuffer sb = new StringBuffer();
145+
for (int i = 0; i < 10; i++) {
146+
sb.append("two " + (1 + 1)); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfComplex]]
147+
// ^^^^^^^^^^^^^^^^
148+
// fix@qfComplex {{Call "append" multiple times.}}
149+
// edit@qfComplex [[sc=16;ec=34]] {{("two ").append(1 + 1)}}
150+
}
151+
return sb.toString();
44152
}
45153

46-
public StringBuilder notChained() {
47-
StringBuilder stringBuilder = new StringBuilder();
48-
stringBuilder.append("text1" + "text2"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf5]]
49-
// ^^^^^^^^^^^^^^^^^
50-
// fix@qf5 {{Call "append" multiple times.}}
51-
// edit@qf5 [[sc=25;ec=44]] {{("text1").append("text2")}}
52-
return stringBuilder;
154+
public void chained() {
155+
for (int i = 0; i < 10; i++) {
156+
String s = new StringBuilder()
157+
.append("text1" + "text2") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfChained]]
158+
// ^^^^^^^^^^^^^^^^^
159+
// fix@qfChained {{Call "append" multiple times.}}
160+
// edit@qfChained [[sc=16;ec=35]] {{("text1").append("text2")}}
161+
.toString();
162+
System.out.println(s);
163+
}
53164
}
54165

55166
public void parameter(StringBuilder passed) {
56-
passed.append("hello " + "hello"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf6]]
57-
// ^^^^^^^^^^^^^^^^^^
58-
// fix@qf6 {{Call "append" multiple times.}}
59-
// edit@qf6 [[sc=18;ec=38]] {{("hello ").append("hello")}}
167+
for (int i = 0; i < 10; i++) {
168+
passed.append("hello " + "hello"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfPassed]]
169+
// ^^^^^^^^^^^^^^^^^^
170+
// fix@qfPassed {{Call "append" multiple times.}}
171+
// edit@qfPassed [[sc=20;ec=40]] {{("hello ").append("hello")}}
172+
}
60173
}
61174

62175
public String call(Iterator<String> iter) {
63-
return new StringBuilder()
64-
.append("<" + iter.next() + "/>") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf7]]
65-
// ^^^^^^^^^^^^^^^^^^^^^^^^
66-
// fix@qf7 {{Call "append" multiple times.}}
67-
// edit@qf7 [[sc=14;ec=40]] {{("<").append(iter.next()).append("/>")}}
68-
.toString();
176+
StringBuffer sb = new StringBuffer();
177+
for (int i = 0; i < 10; i++) {
178+
sb.append("<" + iter.next() + "/>"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfCall]]
179+
// ^^^^^^^^^^^^^^^^^^^^^^^^
180+
// fix@qfCall {{Call "append" multiple times.}}
181+
// edit@qfCall [[sc=16;ec=42]] {{("<").append(iter.next()).append("/>")}}
182+
}
183+
return sb.toString();
69184
}
70185

71186
public String nestedCall(String s) {
72-
return new StringBuilder()
73-
.append(s.toLowerCase())
74-
.toString();
187+
StringBuffer sb = new StringBuffer();
188+
for (int i = 0; i < 10; i++) {
189+
sb.append(s.toLowerCase());
190+
}
191+
return sb.toString();
75192
}
76193

77194
public String stringBufferAppendSimple() {
78-
return new StringBuffer()
79-
.append("text1")
80-
.append("text2")
81-
.toString();
195+
StringBuffer stringBuffer = new StringBuffer();
196+
for (int i = 0; i < 10; i++) {
197+
stringBuffer.append("text1")
198+
.append("text2");
199+
}
200+
return stringBuffer.toString();
201+
}
202+
203+
public String stringBufferConcatSimple() {
204+
StringBuffer stringBuffer = new StringBuffer();
205+
for (int i = 0; i < 10; i++) {
206+
stringBuffer.append("text1" + "text2"); // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qfBuffer]]
207+
// ^^^^^^^^^^^^^^^^^
208+
// fix@qfBuffer {{Call "append" multiple times.}}
209+
// edit@qfBuffer [[sc=26;ec=45]] {{("text1").append("text2")}}
210+
}
211+
return stringBuffer.toString();
82212
}
83213

84214
public String appendWithStartEnd() {
85215
// Breaking this up would require changing the second and third argument.
86-
return new StringBuilder()
87-
.append("text1" + "text2" , 2, 3)
88-
.toString();
216+
StringBuffer sb = new StringBuffer();
217+
for (int i = 0; i < 10; i++) {
218+
sb.append("text1" + "text2", 2, 3);
219+
}
220+
return sb.toString();
89221
}
90222

91-
public String stringBufferConcatSimple() {
92-
return new StringBuffer()
93-
.append("text1" + "text2") // Noncompliant {{Use multiple calls to "append" instead of string concatenation.}} [[quickfixes=qf101]]
94-
// ^^^^^^^^^^^^^^^^^
95-
// fix@qf101 {{Call "append" multiple times.}}
96-
// edit@qf101 [[sc=14;ec=33]] {{("text1").append("text2")}}
97-
.toString();
223+
public String crossesMethodBoundary() {
224+
// FN: we do not analyze across method calls
225+
StringBuffer sb = new StringBuffer();
226+
for (int i = 0; i < 10; i++) {
227+
anotherMethod(sb);
228+
}
229+
return sb.toString();
230+
}
231+
232+
private void anotherMethod(StringBuffer sb) {
233+
sb.append("text1" + "text2");
98234
}
99235
}

0 commit comments

Comments
 (0)