S1948 should allow public final collection

Clean Code Report False-positive / False-negative...
, ,
1
  • What language is this for?
    Java
  • Which rule?
    S1948
  • Why do you believe it’s a false-positive/false-negative?
    public final List<String> list should have the same behavior has private List<String> list with getter/setter.
    I know that this rule is only valid for private collection today, by I don’t understand why it’s not valid for public final one.
  • Are you using
    • Stand alone SonarLint 10.2.1.77304 - Intellij Ultimate
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
    S1948 report an issue here
public class Test implements Serializable {
    public final List<String> list; // warning here

    public Test(List<String> list) {
        this.list = list;
    }
}
4

Hey @Paul_Noferi,

Thank you for reporting the issue. I can reproduce the false positive on my side, but it looks like the problem has to do with the type of the field being a collection and being a public final field.

In the following example, we only (mistakenly) raise on the second field but not on the first.

class CommunityPost implements Serializable {
  private static final long serialVersionUID = 1L;

  public final String username;
  public final List<String> messages; // False positive

  CommunityPost(String username, List<String> messages) {
    this.username = username;
    this.messages = messages;
  }
}

Assuming there is no attempt to modify the field’s value through reflection outside of a deserialization process, S1948 should not raise an issue on the snippet you shared.

I created a ticket to track the issue.

Best,

Dorian

8

Hi Doran and thank’s for your answer
I saw the answer from jbeleites on the ticket you opened, and I’m wondering:

If public final collection are not safe enough due to type erasure, then why no warning is raised in the following case:

class CommunityPost implements Serializable {
  private static final long serialVersionUID = 1L;

  public final String username;
  private final List<String> messages;

  CommunityPost(String username, List<String> messages) {
    this.username = username;
    this.messages = messages;
  }

  public List<String> getMessages() {
    return messages;
  }
}

I can still access to the messages list and insert non Serializable object inside.

class NonSerializable {
}

void demo() {
  CommunityPost post = new CommunityPost("user", new ArrayList<>());
  List list = post.getMessages();
  list.add(new NonSerializable())
}

Did I miss something ? I’m wondering what is the best practice in this case as the only way to make this list type safe ?