SONARJAVA-5622 extend S7158 to work with all CharSequence not just Srings (#5196)

Author: erwan-serandour

java-checks-test-sources/default/src/main/files/non-compiling/java/lang/String.java

@@ -0,0 +1,17 @@
+package java.lang;
+
+public class String {
+
+  public boolean isEmpty() {
+    return true;
+  }
+
+  int length() {
+    return 0;
+  }
+
+  public void samples(){
+    boolean b = length() > 0; // Noncompliant
+  }
+}
+

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

@@ -0,0 +1,89 @@
+package checks;
+
+import java.nio.CharBuffer;
+
+public class CharSequenceIsEmptyCheckSample {
+
+  public boolean testStringBuilder(StringBuilder sb1, StringBuilder sb2) {
+    boolean b;
+
+    b = sb1.length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "StringBuilder" is empty or not.}}
+    //  ^^^^^^^^^^^^^^^^^
+    b = sb2.length() <= 0; // Noncompliant [[quickfixes=qf2]]
+    // fix@qf2 {{Replace with "isEmpty()"}}
+    // edit@qf2 [[sc=13;ec=26]]{{isEmpty()}}
+    
+
+    b = 0 >= sb1.length(); // Noncompliant
+    b = 1 > sb2.length();  // Noncompliant
+    
+    return b;
+  }
+  
+
+  public boolean testStringBuffer(StringBuffer buf1, StringBuffer buf2) {
+    boolean b;
+
+    b = buf1.length() == 0;  // Noncompliant {{Use "isEmpty()" to check whether a "StringBuffer" is empty or not.}}
+    //  ^^^^^^^^^^^^^^^^^^
+    b = buf2.length() < 1; // Noncompliant [[quickfixes=qf3]]
+    // fix@qf3 {{Replace with "isEmpty()"}}
+    // edit@qf3 [[sc=14;ec=26]]{{isEmpty()}}
+
+
+    b = 0 != buf1.length(); // Noncompliant
+    b = 1 <= buf2.length(); // Noncompliant
+    
+    return b;
+  }
+
+  public boolean testCharBuffer(CharBuffer cb1, CharBuffer cb2) {
+    boolean b;
+
+    b = cb1.length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "CharBuffer" is empty or not.}}
+    //  ^^^^^^^^^^^^^^^^^
+    b = cb2.length() > 0;  // Noncompliant [[quickfixes=qf4]]
+    // fix@qf4 {{Replace with "isEmpty()"}}
+    // edit@qf4 [[sc=9;ec=9]]{{!}}
+    // edit@qf4 [[sc=13;ec=25]]{{isEmpty()}}
+    
+
+    b = 0 == cb1.length(); // Noncompliant
+    b = 0 < cb2.length();  // Noncompliant
+    
+    return b;
+  }
+
+  
+
+  public boolean testCharSequence(CharSequence cs1, CharSequence cs2) {
+    boolean b;
+
+    b = cs1.length() == 0; // Noncompliant {{Use "isEmpty()" to check whether a "CharSequence" is empty or not.}}
+    //  ^^^^^^^^^^^^^^^^^
+    b = cs2.length() >= 1; // Noncompliant [[quickfixes=qf5]]
+    // fix@qf5 {{Replace with "isEmpty()"}}
+    // edit@qf5 [[sc=9;ec=9]]{{!}}
+    // edit@qf5 [[sc=13;ec=26]]{{isEmpty()}}
+    
+
+    b = 0 == cs1.length(); // Noncompliant
+    b = 1 <= cs2.length(); // Noncompliant
+    
+    // Proper way using isEmpty
+    b = cs1.isEmpty();
+    b = !cs2.isEmpty();
+    
+    return b;
+  }
+
+  public void nonRelated(Foo f){
+    boolean b = f.length() == 0;
+  }
+
+  static class Foo {
+    int length(){
+      return 0;
+    }
+  }
+}

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

@@ -96,9 +96,6 @@ public boolean sample(String s, String t) {
 
     b = 1 < 0;
 
-    // StringBuilder does not have `isEmpty()`
-    StringBuilder stringBuilder = new StringBuilder();
-    b = stringBuilder.length() == 0;
 
     return b;
   }

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

@@ -32,23 +32,30 @@
 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.MemberSelectExpressionTree;
 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;
 
 /**
- * Implement rule that <pre>String.length() == 0</pre> should be replaced with
- * <pre>String.isEmpty()</pre>.
+ * Implement rule that <pre>CharSequence.length() == 0</pre> should be replaced with
+ * <pre>CharSequence.isEmpty()</pre>.
  */
 @Rule(key = "S7158")
 public class StringIsEmptyCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
 
