deprecated reflect mode has been replaced with import mode

[tulzke]

resolves #175
resolves #197
resolves #128

It is impossible to create an mock for a generic interface via reflect mode, because it is impossible to compile a generic type without instantiation.
This PR replaces the reflect mod for parsing using go/types.

All exists mocks have been regenerated and the tests have been passed. But since this radically changes the behavior of reflect mode, I would be grateful if there are those who want to add additional test cases that I did not provide.

We can also come up with another name instead of import mode.

benefits:

• generation mocks for generic interfaces
• generation mocks for aliases to interfaces
• correct names for method arguments

[JacobOaks]

Hey! Thank you for this PR! We're excited about this change. We took a first pass and have some initial comments. We're going to do some additional testing on this and come back soon for more review.

One high level comment I wanted to raise was - with Go 1.23, go/types introduced Alias. Can we add test cases that try to load aliases to interfaces and verify this will work?

[tulzke]

One high level comment I wanted to raise was - with Go 1.23, go/types introduced Alias. Can we add test cases that try to load aliases to interfaces and verify this will work?

This already works without explicitly specifying an alias. This works thanks to the Underlying method.

Example of generated mock for alias:

mock/mockgen/internal/tests/import_mode/mock/interfaces.go

Lines 297 to 434 in 703a35f

	type MockHuman struct {
	ctrl *gomock.Controller
	recorder *MockHumanMockRecorder
	}
	// MockHumanMockRecorder is the mock recorder for MockHuman.
	type MockHumanMockRecorder struct {
	mock *MockHuman
	}
	// NewMockHuman creates a new mock instance.
	func NewMockHuman(ctrl *gomock.Controller) *MockHuman {
	mock := &MockHuman{ctrl: ctrl}
	mock.recorder = &MockHumanMockRecorder{mock}
	return mock
	}
	// EXPECT returns an object that allows the caller to indicate expected use.
	func (m *MockHuman) EXPECT() *MockHumanMockRecorder {
	return m.recorder
	}
	// ISGOMOCK indicates that this struct is a gomock mock.
	func (m *MockHuman) ISGOMOCK() struct{} {
	return struct{}{}
	}
	// Breathe mocks base method.
	func (m *MockHuman) Breathe() {
	m.ctrl.T.Helper()
	m.ctrl.Call(m, "Breathe")
	}
	// Breathe indicates an expected call of Breathe.
	func (mr *MockHumanMockRecorder) Breathe() *MockHumanBreatheCall {
	mr.mock.ctrl.T.Helper()
	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Breathe", reflect.TypeOf((*MockHuman)(nil).Breathe))
	return &MockHumanBreatheCall{Call: call}
	}
	// MockHumanBreatheCall wrap *gomock.Call
	type MockHumanBreatheCall struct {
	*gomock.Call
	}
	// Return rewrite *gomock.Call.Return
	func (c *MockHumanBreatheCall) Return() *MockHumanBreatheCall {
	c.Call = c.Call.Return()
	return c
	}
	// Do rewrite *gomock.Call.Do
	func (c *MockHumanBreatheCall) Do(f func()) *MockHumanBreatheCall {
	c.Call = c.Call.Do(f)
	return c
	}
	// DoAndReturn rewrite *gomock.Call.DoAndReturn
	func (c *MockHumanBreatheCall) DoAndReturn(f func()) *MockHumanBreatheCall {
	c.Call = c.Call.DoAndReturn(f)
	return c
	}
	// Eat mocks base method.
	func (m *MockHuman) Eat(foods ...import_mode.Food) {
	m.ctrl.T.Helper()
	varargs := []any{}
	for _, a := range foods {
	varargs = append(varargs, a)
	}
	m.ctrl.Call(m, "Eat", varargs...)
	}
	// Eat indicates an expected call of Eat.
	func (mr *MockHumanMockRecorder) Eat(foods ...any) *MockHumanEatCall {
	mr.mock.ctrl.T.Helper()
	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Eat", reflect.TypeOf((*MockHuman)(nil).Eat), foods...)
	return &MockHumanEatCall{Call: call}
	}
	// MockHumanEatCall wrap *gomock.Call
	type MockHumanEatCall struct {
	*gomock.Call
	}
	// Return rewrite *gomock.Call.Return
	func (c *MockHumanEatCall) Return() *MockHumanEatCall {
	c.Call = c.Call.Return()
	return c
	}
	// Do rewrite *gomock.Call.Do
	func (c *MockHumanEatCall) Do(f func(...import_mode.Food)) *MockHumanEatCall {
	c.Call = c.Call.Do(f)
	return c
	}
	// DoAndReturn rewrite *gomock.Call.DoAndReturn
	func (c *MockHumanEatCall) DoAndReturn(f func(...import_mode.Food)) *MockHumanEatCall {
	c.Call = c.Call.DoAndReturn(f)
	return c
	}
	// Sleep mocks base method.
	func (m *MockHuman) Sleep(duration time.Duration) {
	m.ctrl.T.Helper()
	m.ctrl.Call(m, "Sleep", duration)
	}
	// Sleep indicates an expected call of Sleep.
	func (mr *MockHumanMockRecorder) Sleep(duration any) *MockHumanSleepCall {
	mr.mock.ctrl.T.Helper()
	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sleep", reflect.TypeOf((*MockHuman)(nil).Sleep), duration)
	return &MockHumanSleepCall{Call: call}
	}
	// MockHumanSleepCall wrap *gomock.Call
	type MockHumanSleepCall struct {
	*gomock.Call
	}
	// Return rewrite *gomock.Call.Return
	func (c *MockHumanSleepCall) Return() *MockHumanSleepCall {
	c.Call = c.Call.Return()
	return c
	}
	// Do rewrite *gomock.Call.Do
	func (c *MockHumanSleepCall) Do(f func(time.Duration)) *MockHumanSleepCall {
	c.Call = c.Call.Do(f)
	return c
	}
	// DoAndReturn rewrite *gomock.Call.DoAndReturn
	func (c *MockHumanSleepCall) DoAndReturn(f func(time.Duration)) *MockHumanSleepCall {
	c.Call = c.Call.DoAndReturn(f)
	return c
	}

