FP on squid:S1948: ImmutableSet in Set field is labelled "not serializable"

java

(Jens Bannmann) #1

S1948 raised an issue with the ruleViolations field in the following class:

public class RuleViolationException extends RuntimeException
{
    private final Set<RuleViolation> ruleViolations; // S1948: Make "ruleViolations" transient or serializable.

    public RuleViolationException(String message, @NonNull Set<RuleViolation> ruleViolations)
    {
        super(message);
        this.ruleViolations = ImmutableSet.copyOf(ruleViolations);
    }

    public RuleViolationException(Set<RuleViolation> ruleViolations)
    {
        this(null, ruleViolations);
    }

    public Set<RuleViolation> getRuleViolations()
    {
        return ruleViolations; // S2384: Return a copy of "ruleViolations"
    }
}

Guava’s ImmutableSet implements Serializable, so I think this is a false positive. Furthermore, according to the definition in SONARJAVA-1225, S1948 should not raise an issue for final fields where the only assignments are Serializable classes, which is the case here (unless I’m overlooking something).

Here’s the link to the SonarCloud issue [might go away, see below], and here’s the original source code at that version.

Note that we will likely change the field type from Set to ImmutableSet to avoid getting an issue from S2384 in getRuleViolations(). That rule only seems to look at the field type, not the assignments. Maybe it should be adapted as well?


(Tibor Blenessy) #2

Hello,

can you share source for RuleViolation? Is it serializable?


(Jens Bannmann) #3

Oh right, it isn’t. Here’s the source code. Sorry, I should have checked this beforehand. So S1948 is likely not reporting a false positive after all.

So what do you think about my point with S2384?


(Jens Bannmann) #4

For reference, here is the corresponding issue.