SONARJAVA-7188 SONARJAVA-4606 Improvements for the rule S7158

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

@@ -0,0 +1,106 @@
+package checks;
+
+public class StringIsBlankCheckSample {
+  public boolean sample(String s, StringBuilder sb) {
+    boolean b;
+
+    // test `strip().length() == 0` and equivalent code
+
+    b = s.strip().length() == 0; // Noncompliant {{Use "isBlank()" to check whether a "String" is blank or not.}} [[quickfixes=qf1]]
+    // fix@qf1 {{Replace with "isBlank()"}}
+    // edit@qf1 [[sc=11;ec=32]]{{isBlank()}}
+
+    b = s.strip().length() <= 0; // Noncompliant [[quickfixes=qf2]]
+    // fix@qf2 {{Replace with "isBlank()"}}
+    // edit@qf2 [[sc=11;ec=32]]{{isBlank()}}
+
+    b = s.strip().length() < 1; // Noncompliant [[quickfixes=qf3]]
+    // fix@qf3 {{Replace with "isBlank()"}}
+    // edit@qf3 [[sc=11;ec=31]]{{isBlank()}}
+
+    // test `strip().length() != 0` and equivalent code
+
+    b = s.strip().length() != 0; // Noncompliant [[quickfixes=qf4]]
+    // fix@qf4 {{Replace with "isBlank()"}}
+    // edit@qf4 [[sc=9;ec=9]]{{!}}
+    // edit@qf4 [[sc=11;ec=32]]{{isBlank()}}
+
+    b = s.strip().length() > 0; // Noncompliant [[quickfixes=qf5]]
+    // fix@qf5 {{Replace with "isBlank()"}}
+    // edit@qf5 [[sc=9;ec=9]]{{!}}
+    // edit@qf5 [[sc=11;ec=31]]{{isBlank()}}
+
+    b = s.strip().length() >= 1; // Noncompliant [[quickfixes=qf6]]
+    // fix@qf6 {{Replace with "isBlank()"}}
+    // edit@qf6 [[sc=9;ec=9]]{{!}}
+    // edit@qf6 [[sc=11;ec=32]]{{isBlank()}}
+
+    // reversed order
+
+    b = 0 == s.strip().length(); // Noncompliant [[quickfixes=qf7]]
+    // fix@qf7 {{Replace with "isBlank()"}}
+    // edit@qf7 [[sc=9;ec=14]]{{}}
+    // edit@qf7 [[sc=16;ec=32]]{{isBlank()}}
+
+    b = 0 != s.strip().length(); // Noncompliant [[quickfixes=qf8]]
+    // fix@qf8 {{Replace with "isBlank()"}}
+    // edit@qf8 [[sc=9;ec=14]]{{!}}
+    // edit@qf8 [[sc=16;ec=32]]{{isBlank()}}
+
+    // extra parentheses
+
+    b = (s.strip().length()) == 0; // Noncompliant [[quickfixes=qf9]]
+    // fix@qf9 {{Replace with "isBlank()"}}
+    // edit@qf9 [[sc=9;ec=10]]{{}}
+    // edit@qf9 [[sc=12;ec=34]]{{isBlank()}}
+
+    // the `strip()` call is not directly chained, so only emptiness can be simplified
+    b = (s.strip()).length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf10]]
+    // fix@qf10 {{Replace with "isEmpty()"}}
+    // edit@qf10 [[sc=21;ec=34]]{{isEmpty()}}
+
+    // chained method calls
+
+    b = s.toUpperCase().strip().length() == 0; // Noncompliant [[quickfixes=qf11]]
+    // fix@qf11 {{Replace with "isBlank()"}}
+    // edit@qf11 [[sc=25;ec=46]]{{isBlank()}}
+
+    b = sb.toString().strip().length() == 0; // Noncompliant [[quickfixes=qf12]]
+    // fix@qf12 {{Replace with "isBlank()"}}
+    // edit@qf12 [[sc=23;ec=44]]{{isBlank()}}
+
+    // `trim()` does not remove the same characters as `isBlank()` considers white space,
+    // so the message and the quick fix spell out what changes
+
+    b = s.trim().length() == 0; // Noncompliant {{Use "isBlank()" to check whether a "String" is blank or not. Use this if Unicode whitespace should be treated as blank and control characters as non-blank.}} [[quickfixes=qf13]]
+    // fix@qf13 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}}
+    // edit@qf13 [[sc=11;ec=31]]{{isBlank()}}
+
+    b = s.trim().length() > 0; // Noncompliant [[quickfixes=qf14]]
+    // fix@qf14 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}}
+    // edit@qf14 [[sc=9;ec=9]]{{!}}
+    // edit@qf14 [[sc=11;ec=30]]{{isBlank()}}
+
+    b = 0 == s.trim().length(); // Noncompliant [[quickfixes=qf15]]
+    // fix@qf15 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}}
+    // edit@qf15 [[sc=9;ec=14]]{{}}
+    // edit@qf15 [[sc=16;ec=31]]{{isBlank()}}
+
+    b = s.toLowerCase().trim().length() == 0; // Noncompliant [[quickfixes=qf16]]
+    // fix@qf16 {{Replace with "isBlank()" (Unicode whitespace is treated as blank, control characters as non-blank)}}
+    // edit@qf16 [[sc=25;ec=45]]{{isBlank()}}
+
+    b = s.trim().length() == 1;
+    b = s.trim().length() >= 8;
+
+    b = s.strip().length() == 1;
+    b = s.strip().length() > 3;
+    b = s.strip().length() >= 8;
+    b = s.strip().length() == sb.length();
+
+    b = s.isBlank();
+    b = !s.isBlank();
+
+    return b;
+  }
+}

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

