Introduce analysistest testdata convention. (#278)

* Introduce analysistest testdata convention. * Add Project Conventions to CONTRIBUTING.md and document this testdata convention. * Move analysistest testdata from **/testdata/src/** to **/testdata/src/*_analysistest/**. * Add go.mod for each *_analysistest * Update relevant analysistest configurations. * Update relevant *_test.go analysistest.Run package target strings. * Update impacted imports. Fixes #274
google · Feb 3, 2021 · 80f8749 · 80f8749
1 parent 15d2c2d
commit 80f8749
Show file tree

Hide file tree

Showing 69 changed files with 104 additions and 76 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -26,3 +26,19 @@ information on using pull requests.
 
 This project follows [Google's Open Source Community
 Guidelines](https://opensource.google/conduct/).
+
+## Project Conventions
+
+### Use Go Modules in `testdata`
+
+The analyzer testing package `analysistest` executes a build of the Go code it tests.
+This treats the `testdata` folder as a synthetic GOPATH, and Go code tested within will look for imported packages in `testdata/src/...`.
+Unfortunately, this tends to break IDE linking, making test development difficult.
+
+To ameliorate this issue, we group a given test's `testdata` into a synthetic root package and use Go modules to identify it for the IDE.
+This allows your IDE to correctly link imports while adhering to the expectation of `analysistest`.
+
+Specifically, `testdata` should be grouped under `testdata/src/NAME_analysistest`, where `NAME` is the name of the analyzer being tested.
+Initialize a trivial Go module file with `go mod init NAME_analysistest`.
+Any imports used within this `testdata` will begin with `NAME_analysistest`.
+See [`levee_analysistest`](internal/pkg/levee/testdata/src/levee_analysistest) for an example. 
diff --git a/...fig/testdata/src/example.com/core/core.go → .../config_analysistest/example/core/core.go b/...fig/testdata/src/example.com/core/core.go → .../config_analysistest/example/core/core.go
diff --git a/...data/src/example.com/crosspkg/crosspkg.go → ...analysistest/example/crosspkg/crosspkg.go b/...data/src/example.com/crosspkg/crosspkg.go → ...analysistest/example/crosspkg/crosspkg.go
@@ -15,9 +15,9 @@
 package crosspkg
 
 import (
-	"example.com/core"
-	"example.com/notcore"
-	necore "notexample.com/core"
+	"config_analysistest/example/core"
+	"config_analysistest/example/notcore"
+	necore "config_analysistest/notexample/core"
 )
 
 func CoreCalls() {

diff --git a/...estdata/src/example.com/exclusion/test.go → ...ig_analysistest/example/exclusion/test.go b/...estdata/src/example.com/exclusion/test.go → ...ig_analysistest/example/exclusion/test.go
diff --git a/...stdata/src/example.com/notcore/notcore.go → ...g_analysistest/example/notcore/notcore.go b/...stdata/src/example.com/notcore/notcore.go → ...g_analysistest/example/notcore/notcore.go
diff --git a/internal/pkg/config/testdata/src/config_analysistest/go.mod b/internal/pkg/config/testdata/src/config_analysistest/go.mod
@@ -0,0 +1,3 @@
+module config_analysistest
+
+go 1.15
diff --git a/.../testdata/src/notexample.com/core/core.go → ...nfig_analysistest/notexample/core/core.go b/.../testdata/src/notexample.com/core/core.go → ...nfig_analysistest/notexample/core/core.go
diff --git a/...data/src/notexample.com/exclusion/test.go → ...analysistest/notexample/exclusion/test.go b/...data/src/notexample.com/exclusion/test.go → ...analysistest/notexample/exclusion/test.go
diff --git a/internal/pkg/config/testdata/test-config.yaml b/internal/pkg/config/testdata/test-config.yaml
@@ -13,15 +13,15 @@
 # limitations under the License.
 ---
 sinks:
-  - packageRE: "^example.com/core"
-    methodRE: "^Sink$"
-  - packageRE: "^example.com/core"
-    methodRE: "^Do$"
-    receiverRE: "^Sinker$"
+  - package: "config_analysistest/example/core"
+    method: "Sink"
+  - package: "config_analysistest/example/core"
+    method: "Do"
+    receiver: "Sinker"
 fieldTags:
   - key: example
     value: sensitive
 exclude:
-  - packageRE: "^example.com/exclusion$"
-    methodRE: "^Foo$"
-  - packageRE: "^notexample.com/exclusion$"
+  - package: "config_analysistest/example/exclusion"
+    method: "Foo"
+  - package: "config_analysistest/notexample/exclusion"
diff --git a/internal/pkg/fieldpropagator/testdata/src/fieldpropagator_analysistest/go.mod b/internal/pkg/fieldpropagator/testdata/src/fieldpropagator_analysistest/go.mod
@@ -0,0 +1,3 @@
+module fieldpropagator_analysistest
+
+go 1.15
diff --git a/...eldpropagator/testdata/src/proto/proto.go → ...eldpropagator_analysistest/proto/proto.go b/...eldpropagator/testdata/src/proto/proto.go → ...eldpropagator_analysistest/proto/proto.go
diff --git a/...eldpropagator/testdata/src/source/test.go → ...eldpropagator_analysistest/source/test.go b/...eldpropagator/testdata/src/source/test.go → ...eldpropagator_analysistest/source/test.go
diff --git a/internal/pkg/fieldpropagator/testdata/test-config.yaml b/internal/pkg/fieldpropagator/testdata/test-config.yaml
@@ -16,12 +16,12 @@ Sources:
   - PackageRE: source
     TypeRE: "^Source$"
     FieldRE: "^data"
-  - Package: proto
+  - PackageRE: proto
     Type: KeyPair
     Field: Private
-  - Package: proto
+  - PackageRE: proto
     Type: TLS
     Field: Cert
-  - Package: proto
+  - PackageRE: proto
     Type: BasicAuth
     Field: Password
diff --git a/internal/pkg/fieldtags/analyzer_test.go b/internal/pkg/fieldtags/analyzer_test.go
@@ -31,7 +31,7 @@ func TestFieldTagsAnalysis(t *testing.T) {
 		t.Error(err)
 	}
 
-	results := analysistest.Run(t, testdata, Analyzer, "example.com/core", "example.com/crosspkg")
+	results := analysistest.Run(t, testdata, Analyzer, "fieldtags_analysistest/core", "fieldtags_analysistest/crosspkg")
 
 	if len(results) != 2 {
 		t.Fatalf("expected 2 results, got %d", len(results))

diff --git a/...ags/testdata/src/example.com/core/core.go → ...a/src/fieldtags_analysistest/core/core.go b/...ags/testdata/src/example.com/core/core.go → ...a/src/fieldtags_analysistest/core/core.go
diff --git a/...data/src/example.com/crosspkg/crosspkg.go → ...eldtags_analysistest/crosspkg/crosspkg.go b/...data/src/example.com/crosspkg/crosspkg.go → ...eldtags_analysistest/crosspkg/crosspkg.go
@@ -14,7 +14,7 @@
 
 package crosspkg
 
-import "example.com/core"
+import "fieldtags_analysistest/core"
 
 type PersonWrapper struct {
 	p          core.Person

diff --git a/internal/pkg/fieldtags/testdata/src/fieldtags_analysistest/go.mod b/internal/pkg/fieldtags/testdata/src/fieldtags_analysistest/go.mod
@@ -0,0 +1,3 @@
+module fieldtags_analysistest
+
+go 1.15
diff --git a/internal/pkg/levee/levee_test.go b/internal/pkg/levee/levee_test.go
@@ -32,7 +32,7 @@ func TestLevee(t *testing.T) {
 	if *debugging {
 		Analyzer.Requires = append(Analyzer.Requires, debug.Analyzer)
 	}
-	analysistest.Run(t, dataDir, Analyzer, "./src/example.com/...")
+	analysistest.Run(t, dataDir, Analyzer, "./src/levee_analysistest/example/...")
 }
 
 func TestLeveeDoesNotCreateReportsForPanicIfPanicingOnTaintedValuesIsAllowed(t *testing.T) {
@@ -43,21 +43,21 @@ func TestLeveeDoesNotCreateReportsForPanicIfPanicingOnTaintedValuesIsAllowed(t *
 	if *debugging {
 		Analyzer.Requires = append(Analyzer.Requires, debug.Analyzer)
 	}
-	analysistest.Run(t, dataDir, Analyzer, "./src/nopanic.com/...")
+	analysistest.Run(t, dataDir, Analyzer, "./src/levee_analysistest/nopanic.com/...")
 }
 
 func TestFormattingWithCustomReportMessage(t *testing.T) {
 	dataDir := analysistest.TestData()
 	if err := Analyzer.Flags.Set("config", dataDir+"/with-custom-message.yaml"); err != nil {
 		t.Error(err)
 	}
-	analysistest.Run(t, dataDir, Analyzer, "./src/custom.message.com/withcustom")
+	analysistest.Run(t, dataDir, Analyzer, "./src/levee_analysistest/custom.message.com/withcustom")
 }
 
 func TestFormattingWithoutCustomReportMessage(t *testing.T) {
 	dataDir := analysistest.TestData()
 	if err := Analyzer.Flags.Set("config", dataDir+"/no-custom-message.yaml"); err != nil {
 		t.Error(err)
 	}
-	analysistest.Run(t, dataDir, Analyzer, "./src/custom.message.com/nocustom")
+	analysistest.Run(t, dataDir, Analyzer, "./src/levee_analysistest/custom.message.com/nocustom")
 }
diff --git a/internal/pkg/levee/testdata/allowpanicontaintedvalues-config.yaml b/internal/pkg/levee/testdata/allowpanicontaintedvalues-config.yaml
@@ -14,9 +14,9 @@
 ---
 AllowPanicOnTaintedValues: true
 Sources:
-  - Package: "nopanic.com/core"
+  - Package: "levee_analysistest/nopanic.com/core"
     Type: "Source"
     Field: "Data"
 Sinks:
-  - Package: "nopanic.com/core"
+  - Package: "levee_analysistest/nopanic.com/core"
     Method: "Sink"
diff --git a/...c/custom.message.com/nocustom/nocustom.go → ...t/custom.message.com/nocustom/nocustom.go b/...c/custom.message.com/nocustom/nocustom.go → ...t/custom.message.com/nocustom/nocustom.go
diff --git a/...stom.message.com/withcustom/withcustom.go → ...stom.message.com/withcustom/withcustom.go b/...stom.message.com/withcustom/withcustom.go → ...stom.message.com/withcustom/withcustom.go
diff --git a/...testdata/src/example.com/core/sanitize.go → ...vee_analysistest/example/core/sanitize.go b/...testdata/src/example.com/core/sanitize.go → ...vee_analysistest/example/core/sanitize.go
diff --git a/...vee/testdata/src/example.com/core/sink.go → ...c/levee_analysistest/example/core/sink.go b/...vee/testdata/src/example.com/core/sink.go → ...c/levee_analysistest/example/core/sink.go
diff --git a/...e/testdata/src/example.com/core/source.go → ...levee_analysistest/example/core/source.go b/...e/testdata/src/example.com/core/source.go → ...levee_analysistest/example/core/source.go
diff --git a/.../src/example.com/tests/arguments/tests.go → ...ysistest/example/tests/arguments/tests.go b/.../src/example.com/tests/arguments/tests.go → ...ysistest/example/tests/arguments/tests.go
@@ -15,7 +15,7 @@
 package arguments
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestSourceFromParamByReference(s *core.Source) {

diff --git a/...data/src/example.com/tests/binop/tests.go → ...analysistest/example/tests/binop/tests.go b/...data/src/example.com/tests/binop/tests.go → ...analysistest/example/tests/binop/tests.go
@@ -17,7 +17,7 @@ package binop
 import (
 	"fmt"
 
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestConcatenatingTaintedAndNonTaintedStrings(prefix string) {

diff --git a/...a/src/example.com/tests/booleans/tests.go → ...lysistest/example/tests/booleans/tests.go b/...a/src/example.com/tests/booleans/tests.go → ...lysistest/example/tests/booleans/tests.go
@@ -15,7 +15,7 @@
 package inlining
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func HasSecret(s core.Source) bool {

diff --git a/...ample.com/tests/callorder/beforesource.go → ...t/example/tests/callorder/beforesource.go b/...ample.com/tests/callorder/beforesource.go → ...t/example/tests/callorder/beforesource.go
@@ -15,7 +15,7 @@
 package callorder
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 // This type should *not* be identified as a Source.

diff --git a/...example.com/tests/callorder/colocation.go → ...est/example/tests/callorder/colocation.go b/...example.com/tests/callorder/colocation.go → ...est/example/tests/callorder/colocation.go
@@ -15,7 +15,7 @@
 package callorder
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestTaintedColocatedArgumentDoesNotReachSinkThatPrecedesColocation() {

diff --git a/...example.com/tests/callorder/multiblock.go → ...est/example/tests/callorder/multiblock.go b/...example.com/tests/callorder/multiblock.go → ...est/example/tests/callorder/multiblock.go
@@ -18,7 +18,7 @@ import (
 	"fmt"
 	"io"
 
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestSinkInIfBeforeTaint(s core.Source, w io.Writer) {

diff --git a/...xample.com/tests/callorder/singleblock.go → ...st/example/tests/callorder/singleblock.go b/...xample.com/tests/callorder/singleblock.go → ...st/example/tests/callorder/singleblock.go
@@ -18,7 +18,7 @@ import (
 	"fmt"
 	"io"
 
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestTaintBeforeSinking(s core.Source, w io.Writer) {

diff --git a/...a/src/example.com/tests/closures/tests.go → ...lysistest/example/tests/closures/tests.go b/...a/src/example.com/tests/closures/tests.go → ...lysistest/example/tests/closures/tests.go
@@ -15,7 +15,7 @@
 package closures
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TesCapturedSourceReachesSinkInClosure() func() {

diff --git a/...c/example.com/tests/collections/arrays.go → ...stest/example/tests/collections/arrays.go b/...c/example.com/tests/collections/arrays.go → ...stest/example/tests/collections/arrays.go
@@ -15,7 +15,7 @@
 package collections
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestArrayLiteralContainingSourceIsTainted(s core.Source) {

diff --git a/...rc/example.com/tests/collections/chans.go → ...istest/example/tests/collections/chans.go b/...rc/example.com/tests/collections/chans.go → ...istest/example/tests/collections/chans.go
@@ -15,7 +15,7 @@
 package collections
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestSourceReceivedFromChannelIsTainted(sources <-chan core.Source) {

diff --git a/...src/example.com/tests/collections/maps.go → ...sistest/example/tests/collections/maps.go b/...src/example.com/tests/collections/maps.go → ...sistest/example/tests/collections/maps.go
@@ -15,7 +15,7 @@
 package collections
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestMapLiteralContainingSourceKeyIsTainted(s core.Source) {

diff --git a/...c/example.com/tests/collections/slices.go → ...stest/example/tests/collections/slices.go b/...c/example.com/tests/collections/slices.go → ...stest/example/tests/collections/slices.go
@@ -15,7 +15,7 @@
 package collections
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestSlices(s core.Source) {

diff --git a/.../example.com/tests/collections/sources.go → ...test/example/tests/collections/sources.go b/.../example.com/tests/collections/sources.go → ...test/example/tests/collections/sources.go
@@ -15,7 +15,7 @@
 package collections
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestSourceMapIsSource() {

diff --git a/...src/example.com/tests/colocation/tests.go → ...sistest/example/tests/colocation/tests.go b/...src/example.com/tests/colocation/tests.go → ...sistest/example/tests/colocation/tests.go
@@ -18,7 +18,7 @@ import (
 	"encoding/json"
 	"reflect"
 
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func colocateString(core.Source, string) {}

diff --git a/...c/example.com/tests/declarations/tests.go → ...stest/example/tests/declarations/tests.go b/...c/example.com/tests/declarations/tests.go → ...stest/example/tests/declarations/tests.go
@@ -16,7 +16,7 @@
 package declarations
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestSourceDeclaredInBody() {

diff --git a/...data/src/example.com/tests/eface/tests.go → ...analysistest/example/tests/eface/tests.go b/...data/src/example.com/tests/eface/tests.go → ...analysistest/example/tests/eface/tests.go
@@ -15,7 +15,7 @@
 package eface
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestEfaceSource(s core.Source) {

diff --git a/.../src/example.com/tests/embedding/tests.go → ...ysistest/example/tests/embedding/tests.go b/.../src/example.com/tests/embedding/tests.go → ...ysistest/example/tests/embedding/tests.go
@@ -15,7 +15,7 @@
 package embedding
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 type EmbedsSource struct {

diff --git a/...xample.com/tests/excludedpackage/tests.go → ...st/example/tests/excludedpackage/tests.go b/...xample.com/tests/excludedpackage/tests.go → ...st/example/tests/excludedpackage/tests.go
@@ -15,7 +15,7 @@
 package excludedpackage
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func Oops(s core.Source) {

diff --git a/...a/src/example.com/tests/extracts/tests.go → ...lysistest/example/tests/extracts/tests.go b/...a/src/example.com/tests/extracts/tests.go → ...lysistest/example/tests/extracts/tests.go
@@ -15,7 +15,7 @@
 package extracts
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func CreateSource() (core.Source, error) {

diff --git a/...ata/src/example.com/tests/fields/tests.go → ...nalysistest/example/tests/fields/tests.go b/...ata/src/example.com/tests/fields/tests.go → ...nalysistest/example/tests/fields/tests.go
@@ -18,7 +18,7 @@ package fields
 import (
 	"strconv"
 
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestFieldAccessors(s core.Source, ptr *core.Source) {

diff --git a/...xample.com/tests/includedpackage/tests.go → ...st/example/tests/includedpackage/tests.go b/...xample.com/tests/includedpackage/tests.go → ...st/example/tests/includedpackage/tests.go
@@ -15,7 +15,7 @@
 package includedpackage
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func Oops(s core.Source) {

diff --git a/...a/src/example.com/tests/inlining/tests.go → ...lysistest/example/tests/inlining/tests.go b/...a/src/example.com/tests/inlining/tests.go → ...lysistest/example/tests/inlining/tests.go
@@ -15,7 +15,7 @@
 package inlining
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func NewSource() *core.Source {

diff --git a/...data/src/example.com/tests/loops/tests.go → ...analysistest/example/tests/loops/tests.go b/...data/src/example.com/tests/loops/tests.go → ...analysistest/example/tests/loops/tests.go
@@ -15,7 +15,7 @@
 package loops
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestTaintInThenBlockInLoopSinkAfterLoop() {

diff --git a/.../example.com/tests/makeinterface/tests.go → ...test/example/tests/makeinterface/tests.go b/.../example.com/tests/makeinterface/tests.go → ...test/example/tests/makeinterface/tests.go
@@ -15,7 +15,7 @@
 package deletethis
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestTaintingInterfaceValueDoesNotTaintContainedValue(s *core.Source, str string) {

diff --git a/...ata/src/example.com/tests/method/tests.go → ...nalysistest/example/tests/method/tests.go b/...ata/src/example.com/tests/method/tests.go → ...nalysistest/example/tests/method/tests.go
@@ -15,7 +15,7 @@
 package method
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestMethodCallOnStaticallyKnownReceiverPropagatesTaint(s core.Source) {

diff --git a/...rc/example.com/tests/namedreturn/tests.go → ...istest/example/tests/namedreturn/tests.go b/...rc/example.com/tests/namedreturn/tests.go → ...istest/example/tests/namedreturn/tests.go
@@ -15,7 +15,7 @@
 package namedreturn
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestNamedReturnValue() (s core.Source) {

diff --git a/...data/src/example.com/tests/panic/tests.go → ...analysistest/example/tests/panic/tests.go b/...data/src/example.com/tests/panic/tests.go → ...analysistest/example/tests/panic/tests.go
@@ -15,7 +15,7 @@
 package panic
 
 import (
-	"example.com/core"
+	"levee_analysistest/example/core"
 )
 
 func TestPanicIsASink(source core.Source) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -18,7 +18,7 @@ import ( @@
     	"fmt"
     	"io"
-    	"example.com/core"
+    	"levee_analysistest/example/core"
     )
     func TestSinkInIfBeforeTaint(s core.Source, w io.Writer) {
@@ Expand Down @@