Refactor the check of a main method in a test program to improve maintainability

Author: luchua-bc

java/ql/src/experimental/Security/CWE/CWE-489/EJBMain.ql

@@ -9,20 +9,14 @@
 
 import java
 import semmle.code.java.J2EE
+import MainLib
 
 /** The `main` method in an Enterprise Java Bean. */
 class EnterpriseBeanMainMethod extends Method {
   EnterpriseBeanMainMethod() {
     this.getDeclaringType() instanceof EnterpriseBean and
-    this.hasName("main") and
-    this.isStatic() and
-    this.getReturnType() instanceof VoidType and
-    this.isPublic() and
-    this.getNumberOfParameters() = 1 and
-    this.getParameter(0).getType() instanceof Array and
-    not this.getDeclaringType().getName().toLowerCase().matches("%test%") and // Simple check to exclude test classes to reduce FPs
-    not this.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") and // Simple check to exclude classes in test packages to reduce FPs
-    not exists(this.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) //  Match test directory structure of build tools like maven
+    isMainMethod(this) and
+    not isTestMethod(this)
   }
 }
 

java/ql/src/experimental/Security/CWE/CWE-489/MainLib.qll

@@ -0,0 +1,27 @@
+/** Definitions related to the main method in a test program. */
+
+import java
+
+/** Holds if `m` is the main method of a Java class with the signature `public static void main(String[] args)`. */
+predicate isMainMethod(Method m) {
+  m.hasName("main") and
+  m.isStatic() and
+  m.getReturnType() instanceof VoidType and
+  m.isPublic() and
+  m.getNumberOfParameters() = 1 and
+  m.getParameter(0).getType() instanceof Array
+}
+
+/**
+ * Holds if `m` is a test method indicated by:
+ *    a) in a test directory such as `src/test/java`
+ *    b) in a test package whose name has the word `test`
+ *    c) in a test class whose name has the word `test`
+ *    d) in a test class implementing a test framework such as JUnit or TestNG
+ */
+predicate isTestMethod(Method m) {
+  m.getDeclaringType().getName().toLowerCase().matches("%test%") or // Simple check to exclude test classes to reduce FPs
+  m.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") or // Simple check to exclude classes in test packages to reduce FPs
+  exists(m.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) or //  Match test directory structure of build tools like maven
+  m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG
+}

java/ql/src/experimental/Security/CWE/CWE-489/WebComponentMain.ql

@@ -9,6 +9,7 @@
 
 import java
 import semmle.code.java.frameworks.Servlets
+import MainLib
 
 /** The java type `javax.servlet.Filter`. */
 class ServletFilterClass extends Class {
@@ -47,15 +48,8 @@ class WebComponentMainMethod extends Method {
           .getASupertype+()
           .hasQualifiedName("org.springframework.webflow.execution", "Action") // Spring actions
     ) and
-    this.hasName("main") and
-    this.isStatic() and
-    this.getReturnType() instanceof VoidType and
-    this.isPublic() and
-    this.getNumberOfParameters() = 1 and
-    this.getParameter(0).getType() instanceof Array and
-    not this.getDeclaringType().getName().toLowerCase().matches("%test%") and // Simple check to exclude test classes to reduce FPs
-    not this.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") and // Simple check to exclude classes in test packages to reduce FPs
-    not exists(this.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) //  Match test directory structure of build tools like maven
+    isMainMethod(this) and
+    not isTestMethod(this)
   }
 }