Skip to content

Commit

Permalink
SONARJAVA-4935 S1192 should not report on individual lines of multi l…
Browse files Browse the repository at this point in the history
…ine 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
  • Loading branch information
kaufco committed Apr 15, 2024
1 parent 3f65e31 commit b0515a4
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 30 deletions.
3 changes: 0 additions & 3 deletions its/ruling/src/test/resources/eclipse-jetty/java-S1192.json
Expand Up @@ -139,9 +139,6 @@
"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java": [
76
],
"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/CertificateValidator.java": [
143
],
"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java": [
1047
],
Expand Down
@@ -1,7 +1,4 @@
{
"jboss-ejb3-tutorial:blob/src/org/jboss/tutorial/blob/bean/LobTesterBean.java": [
68
],
"jboss-ejb3-tutorial:callbacks/src/org/jboss/tutorial/callback/client/Client.java": [
40
],
Expand Down
22 changes: 1 addition & 21 deletions its/ruling/src/test/resources/sonar-server/java-S1192.json
Expand Up @@ -8,9 +8,6 @@
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/component/ComponentFinder.java": [
147
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/component/ws/SearchProjectsAction.java": [
144
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/webhook/WebhookPayloadFactoryImpl.java": [
79
],
Expand All @@ -26,22 +23,6 @@
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java": [
121
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/measure/ws/ComponentTreeAction.java": [
150
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/permission/index/PermissionIndexerDao.java": [
111,
111,
112,
113,
114,
117,
119,
120,
121,
122,
123
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/permission/ws/ProjectWsRef.java": [
42
],
Expand All @@ -55,8 +36,7 @@
72
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/plugins/ws/UpdatesAction.java": [
71,
73
71
],
"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java": [
146
Expand Down
@@ -1,5 +1,7 @@
package checks;

import javax.annotation.Nullable;

public class StringLiteralDuplicatedCheck {

public void f() {
Expand Down Expand Up @@ -47,7 +49,7 @@ public ConstantAlreadyDefined setProject(Proj project) {
}

private void setFieldValue(String projectName, Object longName) {

}

public ConstantAlreadyDefined setProject(String projectKey, String projectName) {
Expand Down Expand Up @@ -76,3 +78,36 @@ class CompleteCoverage {

private static final String NOT_USED = "this constant is not used anywhere";
}

class IgnoreLiteralFragments {

private final String sqlStatement1 =
" SELECT " + // Compliant, because part of fragmented literal
" c.id, " + // Compliant, because part of fragmented literal
" c.name, " +
" FROM customers c " + // Compliant, because part of fragmented literal
" WHERE max_number IS NULL";

private final String sqlStatement2 =
" SELECT " +
" c.id, " +
" c.age, " +
" FROM customers c ";

private final String sqlStatement3 =
" SELECT " +
" c.id, " +
" c.name, " +
" c.birthDate, " +
" FROM customers c " +
" WHERE max_number IS NULL";
}

class Coverage {

@Nullable
Object coverAnnotations = null;

private final String prevLeftNull = "SELECT" + 3;
private final String prevRightNull = 3 + "SELECT";
}
Expand Up @@ -24,6 +24,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.java.model.LiteralUtils;
Expand All @@ -32,6 +33,7 @@
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.tree.AnnotationTree;
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.LiteralTree;
import org.sonar.plugins.java.api.tree.MethodTree;
Expand Down Expand Up @@ -90,13 +92,55 @@ private static List<JavaFileScannerContext.Location> secondaryLocations(Collecti
public void visitLiteral(LiteralTree tree) {
if (tree.is(Tree.Kind.STRING_LITERAL, Tree.Kind.TEXT_BLOCK)) {
String literal = tree.value();
if (literal.length() >= MINIMAL_LITERAL_LENGTH) {
if (literal.length() >= MINIMAL_LITERAL_LENGTH && !isStringLiteralFragment(tree)) {
String stringValue = LiteralUtils.getAsStringValue(tree).replace("\\n", "\n");
occurrences.computeIfAbsent(stringValue, key -> new ArrayList<>()).add(tree);
}
}
}

private static boolean isStringLiteralFragment(ExpressionTree tree) {
return isStringLiteral(tree) && (isStringLiteral(getNextOperand(tree)) || isStringLiteral(getPreviousOperand(tree)));
}

private static boolean isStringLiteral(@Nullable Tree tree) {
return tree != null && tree.is(Tree.Kind.STRING_LITERAL);
}

@Nullable
private static ExpressionTree getNextOperand(ExpressionTree tree) {
var binary = asPlusExpression(tree.parent());
if (binary == null) {
return null;
}
if (tree == binary.leftOperand()) {
return binary.rightOperand();
} else {
binary = asPlusExpression(binary.parent());
return binary != null ? binary.rightOperand() : null;
}
}

@Nullable
private static ExpressionTree getPreviousOperand(ExpressionTree tree) {
var binary = asPlusExpression(tree.parent());
if (binary == null) {
return null;
}
if (tree == binary.leftOperand()) {
return null;
} else {
var left = binary.leftOperand();
binary = asPlusExpression(left);
return binary != null ? binary.rightOperand() : binary;
}
}

@Nullable
private static BinaryExpressionTree asPlusExpression(Tree tree) {
return tree.is(Tree.Kind.PLUS) ? (BinaryExpressionTree) tree : null;
}

@Override
public void visitVariable(VariableTree tree) {
ExpressionTree initializer = tree.initializer();
Expand All @@ -111,7 +155,7 @@ public void visitVariable(VariableTree tree) {

@Override
public void visitMethod(MethodTree tree) {
if(ModifiersUtils.hasModifier(tree.modifiers(), Modifier.DEFAULT)) {
if (ModifiersUtils.hasModifier(tree.modifiers(), Modifier.DEFAULT)) {
//Ignore default methods to avoid catch-22 with S1214
return;
}
Expand Down

0 comments on commit b0515a4

Please sign in to comment.