And, yes, i tested it in compile-time:

mock/mockgen/internal/tests/import_mode/mock/interfaces_test.go

Line 21 in 703a35f

var human import_mode.Human = &MockHuman{}

Generated mock successfully implements alias.

[JacobOaks]

One high level comment I wanted to raise was - with Go 1.23, go/types introduced Alias. Can we add test cases that try to load aliases to interfaces and verify this will work?

This already works without explicitly specifying an alias. This works thanks to the Underlying method.

@tulzke, when I run mockgen's tests on go1.23.1, or with GODEBUG=gotypesalias=1, I get the following failure:

$ GODEBUG=gotypesalias=1 go test ./...
...
                Error:          Received unexpected error:
                                extract interfaces from package: parse interface: error parsing type go.uber.org/mock/mockgen/internal/tests/import_mode.Human = go.uber.org/mock/mockgen/internal/tests/import_mode.Primate: Human is not an interface. it is a interface{Breathe(); Sleep(duration time.Duration); go.uber.org/mock/mockgen/internal/tests/import_mode.Eater}
...

This is because obj in parseInterface is a *types.Alias, but being asserted to a *types.Named. types.Unalias can be applied here to get to the underlying *types.Named

[JacobOaks]

Hey @tulzke - thanks for the updates and sorry for the delay. Took another pass.

[JacobOaks]

Forgot to add - since types.Unalias was added in 1.22, we will probably want to hide usages of it behind a build tag.

[tulzke]

Forgot to add - since types.Unalias was added in 1.22, we will probably want to hide usages of it behind a build tag.

Why we can't just update go up to 1.23 in go.mod? This is the latest stable version. If we make different implementations for different versions of Go, it will be noticeably more difficult to maintain.

[JacobOaks]

Why we can't just update go up to 1.23 in go.mod? This is the latest stable version. If we make different implementations for different versions of Go, it will be noticeably more difficult to maintain.

I'm gonna ask other maintainers their thoughts here because I'm actually not super sure myself. cc: @sywhang @r-hang @tchung1118.

I believe using 1.23 in go.mod would force anybody who wants to use the next release to upgrade to go1.23. We can technically bump the version of go.mod to 1.22, since that aligns with our release policy of supporting the two latest versions, and then we can reference types.Alias freely here. But since gomock is used widely and this is a simple enough difference to hide behind build tags, it would be nice to use them instead of alienating pre-1.22 users from our next release. I don't think we would need to duplicate a lot of code, I think both references to types.Alias/types.Unalias can be one-line functions with different definitions between the versions, but I agree it is technically a maintenance burden, especially for the switch case.

