EI_EXPOSE_REP false positive

[oliveryasuna]

The below should not report EI_EXPOSE_REP, but does.

public enum UIState implements {

  INITIALIZING(Collections.emptySet()),

  RUNNING(Collections.singleton(INITIALIZING)),

  TERMINATED(Collections.singleton(RUNNING));

  UIState(final Set<UIState> transitions) {
    this.transitions = Collections.unmodifiableSet(transitions);
  }

  private final Set<UIState> transitions;

  public final Set<UIState> transitions() {
    return transitions;
  }

}

Changing to the following, it is not reported.

public enum UIState implements {

  INITIALIZING(Collections.emptySet()),

  RUNNING(Collections.singleton(INITIALIZING)),

  TERMINATED(Collections.singleton(RUNNING));

  UIState(final Set<UIState> transitions) {
    this.transitions = transitions;
  }

  private final Set<UIState> transitions;

  public final Set<UIState> transitions() {
    return Collections.unmodifiableSet(transitions);
  }

}

[welcome]

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.

[baloghadamsoftware]

Hello! You are right, this is indeed a false positive. The reason for the report is that the FindReturnRef detector checks the methods one by one: if it finds a return statement which returns a private variable whose static type is (probably) mutable then it reports the bug. What the detector is missing is that it should check each assignment to the attribute and not report the bug if all the assignments are immutable such as Collections.unmodifiable*() which means that its dynamic type is always immutable. However, this requires some fundamental changes in the detector. I will think a bit about the best possible solution.

[osiegmar]

Similar to #1771

[nmatt]

I'd like to add that this makes EI_EXPOSE_REP close to useless, since having e.g. a private field List that was constructed as unmodifiable in the constructor, is the standard way to prevent exposing mutable collection-valued fields.