From eb6764164a8d567c950e3ab60d8d3885cf40c7da Mon Sep 17 00:00:00 2001 From: Boston Cartwright Date: Mon, 28 Oct 2024 12:50:17 -0600 Subject: [PATCH] fix: import and arg collision (#219) This PR fixes an issue where if the name of an argument to a method that is being mocked collides with the name of a package that needs to be imported, mockgen could generate uncompilable code. Resolves #218 --- .../import_collision/internalpackage/foo.go | 3 + .../tests/import_collision/p2/mocks/mocks.go | 55 +++++++++++++++++++ .../internal/tests/import_collision/p2/p2.go | 12 ++++ .../tests/package_mode/mock/interfaces.go | 8 +-- mockgen/mockgen.go | 18 +++++- 5 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 mockgen/internal/tests/import_collision/internalpackage/foo.go create mode 100644 mockgen/internal/tests/import_collision/p2/mocks/mocks.go create mode 100644 mockgen/internal/tests/import_collision/p2/p2.go diff --git a/mockgen/internal/tests/import_collision/internalpackage/foo.go b/mockgen/internal/tests/import_collision/internalpackage/foo.go new file mode 100644 index 0000000..eaedd5b --- /dev/null +++ b/mockgen/internal/tests/import_collision/internalpackage/foo.go @@ -0,0 +1,3 @@ +package internalpackage + +type FooExported struct{} diff --git a/mockgen/internal/tests/import_collision/p2/mocks/mocks.go b/mockgen/internal/tests/import_collision/p2/mocks/mocks.go new file mode 100644 index 0000000..3b8fca6 --- /dev/null +++ b/mockgen/internal/tests/import_collision/p2/mocks/mocks.go @@ -0,0 +1,55 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: go.uber.org/mock/mockgen/internal/tests/import_collision/p2 (interfaces: Mything) +// +// Generated by this command: +// +// mockgen -destination=mocks/mocks.go -package=internalpackage . Mything +// + +// Package internalpackage is a generated GoMock package. +package internalpackage + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" + internalpackage "go.uber.org/mock/mockgen/internal/tests/import_collision/internalpackage" +) + +// MockMything is a mock of Mything interface. +type MockMything struct { + ctrl *gomock.Controller + recorder *MockMythingMockRecorder + isgomock struct{} +} + +// MockMythingMockRecorder is the mock recorder for MockMything. +type MockMythingMockRecorder struct { + mock *MockMything +} + +// NewMockMything creates a new mock instance. +func NewMockMything(ctrl *gomock.Controller) *MockMything { + mock := &MockMything{ctrl: ctrl} + mock.recorder = &MockMythingMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockMything) EXPECT() *MockMythingMockRecorder { + return m.recorder +} + +// DoThat mocks base method. +func (m *MockMything) DoThat(arg0 int) internalpackage.FooExported { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DoThat", arg0) + ret0, _ := ret[0].(internalpackage.FooExported) + return ret0 +} + +// DoThat indicates an expected call of DoThat. +func (mr *MockMythingMockRecorder) DoThat(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DoThat", reflect.TypeOf((*MockMything)(nil).DoThat), arg0) +} diff --git a/mockgen/internal/tests/import_collision/p2/p2.go b/mockgen/internal/tests/import_collision/p2/p2.go new file mode 100644 index 0000000..e6200b7 --- /dev/null +++ b/mockgen/internal/tests/import_collision/p2/p2.go @@ -0,0 +1,12 @@ +package p2 + +//go:generate mockgen -destination=mocks/mocks.go -package=internalpackage . Mything + +import ( + "go.uber.org/mock/mockgen/internal/tests/import_collision/internalpackage" +) + +type Mything interface { + // issue here, is that the variable has the same name as an imported package. + DoThat(internalpackage int) internalpackage.FooExported +} diff --git a/mockgen/internal/tests/package_mode/mock/interfaces.go b/mockgen/internal/tests/package_mode/mock/interfaces.go index 06061a7..60a0ad4 100644 --- a/mockgen/internal/tests/package_mode/mock/interfaces.go +++ b/mockgen/internal/tests/package_mode/mock/interfaces.go @@ -654,17 +654,17 @@ func (c *MockCarFuelTankCall[FuelType]) DoAndReturn(f func() cars.FuelTank[FuelT } // Refuel mocks base method. -func (m *MockCar[FuelType]) Refuel(fuel FuelType, volume int) error { +func (m *MockCar[FuelType]) Refuel(arg0 FuelType, volume int) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Refuel", fuel, volume) + ret := m.ctrl.Call(m, "Refuel", arg0, volume) ret0, _ := ret[0].(error) return ret0 } // Refuel indicates an expected call of Refuel. -func (mr *MockCarMockRecorder[FuelType]) Refuel(fuel, volume any) *MockCarRefuelCall[FuelType] { +func (mr *MockCarMockRecorder[FuelType]) Refuel(arg0, volume any) *MockCarRefuelCall[FuelType] { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Refuel", reflect.TypeOf((*MockCar[FuelType])(nil).Refuel), fuel, volume) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Refuel", reflect.TypeOf((*MockCar[FuelType])(nil).Refuel), arg0, volume) return &MockCarRefuelCall[FuelType]{Call: call} } diff --git a/mockgen/mockgen.go b/mockgen/mockgen.go index b5365de..75d1a69 100644 --- a/mockgen/mockgen.go +++ b/mockgen/mockgen.go @@ -758,6 +758,17 @@ func (g *generator) GenerateMockReturnCallMethod(intf *model.Interface, m *model return nil } +// nameExistsAsPackage returns true if the name exists as a package name. +// This is used to avoid name collisions when generating mock method arguments. +func (g *generator) nameExistsAsPackage(name string) bool { + for _, symbolName := range g.packageMap { + if symbolName == name { + return true + } + } + return false +} + func (g *generator) getArgNames(m *model.Method, in bool) []string { var params []*model.Parameter if in { @@ -766,16 +777,19 @@ func (g *generator) getArgNames(m *model.Method, in bool) []string { params = m.Out } argNames := make([]string, len(params)) + for i, p := range params { name := p.Name - if name == "" || name == "_" { + + if name == "" || name == "_" || g.nameExistsAsPackage(name) { name = fmt.Sprintf("arg%d", i) } argNames[i] = name } if m.Variadic != nil && in { name := m.Variadic.Name - if name == "" { + + if name == "" || g.nameExistsAsPackage(name) { name = fmt.Sprintf("arg%d", len(params)) } argNames = append(argNames, name)