[ebilling]

As moving to go1.23 would be considered a breaking change, it might make sense to release as v2 which would force users to move forward manually. That would allow the old system to work with older versions but force users to move forward with a "modern" version. I've been watching/waiting/using this change for a few weeks now. For users that want to stay up-to-date with Go versions and latest protoc changes, not merging this change is causing real headaches.

[JacobOaks]

Chatted about build tags vs. go.mod update internally - we think it's fine to update go.mod to 1.22 which will allow this PR to merge without build tags.

This is technically in line with our release policy and will avoid some ugly code in the big switch statement as @tulzke mentioned.

As moving to go1.23 would be considered a breaking change, it might make sense to release as v2 which would force users to move forward manually. That would allow the old system to work with older versions but force users to move forward with a "modern" version.

As a policy, we don't typically do 2.0 releases. If something is truly a breaking change we don't usually even consider it. In this case however - I'm not sure dropping support for Go versions that aren't even supported themselves upstream qualifies as a breaking change, so I think it's fine to update go.mod.

[JacobOaks]

Hey @tulzke - this is looking great! Thanks for your updates. I did a final fine-toothed review over this, but other than these last couple of comments/questions this LGTM.

[JacobOaks]

Can you expand on why this is needed? Ideally, we'd want to support Go 1.22 w/o the GODEBUG variable set.

[tulzke]

I have already generated mocks for aliases. Without gotypesalias=1, mocks are generated without aliases.
Example from failed test:

< func (c *MockEarthHumanPopulationCall) Return(arg0 int) *MockEarthHumanPopulationCall {
---
> func (c *MockEarthHumanPopulationCall) Return(arg0 import_mode.HumansCount) *MockEarthHumanPopulationCall {

HumansCount here - alias for int. On 1.23 and 1.22 with GODEBUG=gotypealias=1 mockgen wiil generate mocks with correct types. On native 1.22 it wil generate int instead of import_mode.HumansCount.

Tests on 1.22 and 1.23 have different behavior now due to aliases.

[JacobOaks]

Understood - let's keep this here then. Can you just add this brief explanation into the test script?

[tulzke]

I have tested it now. And with go 1.22 in go mod it does not work with aliases as i expected.

$ go version                                                       
go version go1.23.1 darwin/arm64
$ cat go.mod | grep "go 1."
go 1.22
$ cd mockgen               
$ go build . && cp mockgen /Users/USERNAME/go/bin/mockgen # copy binary to PATH
$ cat internal/tests/import_mode/go.mod | grep "go 1." #checking go version in nested go module 
go 1.23
$ cd internal/tests/import_mode                       
$ cat interfaces.go| grep "AddHumans" 
        AddHumans(HumansCount) []Human
$ go generate ./...
$ cat mock/interfaces.go | grep "AddHumans(arg0" #must have HumansCount and Human instead of int and Primate
func (m *MockEarth) AddHumans(arg0 int) []import_mode.Primate {
func (mr *MockEarthMockRecorder) AddHumans(arg0 any) *MockEarthAddHumansCall {

In my case it works only after i changed go version in go.mod and rebuild binary.

Do you have any ideas how to support go 1.22 and alias types together?

[JacobOaks]

Hey @tulzke - I'm sorry, I mistakenly overrode your remote branch's main with the origin's main while trying to fix the go1.22 alias issue.

I think I'm able to re-instate it/re-open the PR how it was by pushing the correct main in my local state to your branch if you grant me permission to do so - or you can force-push your local main to tulzke/main if you have your local state still correct and then I think I can re-open/reinstate the PR. Again, really sorry about the mistake.

[tulzke]

Hey @tulzke - I'm sorry, I mistakenly overrode your remote branch's main with the origin's main while trying to fix the go1.22 alias issue.

I think I'm able to re-instate it/re-open the PR how it was by pushing the correct main in my local state to your branch if you grant me permission to do so - or you can force-push your local main to tulzke/main if you have your local state still correct and then I think I can re-open/reinstate the PR. Again, really sorry about the mistake.

I can't re-open this PR, so i created the new one:
#207