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

[romainbrenguier]

Summary

• Detect calls to Collections.sort() and recommend using List.sort() instead (available since Java 8)
• Extends AbstractMethodDetection with MethodMatchers for both 1-arg and 2-arg overloads
• Provides a quick fix to transform Collections.sort(list) to list.sort(null) and Collections.sort(list, comparator) to list.sort(comparator)

Test plan

• Unit tests pass with semantic analysis (verifyIssues())
• Unit tests pass without semantic analysis (verifyIssues())
• Ruling tests updated (auto-generated PR if needed)

🤖 Generated with Claude Code

Agent workflow

PR created using uv run new_rule_implementation.py S9412 -j SONARJAVA-7043 -a claude -g AGENTS.md .claude/skills/new-rule/SKILL.md
Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier

Iterated on the PR with uv run ci_loop.py for 2 iterations.
✔️ The PR is now ready for review.

Addressed review comments in pr_report_6221.md using uv run address_reviews.py pr_report_6221.md

[hashicorp-vault-sonar-prod]

SONARJAVA-7043

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6222

Please review and merge it into your branch.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6222

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

Detected changes in 3 rule files: 0 issues removed, 15 issues added.

S9412 (java) on eclipse-jetty - 0 issues removed, 4 issues added - new ruling file

Added jetty-server/src/test/java/org/eclipse/jetty/server/ProxyCustomizerTest.java (line 97)

(source file not found at this revision: jetty-server/src/test/java/org/eclipse/jetty/server/ProxyCustomizerTest.java)

Added jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java (line 792)

(source file not found at this revision: jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java (line 840)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/PathWatcher.java)

Added jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java (line 693)

(source file not found at this revision: jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java)

S9412 (java) on guava - 0 issues removed, 4 issues added - new ruling file

Added src/com/google/common/collect/ImmutableMultimap.java (line 280)

       275 |      */
       276 |     public ImmutableMultimap<K, V> build() {
       277 |       if (valueComparator != null) {
       278 |         for (Collection<V> values : builderMultimap.asMap().values()) {
       279 |           List<V> list = (List<V>) values;
>>>    280 |           Collections.sort(list, valueComparator);
       281 |         }
       282 |       }
       283 |       if (keyComparator != null) {
       284 |         Multimap<K, V> sortedCopy = new BuilderMultimap<K, V>();
       285 |         List<Map.Entry<K, Collection<V>>> entries =

Added src/com/google/common/collect/Ordering.java (line 698)

       693 |     if (k == 0 || !elements.hasNext()) {
       694 |       return ImmutableList.of();
       695 |     } else if (k >= Integer.MAX_VALUE / 2) {
       696 |       // k is really large; just do a straightforward sorted-copy-and-sublist
       697 |       ArrayList<E> list = Lists.newArrayList(elements);
>>>    698 |       Collections.sort(list, this);
       699 |       if (list.size() > k) {
       700 |         list.subList(k, list.size()).clear();
       701 |       }
       702 |       list.trimToSize();
       703 |       return Collections.unmodifiableList(list);

Added src/com/google/common/collect/RegularImmutableTable.java (line 125)

       120 |           }
       121 |           return (columnComparator == null) ? 0
       122 |               : columnComparator.compare(cell1.getColumnKey(), cell2.getColumnKey());
       123 |         }
       124 |       };
>>>    125 |       Collections.sort(cells, comparator);
       126 |     }
       127 |     return forCellsInternal(cells, rowComparator, columnComparator);
       128 |   }
       129 | 
       130 |   static <R, C, V> RegularImmutableTable<R, C, V> forCells(

Added src/com/google/common/util/concurrent/ServiceManager.java (line 597)

       592 |           }
       593 |         }
       594 |       } finally {
       595 |         monitor.leave();
       596 |       }
>>>    597 |       Collections.sort(loadTimes, Ordering.natural()
       598 |           .onResultOf(new Function<Entry<Service, Long>, Long>() {
       599 |             @Override public Long apply(Map.Entry<Service, Long> input) {
       600 |               return input.getValue();
       601 |             }
       602 |           }));

S9412 (java) on sonar-server - 0 issues removed, 7 issues added - new ruling file

Added src/main/java/org/sonar/server/computation/task/projectanalysis/source/DuplicationLineReader.java (line 69)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/source/DuplicationLineReader.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/source/SymbolsLineReader.java (line 58)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/source/SymbolsLineReader.java)

Added src/main/java/org/sonar/server/computation/task/projectanalysis/source/SymbolsLineReader.java (line 83)

(source file not found at this revision: src/main/java/org/sonar/server/computation/task/projectanalysis/source/SymbolsLineReader.java)

Added src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java (line 75)

(source file not found at this revision: src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java)

Added src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java (line 78)

(source file not found at this revision: src/main/java/org/sonar/server/duplication/ws/DuplicationsParser.java)

Added src/main/java/org/sonar/server/es/IndexDefinitionHash.java (line 79)

(source file not found at this revision: src/main/java/org/sonar/server/es/IndexDefinitionHash.java)

Added src/test/java/org/sonar/server/computation/task/projectanalysis/duplication/TextBlockTest.java (line 92)

(source file not found at this revision: src/test/java/org/sonar/server/computation/task/projectanalysis/duplication/TextBlockTest.java)

[nathsou]

LGTM, two non-blocking remarks on the quick fix.

[nathsou]

The quick fix title embeds the full replacement text. With an inline multi-line lambda or anonymous Comparator (Collections.sort(list, (a, b) -> { ... })), the whole block ends up in the IDE quick fix menu entry. A fixed title such as Replace with "List.sort()" would avoid that.

[romainbrenguier]

Addressed in 9ae90fc: the quick fix title is now the fixed Replace with "List.sort()".

[nathsou]

Collections.sort(List<T>) enforces T extends Comparable<? super T> at compile time, while list.sort(null) does not: if the element type later stops being Comparable, a compile error becomes a runtime ClassCastException. list.sort(Comparator.naturalOrder()) keeps the type check (at the cost of an import in the quick fix). This follows the RSPEC's "How to fix it" section, so it may be better settled on SonarSource/rspec#8233.

[romainbrenguier]

Kept list.sort(null) for now: it matches the RSPEC's "How to fix it" section and avoids adding an import in the quick fix. I agree the compile-time check is worth keeping, so I'll raise it on SonarSource/rspec#8233. If the RSPEC moves to Comparator.naturalOrder(), we'll update the quick fix to match.

[gitar-bot]

Code Review ✅ Approved 3 closed / 3 findings

🟡 Medium risk · Adds a Java rule and automated quick fix for collection sorting calls.

Implements rule S9412 to detect Collections.sort() calls and recommend List.sort() instead, with quick fixes to transform the code. Addresses Java 8 version gating, quick fix parenthesization, and test coverage for the quick fix transformation.

✅ 3 closed

✅ Bug: Rule isn't limited to Java 8+, though its description says it is

📄 java-checks/src/main/java/org/sonar/java/checks/CollectionsSortCheck.java:29-30 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9412.html:1-4 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9412.json:9-13
The rule description says the issue only applies "in Java 8 or later", and the rule is tagged java8. But CollectionsSortCheck doesn't implement JavaVersionAwareVisitor. On a project compiled for Java 7 or earlier it will still flag every Collections.sort call, and its quick fix will suggest list.sort(...), which doesn't exist in those versions. Other checks tagged java8 already handle this: about 20 in this module (e.g. DiamondOperatorCheck, ReplaceGuavaWithJavaCheck, LambdaSingleExpressionCheck) implement JavaVersionAwareVisitor with isJava8Compatible(). Add the same gate here, and add a test with .withJavaVersion(7) + verifyNoIssues().

✅ Bug: Quick fix produces wrong code when the list argument needs parentheses

📄 java-checks/src/main/java/org/sonar/java/checks/CollectionsSortCheck.java:57-68
The quick fix builds its replacement as listText + ".sort(...)", where listText is the list argument's source text copied as-is. That breaks when the argument has lower precedence than member access. Collections.sort(flag ? a : b) becomes flag ? a : b.sort(null), which only sorts b and usually doesn't compile. Collections.sort((List<String>) obj) becomes (List<String>) obj.sort(null), which casts the void result instead of the list. Assignments and lambdas break the same way. Wrap the list text in parentheses unless it is a simple expression (identifier, member select, method call, array access, parenthesized expression, new ...). QuickFixHelper already has a similar helper, addParenthesisIfRequired.

✅ Quality: Sample file never checks the quick fix

📄 java-checks-test-sources/default/src/main/java/checks/CollectionsSortCheckSample.java:13-18 📄 java-checks/src/main/java/org/sonar/java/checks/CollectionsSortCheck.java:57-69
The rule adds a quick fix, and S9412.json declares "quickfix": "targeted". But CollectionsSortCheckSample.java has no [[quickfixes=...]], // fix@ or // edit@ annotations, so verifyIssues() never checks the edits. Neither the one-argument path (.sort(null)) nor the two-argument path is tested, which is how the precedence bug above went unnoticed. Add quick-fix expectations for both overloads, plus a case with a complex list expression.

Review coverage

🧪 Functional validation 2 of 4 objectives covered

📋 Rules No rules evaluated

🤖 Auto-approval Not enabled · Set up

Implementation Status ◻️ 2 of 4 objectives covered

◻️ SONARJAVA-7043 - 2 of 4 objectives covered

This PR implements rule S9412 to detect "Collections.sort()" calls and provide a quick fix, while excluding support for import removal and custom @SuppressWarnings handling.

Other objectives on this issue, possibly covered elsewhere:

• ◻️ Respect @SuppressWarnings where applicable
• ◻️ Remove the Collections import when it is no longer used

✅ 2 covered here

• ✅ Detect calls to Collections.sort() that can be replaced with list.sort()
• ✅ Provide a quick fix named "Replace with List.sort()" for Collections.sort() calls

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube