Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefix matching behavior of SuppressFBWarnings makes it impossible to suppress just one warning #2938

Open
tayloj opened this issue Apr 10, 2024 · 2 comments

Comments

@tayloj
Copy link

tayloj commented Apr 10, 2024

Summary

The @SuppressFBWarnings(value = "ABCDE", justification = "...") suppresses warnings for all rules whose names begin with ABCDE, not just the rule ABCDE. This is a problem because because there are rules whose names are proper prefixes of other rules' names, making it impossible to suppress just the one rule without suppressing the other.

In the example below, it's impossible to suppress EI_EXPOSE_REP without also suppressing EI_EXPOSE_REP2.

Javadoc on the SuppressFBWarnings annotation suggests that value is a pattern (which may explain the prefix matching behavior) but experimentation shows that line anchors like ^ and $ aren't handled, so this can't be handled by adding $ at the end.

Details and Reproducing

Using the maven spotbugs plugin (version 4.8.2), I get two errors when analyzing a simple POJO with a mutable List field (and this is expected):

[ERROR] Medium: org.example.ListWrapper.getNames() may expose internal representation by returning ListWrapper.names [org.example.ListWrapper] At ListWrapper.java:[line 11] EI_EXPOSE_REP
[ERROR] Medium: org.example.ListWrapper.setNames(List) may expose internal representation by storing an externally mutable object into ListWrapper.names [org.example.ListWrapper] At ListWrapper.java:[line 13] EI_EXPOSE_REP2
package org.example;

import java.util.List;

public class ListWrapper {

  private List<String> names;

  public List<String> getNames() { return names; }

  public void setNames(final List<String> names) { this.names = names; }
}

This is expected, but I'm OK with it, so I'd like to use @SuppressFBWarnings. Note that the warning for the getter is EI_EXPOSE_REP, and for the setter is EI_EXPOSE_REP2. That is, the annotations should be:

  @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "It's OK")
  public List<String> getNames() { return names; }

  @SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "It's OK")
  public void setNames(final List<String> names) { this.names = names; }

This makes the errors disappear. However, by a copy and paste mistake, I initially used EI_EXPOSE_REP in both annotations, and still both errors disappear. A bit more testing shows that that if the value is any prefix of the rule name, the errors are suppressed. For instance:

  @SuppressFBWarnings(value = "EI_EX", justification = "It's OK")
  public List<String> getNames() { return names; }

  @SuppressFBWarnings(value = "EI_EXPO", justification = "It's OK")
  public void setNames(final List<String> names) { this.names = names; }

That's (sort of) consistent with the documentation on the SuppressFBWarnings value field, which says that value can be a pattern:

    /**
     * The set of FindBugs warnings that are to be suppressed in
     * annotated element. The value can be a bug category, kind or pattern.
     */
    String[] value() default {};

However, given that some rule names, like EI_EXPOSE_REP are proper prefixes of other rule names, like EI_EXPOSE_REP2, it means that it's impossible to suppress warnings for just EI_EXPOSE_REP without also suppressing for EI_EXPOSE_REP2. I tried experimenting with the "pattern" aspect of it (e.g., checking whether I could use an end-of-line anchor with EI_EXPOSE_REP$), but that didn't work either.

I'm using spotbugs and spotbugs-annotations 4.8.2 on Java 17.0.6.

Any thoughts or workarounds?

Copy link

welcome bot commented Apr 10, 2024

Thanks for opening your first issue here! 😃
Please check our contributing guideline. Especially when you report a problem, make sure you share a Minimal, Complete, and Verifiable example to reproduce it in this issue.

@iloveeclipse
Copy link
Member

iloveeclipse commented Apr 10, 2024

Any thoughts or workarounds?

I guess you have to dig into the spotbugs source to fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants