-
Notifications
You must be signed in to change notification settings - Fork 629
Description
I'm seeing some different behavior in SpotBugs 4.9.2 after upgrading from 4.9.1 (via maven-spotbugs-plugin 4.9.2.0 and 4.9.2.0. I actually tried using 4.9.3.0, where I first saw the problem, and then tracked it down to the change from .1 to .2.) Here's a simple record that has EI_EXPOSE_REP and EI_EXPOSE_REP2 problems:
package example;
import java.util.List;
public record MyRecord(String name, List<String> values) {}
Behavior in 4.9.1
SpotBugs (via maven-spotbugs-plugin, 4.9.1.0) reports the EI_EXPOSE_REP, and misses the EI_EXPOSE_REP2:
Medium: example.MyRecord.values() may expose internal representation by returning MyRecord.values [example.MyRecord] At MyRecord.java:[line 5] EI_EXPOSE_REP
In 4.9.1, this can be resolved with by adding a @SuppressFBWarnings
to the values field:
public record MyRecord(String name, @SuppressFBWarnings("EI_EXPOSE_REP") List<String> values) {}
Behavior in 4.9.2
However, in 4.9.2 (via maven-spotbugs-plugin, 4.9.2.0), the original code results in two warnings (which I think is an improvement, since EI_EXPOSE_REP2 should apply here, too, since someRecord.values().add(...)
would change the internal representation):
[ERROR] Medium: example.MyRecord.values() may expose internal representation by returning MyRecord.values [example.MyRecord] At MyRecord.java:[line 5] EI_EXPOSE_REP
[ERROR] Medium: new example.MyRecord(String, List) may expose internal representation by storing an externally mutable object into MyRecord.values [example.MyRecord] At MyRecord.java:[line 5] EI_EXPOSE_REP2
Adding @SuppressFBWarnings({"EI_EXPOSE_REP","EI_EXPOSE_REP2"})
to the original code doesn't satisfy SpotBugs 4.9.2, though. Instead, the annotation is recognizes as being on the field, the accessor method, and the constructor parameter, but as useless in all three places.
[ERROR] Medium: Suppressing annotation on the field example.MyRecord.values is unnecessary [example.MyRecord] In MyRecord.java US_USELESS_SUPPRESSION_ON_FIELD
[ERROR] Medium: Suppressing annotation on the method example.MyRecord.values() is unnecessary [example.MyRecord] At MyRecord.java:[line 6] US_USELESS_SUPPRESSION_ON_METHOD
[ERROR] Medium: Suppressing annotation on the parameter 3 of the method new example.MyRecord(String, List) is unnecessary [example.MyRecord] At MyRecord.java:[line 6] US_USELESS_SUPPRESSION_ON_METHOD_PARAMETER
(Also note, in the last warning, that's it's reported as "parameter 3". I'd have expected either "parameter 2" or "parameter 1", depending on whether parameter indexing is 1-based or 0-based.)
I can handle the EI_EXPOSE_REP by manually defining the accessor method and annotating it, but that's obviously undesirable since part of the appeal of records is automatically-defined accessors:
@SuppressFBWarnings("EI_EXPOSE_REP")
public List<String> values() {
return values;
}
Alternatively, I can address REP and REP2 by copying within a constructor, which is actually what I'd typically want to do, but that doesn't allow for the case where I actually want internal representations exposed (even if it's risky):
public MyRecord {
values = List.copyOf(values);
}
Is there some way in 4.9.2 and onward to suppress the EI_EXPOSE_REP and EI_EXPOSE_REP2 warnings on the values
field?