deduplicate methods to allow overlapping methods on embedded interfaces (#498)

marten-seemann · web-flow · commit b9a8584115bc · 2021-01-28T15:47:31.000-07:00
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -47,4 +47,9 @@ jobs:
         ./ci/check_panic_handling.sh
 
     - name: Run Go tests
-      run: go test -v ./...
+      run: |
+        for i in $(find $PWD -name go.mod); do
+          pushd $(dirname $i)
+          go test ./...
+          popd
+        done 
diff --git a/ci/check_go_generate.sh b/ci/check_go_generate.sh
@@ -18,7 +18,13 @@ trap cleanup EXIT
 
 cp -r . "${TEMP_DIR}/"
 cd $TEMP_DIR
-go generate ./...
+
+for i in $(find $PWD -name go.mod); do
+    pushd $(dirname $i)
+    go generate ./...
+    popd
+done 
+
 if ! diff -r . "${BASE_DIR}"; then
     echo
     echo "The generated files aren't up to date."
diff --git a/ci/check_go_mod.sh b/ci/check_go_mod.sh
@@ -3,7 +3,11 @@
 
 set -euo pipefail
 
-go mod tidy
+for i in $(find $PWD -name go.mod); do
+    pushd $(dirname $i)
+    go mod tidy
+    popd
+done 
 
 if [ ! -z "$(git status --porcelain)" ]; then
     git status
diff --git a/mockgen/internal/tests/overlapping_methods/go.mod b/mockgen/internal/tests/overlapping_methods/go.mod
@@ -0,0 +1,5 @@
+module github.com/golang/mock/mockgen/internal/tests/overlapping_methods
+
+go 1.14
+
+require github.com/golang/mock v1.4.4
diff --git a/mockgen/internal/tests/overlapping_methods/go.sum b/mockgen/internal/tests/overlapping_methods/go.sum
@@ -0,0 +1,8 @@
+github.com/golang/mock v1.4.4 h1:l75CXGRSwbaYNpl/Z2X1XIIAMSCquvXgpVZDhwEIJsc=
+github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4=
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
diff --git a/mockgen/internal/tests/overlapping_methods/interfaces.go b/mockgen/internal/tests/overlapping_methods/interfaces.go
@@ -0,0 +1,13 @@
+// +build go1.14
+
+package overlap
+
+type ReadCloser interface {
+	Read([]byte) (int, error)
+	Close() error
+}
+
+type WriteCloser interface {
+	Write([]byte) (int, error)
+	Close() error
+}
diff --git a/mockgen/internal/tests/overlapping_methods/mock.go b/mockgen/internal/tests/overlapping_methods/mock.go
diff --git a/mockgen/internal/tests/overlapping_methods/overlap.go b/mockgen/internal/tests/overlapping_methods/overlap.go
@@ -0,0 +1,9 @@
+// +build go1.14
+
+package overlap
+
+//go:generate mockgen -package overlap -destination mock.go -source overlap.go -aux_files github.com/golang/mock/mockgen/internal/tests/overlapping_methods=interfaces.go
+type ReadWriteCloser interface {
+	ReadCloser
+	WriteCloser
+}
diff --git a/mockgen/internal/tests/overlapping_methods/overlap_test.go b/mockgen/internal/tests/overlapping_methods/overlap_test.go
@@ -0,0 +1,21 @@
+// +build go1.14
+
+package overlap
+
+import (
+	"errors"
+	"testing"
+
+	gomock "github.com/golang/mock/gomock"
+)
+
+// TestValidInterface assesses whether or not the generated mock is valid
+func TestValidInterface(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	s := NewMockReadWriteCloser(ctrl)
+	s.EXPECT().Close().Return(errors.New("test"))
+
+	s.Close()
+}
diff --git a/mockgen/mockgen_test.go b/mockgen/mockgen_test.go
@@ -237,10 +237,12 @@ func TestGenerateMockInterface_Helper(t *testing.T) {
 				}
 			}
 