-  private static final MethodMatchers LENGTH_METHOD = MethodMatchers.create()
+  private static final MethodMatchers STRING_LENGTH_METHOD = MethodMatchers.create()
     .ofTypes("java.lang.String")
     .names("length")
     .addWithoutParametersMatcher()
     .build();
 
+  private static final MethodMatchers CHARSEQUENCE_LENGTH_METHOD = MethodMatchers.create()
+    .ofSubTypes("java.lang.CharSequence")
+    .names("length")
+    .addWithoutParametersMatcher()
+    .build();
+
   private static final Kind[] TARGETED_BINARY_OPERATOR_TREES = {
     Kind.EQUAL_TO,
     Kind.NOT_EQUAL_TO,
@@ -71,12 +78,14 @@ private static boolean isEmptinessCheck(ComparisonType comparisonType) {
     return comparisonType == ComparisonType.IS_EMPTY || comparisonType == ComparisonType.IS_NOT_EMPTY;
   }
 
-  // `String.isEmpty()` is available since Java 6.
+
+  // `String.isEmpty()` is available since Java 6, but `CharSequence.isEmpty()` since Java 15
   @Override
   public boolean isCompatibleWithJavaVersion(JavaVersion version) {
     return version.isJava6Compatible();
   }
 
+
   @Override
   public List<Kind> nodesToVisit() {
     return List.of(TARGETED_BINARY_OPERATOR_TREES);
@@ -112,7 +121,7 @@ public void visitNode(Tree tree) {
         .newIssue(context)
         .forRule(this)
         .onTree(tree)
-        .withMessage("Use isEmpty() to check whether a string is empty or not.")
+        .withMessage("Use \"isEmpty()\" to check whether a \"" + ownerName(lengthCall) + "\" is empty or not.")
         .withQuickFix(() -> getQuickFix(tree, lengthCall, comparisonType))
         .report();
     }
@@ -144,10 +153,20 @@ private static JavaQuickFix getQuickFix(Tree comparisonExpression, MethodInvocat
     return builder.build();
   }
 
+  private static String ownerName(MethodInvocationTree lengthCall) {
+    if (lengthCall.methodSelect() instanceof MemberSelectExpressionTree sel) {
+      return sel.expression().symbolType().name();
+    } else {
+      return lengthCall.methodSymbol().owner().name();
+    }
+  }
+
   @Nullable
-  private static MethodInvocationTree getLengthCall(ExpressionTree tree) {
-    if (tree instanceof MethodInvocationTree mit && LENGTH_METHOD.matches(mit)) {
-      return mit;
+  private MethodInvocationTree getLengthCall(ExpressionTree tree) {
+    if (tree instanceof MethodInvocationTree mit) {
+      if (STRING_LENGTH_METHOD.matches(mit) || (context.getJavaVersion().isJava15Compatible() && CHARSEQUENCE_LENGTH_METHOD.matches(mit))) {
+        return mit;
+      }
     }
     return null;
   }

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

@@ -20,17 +20,45 @@
 import org.sonar.java.checks.verifier.CheckVerifier;
 
 import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath;
 
 class StringIsEmptyCheckTest {
 
   @Test
-  void test() {
+  void string() {
     CheckVerifier.newVerifier()
       .onFile(mainCodeSourcesPath("checks/StringIsEmptyCheckSample.java"))
       .withCheck(new StringIsEmptyCheck())
       .verifyIssues();
   }
 
+  @Test
+  void string_15() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/StringIsEmptyCheckSample.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .withJavaVersion(15)
+      .verifyIssues();
+  }
+
+  @Test
+  void charSequence_15() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/CharSequenceIsEmptyCheckSample.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .withJavaVersion(15)
+      .verifyIssues();
+  }
+
+  @Test
+  void charSequence_8() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/CharSequenceIsEmptyCheckSample.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .withJavaVersion(8)
+      .verifyNoIssues();
+  }
+
   @Test
   void testOlderJavaVersion() {
     CheckVerifier.newVerifier()
@@ -39,4 +67,12 @@ void testOlderJavaVersion() {
       .withJavaVersion(5)
       .verifyNoIssues();
   }
+
+  @Test
+  void without_explicit_receiver() {
+    CheckVerifier.newVerifier()
+      .onFile(nonCompilingTestSourcesPath("java/lang/String.java"))
+      .withCheck(new StringIsEmptyCheck())
+      .verifyIssues();
+  }
 }