Skip to content

Commit b0515a4

Browse files
authored
SONARJAVA-4935 S1192 should not report on individual lines of multi line string literal (#4767)
* SONARJAVA-4935 S1192 should not report on individual lines of multi line string literal * Ruling and coverage * Internal contributor ruling * Review comments
1 parent 3f65e31 commit b0515a4

5 files changed

Lines changed: 83 additions & 30 deletions

File tree

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,6 @@
139139
"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java": [
140140
76
141141
],
142-
"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/CertificateValidator.java": [
143-
143
144-
],
145142
"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java": [
146143
1047
147144
],

its/ruling/src/test/resources/jboss-ejb3-tutorial/java-S1192.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
11
{
2-
"jboss-ejb3-tutorial:blob/src/org/jboss/tutorial/blob/bean/LobTesterBean.java": [
3-
68
4-
],
52
"jboss-ejb3-tutorial:callbacks/src/org/jboss/tutorial/callback/client/Client.java": [
63
40
74
],

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

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@
88
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/component/ComponentFinder.java": [
99
147
1010
],
11-
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/component/ws/SearchProjectsAction.java": [
12-
144
13-
],
1411
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPayloadFactoryImpl.java": [
1512
79
1613
],
@@ -26,22 +23,6 @@
2623
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java": [
2724
121
2825
],
29-
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/measure/ws/ComponentTreeAction.java": [
30-
150
31-
],
32-
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/permission/index/PermissionIndexerDao.java": [
33-
111,
34-
111,
35-
112,
36-
113,
37-
114,
38-
117,
39-
119,
40-
120,
41-
121,
42-
122,
43-
123
44-
],
4526
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/permission/ws/ProjectWsRef.java": [
4627
42
4728
],
@@ -55,8 +36,7 @@
5536
72
5637
],
5738
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/plugins/ws/UpdatesAction.java": [
58-
71,
59-
73
39+
71
6040
],
6141
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java": [
6242
146

java-checks-test-sources/default/src/main/java/checks/StringLiteralDuplicatedCheck.java

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package checks;
22

3+
import javax.annotation.Nullable;
4+
35
public class StringLiteralDuplicatedCheck {
46

57
public void f() {
@@ -47,7 +49,7 @@ public ConstantAlreadyDefined setProject(Proj project) {
4749
}
4850

4951
private void setFieldValue(String projectName, Object longName) {
50-
52+
5153
}
5254

5355
public ConstantAlreadyDefined setProject(String projectKey, String projectName) {
@@ -76,3 +78,36 @@ class CompleteCoverage {
7678

7779
private static final String NOT_USED = "this constant is not used anywhere";
7880
}
81+
82+
class IgnoreLiteralFragments {
83+
84+
private final String sqlStatement1 =
85+
" SELECT " + // Compliant, because part of fragmented literal
86+
" c.id, " + // Compliant, because part of fragmented literal
87+
" c.name, " +
88+
" FROM customers c " + // Compliant, because part of fragmented literal
89+
" WHERE max_number IS NULL";
90+
91+
private final String sqlStatement2 =
92+
" SELECT " +
93+
" c.id, " +
94+
" c.age, " +
95+
" FROM customers c ";
96+
97+
private final String sqlStatement3 =
98+
" SELECT " +
99+
" c.id, " +
100+
" c.name, " +
101+
" c.birthDate, " +
102+
" FROM customers c " +
103+
" WHERE max_number IS NULL";
104+
}
105+
106+
class Coverage {
107+
108+
@Nullable
109+
Object coverAnnotations = null;
110+
111+
private final String prevLeftNull = "SELECT" + 3;
112+
private final String prevRightNull = 3 + "SELECT";
113+
}

java-checks/src/main/java/org/sonar/java/checks/StringLiteralDuplicatedCheck.java

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.HashMap;
2525
import java.util.List;
2626
import java.util.Map;
27+
import javax.annotation.Nullable;
2728
import org.sonar.check.Rule;
2829
import org.sonar.check.RuleProperty;
2930
import org.sonar.java.model.LiteralUtils;
@@ -32,6 +33,7 @@
3233
import org.sonar.plugins.java.api.JavaFileScannerContext;
3334
import org.sonar.plugins.java.api.tree.AnnotationTree;
3435
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
36+
import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
3537
import org.sonar.plugins.java.api.tree.ExpressionTree;
3638
import org.sonar.plugins.java.api.tree.LiteralTree;
3739
import org.sonar.plugins.java.api.tree.MethodTree;
@@ -90,13 +92,55 @@ private static List<JavaFileScannerContext.Location> secondaryLocations(Collecti
9092
public void visitLiteral(LiteralTree tree) {
9193
if (tree.is(Tree.Kind.STRING_LITERAL, Tree.Kind.TEXT_BLOCK)) {
9294
String literal = tree.value();
93-
if (literal.length() >= MINIMAL_LITERAL_LENGTH) {
95+
if (literal.length() >= MINIMAL_LITERAL_LENGTH && !isStringLiteralFragment(tree)) {
9496
String stringValue = LiteralUtils.getAsStringValue(tree).replace("\\n", "\n");
9597
occurrences.computeIfAbsent(stringValue, key -> new ArrayList<>()).add(tree);
9698
}
9799
}
98100
}
99101

102+
private static boolean isStringLiteralFragment(ExpressionTree tree) {
103+
return isStringLiteral(tree) && (isStringLiteral(getNextOperand(tree)) || isStringLiteral(getPreviousOperand(tree)));
104+
}
105+
106+
private static boolean isStringLiteral(@Nullable Tree tree) {
107+
return tree != null && tree.is(Tree.Kind.STRING_LITERAL);
108+
}
109+
110+
@Nullable
111+
private static ExpressionTree getNextOperand(ExpressionTree tree) {
112+
var binary = asPlusExpression(tree.parent());
113+
if (binary == null) {
114+
return null;
115+
}
116+
if (tree == binary.leftOperand()) {
117+
return binary.rightOperand();
118+
} else {
119+
binary = asPlusExpression(binary.parent());
120+
return binary != null ? binary.rightOperand() : null;
121+
}
122+
}
123+
124+
@Nullable
125+
private static ExpressionTree getPreviousOperand(ExpressionTree tree) {
126+
var binary = asPlusExpression(tree.parent());
127+
if (binary == null) {
128+
return null;
129+
}
130+
if (tree == binary.leftOperand()) {
131+
return null;
132+
} else {
133+
var left = binary.leftOperand();
134+
binary = asPlusExpression(left);
135+
return binary != null ? binary.rightOperand() : binary;
136+
}
137+
}
138+
139+
@Nullable
140+
private static BinaryExpressionTree asPlusExpression(Tree tree) {
141+
return tree.is(Tree.Kind.PLUS) ? (BinaryExpressionTree) tree : null;
142+
}
143+
100144
@Override
101145
public void visitVariable(VariableTree tree) {
102146
ExpressionTree initializer = tree.initializer();
@@ -111,7 +155,7 @@ public void visitVariable(VariableTree tree) {
111155

112156
@Override
113157
public void visitMethod(MethodTree tree) {
114-
if(ModifiersUtils.hasModifier(tree.modifiers(), Modifier.DEFAULT)) {
158+
if (ModifiersUtils.hasModifier(tree.modifiers(), Modifier.DEFAULT)) {
115159
//Ignore default methods to avoid catch-22 with S1214
116160
return;
117161
}

0 commit comments

Comments
 (0)