SONARJAVA-7043: Implemented rule S9412 "Collections.sort()" should not be used

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

@@ -0,0 +1,14 @@
+{
+"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ProxyCustomizerTest.java": [
+97
+],
+"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java": [
+792
+],
+"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java": [
+840
+],
+"org.eclipse.jetty:jetty-project:jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java": [
+693
+]
+}

its/ruling/src/test/resources/expected/guava/java-S9412.json

@@ -0,0 +1,14 @@
+{
+"com.google.guava:guava:src/com/google/common/collect/ImmutableMultimap.java": [
+280
+],
+"com.google.guava:guava:src/com/google/common/collect/Ordering.java": [
+698
+],
+"com.google.guava:guava:src/com/google/common/collect/RegularImmutableTable.java": [
+125
+],
+"com.google.guava:guava:src/com/google/common/util/concurrent/ServiceManager.java": [
+597
+]
+}

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

@@ -0,0 +1,19 @@
+{
+"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/source/DuplicationLineReader.java": [
+69
+],
+"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/source/SymbolsLineReader.java": [
+58,
+83
+],
+"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java": [
+75,
+78
+],
+"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/es/IndexDefinitionHash.java": [
+79
+],
+"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/duplication/TextBlockTest.java": [
+92
+]
+}

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

@@ -0,0 +1,75 @@
+package checks;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedList;
+import java.util.List;
+
+class CollectionsSortCheckSample {
+
+  void noncompliant(List<String> myList, Comparator<String> myComparator) {
+    Collections.sort(myList); // Noncompliant {{Replace this "Collections.sort()" with "List.sort()".}} [[quickfixes=qf1]]
+//              ^^^^
+    // fix@qf1 {{Replace with "List.sort()"}}
+    // edit@qf1 [[sc=5;ec=29]] {{myList.sort(null)}}
+    Collections.sort(myList, Comparator.naturalOrder()); // Noncompliant {{Replace this "Collections.sort()" with "List.sort()".}} [[quickfixes=qf2]]
+//              ^^^^
+    // fix@qf2 {{Replace with "List.sort()"}}
+    // edit@qf2 [[sc=5;ec=56]] {{myList.sort(Comparator.naturalOrder())}}
+    Collections.sort(myList, myComparator); // Noncompliant {{Replace this "Collections.sort()" with "List.sort()".}} [[quickfixes=qf3]]
+//              ^^^^
+    // fix@qf3 {{Replace with "List.sort()"}}
+    // edit@qf3 [[sc=5;ec=43]] {{myList.sort(myComparator)}}
+  }
+
+  void noncompliantSubtypes() {
+    ArrayList<String> arrayList = new ArrayList<>();
+    Collections.sort(arrayList); // Noncompliant
+//              ^^^^
+
+    LinkedList<Integer> linkedList = new LinkedList<>();
+    Collections.sort(linkedList); // Noncompliant
+//              ^^^^
+  }
+
+  void noncompliantMethodReturn() {
+    Collections.sort(getList()); // Noncompliant
+//              ^^^^
+  }
+
+  void noncompliantComplexExpression(boolean flag, List<String> a, List<String> b) {
+    Collections.sort(flag ? a : b); // Noncompliant [[quickfixes=qf4]]
+//              ^^^^
+    // fix@qf4 {{Replace with "List.sort()"}}
+    // edit@qf4 [[sc=5;ec=35]] {{(flag ? a : b).sort(null)}}
+  }
+
+  void noncompliantFullyQualified(List<String> myList) {
+    java.util.Collections.sort(myList); // Noncompliant
+  }
+
+  void compliant(List<String> myList, Comparator<String> comparator) {
+    myList.sort(null);
+    myList.sort(comparator);
+    Collections.shuffle(myList);
+    Collections.reverse(myList);
+    Collections.unmodifiableList(myList);
+    int[] array = {3, 1, 2};
+    Arrays.sort(array);
+  }
+
+  void compliantCustomClass(List<String> list) {
+    MyCollections.sort(list);
+  }
+
+  private List<String> getList() {
+    return new ArrayList<>();
+  }
+
+  static class MyCollections {
+    static <T> void sort(List<T> list) {
+    }
+  }
+}

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

@@ -0,0 +1,91 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import java.util.EnumSet;
+import java.util.Set;
+import org.sonar.check.Rule;
+import org.sonar.java.checks.helpers.QuickFixHelper;
+import org.sonar.java.checks.methods.AbstractMethodDetection;
+import org.sonar.java.model.ExpressionUtils;
+import org.sonar.java.reporting.JavaQuickFix;
+import org.sonar.java.reporting.JavaTextEdit;
+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.ExpressionTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.Tree;
+
+@Rule(key = "S9412")
+public class CollectionsSortCheck extends AbstractMethodDetection implements JavaVersionAwareVisitor {
+
+  private static final Set<Tree.Kind> SAFE_KINDS = EnumSet.of(
+    Tree.Kind.IDENTIFIER,
+    Tree.Kind.MEMBER_SELECT,
+    Tree.Kind.METHOD_INVOCATION,
+    Tree.Kind.ARRAY_ACCESS_EXPRESSION,
+    Tree.Kind.PARENTHESIZED_EXPRESSION,
+    Tree.Kind.NEW_CLASS);
+
+  @Override
+  public boolean isCompatibleWithJavaVersion(JavaVersion version) {
+    return version.isJava8Compatible();
+  }
+
+  private static final String MESSAGE = "Replace this \"Collections.sort()\" with \"List.sort()\".";
+
+  @Override
+  protected MethodMatchers getMethodInvocationMatchers() {
+    return MethodMatchers.create()
+      .ofTypes("java.util.Collections")
+      .names("sort")
+      .addParametersMatcher("java.util.List")
+      .addParametersMatcher("java.util.List", "java.util.Comparator")
+      .build();
+  }
+
+  @Override
+  protected void onMethodInvocationFound(MethodInvocationTree mit) {
+    ExpressionTree listArgument = mit.arguments().get(0);
+    boolean hasTwoArgs = mit.arguments().size() == 2;
+
+    QuickFixHelper.newIssue(context)
+      .forRule(this)
+      .onTree(ExpressionUtils.methodName(mit))
+      .withMessage(MESSAGE)
+      .withQuickFix(() -> buildQuickFix(mit, listArgument, hasTwoArgs))
+      .report();
+  }
+
+  private JavaQuickFix buildQuickFix(MethodInvocationTree mit, ExpressionTree listArgument, boolean hasTwoArgs) {
+    String listText = QuickFixHelper.contentForTree(listArgument, context);
+    if (!listArgument.is(SAFE_KINDS.toArray(new Tree.Kind[0]))) {
+      listText = "(" + listText + ")";
+    }
+    String replacement;
+    if (hasTwoArgs) {
+      String comparatorText = QuickFixHelper.contentForTree(mit.arguments().get(1), context);
+      replacement = listText + ".sort(" + comparatorText + ")";
+    } else {
+      replacement = listText + ".sort(null)";
+    }
+    return JavaQuickFix.newQuickFix("Replace with \"List.sort()\"")
+      .addTextEdit(JavaTextEdit.replaceTree(mit, replacement))
+      .build();
+  }
+}

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

@@ -0,0 +1,55 @@
+/*
+ * SonarQube Java
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.java.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+
+import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
+
+class CollectionsSortCheckTest {
+
+  @Test
+  void test() {
+    createCheckVerifier()
+      .withJavaVersion(8)
+      .verifyIssues();
+  }
+
+  @Test
+  void test_no_issues_for_older_versions() {
+    createCheckVerifier()
+      .withJavaVersion(7)
+      .verifyNoIssues();
+  }
+
+  @Test
+  void test_without_semantic() {
+    CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/CollectionsSortCheckSample.java"))
+      .withCheck(new CollectionsSortCheck())
+      .withJavaVersion(8)
+      .withoutSemantic()
+      .verifyIssues();
+  }
+
+  private static CheckVerifier createCheckVerifier() {
+    return CheckVerifier.newVerifier()
+      .onFile(mainCodeSourcesPath("checks/CollectionsSortCheckSample.java"))
+      .withCheck(new CollectionsSortCheck());
+  }
+}

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9412.html

@@ -0,0 +1,48 @@
+<p>This rule raises an issue when a standalone utility function for sorting is called on a collection object in language versions where the collection
+type provides its own instance method for sorting.</p>
+<p>In Java, this specifically refers to calling <code>Collections.sort()</code> on a <code>List</code> object in Java 8 or later, where
+<code>List.sort()</code> should be used instead.</p>
+<h2>Why is this an issue?</h2>
+<p>In earlier versions of the language, sorting a collection required using a static utility method from a helper class. This approach followed the
+pattern of having utility functions in separate classes for operations on data structures.</p>
+<p>In later versions, the collection interface itself gained its own sorting method. This change embraced a more object-oriented design where the
+collection can be sorted directly on the instance.</p>
+<p>Using the static utility sorting function in modern code is outdated for several reasons:</p>
+<ul>
+  <li><strong>Readability</strong>: <code>myCollection.sort(comparator)</code> reads more naturally than <code>UtilityClass.sort(myCollection,
+  comparator)</code>. The object performing the action comes first, making the code flow clearer.</li>
+  <li><strong>API design</strong>: The newer approach aligns with modern design patterns that favor adding methods directly to interfaces, reducing
+  the need for separate utility classes.</li>
+  <li><strong>Consistency</strong>: Using the instance method makes the code consistent with other modern APIs in the language.</li>
+</ul>
+<p>While the static utility sorting function still works correctly and has the same behavior, using it signals that the code hasn’t been updated to
+take advantage of newer language features.</p>
+<p>In Java specifically, this refers to using <code>Collections.sort()</code> versus the <code>List.sort()</code> method that was introduced in Java
+8.</p>
+<h3>What is the potential impact?</h3>
+<p>Using a utility class method to sort a collection instead of the collection’s own sorting method has minimal practical impact on the application’s
+behavior, as both methods produce the same result. However, it affects code maintainability:</p>
+<ul>
+  <li>The code appears outdated, suggesting it may not be taking advantage of other modern language features.</li>
+  <li>New team members familiar with modern language practices may find the code less intuitive.</li>
+  <li>The codebase becomes inconsistent if some parts use the collection’s instance method while others use the utility class method.</li>
+</ul>
+<h2>How to fix it</h2>
+<p>Replace <code>Collections.sort(myList)</code> with <code>myList.sort(null)</code>. When sorting with the natural order, pass <code>null</code> as
+the comparator to the <code>sort()</code> method.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+Collections.sort(myList); // Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+myList.sort(null);
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>Java SE Documentation - <a href="https://docs.oracle.com/javase/8/docs/api/java/util/List.html#sort-java.util.Comparator-">List (Java Platform
+  SE 8)</a></li>
+</ul>
+

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9412.json

@@ -0,0 +1,25 @@
+{
+  "title": "\"Collections.sort()\" should not be used",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "java8",
+    "obsolete",
+    "convention"
+  ],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-9412",
+  "sqKey": "S9412",
+  "scope": "All",
+  "quickfix": "targeted",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "LOW"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}