SONARJAVA-4825 S6876: SequencedCollection reversed view should be use…

…d for reverse iteration order (#4717)

its/autoscan/src/test/resources/autoscan/diffs/diff_S6876.json

@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S6876",
+  "hasTruePositives": true,
+  "falseNegatives": 0,
+  "falsePositives": 0
+}

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

@@ -0,0 +1,89 @@
+package checks;
+
+import java.util.List;
+import java.util.ListIterator;
+
+public class ReversedMethodSequencedCollectionCheckSample {
+
+  void printLastToFirst(List<String> list, ListIterator<String> listIterator) {
+    for (var it = list.listIterator(list.size()); it.hasPrevious(); ) { // Noncompliant[[sc=5;ec=8]] {{Use the "reversed()" method
+      // instead of manually iterating the list in reverse.}}
+      var element = it.previous();
+    }
+
+    // for each statement
+    for (var element : list) { // Compliant
+
+    }
+
+    int size = list.size();
+    for (var it = list.listIterator(size); it.hasPrevious(); ) { // Compliant - size is unknown
+      var element = it.previous();
+    }
+
+    // for each statement
+    for (var element : list.reversed()) { // Compliant
+    }
+
+  }
+
+  void coverage(List<String> list, ListIterator<String> iterator) {
+    boolean isTrue = false;
+    for (var it = list.listIterator(list.size()); isTrue; ) { // Compliant - coverage
+      var element = it.previous();
+    }
+
+    for (var it = list.listIterator(list.size()); getBoolean(); ) { // Compliant - coverage
+      var element = it.previous();
+    }
+
+    for (; iterator.hasPrevious(); ) { // Compliant
+      var element = iterator.previous();
+    }
+
+    for (int a; iterator.hasPrevious(); ) { // Compliant - coverage
+      var element = iterator.previous();
+    }
+
+    int c = 0;
+    for (c++; iterator.hasPrevious(); ) { // Compliant - coverage
+      var element = iterator.previous();
+    }
+
+    for (int a = getInt(); iterator.hasPrevious(); ) { // Compliant - coverage
+      var element = iterator.previous();
+    }
+
+    for (int a = 1; iterator.hasPrevious(); ) { // Compliant - coverage
+      var element = iterator.previous();
+    }
+
+    for (var it = list.listIterator(getInt()); it.hasPrevious(); ) { // Compliant - coverage
+      var element = it.previous();
+    }
+
+    for (var it = list.listIterator(5); it.hasPrevious(); ) { // Compliant - coverage
+      var element = it.previous();
+    }
+
+    for (var it = list.listIterator(list.size()); it.hasPrevious(); ) { // Noncompliant
+    }
+
+    for (var it = list.listIterator(list.size()); it.hasPrevious(); ) { // Noncompliant - FP it.previous() is called more than once
+      it.previous();
+      it.previous();
+    }
+
+    for (var it = list.listIterator(list.size()); ; ) { // Compliant
+      var element = it.previous();
+    }
+  }
+
+  boolean getBoolean() {
+    return false;
+  }
+
+  int getInt() {
+    return 0;
+  }
+}

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

@@ -0,0 +1,99 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.java.checks;
+
+import java.util.List;
+import org.sonar.check.Rule;
+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.ForStatementTree;
+import org.sonar.plugins.java.api.tree.ListTree;
+import org.sonar.plugins.java.api.tree.MethodInvocationTree;
+import org.sonar.plugins.java.api.tree.StatementTree;
+import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.VariableTree;
+
+@Rule(key = "S6876")
+public class ReversedMethodSequencedCollectionCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
+
+  private static final String ISSUE_MESSAGE = "Use the \"reversed()\" method instead of manually iterating the list in reverse.";
+
+  private static final MethodMatchers LIST_ITERATOR_MATCHER = MethodMatchers.create()
+    .ofTypes("java.util.List")
+    .names("listIterator")
+    .addParametersMatcher("int")
+    .build();
+
+  private static final MethodMatchers LIST_SIZE_MATCHER = MethodMatchers.create()
+    .ofTypes("java.util.List")
+    .names("size")
+    .addWithoutParametersMatcher()
+    .build();
+
+  private static final MethodMatchers LIST_HAS_PREVIOUS_MATCHER = MethodMatchers.create()
+    .ofTypes("java.util.ListIterator")
+    .names("hasPrevious")
+    .addWithoutParametersMatcher()
+    .build();
+
+  @Override
+  public boolean isCompatibleWithJavaVersion(JavaVersion version) {
+    return version.isJava21Compatible();
+  }
+
+  @Override
+  public List<Tree.Kind> nodesToVisit() {
+    return List.of(Tree.Kind.FOR_STATEMENT);
+  }
+
+  @Override
+  public void visitNode(Tree tree) {
+    var forStatement = (ForStatementTree) tree;
+
+    ListTree<StatementTree> initializerStatements = forStatement.initializer();
+    if (initializerStatements.size() != 1) {
+      return;
+    }
+    var initializer = initializerStatements.get(0);
+    var condition = forStatement.condition();
+    if (condition == null) {
+      return;
+    }
+
+    if (isInitializerListIteratorFromLast(initializer) && isConditionHasPrevious(condition)) {
+      reportIssue(forStatement.forKeyword(), ISSUE_MESSAGE);
+    }
+  }
+
+  private static boolean isInitializerListIteratorFromLast(Tree initializer) {
+    return initializer instanceof VariableTree variable
+      && variable.initializer() instanceof MethodInvocationTree variableInitializer
+      && LIST_ITERATOR_MATCHER.matches(variableInitializer)
+      && variableInitializer.arguments().get(0) instanceof MethodInvocationTree arg
+      && LIST_SIZE_MATCHER.matches(arg);
+  }
+
+  private static boolean isConditionHasPrevious(Tree condition) {
+    return condition instanceof MethodInvocationTree invocation && LIST_HAS_PREVIOUS_MATCHER.matches(invocation);
+  }
+
+}

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

@@ -0,0 +1,46 @@
+/*
+ * SonarQube Java
+ * Copyright (C) 2012-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * 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 GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.java.checks;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.java.checks.verifier.CheckVerifier;
+import org.sonar.java.checks.verifier.TestUtils;
+
+class ReversedMethodSequencedCollectionCheckTest {
+
+  @Test
+  void test() {
+    CheckVerifier.newVerifier()
+      .onFile(TestUtils.mainCodeSourcesPath("checks/ReversedMethodSequencedCollectionCheckSample.java"))
+      .withCheck(new ReversedMethodSequencedCollectionCheck())
+      .withJavaVersion(21)
+      .verifyIssues();
+  }
+
+  @Test
+  void test_before_21() {
+    CheckVerifier.newVerifier()
+      .onFile(TestUtils.mainCodeSourcesPath("checks/ReversedMethodSequencedCollectionCheckSample.java"))
+      .withCheck(new ReversedMethodSequencedCollectionCheck())
+      .withJavaVersion(20)
+      .verifyNoIssues();
+  }
+
+}

sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckList.java

@@ -332,6 +332,7 @@
 import org.sonar.java.checks.ReturnOfBooleanExpressionsCheck;
 import org.sonar.java.checks.ReuseRandomCheck;
 import org.sonar.java.checks.ReverseSequencedCollectionCheck;
+import org.sonar.java.checks.ReversedMethodSequencedCollectionCheck;
 import org.sonar.java.checks.RightCurlyBraceDifferentLineAsNextBlockCheck;
 import org.sonar.java.checks.RightCurlyBraceSameLineAsNextBlockCheck;
 import org.sonar.java.checks.RightCurlyBraceStartLineCheck;
@@ -1052,6 +1053,7 @@ public final class CheckList {
     ReturnOfBooleanExpressionsCheck.class,
     ReuseRandomCheck.class,
     ReverseSequencedCollectionCheck.class,
+    ReversedMethodSequencedCollectionCheck.class,
     RightCurlyBraceDifferentLineAsNextBlockCheck.class,
     RightCurlyBraceSameLineAsNextBlockCheck.class,
     RightCurlyBraceStartLineCheck.class,

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

@@ -0,0 +1,36 @@
+<h2>Why is this an issue?</h2>
+<p>Java 21 introduces the new Sequenced Collections API, which is applicable to all collections with a defined sequence on their elements, such as
+<code>LinkedList</code>, <code>TreeSet</code>, and others (see <a href="https://openjdk.org/jeps/431">JEP 431</a>). For projects using Java 21 and
+onwards, this API should be utilized instead of workaround implementations that were necessary before Java 21.</p>
+<p>This rule reports when a collection is iterated in reverse through explicit implementation or workarounds, instead of using the reversed view of
+the collection.</p>
+<h2>How to fix it</h2>
+<p>Replace the reported statement with a forward-iteration over the reversed view of the collection.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+void printLastToFirst(List&lt;String&gt; list) {
+  for (var it = list.listIterator(list.size()); it.hasPrevious();) {
+    var element = it.previous();
+    System.out.println(element);
+  }
+}
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+void printLastToFirst(List&lt;String&gt; list) {
+  for (var element: list.reversed()) {
+    System.out.println(element);
+  }
+}
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Java Documentation - <a href="https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/SequencedCollection.html">Interface
+  SequencedCollection</a> </li>
+  <li> OpenJDK - <a href="https://openjdk.org/jeps/431">JEP 431: Sequenced Collections</a> </li>
+  <li> Java Documentation - <a
+  href="https://docs.oracle.com/en/java/javase/21/core/creating-sequenced-collections-sets-and-maps.html#GUID-DCFE1D88-A0F5-47DE-A816-AEDA50B97523">Creating Sequenced Collections, Sets, and Maps</a> </li>
+</ul>
+

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

@@ -0,0 +1,23 @@
+{
+  "title": "Reverse iteration should utilize reversed view",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "java21"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6876",
+  "sqKey": "S6876",
+  "scope": "All",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "HIGH"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

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

@@ -499,6 +499,7 @@
     "S6857",
     "S6862",
     "S6863",
+    "S6876",
     "S6877",
     "S6878",
     "S6885",