EI_EXPOSE_REP2 being reported based off purely on method name

[pnsantos]

Not really sure if this is a bug or a feature, but it seems spotbugs (4.3+) is reporting EI_EXPOSE_REP2 purely based off the method name. For example I have the following:

public class SomeClass {

  public CompletableFuture<Void> delete(String id) {
    // delete something in the db
  }

}

and then

public class AnotherClass {

  private final SomeClass repo;

  public AnotherClass(SomeClass repo) {
    this.repo = repo;
  }

}

• Spotbugs 4.2 reports no violations
• Spotbugs 4.3+ reports EI_EXPOSE_REP2 (tested all the way to 4.4.2)

However if I rename delete to something else (say deleete) then it no longer reports EI_EXPOSE_REP2 which makes me think it's just looking at the method name?

Kinda makes this Bug useless... All our projects are suddenly reporting hundreds of EI_EXPOSE_REP2

Is this expected behaviour?

[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.

[pensionarchitects-ben]

We also have a problem with this, we upgraded to spotbugs 4.5.3 (from 4.0.0) and suddenly hundreds of errors while most of them are false positives. we disabled the warning for now. Subscribed to hear about progress on this.

[brunomuniz-encora]

Same here. Lots of reports of EI_EXPOSE_REP2 with the new versions of spotbugs, some might be false positives. Also subscribing to hear about progress on this.

[rratliff]

I found a workaround. Using the original poster's example:

public class AnotherClass {

  private final SomeClass repo;

  public AnotherClass(SomeClass repo) {
    this.repo = Objects.requireNonNull(repo, "repo");
  }

}

It seems wrapping it in Objects.requireNonNull() is sufficient to thwart the detection of this "bug."

[gtoison]

MutableClasses.looksLikeASetter is used to detect a mutable class, it checks if a method name starts with one of the suspicious prefixes and if the return type is different compared to the parameter type:

spotbugs/spotbugs/src/main/java/edu/umd/cs/findbugs/util/MutableClasses.java

Lines 103 to 108 in 2fdbb01

	public static boolean looksLikeASetter(String methodName, String classSig, String retSig) {
	if (Objects.equals(classSig, retSig)) {
	return false;
	}
	return SETTER_LIKE_PREFIXES.stream().anyMatch(name -> methodName.startsWith(name));

spotbugs/spotbugs/src/main/java/edu/umd/cs/findbugs/util/MutableClasses.java

Lines 62 to 64 in 2fdbb01

	private static final List<String> SETTER_LIKE_PREFIXES = Arrays.asList(
	"set", "put", "add", "insert", "delete", "remove", "erase", "clear", "push", "pop",
	"enqueue", "dequeue", "write", "append", "replace");

I think the heuristic is a bit too aggressive here: considering the interest in this issue, there are many false positives detected.
@baloghadamsoftware what do you think?

[hazendaz]

fixed via #2514

[gtoison]

Hello @hazendaz, the issue is not fixed unfortunately: #2514 mitigates the problem for enums and records, but other classes might be recognized as mutable solely because they have a method whose name starts with one of the suspcious prefixes:

spotbugs/spotbugs/src/main/java/edu/umd/cs/findbugs/util/MutableClasses.java

Lines 62 to 64 in 2fdbb01

	private static final List<String> SETTER_LIKE_PREFIXES = Arrays.asList(
	"set", "put", "add", "insert", "delete", "remove", "erase", "clear", "push", "pop",
	"enqueue", "dequeue", "write", "append", "replace");

One way to improve would be to let the users supply their own "setter like prefixes"?