java:S6204 should not raise an issue when addFirst/-Last, removeFirst/-Last is called on the list

[Jmanom]

Rule S6204 intent is to use unmutable lists instead of mutable lists whenever possible. Specifically, the rule correctly suggests replacing stream.collect(Collectors.toList()); with stream.toList(); when there is no modification detected on the returned list, and thus, it is incorrect to define it as a mutable list.

To this end the rule recognizes several methods on List that constitute a modification and allows like e.g. add, set and others

Beginning with Java 21 List implements SequencedCollection and 4 additional methods modify the list and the rule must not suggest the use of Stream::toList().

i.e. List::addFirst, List::addLast, List::removeFirst, List::removeLast

Suggested implementation, see PR:

SONARJAVA java:S6204 should not raise an issue when addFirst/addLast/removeFirst/removeLast is called on the list

___________________

Discussion:

This is similar to the already solved case for removeIf:

java:S6204 List.removeIf(Predicate) is not considered a modification

SonarLint for Eclipse 7.1.0.39158 Eclipse 2021-09 (4.21.0), build id: 20210910-1417 openjdk version “16.0.2” 2021-07-20, OpenJDK 64-Bit Server VM Temurin-16.0.2+7 (build 16.0.2+7, mixed mode, sharing) When collecting a Stream with Collectors.toList() in Java 16+, S6204 suggests to use the new method Stream.toList() to collect to an unmodifyable List, apart from cases where the List is modified afterwards. List.removeIf(Predicate) seems to be not considered a modification and gives a false-posi…

Jira

Several other open issues discuss this rule java:S6204 but not specifically the use of these methods:

[java:S6204] Improve suggestions

Qube: Community 9.9 The Java rule S6204 suggests to replace the use of new ArrayList<>().stream().collect( Collectors.toList() ); with new ArrayList<>().stream().toList(); More precisely it suggests “… when unmodifiable list needed”. In the inline hint from Lint it just says “Replace this usage of ‘Stream.collect(Collectors.toList())’ with ‘Stream.toList()’”. This leads you in a wrong direction. Hence the text should be improved. First of all the Java API is a bit odd here. Stream.toList(…

java:S6204 suggestion contradicts javadoc -- modifiable list

The rule java:S6204 suggests .collect(Collectors.toList()) to create a modifiable list, but the javadoc for Collectors.toList states that it does not make any guarantees about the returned list being modifiable. Link to documentation: https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Collectors.html#toList() It feels like an API design mistake. If I want a modifiable list, I have to use toCollection and specify the concrete list I want, it seems.

[ganncamp (G Ann Campbell)]

Hi,

Can you give your context for this false-negative? I.e. are you on SonarQube Cloud? SonarQube for IDE (flavor and version)? SonarQube self-managed (flavor and version)?

 
Thx,
Ann

[Jmanom]

Hi,

I have seen it on our clients self hosted SonarQube Server, Community Build v25.12.0.11709.
Is that information useful?

As you might have noticed, I have already sent a PR including tests for this (SONARJAVA java:S6204 should not raise an issue when addFirst/addLast/removeFirst/removeLast is called on the list by Jmanom · Pull Request #5382 · SonarSource/sonar-java · GitHub).
Maybe that clarifies more than my overly wordy description.

[ganncamp (G Ann Campbell)]

Hi,

Thanks for the context. Yes, it’s very helpful!

I’ll flag this for the language experts.

 
Ann