Recognise more testing frameworks

change-notes/1.24/analysis-go.md

@@ -4,8 +4,9 @@
 
 * Alert suppression can now be done with single-line block comments (`/* ... */`) as well as line comments (`// ...`).
 * Analysis of flow through fields has been improved.
-* More sources of untrusted input as well as vulnerable sinks are modelled, which may lead to more results from the security queries.
+* Detection of test code has been improved. LGTM will not show alerts in test code by default.
 * Go 1.14 library changes have been modeled.
+* More sources of untrusted input as well as vulnerable sinks are modelled, which may lead to more results from the security queries.
 
 ## New queries
 

ql/src/Security/CWE-295/DisabledCertificateCheck.ql

@@ -115,5 +115,5 @@ where
     becomesPartOf*(base, init)
   ) and
   // exclude results in test code
-  exists(File fl | fl = w.getFile() | not fl = any(TestCase tc).getFile())
+  exists(File fl | fl = w.getFile() | not fl instanceof TestFile)
 select w, "InsecureSkipVerify should not be used in production code."

ql/src/filters/ClassifyFiles.ql

@@ -21,7 +21,7 @@ string generatorCommentRegex() {
 
 predicate classify(File f, string category) {
   // tests
-  f = any(TestCase tc).getFile() and
+  f instanceof TestFile and
   category = "test"
   or
   // vendored code

ql/src/semmle/go/frameworks/Testing.qll

@@ -27,16 +27,59 @@ module TestCase {
   /** A `go test` style test (including benchmarks and examples). */
   private class GoTestFunction extends Range, FuncDef {
     GoTestFunction() {
-      getName().regexpMatch("Test[^a-z].*") and
+      getName().regexpMatch("Test(?![a-z]).*") and
       getNumParameter() = 1 and
       getParameter(0).getType().(PointerType).getBaseType().hasQualifiedName("testing", "T")
       or
-      getName().regexpMatch("Benchmark[^a-z].*") and
+      getName().regexpMatch("Benchmark(?![a-z]).*") and
       getNumParameter() = 1 and
       getParameter(0).getType().(PointerType).getBaseType().hasQualifiedName("testing", "B")
       or
-      getName().regexpMatch("Example[^a-z].*") and
+      getName().regexpMatch("Example(?![a-z]).*") and
       getNumParameter() = 0
     }
   }
 }
+
+/**
+ * A file that contains test cases or is otherwise used for testing.
+ *
+ * Extend this class to refine existing models of testing frameworks. If you want to model new
+ * frameworks, extend `TestFile::Range` instead.
+ */
+class TestFile extends File {
+  TestFile::Range self;
+
+  TestFile() { this = self }
+}
+
+/** Provides classes for working with test files. */
+module TestFile {
+  /**
+   * A file that contains test cases or is otherwise used for testing.
+   *
+   * Extend this class to model new testing frameworks. If you want to refine existing models,
+   * extend `TestFile` instead.
+   */
+  abstract class Range extends File { }
+
+  /** A file containing at least one test case. */
+  private class FileContainingTestCases extends Range {
+    FileContainingTestCases() { this = any(TestCase tc).getFile() }
+  }
+
+  /** A file that imports a well-known testing framework. */
+  private class FileImportingTestingFramework extends Range {
+    FileImportingTestingFramework() {
+      exists(string pkg, ImportSpec is |
+        is.getPath() = pkg and
+        is.getFile() = this
+      |
+        pkg = "net/http/httptest" or
+        pkg = "gen/thrifttest" or
+        pkg = "github.com/onsi/ginkgo" or
+        pkg = "github.com/onsi/gomega"
+      )
+    }
+  }
+}

ql/test/query-tests/filters/ClassifyFiles/ClassifyFiles.expected

@@ -1,2 +1,4 @@
 | hello2.go:0:0:0:0 | hello2.go | generated |
 | hello.go:0:0:0:0 | hello.go | generated |
+| httptest.go:0:0:0:0 | httptest.go | test |
+| test1.go:0:0:0:0 | test1.go | test |

ql/test/query-tests/filters/ClassifyFiles/TestCase.expected

@@ -0,0 +1,6 @@
+| test1.go:5:1:5:26 | function declaration |
+| test1.go:7:1:7:27 | function declaration |
+| test1.go:15:1:15:31 | function declaration |
+| test1.go:17:1:17:32 | function declaration |
+| test1.go:25:1:25:17 | function declaration |
+| test1.go:27:1:27:18 | function declaration |

ql/test/query-tests/filters/ClassifyFiles/TestCase.ql

@@ -0,0 +1,4 @@
+import go
+
+from TestCase tc
+select tc

ql/test/query-tests/filters/ClassifyFiles/httptest.go

@@ -0,0 +1,5 @@
+package main
+
+import "net/http/httptest"
+
+func setup(server *httptest.Server) {}

ql/test/query-tests/filters/ClassifyFiles/test1.go

@@ -0,0 +1,31 @@
+package main
+
+import "testing"
+
+func Test(t *testing.T) {} // test case
+
+func Test2(t *testing.T) {} // test case
+
+func TestMustHaveSingleParameter(t *testing.T, b bool) {} // not a test case
+
+func TestParameterMustHaveCorrectType(b *testing.B) {} // not a test case
+
+func TestsDoNotLookLikeThis(t *testing.T) {} // not a test case
+
+func Benchmark(b *testing.B) {} // test case
+
+func Benchmark2(b *testing.B) {} // test case
+
+func BenchmarkMustHaveSingleParameter(b *testing.B, flag bool) {} // not a test case
+
+func BenchmarkParameterMustHaveCorrectType(t *testing.T) {} // not a test case
+
+func BenchmarksDoNotLookLikeThis(b *testing.B) {} // not a test case
+
+func Example() {} // test case
+
+func Example2() {} // test case
+
+func ExampleMustNotHaveParameter(t *testing.T) {} // not a test case
+
+func ExamplesDoNotLookLikeThis() {} // not a test case