-			if err := g.GenerateMockInterface(&model.Interface{
-				Name:    "Somename",
-				Methods: test.Methods,
-			}, "somepackage"); err != nil {
+			intf := &model.Interface{Name: "Somename"}
+			for _, m := range test.Methods {
+				intf.AddMethod(m)
+			}
+
+			if err := g.GenerateMockInterface(intf, "somepackage"); err != nil {
 				t.Fatal(err)
 			}
 
diff --git a/mockgen/model/model.go b/mockgen/model/model.go
@@ -71,6 +71,16 @@ func (intf *Interface) addImports(im map[string]bool) {
 	}
 }
 
+// AddMethod adds a new method, deduplicating by method name.
+func (intf *Interface) AddMethod(m *Method) {
+	for _, me := range intf.Methods {
+		if me.Name == m.Name {
+			return
+		}
+	}
+	intf.Methods = append(intf.Methods, m)
+}
+
 // Method is a single method of an interface.
 type Method struct {
 	Name     string
@@ -311,7 +321,7 @@ func InterfaceFromInterfaceType(it reflect.Type) (*Interface, error) {
 			return nil, err
 		}
 
-		intf.Methods = append(intf.Methods, m)
+		intf.AddMethod(m)
 	}
 
 	return intf, nil
diff --git a/mockgen/parse.go b/mockgen/parse.go
@@ -267,7 +267,7 @@ func (p *fileParser) parseInterface(name, pkg string, it *ast.InterfaceType) (*m
 			if err != nil {
 				return nil, err
 			}
-			intf.Methods = append(intf.Methods, m)
+			intf.AddMethod(m)
 		case *ast.Ident:
 			// Embedded interface in this package.
 			ei := p.auxInterfaces[pkg][v.String()]
@@ -291,8 +291,9 @@ func (p *fileParser) parseInterface(name, pkg string, it *ast.InterfaceType) (*m
 				}
 			}
 			// Copy the methods.
-			// TODO: apply shadowing rules.
-			intf.Methods = append(intf.Methods, eintf.Methods...)
+			for _, m := range eintf.Methods {
+				intf.AddMethod(m)
+			}
 		case *ast.SelectorExpr:
 			// Embedded interface in another package.
 			fpkg, sel := v.X.(*ast.Ident).String(), v.Sel.String()
@@ -333,7 +334,9 @@ func (p *fileParser) parseInterface(name, pkg string, it *ast.InterfaceType) (*m
 			}
 			// Copy the methods.
 			// TODO: apply shadowing rules.
-			intf.Methods = append(intf.Methods, eintf.Methods...)
+			for _, m := range eintf.Methods {
+				intf.AddMethod(m)
+			}
 		default:
 			return nil, fmt.Errorf("don't know how to mock method of type %T", field.Type)
 		}

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +module github.com/golang/mock/mockgen/internal/tests/overlapping_methods
++
 +go 1.14
++
 +require github.com/golang/mock v1.4.4
Original file line number	Diff line number	Diff line change
`@@ -237,10 +237,12 @@ func TestGenerateMockInterface_Helper(t *testing.T) {`
`237`	`237`	`}`
`238`	`238`	`}`
`239`	`239`
`240`		`- if err := g.GenerateMockInterface(&model.Interface{`
`241`		`- Name: "Somename",`
`242`		`- Methods: test.Methods,`
`243`		`- }, "somepackage"); err != nil {`
	`240`	`+ intf := &model.Interface{Name: "Somename"}`
	`241`	`+ for _, m := range test.Methods {`
	`242`	`+ intf.AddMethod(m)`
	`243`	`+ }`
	`244`	`+`
	`245`	`+ if err := g.GenerateMockInterface(intf, "somepackage"); err != nil {`
`244`	`246`	`t.Fatal(err)`
`245`	`247`	`}`
`246`	`248`