@@ -0,0 +1,29 @@
+package checks;
+
+public class StringIsBlankCheckSample_java10 {
+  public boolean sample(String s) {
+    boolean b;
+
+    // `String.isBlank()` does not exist before Java 11, so these stay emptiness checks on the trimmed string
+
+    b = s.strip().length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf1]]
+    // fix@qf1 {{Replace with "isEmpty()"}}
+    // edit@qf1 [[sc=19;ec=32]]{{isEmpty()}}
+
+    b = s.strip().length() > 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf2]]
+    // fix@qf2 {{Replace with "isEmpty()"}}
+    // edit@qf2 [[sc=9;ec=9]]{{!}}
+    // edit@qf2 [[sc=19;ec=31]]{{isEmpty()}}
+
+    b = s.trim().length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf3]]
+    // fix@qf3 {{Replace with "isEmpty()"}}
+    // edit@qf3 [[sc=18;ec=31]]{{isEmpty()}}
+
+    b = 0 == s.trim().length(); // Noncompliant {{Use "isEmpty()" to check whether a "String" is empty or not.}} [[quickfixes=qf4]]
+    // fix@qf4 {{Replace with "isEmpty()"}}
+    // edit@qf4 [[sc=9;ec=14]]{{}}
+    // edit@qf4 [[sc=23;ec=31]]{{isEmpty()}}
+
+    return b;
+  }
+}

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

