New detector for MET07-J

CHANGELOG.md

@@ -16,6 +16,9 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 
 - tbd
 
+### Added
+- New detector `FindHiddenMehtod` for bug type `HSM_HIDING_METHOD`. This bug is reported whenever a subclass method hides the static method of super class. (See [SEI CERT MET07-J] (https://wiki.sei.cmu.edu/confluence/display/java/MET07-J.+Never+declare+a+class+method+that+hides+a+method+declared+in+a+superclass+or+superinterface)).
+
 ## 4.8.1 - 2023-11-06
 
 ### Fixed

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindHiddenMethodTest.java

@@ -0,0 +1,177 @@
+package edu.umd.cs.findbugs.detect;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+
+import org.junit.jupiter.api.Test;
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+
+class FindHiddenMethodTest extends AbstractIntegrationTest {
+    private final String BUG_TYPE = "HSM_HIDING_METHOD";
+
+    @Test
+    void testBadFindHiddenMethodTest() {
+        performAnalysis("findhiddenmethodtest/GrantAccessStatic.class",
+                "findhiddenmethodtest/BadFindHiddenMethodTest.class");
+        assertHSBCBug("displayAccountStatus", 15);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest2() {
+        performAnalysis("findhiddenmethodtest/BadSuperClass.class",
+                "findhiddenmethodtest/BadHidingVsOverriding.class");
+        assertHSBCBug("methodHiding", 19);
+    }
+
+
+    @Test
+    void testBadFindHiddenMethodTest3() {
+        performAnalysis("findhiddenmethodtest/SuperBadInEqualMultipleMethod.class",
+                "findhiddenmethodtest/BadInEqualMultipleStaticMethod.class");
+        assertHSBCBug("display", 25);
+        assertHSBCBug("display2", 29);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest4() {
+        performAnalysis("findhiddenmethodtest/BadMainMethodCheck.class",
+                "findhiddenmethodtest/BadSuperClassWithMain.class");
+        assertHSBCBug("main", 18);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest5() {
+        performAnalysis("findhiddenmethodtest/BadGrandClass.class",
+                "findhiddenmethodtest/BadParentClass.class",
+                "findhiddenmethodtest/BadMultiLevelInheritance.class");
+        assertHSBCBug("display", 20);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest6() {
+        performAnalysis("findhiddenmethodtest/SuperBadMultipleMethods.class",
+                "findhiddenmethodtest/BadMultipleStaticMethods.class");
+        assertHSBCBug("display", 19);
+        assertHSBCBug("display2", 23);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest7() {
+        performAnalysis("findhiddenmethodtest/SuperBadNonOrderedMultipleStaticMethods.class",
+                "findhiddenmethodtest/BadNonOrderedMultipleStaticMethods.class");
+        assertHSBCBug("display", 24);
+        assertHSBCBug("display2", 28);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest8() {
+        performAnalysis("findhiddenmethodtest/BadOuterInnerClass.class",
+                "findhiddenmethodtest/BadOuterInnerClass$BadInner.class");
+        assertHSBCBug("badMethod", 15);
+    }
+
+    @Test
+    void testBadFindHiddenMethodTest9() {
+        performAnalysis("findhiddenmethodtest/SuperBadProtected.class",
+                "findhiddenmethodtest/BadProtected.class");
+        assertHSBCBug("display", 15);
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest() {
+        performAnalysis("findhiddenmethodtest/GrantAccess.class",
+                "findhiddenmethodtest/GoodFindHiddenMethodTest.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest2() {
+        performAnalysis("findhiddenmethodtest/SuperGoodInEqualMultipleMethod.class",
+                "findhiddenmethodtest/GoodInEqualNonStaticMethods.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest3() {
+        performAnalysis("findhiddenmethodtest/GoodSuperClassWithMain.class",
+                "findhiddenmethodtest/GoodMainMethodCheck.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest4() {
+        performAnalysis("findhiddenmethodtest/GoodGrandNonStatic.class",
+                "findhiddenmethodtest/GoodParentNonStatic.class",
+                "findhiddenmethodtest/GoodMultiLevelInheritanceNonStatic.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest5() {
+        performAnalysis("findhiddenmethodtest/GoodGrandClassPrivate.class",
+                "findhiddenmethodtest/GoodParentClassPrivate.class",
+                "findhiddenmethodtest/GoodMultiLevelInheritancePrivate.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest6() {
+        performAnalysis("findhiddenmethodtest/SuperMultipleNonStatic.class",
+                "findhiddenmethodtest/GoodMultipleNonStaticMethods.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest7() {
+        performAnalysis("findhiddenmethodtest/SuperGoodMultipleStaticMethod.class",
+                "findhiddenmethodtest/GoodMultipleStaticMethod.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest8() {
+        performAnalysis("findhiddenmethodtest/SuperGoodNonOrderedMultipleStaticMethods.class",
+                "findhiddenmethodtest/GoodNonOrderedMultipleNonStaticMethods.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest9() {
+        performAnalysis("findhiddenmethodtest/GoodOuterInnerClassNonStatic.class",
+                "findhiddenmethodtest/GoodOuterInnerClassNonStatic$GoodInnerNonStaticGood.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest10() {
+        performAnalysis("findhiddenmethodtest/GoodOuterInnerClassPrivate.class",
+                "findhiddenmethodtest/GoodOuterInnerClassPrivate$GoodInnerClassPrivate.class");
+        assertNoHSBCBug();
+    }
+
+    @Test
+    void testGoodFindHiddenMethodTest11() {
+        performAnalysis("findhiddenmethodtest/SuperGoodProtected.class",
+                "findhiddenmethodtest/GoodProtected.class");
+        assertNoHSBCBug();
+    }
+
+    private void assertNoHSBCBug() {
+        final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
+                .bugType(BUG_TYPE)
+                .build();
+        assertThat(getBugCollection(), containsExactly(0, bugTypeMatcher));
+    }
+
+    private void assertHSBCBug(String methodName, int lineNumber) {
+        final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
+                .bugType(BUG_TYPE)
+                .inMethod(methodName)
+                .atLine(lineNumber)
+                .build();
+        assertThat(getBugCollection(), hasItem(bugTypeMatcher));
+    }
+}

spotbugs/etc/findbugs.xml

@@ -682,6 +682,9 @@
           <Detector class="edu.umd.cs.findbugs.detect.DontReusePublicIdentifiers" speed="fast"
                     reports="PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_CLASS_NAMES,PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_FIELD_NAMES,PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_METHOD_NAMES,PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_LOCAL_VARIABLE_NAMES"
                     disabled="true"/>
+          <Detector class="edu.umd.cs.findbugs.detect.FindHiddenMethod" speed="fast"
+                    reports="HSM_HIDING_METHOD"/>
+
           <!-- Bug Categories -->
           <BugCategory category="NOISE" hidden="true"/>
 
@@ -714,6 +717,7 @@
           <BugCode abbrev="ASE"/>
           <BugCode abbrev="VSC"/>
           <BugCode abbrev="DA"/>
+          <BugCode abbrev="HSM"/>
           <!-- Bug patterns -->
           <BugPattern abbrev="JUA" type="JUA_DONT_ASSERT_INSTANCEOF_IN_TESTS" category="BAD_PRACTICE" experimental="true" />
           <BugPattern abbrev="AA" type="AA_ASSERTION_OF_ARGUMENTS" category="BAD_PRACTICE" />
@@ -1320,4 +1324,5 @@
           <BugPattern abbrev="PI" type="PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_METHOD_NAMES" category="BAD_PRACTICE"/>
           <BugPattern abbrev="PI" type="PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_LOCAL_VARIABLE_NAMES" category="BAD_PRACTICE"/>
 
+          <BugPattern abbrev="HSM" type="HSM_HIDING_METHOD" category="CORRECTNESS"/>
 </FindbugsPlugin>

spotbugs/etc/messages.xml

@@ -1819,6 +1819,23 @@ factory pattern to create these objects.</p>
       ]]>
     </Details>
   </Detector>
+  <Detector class="edu.umd.cs.findbugs.detect.FindHiddenMethod">
+    <Details>
+      <![CDATA[
+            <p>
+              Hiding happens when a sub class defines a static method
+              with the same header (signature plus return type) as any of its super classes.
+              In the event of method hiding, the invoked method is determined based on the specific qualified name or
+              method invocation expression used at the calling site.
+              The results are often unexpected, although the Java language provides unambiguous rules for method invocation for method hiding.
+              Moreover, method hiding and method overriding is often confused by programmers.
+              Consequently, programmers should avoid the method hiding.
+              Programmer should declare the respective method non-static or restrict it as private to eradicate the problem.
+            </p>
+      ]]>
+    </Details>
+  </Detector>
+
   <!--
   **********************************************************************
   BugPatterns
@@ -9005,6 +9022,25 @@ Using floating-point variables should not be used as loop counters, as they are
         ]]>
     </Details>
   </BugPattern>
+  <BugPattern type="HSM_HIDING_METHOD">
+    <ShortDescription>Method hiding must be avoided.</ShortDescription>
+    <LongDescription>The method '{1}' in class '{2}' hides a method in class '{3}'. Declare the respective methods non-static or private to eradicate the problem.</LongDescription>
+    <Details>
+      <![CDATA[
+        Hiding happens when a sub class defines a static method
+        with same header (signature plus return type) as in any of the super classes.
+        In case of method hiding the invoked method is determined based on the specific qualified name or
+        method invocation expression used at the calling site.
+        The results are often unexpected, although the Java language provides unambiguous rules for method invocation for method hiding.
+        Moreover, method hiding and method overriding is often confused by programmers.
+        Consequently, programmers must avoid the method hiding.
+        Programmer should declare the respective methods non-static or restrict it as private to eradicate the problem.
+        <p>
+        See SEI CERT rule <a href="https://wiki.sei.cmu.edu/confluence/display/java/MET07-J.+Never+declare+a+class+method+that+hides+a+method+declared+in+a+superclass+or+superinterface">MET07-J. Never declare a class method that hides a method declared in a superclass or superinterface</a>.
+        </p>
+        ]]>
+    </Details>
+  </BugPattern>
   <BugPattern type="PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_CLASS_NAMES">
         <ShortDescription>Do not reuse public identifiers from JSL as class name</ShortDescription>
         <LongDescription>The name of the class {0} shadows the publicly available identifier from the Java Standard Library.</LongDescription>
@@ -9263,4 +9299,5 @@ Using floating-point variables should not be used as loop counters, as they are
   <BugCode abbrev="VSC">Vulnerable security check performing methods</BugCode>
   <BugCode abbrev="AA">Misuse of assertions for checking arguments of public methods</BugCode>
   <BugCode abbrev="PI">Do not reuse public identifiers from Java Standard Library</BugCode>
+  <BugCode abbrev="HSM">Avoid method hiding</BugCode>
 </MessageCollection>