Skip to content

Commit

Permalink
QuickFix for S7158 (String.isEmpty() instead of length check).
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomasz-tylenda-sonarsource
tomasz-tylenda-sonarsource committed Nov 27, 2024
1 parent b097d2b commit 4fbe223
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 26 deletions.
81 changes: 66 additions & 15 deletions java-checks-test-sources/default/src/main/java/checks/StringIsEmptyCheckSample.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,82 @@ public boolean sample(String s, String t) {
boolean b;

// test `length() == 0` and equivalent code
b = s.length() == 0; // Noncompliant
b = s.length() <= 0; // Noncompliant
b = s.length() < 1; // Noncompliant

b = s.length() == 0; // Noncompliant [[quickfixes=qf1]]
// fix@qf1 {{Replace with "isEmpty()"}}
// edit@qf1 [[sc=11;ec=24]]{{isEmpty()}}

b = s.length() <= 0; // Noncompliant [[quickfixes=qf2]]
// fix@qf2 {{Replace with "isEmpty()"}}
// edit@qf2 [[sc=11;ec=24]]{{isEmpty()}}

b = s.length() < 1; // Noncompliant [[quickfixes=qf3]]
// fix@qf3 {{Replace with "isEmpty()"}}
// edit@qf3 [[sc=11;ec=23]]{{isEmpty()}}

// test `length() != 0` and equivalent code
b = s.length() != 0; // Noncompliant
b = s.length() > 0; // Noncompliant
b = s.length() >= 1; // Noncompliant
b = s.length() != 0; // Noncompliant [[quickfixes=qf4]]
// fix@qf4 {{Replace with "isEmpty()"}}
// edit@qf4 [[sc=9;ec=9]]{{!}}
// edit@qf4 [[sc=11;ec=24]]{{isEmpty()}}

b = s.length() > 0; // Noncompliant [[quickfixes=qf5]]
// fix@qf5 {{Replace with "isEmpty()"}}
// edit@qf5 [[sc=9;ec=9]]{{!}}
// edit@qf5 [[sc=11;ec=23]]{{isEmpty()}}

b = s.length() >= 1; // Noncompliant [[quickfixes=qf6]]
// fix@qf6 {{Replace with "isEmpty()"}}
// edit@qf6 [[sc=9;ec=9]]{{!}}
// edit@qf6 [[sc=11;ec=24]]{{isEmpty()}}

// reversed order
b = 0 == s.length(); // Noncompliant
b = 0 >= s.length(); // Noncompliant
b = 1 > s.length(); // Noncompliant
b = 0 != s.length(); // Noncompliant
b = 0 < s.length(); // Noncompliant
b = 1 <= s.length(); // Noncompliant

b = 0 == s.length(); // Noncompliant [[quickfixes=qf7]]
// fix@qf7 {{Replace with "isEmpty()"}}
// edit@qf7 [[sc=9;ec=14]]{{}}
// edit@qf7 [[sc=16;ec=24]]{{isEmpty()}}

b = 0 >= s.length(); // Noncompliant [[quickfixes=qf8]]
// fix@qf8 {{Replace with "isEmpty()"}}
// edit@qf8 [[sc=9;ec=14]]{{}}
// edit@qf8 [[sc=16;ec=24]]{{isEmpty()}}

b = 1 > s.length(); // Noncompliant [[quickfixes=qf9]]
// fix@qf9 {{Replace with "isEmpty()"}}
// edit@qf9 [[sc=9;ec=13]]{{}}
// edit@qf9 [[sc=15;ec=23]]{{isEmpty()}}

b = 0 != s.length(); // Noncompliant [[quickfixes=qf10]]
// fix@qf10 {{Replace with "isEmpty()"}}
// edit@qf10 [[sc=9;ec=14]]{{!}}
// edit@qf10 [[sc=16;ec=24]]{{isEmpty()}}

b = 0 < s.length(); // Noncompliant [[quickfixes=qf11]]
// fix@qf11 {{Replace with "isEmpty()"}}
// edit@qf11 [[sc=9;ec=13]]{{!}}
// edit@qf11 [[sc=15;ec=23]]{{isEmpty()}}

b = 1 <= s.length(); // Noncompliant [[quickfixes=qf12]]
// fix@qf12 {{Replace with "isEmpty()"}}
// edit@qf12 [[sc=9;ec=14]]{{!}}
// edit@qf12 [[sc=16;ec=24]]{{isEmpty()}}

// extra parentheses
b = (s.length()) == 0; // Noncompliant
b = (s.length()) == 0; // Noncompliant [[quickfixes=qf13]]
// fix@qf13 {{Replace with "isEmpty()"}}
// edit@qf13 [[sc=9;ec=10]]{{}}
// edit@qf13 [[sc=12;ec=26]]{{isEmpty()}}

// chained method calls
b = s.toUpperCase().length() == 0; // Noncompliant
b = s.toUpperCase().length() == 0; // Noncompliant [[quickfixes=qf14]]
// fix@qf14 {{Replace with "isEmpty()"}}
// edit@qf14 [[sc=25;ec=38]]{{isEmpty()}}

// problem in a nested expression
b = "abc".equals(s) || s.length() == 0; // Noncompliant
b = "abc".equals(s) || s.length() == 0; // Noncompliant [[quickfixes=qf15]]
// fix@qf15 {{Replace with "isEmpty()"}}
// edit@qf15 [[sc=30;ec=43]]{{isEmpty()}}

b = s.length() == 1;
b = s.length() > 3;
Expand Down
111 changes: 100 additions & 11 deletions java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,21 @@
package org.sonar.java.checks;

import java.util.List;
import javax.annotation.Nullable;
import org.sonar.check.Rule;
import org.sonar.java.checks.helpers.QuickFixHelper;
import org.sonar.java.model.ExpressionUtils;
import org.sonar.java.model.LiteralUtils;
import org.sonar.java.reporting.AnalyzerMessage;
import org.sonar.java.reporting.JavaQuickFix;
import org.sonar.java.reporting.JavaTextEdit;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.JavaVersion;
import org.sonar.plugins.java.api.JavaVersionAwareVisitor;
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.tree.BinaryExpressionTree;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.Tree;
import org.sonar.plugins.java.api.tree.Tree.Kind;
Expand All @@ -55,6 +61,19 @@ public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements J
Kind.GREATER_THAN_OR_EQUAL_TO
};

private enum ComparisonType {
// for example, `s.length() == 0`
IS_EMPTY,
// for example, `s.length() > 0`
IS_NOT_EMPTY,
// for example, `s.length() > 80`
OTHER
}

private static boolean isEmptinessCheck(ComparisonType comparisonType) {
return comparisonType == ComparisonType.IS_EMPTY || comparisonType == ComparisonType.IS_NOT_EMPTY;
}

// `String.isEmpty()` is available since Java 6.
@Override
public boolean isCompatibleWithJavaVersion(JavaVersion version) {
Expand All @@ -78,29 +97,99 @@ public void visitNode(Tree tree) {
boolean rightIsZero = LiteralUtils.isZero(right);
boolean rightIsOne = LiteralUtils.isOne(right);

if ((isLengthCall(left) && isComparisonOnRight(bet, rightIsZero, rightIsOne))
|| (isLengthCall(right) && isComparisonOnLeft(leftIsZero, leftIsOne, bet))) {
reportIssue(tree, "Use isEmpty() to check whether a string is empty or not.");
final MethodInvocationTree lengthCall;
final ComparisonType comparisonType;

// First try `s.length() OPERATOR VALUE`, then try `VALUE OPERATOR s.length()`.
MethodInvocationTree mit = getLengthCall(left);
if (mit != null) {
lengthCall = mit;
comparisonType = getComparisonOnRight(bet, rightIsZero, rightIsOne);
} else {
lengthCall = getLengthCall(right);
comparisonType = lengthCall != null ? getComparisonOnLeft(leftIsZero, leftIsOne, bet) : null;
}

if (lengthCall != null && isEmptinessCheck(comparisonType)) {
QuickFixHelper
.newIssue(context)
.forRule(this)
.onTree(tree)
.withMessage("Use isEmpty() to check whether a string is empty or not.")
.withQuickFix(() -> getQuickFix(tree, lengthCall, comparisonType))
.report();
}
}

private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation, ComparisonType comparisonType) {
// There are two cases to deal with:
// s.length() OP CONST
// CONST OP s.length()
JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix("Replace with \"isEmpty()\"");

// Replace "[CONST OP]" with "!"/"".
AnalyzerMessage.TextSpan prefixSpan = AnalyzerMessage.textSpanBetween(comparisonExpression, true, lengthInvocation, false);
String prefixReplacement = comparisonType == ComparisonType.IS_NOT_EMPTY ? "!" : "";
if (!prefixReplacement.isEmpty() || !prefixSpan.isEmpty()) {
builder.addTextEdit(JavaTextEdit.replaceTextSpan(prefixSpan, prefixReplacement));
}

// Replace "length() [OP CONST]" with "isEmpty()".
IdentifierTree lengthIdentifier = ExpressionUtils.methodName(lengthInvocation);
builder.addTextEdit(JavaTextEdit.replaceBetweenTree(
lengthIdentifier,
true,
comparisonExpression,
true,
"isEmpty()"
));

return builder.build();
}

private static boolean isLengthCall(ExpressionTree tree) {
return tree instanceof MethodInvocationTree mit && LENGTH_METHOD.matches(mit);
@Nullable
private static MethodInvocationTree getLengthCall(ExpressionTree tree) {
if (tree instanceof MethodInvocationTree mit && LENGTH_METHOD.matches(mit)) {
return mit;
}
return null;
}

/**
* Check the comparison for <pre>length() OP VALUE</pre>.
*/
private static boolean isComparisonOnRight(BinaryExpressionTree tree, boolean rightIsZero, boolean rightIsOne) {
return (tree.is(Kind.EQUAL_TO, Kind.LESS_THAN_OR_EQUAL_TO, Kind.NOT_EQUAL_TO, Kind.GREATER_THAN) && rightIsZero)
|| (tree.is(Kind.LESS_THAN, Kind.GREATER_THAN_OR_EQUAL_TO) && rightIsOne);
private static ComparisonType getComparisonOnRight(BinaryExpressionTree tree, boolean rightIsZero, boolean rightIsOne) {
if (tree.is(Kind.EQUAL_TO, Kind.LESS_THAN_OR_EQUAL_TO) && rightIsZero) {
return ComparisonType.IS_EMPTY;
}
if (tree.is(Kind.NOT_EQUAL_TO, Kind.GREATER_THAN) && rightIsZero) {
return ComparisonType.IS_NOT_EMPTY;
}
if (tree.is(Kind.LESS_THAN) && rightIsOne) {
return ComparisonType.IS_EMPTY;
}
if (tree.is(Kind.GREATER_THAN_OR_EQUAL_TO) && rightIsOne) {
return ComparisonType.IS_NOT_EMPTY;
}
return ComparisonType.OTHER;
}

/**
* Check the comparison for <pre>VALUE OP length()</pre>.
*/
private static boolean isComparisonOnLeft(boolean leftIsZero, boolean leftIsOne, BinaryExpressionTree tree) {
return (leftIsZero && tree.is(Kind.EQUAL_TO, Kind.GREATER_THAN_OR_EQUAL_TO, Kind.NOT_EQUAL_TO, Kind.LESS_THAN))
|| (leftIsOne && tree.is(Kind.GREATER_THAN, Kind.LESS_THAN_OR_EQUAL_TO));
private static ComparisonType getComparisonOnLeft(boolean leftIsZero, boolean leftIsOne, BinaryExpressionTree tree) {
if (leftIsZero && tree.is(Kind.EQUAL_TO, Kind.GREATER_THAN_OR_EQUAL_TO)) {
return ComparisonType.IS_EMPTY;
}
if (leftIsZero && tree.is(Kind.NOT_EQUAL_TO, Kind.LESS_THAN)) {
return ComparisonType.IS_NOT_EMPTY;
}
if (leftIsOne && tree.is(Kind.GREATER_THAN)) {
return ComparisonType.IS_EMPTY;
}
if (leftIsOne && tree.is(Kind.LESS_THAN_OR_EQUAL_TO)) {
return ComparisonType.IS_NOT_EMPTY;
}
return ComparisonType.OTHER;
}
}

0 comments on commit 4fbe223

Please sign in to comment.