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

New false positive for MS_EXPOSE_REP since 4.3.0 #1747

Closed
uhafner opened this issue Oct 8, 2021 · 4 comments
Closed

New false positive for MS_EXPOSE_REP since 4.3.0 #1747

uhafner opened this issue Oct 8, 2021 · 4 comments

Comments

@uhafner
Copy link

uhafner commented Oct 8, 2021

Since release 4.3.0 I get a false positive for MS_EXPOSE_REP. SpotBugs reports a variable that is created with Collections.createImmutableSet() as problematic.

Affected Code:
https://github.com/jenkinsci/analysis-model/blob/master/src/main/java/edu/hm/hafner/analysis/Severity.java#L40

SpotBugs Warning: Public static edu.hm.hafner.analysis.Severity.getPredefinedValues() may expose internal representation by returning Severity.ALL_SEVERITIES

Details in our CI: https://ci.jenkins.io/job/Plugins/job/analysis-model/view/change-requests/job/PR-687/3/spotbugs/type.-1489577541/source.610ba3d7-4084-4194-a362-f73a2a8a575d/#142

@uhafner uhafner changed the title New false positive for MS_EXPOSE_REP in 4.3.0 New false positive for MS_EXPOSE_REP since 4.3.0 Oct 8, 2021
@jbindel
Copy link

jbindel commented Apr 6, 2022

A simple test case of a false positive is small:

import java.util.Arrays;
import java.util.Collections;
import java.util.List;

public class Foo
{
	private static final List<String> ulist = Collections.unmodifiableList(Arrays.asList("one", "two"));

	public static List<String> getUlist() { return ulist; }
}

SpotBugs starting with release 4.3.0 generates this warning:

M V MS: Public static Foo.getUlist() may expose internal representation by returning ulist At Foo.java:[line 9]

The current detector, FindReturnRef seems to look at the returned List<String> type and pass it to MutableClasses, which does not have enough context about the actual type of the final class variable, which is immutable.

@Apache9
Copy link

Apache9 commented Sep 15, 2022

When upgrading from 4.2.2 to 4.7.2, we also faced the same problem, lots of getInstance method for a singleton classes are reported as MS_EXPOSE_REP.

@gtoison
Copy link
Contributor

gtoison commented Sep 6, 2023

@uhafner and @jbindel this was fixed by @baloghadamsoftware in #2141

@Apache9 There's an outstanding issue (#1797) that the heuristic to detect mutable classes looks for some prefixes in method names that look like setters, for instance "set" but also "push", "write", etc.

@gtoison gtoison closed this as completed Sep 6, 2023
@Apache9
Copy link

Apache9 commented Sep 6, 2023

Thanks for the reminding. Will take a look at #1797

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

4 participants