diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S6876.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6876.json new file mode 100644 index 00000000000..63f773cda45 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S6876.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S6876", + "hasTruePositives": true, + "falseNegatives": 0, + "falsePositives": 0 +} diff --git a/java-checks-test-sources/default/src/main/java/checks/ReversedMethodSequencedCollectionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/ReversedMethodSequencedCollectionCheckSample.java new file mode 100644 index 00000000000..06af41dde9d --- /dev/null +++ b/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 list, ListIterator 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 list, ListIterator 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; + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/ReversedMethodSequencedCollectionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ReversedMethodSequencedCollectionCheck.java new file mode 100644 index 00000000000..24e444bdc21 --- /dev/null +++ b/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 nodesToVisit() { + return List.of(Tree.Kind.FOR_STATEMENT); + } + + @Override + public void visitNode(Tree tree) { + var forStatement = (ForStatementTree) tree; + + ListTree 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); + } + +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/ReversedMethodSequencedCollectionCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/ReversedMethodSequencedCollectionCheckTest.java new file mode 100644 index 00000000000..1743e39fcf2 --- /dev/null +++ b/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(); + } + +} diff --git a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckList.java b/sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckList.java index c052f99872d..1e8c6aeda48 100644 --- a/sonar-java-plugin/src/main/java/org/sonar/plugins/java/CheckList.java +++ b/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, diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6876.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6876.html new file mode 100644 index 00000000000..4cfb9623c69 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6876.html @@ -0,0 +1,36 @@ +

Why is this an issue?

+

Java 21 introduces the new Sequenced Collections API, which is applicable to all collections with a defined sequence on their elements, such as +LinkedList, TreeSet, and others (see JEP 431). For projects using Java 21 and +onwards, this API should be utilized instead of workaround implementations that were necessary before Java 21.

+

This rule reports when a collection is iterated in reverse through explicit implementation or workarounds, instead of using the reversed view of +the collection.

+

How to fix it

+

Replace the reported statement with a forward-iteration over the reversed view of the collection.

+

Code examples

+

Noncompliant code example

+
+void printLastToFirst(List<String> list) {
+  for (var it = list.listIterator(list.size()); it.hasPrevious();) {
+    var element = it.previous();
+    System.out.println(element);
+  }
+}
+
+

Compliant solution

+
+void printLastToFirst(List<String> list) {
+  for (var element: list.reversed()) {
+    System.out.println(element);
+  }
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6876.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S6876.json new file mode 100644 index 00000000000..e125fecd828 --- /dev/null +++ b/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" + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index 8e5f0407fbf..647fff71924 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/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",