java:S6204 Code in Constructor is not considered a modification

  • SonarLint 7.4.0.60471
  • IntelliJ Build #IU-223.8617.56, built on January 26, 2023
  • openjdk version OpenJDK 64-Bit Server VM Zulu17.40+19-CA (build 17.0.6+10-LTS, mixed mode, sharing)

When collecting a Stream with Collectors.toList() in Java 16+, S6204 suggests using the new method Stream.toList() to collect to an unmodifiable List. Code in constructors seems to be not considered a modification and gives a false-positive:

import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class MyClass {
   private MyClass(List<String> list) {
      list.sort(Comparator.naturalOrder()); //mutation here
   }

   public static void factory() {
      // Replace this usage of 'Stream.collect(Collectors.toList())' with 'Stream.toList()'
      new MyClass(Stream.of("whatever").collect(Collectors.toList()));
   }
}

Hi,

Welcome to the community!

The latest version is 8.0.0.63273. Could you upgrade and see if this is still replicable?

 
Thx,
Ann

Retested with 8.0.0.63273. Still the same issue.

1 Like

Hi,

Thanks for checking. I’ve flagged this for team attention.

 
Ann

Hi again,

Actually, it seems that we’ve already created a ticket for this:

Hopefully it’ll be fixed soon.

 
:smiley:
Ann

This ticket has now been closed, and it is not clear why when looking at the PR in question

Hi,

The ticket is Canceled / Won’t Fix with a comment of:

We came to the conclusion not to fix this FP, mainly because it would also eliminate most TPs compared to only 0.2% FPs at the moment.

 
HTH,
Ann

Hello,

Rather late to the game but as a side note it is risky to assume that the Collectors.toList() will return a mutable List. From the javadoc :
" There are no guarantees on the type, mutability, serializability, or thread-safety of the List returned"

So even though this may be a FP as the code currently works it may not be a best practice.

When we want to be sure that we actually want a mutable list we use :

collect(Collectors.toCollection(ArrayList::new));

When using that implementation it is not highlighted by rule S6204.

Hope this helps.

Kind regards

1 Like

Hi Ann, and thanks for the quick reply.

To be honest, this reply is disappointing. In my workplace, due to working in developing Cloud infrastructure, and due to issues related with the multi-team way-of-work, I can guarantee you that 100% of our FPs (False Positives) are not being flagged by our Sonarqube admin.

It’s also common that FPs will be suppressed after discussion with the team.

Given the non-trivial difference in difficulty of reporting FPs, I have to assume that your teams are aware that any % of FPs are certainly much more common in reality.

Also, is “implementation complexity” a good reason to justify the abandonment of a bugfix?

With respect,
Alex

Hi @PolyAlex,

Thanks for the honest feedback – appreciate it! You’re right, we are aware that S6204 can be problematic.

S6204 is part of a group of 20 rules, we want to improve this year. We will take your feedback into account when working on it.

Thanks again for flagging this!

2 Likes