From e36794b0ef332ee4d5896efe3035b9e6f1daa044 Mon Sep 17 00:00:00 2001 From: joaks Date: Wed, 23 Oct 2024 22:55:23 +0000 Subject: [PATCH 1/2] Package Mode: Use aliases when used in source v0.5.0 included #207, which replaced reflect mode with package mode. One issue with package mode that came up (ref: #216) was that generated mocks for interfaces that referred to alias types were referring to the aliases' underlying names instead. e.g., source: ```go import "github.com/tikv/client-go/v2/internal/apicodec" ... type Codec = apicodec.Codec type Foo interface{ Bar() Codec } ``` mock: ```go func (m *MockFoo) Bar() apicodec.Codec { // This is a problem, since apicodec is an internal package. // ... } ``` While technically this problem is solved in Go 1.23 with explicit alias types representation, (indeed, if you run mockgen on the example in the linked issue with `GODEBUG=gotypesalias=1`, you get the expected behavior) since we support the last two versions, we can't bump `go.mod` to 1.23 yet. This leaves us with the old behavior, where `go/types` does not track alias types. You can tell if an object is an alias, but not a type itself, and there is no way to retrieve the object of interest at the point where we are recursively parsing method types. This PR works around this issue (temporarily) by using syntax information to find all references to aliases in the source package. When we find one, we record it in a mapping of underlying type -> alias name. Later, while we parse the type tree, we replace any underlying types in the mapping with their alias names. The unexpected side effect of this is that _all_ references to the underlying type in the generated mocks will be replaced with the alias, even if the source used the underlying name. This is fine because: * If the alias is in the mapping, it was used at least once, which means its accessible. * From a type-checking perspective, aliases and their underlying types are equivalent. With this PR, the mocks get generated correctly now: ```go func (m *MockFoo) Bar() Codec { // ... } ``` Once we can bump `go.mod` to 1.23, we should definitely remove this, since the new type alias type nodes solve this problem automatically. --- mockgen/internal/tests/alias/interfaces.go | 47 ++ .../internal/tests/alias/mock/interfaces.go | 448 ++++++++++++++++++ .../tests/alias/mock/interfaces_test.go | 22 + .../internal/tests/alias/subpkg/interfaces.go | 11 + .../tests/package_mode/mock/interfaces.go | 10 +- mockgen/package_mode.go | 116 ++++- mockgen/package_mode_test.go | 157 +++++- 7 files changed, 802 insertions(+), 9 deletions(-) create mode 100644 mockgen/internal/tests/alias/interfaces.go create mode 100644 mockgen/internal/tests/alias/mock/interfaces.go create mode 100644 mockgen/internal/tests/alias/mock/interfaces_test.go create mode 100644 mockgen/internal/tests/alias/subpkg/interfaces.go diff --git a/mockgen/internal/tests/alias/interfaces.go b/mockgen/internal/tests/alias/interfaces.go new file mode 100644 index 0000000..699dadc --- /dev/null +++ b/mockgen/internal/tests/alias/interfaces.go @@ -0,0 +1,47 @@ +package alias + +//go:generate mockgen -typed -package=mock -destination=mock/interfaces.go . Fooer,FooerAlias,Barer,BarerAlias,Bazer,QuxerConsumer,QuuxerConsumer + +import "go.uber.org/mock/mockgen/internal/tests/alias/subpkg" + +// Case 1: A interface that has alias references in this package +// should still be generated for its underlying name, i.e., MockFooer, +// even though we have the alias replacement logic. +type Fooer interface { + Foo() +} + +// Case 2: Generating a mock for an alias type. +type FooerAlias = Fooer + +// Case 3: Generate mock for an interface that takes in alias parameters +// and returns alias results. +type Barer interface{ + Bar(FooerAlias) FooerAlias +} + +// Case 4: Combination of cases 2 & 3. +type BarerAlias = Barer + +// Case 5: Generate mock for an interface that actually returns +// the underlying type. This will generate mocks that use the alias, +// but that should be fine since they should be interchangeable. +type Bazer interface{ + Baz(Fooer) Fooer +} + +// Case 6: Generate mock for a type that refers to an alias defined in this package +// for a type from another package. +// The generated methods should use the alias defined here. +type QuxerAlias = subpkg.Quxer + +type QuxerConsumer interface{ + Consume(QuxerAlias) QuxerAlias +} + +// Case 7: Generate mock for a type that refers to an alias defined in another package +// for an unexported type in that other package. +// The generated method should only use the alias, not the unexported underlying name. +type QuuxerConsumer interface{ + Consume(subpkg.Quuxer) subpkg.Quuxer +} diff --git a/mockgen/internal/tests/alias/mock/interfaces.go b/mockgen/internal/tests/alias/mock/interfaces.go new file mode 100644 index 0000000..4bf739c --- /dev/null +++ b/mockgen/internal/tests/alias/mock/interfaces.go @@ -0,0 +1,448 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: go.uber.org/mock/mockgen/internal/tests/alias (interfaces: Fooer,FooerAlias,Barer,BarerAlias,Bazer,QuxerConsumer,QuuxerConsumer) +// +// Generated by this command: +// +// mockgen -typed -package=mock -destination=mock/interfaces.go . Fooer,FooerAlias,Barer,BarerAlias,Bazer,QuxerConsumer,QuuxerConsumer +// + +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" + alias "go.uber.org/mock/mockgen/internal/tests/alias" + subpkg "go.uber.org/mock/mockgen/internal/tests/alias/subpkg" +) + +// MockFooer is a mock of Fooer interface. +type MockFooer struct { + ctrl *gomock.Controller + recorder *MockFooerMockRecorder + isgomock struct{} +} + +// MockFooerMockRecorder is the mock recorder for MockFooer. +type MockFooerMockRecorder struct { + mock *MockFooer +} + +// NewMockFooer creates a new mock instance. +func NewMockFooer(ctrl *gomock.Controller) *MockFooer { + mock := &MockFooer{ctrl: ctrl} + mock.recorder = &MockFooerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFooer) EXPECT() *MockFooerMockRecorder { + return m.recorder +} + +// Foo mocks base method. +func (m *MockFooer) Foo() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Foo") +} + +// Foo indicates an expected call of Foo. +func (mr *MockFooerMockRecorder) Foo() *MockFooerFooCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Foo", reflect.TypeOf((*MockFooer)(nil).Foo)) + return &MockFooerFooCall{Call: call} +} + +// MockFooerFooCall wrap *gomock.Call +type MockFooerFooCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockFooerFooCall) Return() *MockFooerFooCall { + c.Call = c.Call.Return() + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockFooerFooCall) Do(f func()) *MockFooerFooCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockFooerFooCall) DoAndReturn(f func()) *MockFooerFooCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockFooerAlias is a mock of FooerAlias interface. +type MockFooerAlias struct { + ctrl *gomock.Controller + recorder *MockFooerAliasMockRecorder + isgomock struct{} +} + +// MockFooerAliasMockRecorder is the mock recorder for MockFooerAlias. +type MockFooerAliasMockRecorder struct { + mock *MockFooerAlias +} + +// NewMockFooerAlias creates a new mock instance. +func NewMockFooerAlias(ctrl *gomock.Controller) *MockFooerAlias { + mock := &MockFooerAlias{ctrl: ctrl} + mock.recorder = &MockFooerAliasMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFooerAlias) EXPECT() *MockFooerAliasMockRecorder { + return m.recorder +} + +// Foo mocks base method. +func (m *MockFooerAlias) Foo() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Foo") +} + +// Foo indicates an expected call of Foo. +func (mr *MockFooerAliasMockRecorder) Foo() *MockFooerAliasFooCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Foo", reflect.TypeOf((*MockFooerAlias)(nil).Foo)) + return &MockFooerAliasFooCall{Call: call} +} + +// MockFooerAliasFooCall wrap *gomock.Call +type MockFooerAliasFooCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockFooerAliasFooCall) Return() *MockFooerAliasFooCall { + c.Call = c.Call.Return() + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockFooerAliasFooCall) Do(f func()) *MockFooerAliasFooCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockFooerAliasFooCall) DoAndReturn(f func()) *MockFooerAliasFooCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockBarer is a mock of Barer interface. +type MockBarer struct { + ctrl *gomock.Controller + recorder *MockBarerMockRecorder + isgomock struct{} +} + +// MockBarerMockRecorder is the mock recorder for MockBarer. +type MockBarerMockRecorder struct { + mock *MockBarer +} + +// NewMockBarer creates a new mock instance. +func NewMockBarer(ctrl *gomock.Controller) *MockBarer { + mock := &MockBarer{ctrl: ctrl} + mock.recorder = &MockBarerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockBarer) EXPECT() *MockBarerMockRecorder { + return m.recorder +} + +// Bar mocks base method. +func (m *MockBarer) Bar(arg0 alias.FooerAlias) alias.FooerAlias { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Bar", arg0) + ret0, _ := ret[0].(alias.FooerAlias) + return ret0 +} + +// Bar indicates an expected call of Bar. +func (mr *MockBarerMockRecorder) Bar(arg0 any) *MockBarerBarCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Bar", reflect.TypeOf((*MockBarer)(nil).Bar), arg0) + return &MockBarerBarCall{Call: call} +} + +// MockBarerBarCall wrap *gomock.Call +type MockBarerBarCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockBarerBarCall) Return(arg0 alias.FooerAlias) *MockBarerBarCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockBarerBarCall) Do(f func(alias.FooerAlias) alias.FooerAlias) *MockBarerBarCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockBarerBarCall) DoAndReturn(f func(alias.FooerAlias) alias.FooerAlias) *MockBarerBarCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockBarerAlias is a mock of BarerAlias interface. +type MockBarerAlias struct { + ctrl *gomock.Controller + recorder *MockBarerAliasMockRecorder + isgomock struct{} +} + +// MockBarerAliasMockRecorder is the mock recorder for MockBarerAlias. +type MockBarerAliasMockRecorder struct { + mock *MockBarerAlias +} + +// NewMockBarerAlias creates a new mock instance. +func NewMockBarerAlias(ctrl *gomock.Controller) *MockBarerAlias { + mock := &MockBarerAlias{ctrl: ctrl} + mock.recorder = &MockBarerAliasMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockBarerAlias) EXPECT() *MockBarerAliasMockRecorder { + return m.recorder +} + +// Bar mocks base method. +func (m *MockBarerAlias) Bar(arg0 alias.FooerAlias) alias.FooerAlias { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Bar", arg0) + ret0, _ := ret[0].(alias.FooerAlias) + return ret0 +} + +// Bar indicates an expected call of Bar. +func (mr *MockBarerAliasMockRecorder) Bar(arg0 any) *MockBarerAliasBarCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Bar", reflect.TypeOf((*MockBarerAlias)(nil).Bar), arg0) + return &MockBarerAliasBarCall{Call: call} +} + +// MockBarerAliasBarCall wrap *gomock.Call +type MockBarerAliasBarCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockBarerAliasBarCall) Return(arg0 alias.FooerAlias) *MockBarerAliasBarCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockBarerAliasBarCall) Do(f func(alias.FooerAlias) alias.FooerAlias) *MockBarerAliasBarCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockBarerAliasBarCall) DoAndReturn(f func(alias.FooerAlias) alias.FooerAlias) *MockBarerAliasBarCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockBazer is a mock of Bazer interface. +type MockBazer struct { + ctrl *gomock.Controller + recorder *MockBazerMockRecorder + isgomock struct{} +} + +// MockBazerMockRecorder is the mock recorder for MockBazer. +type MockBazerMockRecorder struct { + mock *MockBazer +} + +// NewMockBazer creates a new mock instance. +func NewMockBazer(ctrl *gomock.Controller) *MockBazer { + mock := &MockBazer{ctrl: ctrl} + mock.recorder = &MockBazerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockBazer) EXPECT() *MockBazerMockRecorder { + return m.recorder +} + +// Baz mocks base method. +func (m *MockBazer) Baz(arg0 alias.FooerAlias) alias.FooerAlias { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Baz", arg0) + ret0, _ := ret[0].(alias.FooerAlias) + return ret0 +} + +// Baz indicates an expected call of Baz. +func (mr *MockBazerMockRecorder) Baz(arg0 any) *MockBazerBazCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Baz", reflect.TypeOf((*MockBazer)(nil).Baz), arg0) + return &MockBazerBazCall{Call: call} +} + +// MockBazerBazCall wrap *gomock.Call +type MockBazerBazCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockBazerBazCall) Return(arg0 alias.FooerAlias) *MockBazerBazCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockBazerBazCall) Do(f func(alias.FooerAlias) alias.FooerAlias) *MockBazerBazCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockBazerBazCall) DoAndReturn(f func(alias.FooerAlias) alias.FooerAlias) *MockBazerBazCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockQuxerConsumer is a mock of QuxerConsumer interface. +type MockQuxerConsumer struct { + ctrl *gomock.Controller + recorder *MockQuxerConsumerMockRecorder + isgomock struct{} +} + +// MockQuxerConsumerMockRecorder is the mock recorder for MockQuxerConsumer. +type MockQuxerConsumerMockRecorder struct { + mock *MockQuxerConsumer +} + +// NewMockQuxerConsumer creates a new mock instance. +func NewMockQuxerConsumer(ctrl *gomock.Controller) *MockQuxerConsumer { + mock := &MockQuxerConsumer{ctrl: ctrl} + mock.recorder = &MockQuxerConsumerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockQuxerConsumer) EXPECT() *MockQuxerConsumerMockRecorder { + return m.recorder +} + +// Consume mocks base method. +func (m *MockQuxerConsumer) Consume(arg0 alias.QuxerAlias) alias.QuxerAlias { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Consume", arg0) + ret0, _ := ret[0].(alias.QuxerAlias) + return ret0 +} + +// Consume indicates an expected call of Consume. +func (mr *MockQuxerConsumerMockRecorder) Consume(arg0 any) *MockQuxerConsumerConsumeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Consume", reflect.TypeOf((*MockQuxerConsumer)(nil).Consume), arg0) + return &MockQuxerConsumerConsumeCall{Call: call} +} + +// MockQuxerConsumerConsumeCall wrap *gomock.Call +type MockQuxerConsumerConsumeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockQuxerConsumerConsumeCall) Return(arg0 alias.QuxerAlias) *MockQuxerConsumerConsumeCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockQuxerConsumerConsumeCall) Do(f func(alias.QuxerAlias) alias.QuxerAlias) *MockQuxerConsumerConsumeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockQuxerConsumerConsumeCall) DoAndReturn(f func(alias.QuxerAlias) alias.QuxerAlias) *MockQuxerConsumerConsumeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// MockQuuxerConsumer is a mock of QuuxerConsumer interface. +type MockQuuxerConsumer struct { + ctrl *gomock.Controller + recorder *MockQuuxerConsumerMockRecorder + isgomock struct{} +} + +// MockQuuxerConsumerMockRecorder is the mock recorder for MockQuuxerConsumer. +type MockQuuxerConsumerMockRecorder struct { + mock *MockQuuxerConsumer +} + +// NewMockQuuxerConsumer creates a new mock instance. +func NewMockQuuxerConsumer(ctrl *gomock.Controller) *MockQuuxerConsumer { + mock := &MockQuuxerConsumer{ctrl: ctrl} + mock.recorder = &MockQuuxerConsumerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockQuuxerConsumer) EXPECT() *MockQuuxerConsumerMockRecorder { + return m.recorder +} + +// Consume mocks base method. +func (m *MockQuuxerConsumer) Consume(arg0 subpkg.Quuxer) subpkg.Quuxer { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Consume", arg0) + ret0, _ := ret[0].(subpkg.Quuxer) + return ret0 +} + +// Consume indicates an expected call of Consume. +func (mr *MockQuuxerConsumerMockRecorder) Consume(arg0 any) *MockQuuxerConsumerConsumeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Consume", reflect.TypeOf((*MockQuuxerConsumer)(nil).Consume), arg0) + return &MockQuuxerConsumerConsumeCall{Call: call} +} + +// MockQuuxerConsumerConsumeCall wrap *gomock.Call +type MockQuuxerConsumerConsumeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockQuuxerConsumerConsumeCall) Return(arg0 subpkg.Quuxer) *MockQuuxerConsumerConsumeCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockQuuxerConsumerConsumeCall) Do(f func(subpkg.Quuxer) subpkg.Quuxer) *MockQuuxerConsumerConsumeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockQuuxerConsumerConsumeCall) DoAndReturn(f func(subpkg.Quuxer) subpkg.Quuxer) *MockQuuxerConsumerConsumeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} diff --git a/mockgen/internal/tests/alias/mock/interfaces_test.go b/mockgen/internal/tests/alias/mock/interfaces_test.go new file mode 100644 index 0000000..43cabdf --- /dev/null +++ b/mockgen/internal/tests/alias/mock/interfaces_test.go @@ -0,0 +1,22 @@ +package mock + +import ( + "go.uber.org/mock/mockgen/internal/tests/alias" +) + +// This checks for type-checking equivalent of mock types. +// If something does not resolve, the tests will not compile. + +var ( + _ alias.Fooer = &MockFooer{} + _ alias.FooerAlias = &MockFooer{} + _ alias.Fooer = &MockFooerAlias{} + _ alias.FooerAlias = &MockFooerAlias{} + _ alias.Barer = &MockBarer{} + _ alias.BarerAlias = &MockBarer{} + _ alias.Barer = &MockBarerAlias{} + _ alias.BarerAlias = &MockBarerAlias{} + _ alias.Bazer = &MockBazer{} + _ alias.QuxerConsumer = &MockQuxerConsumer{} + _ alias.QuuxerConsumer = &MockQuuxerConsumer{} +) diff --git a/mockgen/internal/tests/alias/subpkg/interfaces.go b/mockgen/internal/tests/alias/subpkg/interfaces.go new file mode 100644 index 0000000..f33f411 --- /dev/null +++ b/mockgen/internal/tests/alias/subpkg/interfaces.go @@ -0,0 +1,11 @@ +package subpkg + +type Quxer interface { + Qux() +} + +type quuxerUnexported interface{ + Quux(Quxer) Quxer +} + +type Quuxer = quuxerUnexported diff --git a/mockgen/internal/tests/package_mode/mock/interfaces.go b/mockgen/internal/tests/package_mode/mock/interfaces.go index 1cb6ffd..06061a7 100644 --- a/mockgen/internal/tests/package_mode/mock/interfaces.go +++ b/mockgen/internal/tests/package_mode/mock/interfaces.go @@ -1382,10 +1382,10 @@ func (m *MockEarth) EXPECT() *MockEarthMockRecorder { } // AddHumans mocks base method. -func (m *MockEarth) AddHumans(arg0 int) []package_mode.Primate { +func (m *MockEarth) AddHumans(arg0 int) []package_mode.Human { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddHumans", arg0) - ret0, _ := ret[0].([]package_mode.Primate) + ret0, _ := ret[0].([]package_mode.Human) return ret0 } @@ -1402,19 +1402,19 @@ type MockEarthAddHumansCall struct { } // Return rewrite *gomock.Call.Return -func (c *MockEarthAddHumansCall) Return(arg0 []package_mode.Primate) *MockEarthAddHumansCall { +func (c *MockEarthAddHumansCall) Return(arg0 []package_mode.Human) *MockEarthAddHumansCall { c.Call = c.Call.Return(arg0) return c } // Do rewrite *gomock.Call.Do -func (c *MockEarthAddHumansCall) Do(f func(int) []package_mode.Primate) *MockEarthAddHumansCall { +func (c *MockEarthAddHumansCall) Do(f func(int) []package_mode.Human) *MockEarthAddHumansCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockEarthAddHumansCall) DoAndReturn(f func(int) []package_mode.Primate) *MockEarthAddHumansCall { +func (c *MockEarthAddHumansCall) DoAndReturn(f func(int) []package_mode.Human) *MockEarthAddHumansCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/mockgen/package_mode.go b/mockgen/package_mode.go index abc9c7a..cbd6dfd 100644 --- a/mockgen/package_mode.go +++ b/mockgen/package_mode.go @@ -4,6 +4,7 @@ import ( "errors" "flag" "fmt" + "go/ast" "go/types" "strings" @@ -17,6 +18,20 @@ var ( type packageModeParser struct { pkgName string + + // Mapping from underlying types to aliases used within the package source. + // + // We prefer to use aliases used in the source rather than underlying type names + // as those may be unexported or internal. + // Once mock is Go1.23+ only, we can remove this + // as the casing for types.Alias will automatically handle this + // in all cases. + aliasReplacements map[types.Type]aliasReplacement +} + +type aliasReplacement struct { + name string + pkg string } func (p *packageModeParser) parsePackage(packageName string, ifaces []string) (*model.Package, error) { @@ -27,6 +42,8 @@ func (p *packageModeParser) parsePackage(packageName string, ifaces []string) (* return nil, fmt.Errorf("load package: %w", err) } + p.buildAliasReplacements(pkg) + interfaces, err := p.extractInterfacesFromPackage(pkg, ifaces) if err != nil { return nil, fmt.Errorf("extract interfaces from package: %w", err) @@ -39,6 +56,90 @@ func (p *packageModeParser) parsePackage(packageName string, ifaces []string) (* }, nil } +// buildAliasReplacements finds and records any references to aliases +// within the given package's source. +// These aliases will be preferred when parsing types +// over the underlying name counterparts, as those may be unexported / internal. +// +// If a type has more than one alias within the source package, +// the latest one to be inspected will be the one used for mapping. +// This is fine, since all aliases and their underlying types are interchangeable +// from a type-checking standpoint. +func (p *packageModeParser) buildAliasReplacements(pkg *packages.Package) { + p.aliasReplacements = make(map[types.Type]aliasReplacement) + + // checkIdent checks if the given identifier exists + // in the given package as an alias, and adds it to + // the alias replacements map if so. + checkIdent := func(pkg *types.Package, ident string) bool { + scope := pkg.Scope() + if scope == nil { + return true + } + obj := scope.Lookup(ident) + if obj == nil { + return true + } + objTypeName, ok := obj.(*types.TypeName) + if !ok { + return true + } + if !objTypeName.IsAlias() { + return true + } + typ := objTypeName.Type() + if typ == nil { + return true + } + p.aliasReplacements[typ] = aliasReplacement{ + name: objTypeName.Name(), + pkg: pkg.Path(), + } + return false + + } + + for _, f := range pkg.Syntax { + fileScope, ok := pkg.TypesInfo.Scopes[f] + if !ok { + continue + } + ast.Inspect(f, func(node ast.Node) bool { + + // Simple identifiers: check if it is an alias + // from the source package. + if ident, ok := node.(*ast.Ident); ok { + return checkIdent(pkg.Types, ident.String()) + } + + // Selector expressions: check if it is an alias + // from the package represented by the qualifier. + selExpr, ok := node.(*ast.SelectorExpr) + if !ok { + return true + } + + x, sel := selExpr.X, selExpr.Sel + xident, ok := x.(*ast.Ident) + if !ok { + return true + } + + xObj := fileScope.Lookup(xident.String()) + pkgName, ok := xObj.(*types.PkgName) + if !ok { + return true + } + + xPkg := pkgName.Imported() + if xPkg == nil { + return true + } + return checkIdent(xPkg, sel.String()) + }) + } +} + func (p *packageModeParser) loadPackage(packageName string) (*packages.Package, error) { var buildFlagsSet []string if *buildFlags != "" { @@ -46,7 +147,7 @@ func (p *packageModeParser) loadPackage(packageName string) (*packages.Package, } cfg := &packages.Config{ - Mode: packages.NeedDeps | packages.NeedImports | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedEmbedFiles, + Mode: packages.NeedDeps | packages.NeedImports | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedEmbedFiles | packages.LoadAllSyntax, BuildFlags: buildFlagsSet, } pkgs, err := packages.Load(cfg, packageName) @@ -193,17 +294,26 @@ func (p *packageModeParser) parseType(t types.Type) (model.Type, error) { return sig, nil case *types.Named, *types.Alias: object := t.(interface{ Obj() *types.TypeName }) + name := object.Obj().Name() var pkg string if object.Obj().Pkg() != nil { pkg = object.Obj().Pkg().Path() } + // If there was an alias to this type used somewhere in the source, + // use that alias instead of the underlying type, + // since the underlying type might be unexported. + if alias, ok := p.aliasReplacements[t]; ok { + name = alias.name + pkg = alias.pkg + } + // TypeArgs method not available for aliases in go1.22 genericType, ok := t.(interface{ TypeArgs() *types.TypeList }) if !ok || genericType.TypeArgs() == nil { return &model.NamedType{ Package: pkg, - Type: object.Obj().Name(), + Type: name, }, nil } @@ -220,7 +330,7 @@ func (p *packageModeParser) parseType(t types.Type) (model.Type, error) { return &model.NamedType{ Package: pkg, - Type: object.Obj().Name(), + Type: name, TypeParams: typeParams, }, nil case *types.Interface: diff --git a/mockgen/package_mode_test.go b/mockgen/package_mode_test.go index 5db836f..4e0ad44 100644 --- a/mockgen/package_mode_test.go +++ b/mockgen/package_mode_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/mock/mockgen/model" ) @@ -322,7 +323,7 @@ func Test_packageModeParser_parsePackage(t *testing.T) { Len: -1, // slice Type: &model.NamedType{ Package: "go.uber.org/mock/mockgen/internal/tests/package_mode", - Type: "Primate", + Type: "Human", }, }, }, @@ -358,3 +359,157 @@ func Test_packageModeParser_parsePackage(t *testing.T) { }) } } + +// This tests the alias replacement behavior of package mode. +// Along with the functionality itself, that can be removed +// once we bump go.mod to 1.23. +func TestAliases(t *testing.T) { + packageName := "go.uber.org/mock/mockgen/internal/tests/alias" + for _, tt := range []struct { + desc string + iface string + expected *model.Interface + }{ + { + desc: "interface with alias references elsewhere", + iface: "Fooer", + expected: &model.Interface{ + Name: "Fooer", + Methods: []*model.Method{{ + Name: "Foo", + }}, + }, + }, + { + desc: "alias to an interface in the same package", + iface: "FooerAlias", + expected: &model.Interface{ + Name: "FooerAlias", + Methods: []*model.Method{{ + Name: "Foo", + }}, + }, + }, + { + desc: "interface that takes/returns aliases from same package", + iface: "Barer", + expected: &model.Interface{ + Name: "Barer", + Methods: []*model.Method{{ + Name: "Bar", + In: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "FooerAlias", + }, + }}, + Out: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "FooerAlias", + }, + }}, + }}, + }, + }, + { + desc: "alias to an interface that takes/returns aliases from same package", + iface: "BarerAlias", + expected: &model.Interface{ + Name: "BarerAlias", + Methods: []*model.Method{{ + Name: "Bar", + In: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "FooerAlias", + }, + }}, + Out: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "FooerAlias", + }, + }}, + }}, + }, + }, + { + desc: "interface that refers to underlying name when alias is referenced in package", + iface: "Bazer", + expected: &model.Interface{ + Name: "Bazer", + Methods: []*model.Method{{ + Name: "Baz", + In: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "FooerAlias", + }, + }}, + Out: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "FooerAlias", + }, + }}, + }}, + }, + }, + { + desc: "interface that refers to an alias to another package type", + iface: "QuxerConsumer", + expected: &model.Interface{ + Name: "QuxerConsumer", + Methods: []*model.Method{{ + Name: "Consume", + In: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "QuxerAlias", + }, + }}, + Out: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias", + Type: "QuxerAlias", + }, + }}, + }}, + }, + }, + { + desc: "interface that refers to another package alias to another package type", + iface: "QuuxerConsumer", + expected: &model.Interface{ + Name: "QuuxerConsumer", + Methods: []*model.Method{{ + Name: "Consume", + In: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias/subpkg", + Type: "Quuxer", + }, + }}, + Out: []*model.Parameter{{ + Type: &model.NamedType{ + Package: "go.uber.org/mock/mockgen/internal/tests/alias/subpkg", + Type: "Quuxer", + }, + }}, + }}, + }, + }, + } { + t.Run(tt.desc, func(t *testing.T) { + var parser packageModeParser + actual, err := parser.parsePackage(packageName, []string{tt.iface}) + require.NoError(t, err) + require.NotNil(t, actual) + require.Len(t, actual.Interfaces, 1) + assert.Equal(t, "alias", actual.Name) + assert.Equal(t, "go.uber.org/mock/mockgen/internal/tests/alias", actual.PkgPath) + assert.Equal(t, tt.expected, actual.Interfaces[0]) + }) + } +} From eb1fdaf547e74c6577847cf343a7f6449ea45d60 Mon Sep 17 00:00:00 2001 From: joaks Date: Mon, 28 Oct 2024 14:34:57 +0000 Subject: [PATCH 2/2] LoadAllSyntax -> LoadSyntax, address comments --- mockgen/package_mode.go | 4 ++-- mockgen/package_mode_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mockgen/package_mode.go b/mockgen/package_mode.go index cbd6dfd..acbe487 100644 --- a/mockgen/package_mode.go +++ b/mockgen/package_mode.go @@ -23,7 +23,7 @@ type packageModeParser struct { // // We prefer to use aliases used in the source rather than underlying type names // as those may be unexported or internal. - // Once mock is Go1.23+ only, we can remove this + // TODO(joaks): Once mock is Go1.23+ only, we can remove this // as the casing for types.Alias will automatically handle this // in all cases. aliasReplacements map[types.Type]aliasReplacement @@ -147,7 +147,7 @@ func (p *packageModeParser) loadPackage(packageName string) (*packages.Package, } cfg := &packages.Config{ - Mode: packages.NeedDeps | packages.NeedImports | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedEmbedFiles | packages.LoadAllSyntax, + Mode: packages.NeedDeps | packages.NeedImports | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedEmbedFiles | packages.LoadSyntax, BuildFlags: buildFlagsSet, } pkgs, err := packages.Load(cfg, packageName) diff --git a/mockgen/package_mode_test.go b/mockgen/package_mode_test.go index 4e0ad44..2455bb1 100644 --- a/mockgen/package_mode_test.go +++ b/mockgen/package_mode_test.go @@ -361,8 +361,8 @@ func Test_packageModeParser_parsePackage(t *testing.T) { } // This tests the alias replacement behavior of package mode. -// Along with the functionality itself, that can be removed -// once we bump go.mod to 1.23. +// TODO(joaks): Update this once we remove the replacement logic +// when we bump go.mod to 1.23. func TestAliases(t *testing.T) { packageName := "go.uber.org/mock/mockgen/internal/tests/alias" for _, tt := range []struct {