Repository: google/go-flow-levee
Pull request #246 (2020-12-30 17:34:20)

Handle pointer methodsets in fieldpropagator
Consider this snippet from the [Go types tutorial](https://github.com/golang/example/tree/master/gotypes):

> A variable of type T has access to all the methods of type *T as well, due to the implicit address-of operation during method calls, but those extra methods are not part of the method set of T.

Currently, the `fieldpropagators` analyzer does not take this into account: it only analyzes the method set of `T`. However, if a method has a pointer receiver, it will be in `*T`'s method set, which means the analyzer will not even consider it.

This PR rectifies this situation by analyzing the method set of `*T` as well.

- [x] Running against a large codebase such as [Kubernetes](https://github.com/kubernetes/kubernetes) does not error out. (See [DEVELOPING.md](https://github.com/google/go-flow-levee/blob/master/DEVELOPING.md) for instructions on how to do that.)
- (N/A) [ ] Appropriate changes to README are included in PR



Patch:
diff --git a/internal/pkg/fieldpropagator/analyzer.go b/internal/pkg/fieldpropagator/analyzer.go
index e93d75a..4d9fc0e 100644
--- a/internal/pkg/fieldpropagator/analyzer.go
+++ b/internal/pkg/fieldpropagator/analyzer.go
@@ -72,11 +72,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		if !ok || !conf.IsSourceType(utils.DecomposeType(ssaType.Type())) {
 			continue
 		}
-		methods := ssaProg.MethodSets.MethodSet(ssaType.Type())
-		for i := 0; i < methods.Len(); i++ {
-			if meth := ssaProg.MethodValue(methods.At(i)); meth != nil {
-				analyzeBlocks(pass, conf, taggedFields, meth)
-			}
+		for _, meth := range methods(ssaProg, ssaType.Type()) {
+			analyzeBlocks(pass, conf, taggedFields, meth)
 		}
 	}
 
@@ -87,6 +84,26 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return ResultType(isFieldPropagator), nil
 }
 
+func methods(ssaProg *ssa.Program, t types.Type) []*ssa.Function {
+	var methods []*ssa.Function
+	// The method sets of T and *T are disjoint.
+	// In Go code, a variable of type T has access to all
+	// the methods of type *T due to an implicit address operation.
+	methods = append(methods, methodValues(ssaProg, t)...)
+	return append(methods, methodValues(ssaProg, types.NewPointer(t))...)
+}
+
+func methodValues(ssaProg *ssa.Program, t types.Type) []*ssa.Function {
+	var methodValues []*ssa.Function
+	mset := ssaProg.MethodSets.MethodSet(t)
+	for i := 0; i < mset.Len(); i++ {
+		if meth := ssaProg.MethodValue(mset.At(i)); meth != nil {
+			methodValues = append(methodValues, meth)
+		}
+	}
+	return methodValues
+}
+
 func analyzeBlocks(pass *analysis.Pass, conf *config.Config, tf fieldtags.ResultType, meth *ssa.Function) {
 	// Function does not return anything
 	if res := meth.Signature.Results(); res == nil || (*res).Len() == 0 {


Test patch:
diff --git a/internal/pkg/fieldpropagator/testdata/src/source/test.go b/internal/pkg/fieldpropagator/testdata/src/source/test.go
index 6d1dbf2..493e45d 100644
--- a/internal/pkg/fieldpropagator/testdata/src/source/test.go
+++ b/internal/pkg/fieldpropagator/testdata/src/source/test.go
@@ -29,7 +29,11 @@ func (s Source) Secret() string { // want Secret:"field propagator identified"
 	return s.secret
 }
 
-func (s Source) Data() string { // want Data:"field propagator identified"
+func (s Source) DataValueReceiver() string { // want DataValueReceiver:"field propagator identified"
+	return s.data
+}
+
+func (s *Source) DataPointerReceiver() string { // want DataPointerReceiver:"field propagator identified"
 	return s.data
 }