diff --git a/CHANGELOG.md b/CHANGELOG.md index fefc5dabbf9..d3541dae77e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2. - Fix XML Output incorrectly escaped in Eclipse Bug Info view ([#2520](https://github.com/spotbugs/spotbugs/pull/2520)) - Updated the MS_EXPOSE_REP description to mention mutable objects, not just arrays ([#1669](https://github.com/spotbugs/spotbugs/issues/1669)) - Described Configuration option frc.suspicious for bug RC_REF_COMPARISON in bug description ([#2297](https://github.com/spotbugs/spotbugs/issues/2297)) +- Fixed FindHEMismatch not reporting HE_SIGNATURE_DECLARES_HASHING_OF_UNHASHABLE_CLASS for some classes ([#2402](https://github.com/spotbugs/spotbugs/issues/2402)) ### Added - New simple name-based AnnotationMatcher for exclude files (now bug annotations store the class java annotations in an attribute called `classAnnotationNames`). For example, use like in an excludeFilter.xml to ignore classes generated by the Immutable framework. This ignores all class, method or field bugs in classes with that annotation. diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue2402Test.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue2402Test.java new file mode 100644 index 00000000000..4ed22aaf4c9 --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/ba/Issue2402Test.java @@ -0,0 +1,32 @@ +package edu.umd.cs.findbugs.ba; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.hamcrest.MatcherAssert.assertThat; + +import org.junit.Test; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; +import edu.umd.cs.findbugs.SortedBugCollection; + +public class Issue2402Test extends AbstractIntegrationTest { + + @Test + public void test() { + performAnalysis("ghIssues/issue2402/TestA.class", + "ghIssues/issue2402/TestA$UMap.class", + "ghIssues/issue2402/TestB.class", + "ghIssues/issue2402/TestB$UMap.class", + "ghIssues/issue2402/TestC.class", + "ghIssues/issue2402/TestC$UMap.class", + "ghIssues/issue2402/TestD.class", + "ghIssues/issue2402/TestD$UMap.class"); + + BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder().bugType("HE_SIGNATURE_DECLARES_HASHING_OF_UNHASHABLE_CLASS").build(); + + SortedBugCollection bugCollection = (SortedBugCollection) getBugCollection(); + assertThat(bugCollection, containsExactly(4, bugTypeMatcher)); + } + +} diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindHEmismatch.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindHEmismatch.java index 0b08ca40729..9dcfeece2ef 100644 --- a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindHEmismatch.java +++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindHEmismatch.java @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -110,7 +111,7 @@ public class FindHEmismatch extends OpcodeStackDetector implements StatelessDete final HashSet nonHashableClasses; - final Map potentialBugs; + final Map potentialBugs; private final BugReporter bugReporter; @@ -563,11 +564,12 @@ private void check(int pos) { if (type.isAbstract() || type.isInterface()) { priority++; } - potentialBugs.put( - type.getClassName(), - new BugInstance(this, "HE_USE_OF_UNHASHABLE_CLASS", priority).addClassAndMethod(this) - .addTypeOfNamedClass(type.getClassName()).describe(TypeAnnotation.UNHASHABLE_ROLE).addCalledMethod(this) - .addSourceLine(this)); + + BugInstance bugInstance = new BugInstance(this, "HE_USE_OF_UNHASHABLE_CLASS", priority).addClassAndMethod(this) + .addTypeOfNamedClass(type.getClassName()).describe(TypeAnnotation.UNHASHABLE_ROLE).addCalledMethod(this) + .addSourceLine(this); + PotentialBugKey key = new PotentialBugKey(type.getClassName(), bugInstance); + potentialBugs.put(key, bugInstance); } @CheckForNull @@ -638,13 +640,14 @@ public void visit(Signature obj) { bug = new BugInstance(this, "HE_SIGNATURE_DECLARES_HASHING_OF_UNHASHABLE_CLASS", priority).addClass(this) .addClass(this).addTypeOfNamedClass(className).describe(TypeAnnotation.UNHASHABLE_ROLE); } - potentialBugs.put(className, bug); + PotentialBugKey key = new PotentialBugKey(className, bug); + potentialBugs.put(key, bug); } @Override public void report() { - for (Map.Entry e : potentialBugs.entrySet()) { - if (!isHashableClassName(e.getKey())) { + for (Map.Entry e : potentialBugs.entrySet()) { + if (!isHashableClassName(e.getKey().className)) { BugInstance bug = e.getValue(); bugReporter.reportBug(bug); @@ -652,4 +655,37 @@ public void report() { } } + + private static class PotentialBugKey { + private final String className; + private final String bugType; + + public PotentialBugKey(String className, BugInstance bugInstance) { + this.className = className; + this.bugType = bugInstance.getType(); + } + + @Override + public int hashCode() { + return Objects.hash(bugType, className); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (obj == null) { + return false; + } + + if (getClass() != obj.getClass()) { + return false; + } + + PotentialBugKey other = (PotentialBugKey) obj; + return Objects.equals(bugType, other.bugType) && Objects.equals(className, other.className); + } + } } diff --git a/spotbugsTestCases/src/java/ghIssues/issue2402/TestA.java b/spotbugsTestCases/src/java/ghIssues/issue2402/TestA.java new file mode 100644 index 00000000000..c3d00eb9db4 --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue2402/TestA.java @@ -0,0 +1,22 @@ +package ghIssues.issue2402; +import java.util.HashMap; +public class TestA { + static class UMap extends HashMap {} + ; + static HashMap m = + new HashMap(); + + static int foo(HashMap map) { + return map.size(); + } + @Override + public boolean equals(Object o) { + return hashCode() == o.hashCode(); + } + public static String add(TestA b, String s) { + return m.put(b, s); + } + public static String get(TestA b) { + return m.get(b); + } +} \ No newline at end of file diff --git a/spotbugsTestCases/src/java/ghIssues/issue2402/TestB.java b/spotbugsTestCases/src/java/ghIssues/issue2402/TestB.java new file mode 100644 index 00000000000..20ff17c740f --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue2402/TestB.java @@ -0,0 +1,22 @@ +package ghIssues.issue2402; +import java.util.HashMap; +public class TestB { + static class UMap extends HashMap {} + ; + static HashMap m = + new HashMap(); + + static int foo(HashMap map) { + return map.size(); + } + @Override + public boolean equals(Object o) { + return hashCode() == o.hashCode(); + } + public static String add(TestB b, String s) { + return m.put(b, s); + } + public static String get(TestB b) { + return m.get(b); + } +} \ No newline at end of file diff --git a/spotbugsTestCases/src/java/ghIssues/issue2402/TestC.java b/spotbugsTestCases/src/java/ghIssues/issue2402/TestC.java new file mode 100644 index 00000000000..9322f88870c --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue2402/TestC.java @@ -0,0 +1,22 @@ +package ghIssues.issue2402; +import java.util.HashMap; +public class TestC { + static class UMap extends HashMap {} + ; + static HashMap m = + new HashMap(); + + static int foo(HashMap map) { + return map.size(); + } + @Override + public boolean equals(Object o) { + return hashCode() == o.hashCode(); + } + public static String add(TestC b, String s) { + return m.put(b, s); + } + public static String get(TestC b) { + return m.get(b); + } +} \ No newline at end of file diff --git a/spotbugsTestCases/src/java/ghIssues/issue2402/TestD.java b/spotbugsTestCases/src/java/ghIssues/issue2402/TestD.java new file mode 100644 index 00000000000..470d69d442c --- /dev/null +++ b/spotbugsTestCases/src/java/ghIssues/issue2402/TestD.java @@ -0,0 +1,22 @@ +package ghIssues.issue2402; +import java.util.HashMap; +public class TestD { + static class UMap extends HashMap {} + ; + static HashMap m = + new HashMap(); + + static int foo(HashMap map) { + return map.size(); + } + @Override + public boolean equals(Object o) { + return hashCode() == o.hashCode(); + } + public static String add(TestD b, String s) { + return m.put(b, s); + } + public static String get(TestD b) { + return m.get(b); + } +} \ No newline at end of file