@@ -82,6 +82,36 @@ public boolean sample(String s, String t) {
     // fix@qf15 {{Replace with "isEmpty()"}}
     // edit@qf15 [[sc=30;ec=43]]{{isEmpty()}}
 
+    // zero written in a non-decimal base
+    b = s.length() == 0x0; // Noncompliant [[quickfixes=qf16]]
+    // fix@qf16 {{Replace with "isEmpty()"}}
+    // edit@qf16 [[sc=11;ec=26]]{{isEmpty()}}
+
+    b = s.length() == 0X0; // Noncompliant [[quickfixes=qf17]]
+    // fix@qf17 {{Replace with "isEmpty()"}}
+    // edit@qf17 [[sc=11;ec=26]]{{isEmpty()}}
+
+    b = s.length() == 00; // Noncompliant [[quickfixes=qf18]]
+    // fix@qf18 {{Replace with "isEmpty()"}}
+    // edit@qf18 [[sc=11;ec=25]]{{isEmpty()}}
+
+    b = s.length() == 0b0; // Noncompliant [[quickfixes=qf19]]
+    // fix@qf19 {{Replace with "isEmpty()"}}
+    // edit@qf19 [[sc=11;ec=26]]{{isEmpty()}}
+
+    b = s.length() == 0B0; // Noncompliant [[quickfixes=qf20]]
+    // fix@qf20 {{Replace with "isEmpty()"}}
+    // edit@qf20 [[sc=11;ec=26]]{{isEmpty()}}
+
+    b = 0x0 != s.length(); // Noncompliant [[quickfixes=qf21]]
+    // fix@qf21 {{Replace with "isEmpty()"}}
+    // edit@qf21 [[sc=9;ec=16]]{{!}}
+    // edit@qf21 [[sc=18;ec=26]]{{isEmpty()}}
+
+    b = s.length() == 0x1;
+    b = s.length() == 01;
+    b = s.length() == 0b1;
+
     b = s.length() == 1;
     b = s.length() > 3;
     b = s.length() <= 10;

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

@@ -39,17 +39,41 @@
 
 /**
  * Implement rule that <pre>CharSequence.length() == 0</pre> should be replaced with
- * <pre>CharSequence.isEmpty()</pre>.
+ * <pre>CharSequence.isEmpty()</pre>, and <pre>String.strip().length() == 0</pre> or
+ * <pre>String.trim().length() == 0</pre> with <pre>String.isBlank()</pre>.
  */
 @Rule(key = "S7158")
 public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
 
+  private static final String JAVA_LANG_STRING = "java.lang.String";
+
+  // `String.isBlank()` is available since Java 11
+  private static final int JAVA_11 = 11;
+
   private static final MethodMatchers STRING_LENGTH_METHOD = MethodMatchers.create()
-    .ofTypes("java.lang.String")
+    .ofTypes(JAVA_LANG_STRING)
     .names("length")
     .addWithoutParametersMatcher()
     .build();
 
+  private static final String TRIM_NAME = "trim";
+
+  private static final MethodMatchers STRING_WHITESPACE_REMOVAL_METHODS = MethodMatchers.create()
+    .ofTypes(JAVA_LANG_STRING)
+    .names("strip", TRIM_NAME)
+    .addWithoutParametersMatcher()
+    .build();
+
+  /**
+   * <pre>trim()</pre> removes every character up to and including <pre>U+0020</pre>, while <pre>isBlank()</pre> relies on
+   * <pre>Character.isWhitespace</pre>. They disagree on control characters below <pre>U+0020</pre> and on Unicode white
+   * space above it, so replacing one with the other can change behaviour.
+   */
+  private static final String TRIM_CAVEAT =
+    "Use this if Unicode whitespace should be treated as blank and control characters as non-blank.";
+
+  private static final String TRIM_FIX_CAVEAT = " (Unicode whitespace is treated as blank, control characters as non-blank)";
+
   private static final MethodMatchers CHARSEQUENCE_LENGTH_METHOD = MethodMatchers.create()
     .ofSubTypes("java.lang.CharSequence")
     .names("length")
@@ -117,21 +141,36 @@ public void visitNode(Tree tree) {
     }
 
     if (lengthCall != null && isEmptinessCheck(comparisonType)) {
+      MethodInvocationTree whitespaceRemovalCall = getWhitespaceRemovalCall(lengthCall);
+
       QuickFixHelper
         .newIssue(context)
         .forRule(this)
         .onTree(tree)
-        .withMessage("Use \"isEmpty()\" to check whether a \"" + ownerName(lengthCall) + "\" is empty or not.")
-        .withQuickFix(() -> getQuickFix(tree, lengthCall, comparisonType))
+        .withMessage(getMessage(lengthCall, whitespaceRemovalCall))
+        .withQuickFix(() -> getQuickFix(tree, lengthCall, whitespaceRemovalCall, comparisonType))
         .report();
     }
   }
 
-  private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation, ComparisonType comparisonType) {
+  private static String getMessage(MethodInvocationTree lengthCall, @Nullable MethodInvocationTree whitespaceRemovalCall) {
+    String adjective = whitespaceRemovalCall != null ? "blank" : "empty";
+    String caveat = isTrimBased(whitespaceRemovalCall) ? (" " + TRIM_CAVEAT) : "";
+
+    return "Use \"" + replacementMethodName(whitespaceRemovalCall) + "\" to check whether a \""
+      + ownerName(lengthCall) + "\" is " + adjective + " or not." + caveat;
+  }
+
+  private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocationTree lengthInvocation,
+    @Nullable MethodInvocationTree whitespaceRemovalCall, ComparisonType comparisonType) {
+    String replacedMethod = replacementMethodName(whitespaceRemovalCall);
+    MethodInvocationTree replacedInvocation = whitespaceRemovalCall != null ? whitespaceRemovalCall : lengthInvocation;
+    String fixDescription = "Replace with \"" + replacedMethod + "\"" + (isTrimBased(whitespaceRemovalCall) ? TRIM_FIX_CAVEAT : "");
+
     // There are two cases to deal with:
     //   s.length() OP CONST
     //   CONST OP s.length()
-    JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix("Replace with \"isEmpty()\"");
+    JavaQuickFix.Builder builder = JavaQuickFix.newQuickFix(fixDescription);
 
     // Replace "[CONST OP]" with "!"/"".
     AnalyzerMessage.TextSpan prefixSpan = AnalyzerMessage.textSpanBetween(comparisonExpression, true, lengthInvocation, false);
@@ -140,19 +179,27 @@ private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocat
       builder.addTextEdit(JavaTextEdit.replaceTextSpan(prefixSpan, prefixReplacement));
     }
 
