From b9a8584115bc0d16e10b15cb07d680c5f68f5eb1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 29 Jan 2021 06:47:31 +0800 Subject: [PATCH] deduplicate methods to allow overlapping methods on embedded interfaces (#498) --- .github/workflows/test.yaml | 7 +- ci/check_go_generate.sh | 8 +- ci/check_go_mod.sh | 6 +- .../internal/tests/overlapping_methods/go.mod | 5 ++ .../internal/tests/overlapping_methods/go.sum | 8 ++ .../tests/overlapping_methods/interfaces.go | 13 ++++ .../tests/overlapping_methods/mock.go | 78 +++++++++++++++++++ .../tests/overlapping_methods/overlap.go | 9 +++ .../tests/overlapping_methods/overlap_test.go | 21 +++++ mockgen/mockgen_test.go | 10 ++- mockgen/model/model.go | 12 ++- mockgen/parse.go | 11 ++- 12 files changed, 176 insertions(+), 12 deletions(-) create mode 100644 mockgen/internal/tests/overlapping_methods/go.mod create mode 100644 mockgen/internal/tests/overlapping_methods/go.sum create mode 100644 mockgen/internal/tests/overlapping_methods/interfaces.go create mode 100644 mockgen/internal/tests/overlapping_methods/mock.go create mode 100644 mockgen/internal/tests/overlapping_methods/overlap.go create mode 100644 mockgen/internal/tests/overlapping_methods/overlap_test.go diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e9d42704..b7210b3d 100644 --- 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 index d906bd58..84f0b6f3 100755 --- 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 index 262f74c5..abe8b3c1 100755 --- 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 new file mode 100644 index 00000000..f502f4a0 --- /dev/null +++ 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 new file mode 100644 index 00000000..11ec9a42 --- /dev/null +++ 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 new file mode 100644 index 00000000..d7780598 --- /dev/null +++ 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 new file mode 100644 index 00000000..20cbc397 --- /dev/null +++ b/mockgen/internal/tests/overlapping_methods/mock.go @@ -0,0 +1,78 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: overlap.go + +// Package overlap is a generated GoMock package. +package overlap + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockReadWriteCloser is a mock of ReadWriteCloser interface. +type MockReadWriteCloser struct { + ctrl *gomock.Controller + recorder *MockReadWriteCloserMockRecorder +} + +// MockReadWriteCloserMockRecorder is the mock recorder for MockReadWriteCloser. +type MockReadWriteCloserMockRecorder struct { + mock *MockReadWriteCloser +} + +// NewMockReadWriteCloser creates a new mock instance. +func NewMockReadWriteCloser(ctrl *gomock.Controller) *MockReadWriteCloser { + mock := &MockReadWriteCloser{ctrl: ctrl} + mock.recorder = &MockReadWriteCloserMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockReadWriteCloser) EXPECT() *MockReadWriteCloserMockRecorder { + return m.recorder +} + +// Close mocks base method. +func (m *MockReadWriteCloser) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close. +func (mr *MockReadWriteCloserMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockReadWriteCloser)(nil).Close)) +} + +// Read mocks base method. +func (m *MockReadWriteCloser) Read(arg0 []byte) (int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Read", arg0) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Read indicates an expected call of Read. +func (mr *MockReadWriteCloserMockRecorder) Read(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Read", reflect.TypeOf((*MockReadWriteCloser)(nil).Read), arg0) +} + +// Write mocks base method. +func (m *MockReadWriteCloser) Write(arg0 []byte) (int, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Write", arg0) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Write indicates an expected call of Write. +func (mr *MockReadWriteCloserMockRecorder) Write(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Write", reflect.TypeOf((*MockReadWriteCloser)(nil).Write), arg0) +} diff --git a/mockgen/internal/tests/overlapping_methods/overlap.go b/mockgen/internal/tests/overlapping_methods/overlap.go new file mode 100644 index 00000000..2baf4ffe --- /dev/null +++ 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 new file mode 100644 index 00000000..234e0445 --- /dev/null +++ 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 index 50fe5ed3..3c3eaae2 100644 --- 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 index c5428c29..d06d5162 100644 --- 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 index dfdad8ab..ef8a9595 100644 --- 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) }