Repository: google/go-flow-levee
Pull request #162 (2020-10-29 17:48:02)

Use funcmatcher for exclusion matchers.
* Add additional tests for exclusion of methods with receivers.

- [x] Tests pass
- [x] Running against a large codebase such as [Kubernetes](https://github.com/kubernetes/kubernetes) does not error out.
- [x] Appropriate changes to README are included in PR

------

Minor refactor.  I was concerned about excluding from exclusion any method attached to a receiver, since we were manually constructing the path for a given function.  Associated tests added.

PurelyApplied: As an aside, good call on adding the reminder to check the docs in the template.  I thought about it going in, did the changes, and then completely forgot to go update the docs.

Patch:
diff --git a/configuration/README.md b/configuration/README.md
index 8b4b0ca..6483030 100644
--- a/configuration/README.md
+++ b/configuration/README.md
@@ -75,12 +75,14 @@ To match only methods without any receiver (i.e., a top-level function), use the
 
 ### Restricting analysis scope
 
-Functions can be explicitly excluded from analysis using regexps:
+Functions can be explicitly excluded from analysis using regexps,
+constructed similarly to those used to identify sanitizers and sinks:
 ```json
 {
   "Exclude": [
     {
-      "PathRE": "^myproject/mypackage\.myfunction$"
+      "PathRE": "^myproject/mypackage$",
+      "MethodRE": "^myfunction$"
     }
   ]
 }
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index ba24abe..236040f 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -44,7 +44,7 @@ type Config struct {
 	Sinks      []funcMatcher
 	Sanitizers []funcMatcher
 	FieldTags  []fieldTagMatcher
-	Exclude    []pathMatcher
+	Exclude    []funcMatcher
 }
 
 type fieldTagMatcher struct {
@@ -76,16 +76,10 @@ func (c Config) IsSourceFieldTag(tag string) bool {
 	return false
 }
 
-type pathMatcher struct {
-	PathRE regexp.Regexp
-}
-
-// IsExcluded determines if a function's fully qualified name (package path + name)
-// matches one of the exclusion patterns in the Config.
+// IsExcluded determines if a function matches one of the exclusion patterns.
 func (c Config) IsExcluded(fn *ssa.Function) bool {
-	path := fmt.Sprintf("%s.%s", fn.Pkg.Pkg.Path(), fn.Name())
 	for _, pm := range c.Exclude {
-		if pm.PathRE.MatchString(path) {
+		if pm.Match(fn) {
 			return true
 		}
 	}


Test patch:
diff --git a/internal/pkg/config/testdata/test-config.json b/internal/pkg/config/testdata/test-config.json
index 6929d63..72704b9 100644
--- a/internal/pkg/config/testdata/test-config.json
+++ b/internal/pkg/config/testdata/test-config.json
@@ -18,10 +18,11 @@
   ],
   "Exclude": [
     {
-      "PathRE": "^example.com/exclusion.Foo$"
+      "PackageRE": "^example.com/exclusion$",
+      "MethodRE": "^Foo$"
     },
     {
-      "PathRE": "^notexample.com/exclusion"
+      "PackageRE": "^notexample.com/exclusion$"
     }
   ]
 }
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/excludedpackage/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/excludedpackage/tests.go
index be9920d..a39d97e 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/excludedpackage/tests.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/excludedpackage/tests.go
@@ -25,3 +25,9 @@ func Oops(s core.Source) {
 func OopsIDidItAgain(s core.Source) {
 	core.Sink(s) // we do not expect a report here, because this package is excluded from analysis
 }
+
+type Receiver int
+
+func (Receiver) OopsWithReceiver(s core.Source) {
+	core.Sink(s) // we do not expect a report here, because this package is excluded from analysis
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/includedpackage/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/includedpackage/tests.go
index 7991440..64cf5e9 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/includedpackage/tests.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/includedpackage/tests.go
@@ -25,3 +25,13 @@ func Oops(s core.Source) {
 func OopsIDidItAgain(s core.Source) {
 	core.Sink(s) // want "a source has reached a sink"
 }
+
+type Receiver int
+
+func (Receiver) OopsWithReceiver(s core.Source) {
+	core.Sink(s) // want "a source has reached a sink"
+}
+
+func (Receiver) OopsWithReceiverExcluded(s core.Source) {
+	core.Sink(s) // we do not expect a report here, because this specific function is excluded from analysis
+}
diff --git a/internal/pkg/levee/testdata/test-config.json b/internal/pkg/levee/testdata/test-config.json
index 115a7ca..862251f 100644
--- a/internal/pkg/levee/testdata/test-config.json
+++ b/internal/pkg/levee/testdata/test-config.json
@@ -20,10 +20,16 @@
   ],
   "Exclude": [
     {
-      "PathRE": "^example.com/tests/excludedpackage"
+      "PackageRE": "^example.com/tests/excludedpackage$"
     },
     {
-      "PathRE": "^example.com/tests/includedpackage.Oops$"
+      "PackageRE": "^example.com/tests/includedpackage$",
+      "MethodRE": "^Oops$"
+    },
+    {
+      "PackageRE": "^example.com/tests/includedpackage$",
+      "MethodRE": "^OopsWithReceiverExcluded$",
+      "ReceiverRE": "^Receiver$"
     }
   ]
 }