-    // Replace "length() [OP CONST]" with "isEmpty()".
-    IdentifierTree lengthIdentifier = ExpressionUtils.methodName(lengthInvocation);
+    // Replace "length() [OP CONST]" with "isEmpty()", or "strip()/trim().length() [OP CONST]" with "isBlank()".
+    IdentifierTree replacedIdentifier = ExpressionUtils.methodName(replacedInvocation);
     builder.addTextEdit(JavaTextEdit.replaceBetweenTree(
-      lengthIdentifier,
+      replacedIdentifier,
       true,
       comparisonExpression,
       true,
-      "isEmpty()"
+      replacedMethod
     ));
 
     return builder.build();
   }
 
+  private static String replacementMethodName(@Nullable MethodInvocationTree whitespaceRemovalCall) {
+    return whitespaceRemovalCall != null ? "isBlank()" : "isEmpty()";
+  }
+
+  private static boolean isTrimBased(@Nullable MethodInvocationTree whitespaceRemovalCall) {
+    return whitespaceRemovalCall != null && TRIM_NAME.equals(ExpressionUtils.methodName(whitespaceRemovalCall).name());
+  }
+
   private static String ownerName(MethodInvocationTree lengthCall) {
     if (lengthCall.methodSelect() instanceof MemberSelectExpressionTree sel) {
       return sel.expression().symbolType().name();
@@ -161,6 +208,21 @@ private static String ownerName(MethodInvocationTree lengthCall) {
     }
   }
 
+  /**
+   * @return the <pre>strip()</pre> or <pre>trim()</pre> invocation the given <pre>length()</pre> call is applied to, or
+   *         null when there is none, when it is not directly chained, or when <pre>String.isBlank()</pre> is not available.
+   */
+  @Nullable
+  private MethodInvocationTree getWhitespaceRemovalCall(MethodInvocationTree lengthCall) {
+    if (context.getJavaVersion().asInt() >= JAVA_11
+      && lengthCall.methodSelect() instanceof MemberSelectExpressionTree sel
+      && sel.expression() instanceof MethodInvocationTree mit
+      && STRING_WHITESPACE_REMOVAL_METHODS.matches(mit)) {
+      return mit;
+    }
+    return null;
+  }
+
   @Nullable
   private MethodInvocationTree getLengthCall(ExpressionTree tree) {
     if (tree instanceof MethodInvocationTree mit) {

java-checks/src/test/java/org/sonar/java/checks/StringIsEmptyCheckTest.java

@@ -41,6 +41,34 @@ void string_15() {
       .verifyIssues();
   }
 
+  @Test
+  void blank_11() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .withJavaVersion(11)
+      .verifyIssues();
+  }
+
+  @Test
+  void blank_11_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .withJavaVersion(11)
+      .withoutSemantic()
+      .verifyIssues();
+  }
+
+  @Test
+  void blank_10() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StringIsBlankCheckSample_java10.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .withJavaVersion(10)
+      .verifyIssues();
+  }
+
   @Test
   void charSequence_15() {
     CheckVerifier.newVerifier()

java-frontend/src/main/java/org/sonar/java/model/LiteralUtils.java

@@ -152,8 +152,19 @@ public static boolean isFalse(Tree tree) {
     return tree.is(Kind.BOOLEAN_LITERAL) && "false".equals(((LiteralTree) tree).value());
   }
 
+  /**
+   * @return true when the expression is an int literal whose value is zero, in any base:
+   *         decimal ({@code 0}), hexadecimal ({@code 0x0}), octal ({@code 00}) or binary ({@code 0b0}).
+   */
   public static boolean isZero(ExpressionTree tree) {
-    return tree.is(Kind.INT_LITERAL) && "0".equals(((LiteralTree) tree).value());
+    if (!tree.is(Kind.INT_LITERAL)) {
+      return false;
+    }
+    try {
+      return intLiteralValue((LiteralTree) tree) == 0;
+    } catch (NumberFormatException e) {
+      return false;
+    }
   }
 
   public static boolean isOne(ExpressionTree tree) {