golang / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
9.26k stars 608 forks source link

Mockgen: Support generating mock for interfaces with generics #621

Closed lingyv-li closed 2 years ago

lingyv-li commented 2 years ago

Requested feature

Mockgen should support generating mocks for interfaces with generics.

Why the feature is needed The Go 1.18 is planned to be released soon, and I noticed that the mockgen tool doesn't support the generic. When a type parameter is in the interface, mockgen will throw an error.

Code example: https://go.dev/play/p/g1G2OvZPQpT?v=gotip

> go run github.com/golang/mock/mockgen -source=generic_mock.go
2022/02/13 10:59:41 Loading input failed: don't know how to mock method of type *ast.IndexExpr
exit status 1

(Optional) Proposed solution

  1. In the code example above, the StudentRepository interface is concrete, so the gererated mock should be concrete too with the type parameters substituted.
  2. The BaseRepository interface is abstract, so the generated mock struct should also have the same type parameters.

(My use case only needs 1, which mockery supports as of today.)

CannibalVox commented 2 years ago

In fact, I've gotten errors generating mocks for interfaces that don't use generics but are located in packages that do use generics. So I think this might be a slightly more serious issue than it appears at first glance.

codyoss commented 2 years ago

Hey, this is planned and will try to land as a fast followup to 1.18. Thank you for the request!

abramk commented 2 years ago

Do you have a rough idea of when we can expect a generics compatible release? This is the main impediment for us to aggressively adopt generic goodness.

CannibalVox commented 2 years ago

FWIW I was able to work around the above issue by moving interfaces out to their own file that don't use generics. That doesn't help for cases where you want the interface itself to use generics, but it did help for my case.

anjmao commented 2 years ago

Haven't looked into Go generics yet, but shouldn't it be possible to rewrite mock with generics and remove the need to generate mocks code?

I imagine it could be similar to C# moq.

service := gomock.New[MyServiceInterface]()
service.EXPECT().GetItem(gomock.Any).Returns(item, nil)
abramk commented 2 years ago

I don't believe the above is possible with go generics. The code has no way of reflecting on the type of the actual parameter. @codyoss do you have an idea of when we can expect generics support?

powerman commented 2 years ago

Looks like it's possible to work around this issue by using type aliases:

// Broken:
type Target interface {
    Method() *generic.Type[int]
}

// Working:
type GenericTypeInt = generic.Type[int]
type Target interface {
    Method() *GenericTypeInt
}
swallowtail62 commented 2 years ago

@powerman It works for me. thank you.

sodul commented 2 years ago

@codyoss Now that Go 1.18 has been out for over a month, is there a timeline update you can share? Thank you!

bradleygore commented 2 years ago

The solution proposed by @powerman doesn't work for generics working with custom types from what I can see:

// generic thing in foo pkg
type Gen [T any] struct {
  Items []T
  // other fields
}

// package bar

import "path/to/foo"

type Thing struct {
  FirstName string
}

type GenericTypeGenThing = foo.Gen[*Thing]

// interface I want to generate a mock for
type Thinger interface {
  Get() GenericTypeGenThing
}

What I see happening is that the generated output is like this for the generic return parameter: foo.Gen[*path/to/bar.Thing]

powerman commented 2 years ago

What I see happening is that the generated output is like this for the generic return parameter: foo.Gen[*path/to/bar.Thing]

At a glance there is a typo in your example code, this line probably should be:

type GenericTypeGenThing = foo.Gen[*Thing] // Added "foo." here.

Unlikely this is the actual problem, because without "foo." it shouldn't compile at all. Anyway, working example in some temp repo would be useful to understand why workaround doesn't works for you.

bradleygore commented 2 years ago

Yeah, that's a typo but not the issue. As you said, it wouldn't have compiled if that were the issue. That's what I get for hastily making a contrived example based on the real one 😝

I appreciate your looking into it!

bradleygore commented 2 years ago

@powerman I have set up a repository with reproduction of a couple different issues with generics and mockgen: gomock-generics-issue

herlon214 commented 2 years ago

I'm looking forward for this.

codyoss commented 2 years ago

Thanks for all the good discussion here folks. I have some time set aside this week to try to implement something here. I will likely put this out in a beta release as I want to make sure this change lands well. Thanks for the patience.

JosiahWitt commented 2 years ago

Caveat: This post is not official by any means!

TL;DR: I added support for generating mocks with generics to my ensure CLI, which generates Go Mock compatible mocks. Feel free to try it out!

The longer version:

A few months ago, I ended up rewriting mockgen as part of the CLI for my ensure test framework. The main motivations for this were:

  1. I found mockgen cumbersome when you wanted to generate a lot of mocks, like we did, since there wasn't a central place to list everything.
  2. To keep mockgen more straightforward, we had a list of packages and their interfaces to generate. However, the reflection option was really slow (we didn't want to list specific files). It ended up taking us over 30 seconds to generate mocks for all our packages. That was after we parallelized the mockgen calls.

My reimplementation solves these issues by:

  1. Using a YAML file to maintain the list of mocks to generate.
  2. Using golang.org/x/tools/go/packages to parse and read the listed packages and interfaces. This takes under a second to generate mocks for the same list of packages!

I just added support for generating mocks with generics (CLI v0.3.0+). It seems to work well, and is well covered by unit tests, but I don't have any codebases with generic interfaces to test it against yet. I ran it against @bradleygore's repo with the following .ensure.yml file, and it seems to do the right thing. Feel free to try it out, and let me know if you run into any issues!

# .ensure.yml file for https://github.com/bradleygore/gomock-generics-issue
mocks:
  packages:
    - path: gomock-generics-issue/iface
      interfaces: [Worker]
    - path: gomock-generics-issue/workers
      interfaces: [CustomWorker, PrimitiveWorker, BigWorker, LittleWorker]
codyoss commented 2 years ago

Hey, if anyone wants to locally checkout my draft PR and try to generate some generic mocks in source mode that would be great! I would like to to see if I need some more test cases 😄 I plan to iterate on reflect mode in a separate PR

herlon214 commented 2 years ago

@codyoss thank you very much! i will do some tests tomorrow morning 🚀

herlon214 commented 2 years ago

@codyoss I tried your generics branch and it panic:

mockgen -package=mock -source=service.go -destination=mock/service.go
panic: interface conversion: ast.Expr is *ast.SelectorExpr, not *ast.Ident

goroutine 1 [running]:
main.(*fileParser).parseGenericType(0x6f3aa0?, {0xc00021a200?, 0xc0002030b0?}, {0x7ca958?, 0xc000202510?}, 0xc00012d3b0?)
        /home/herlon/dev/codyoss/mock/mockgen/generic_go118.go:38 +0x23f
main.(*fileParser).parseType(0xc00012dd10?, {0xc00021a200?, 0x3a?}, {0x7ca958?, 0xc000202510?}, 0x8?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:545 +0xbd6
main.(*fileParser).parseType(0xc00012dd10, {0xc00021a200, 0x3a}, {0x7cabc8?, 0xc000208180?}, 0x8?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:532 +0x5c9
main.(*fileParser).parseFieldList(0xc000218330?, {0xc00021a200, 0x3a}, {0xc00020a0c0, 0x2, 0x50?}, 0xc00020a2c0?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:435 +0x13c
main.(*fileParser).parseFunc(0x6f6080?, {0xc00021a200?, 0x3a?}, 0xc0002126e0, 0x51ccbf?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:412 +0x345
main.(*fileParser).parseInterface(0xc00012dd10, {0xc000218054, 0x7}, {0xc00021a200, 0x3a}, 0xc000202e70)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:300 +0x769
main.(*fileParser).parseFile(0xc00012dd10, {0xc00021a200, 0x3a}, 0xc00020e100)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:232 +0x2d1
main.sourceMode({0x7fff0baf7c9e, 0x22})
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:90 +0x40d
main.main()
        /home/herlon/dev/codyoss/mock/mockgen/mockgen.go:81 +0x2ea

I'm trying to generate a mock for an interface which uses generic structs:

// service.go
type Service interface {
    Create(ctx context.Context, params Params, input Input) error
    Update(ctx context.Context, params Params, input Input) error
    List(ctx context.Context, params Params, filters Filters) (*controller.Results[Output], error)
    Fetch(ctx context.Context, params Params) (*controller.Result[Output], error)
    Delete(ctx context.Context, params Params) error
}

In the case above my generic struct is controller.Result[Output] and controller.Results[Output].


Using specifically the commit 90e3327ab5fb3e24df8dc48272b1c2e11a9d86d0 it panics with:

2022/05/06 15:20:06 Loading input failed: service.go:14:6: failed parsing returns: don't know how to parse type *ast.IndexExpr
codyoss commented 2 years ago

@herlon214 Thank you for taking the time to try out the branch. I think you may have missed a step in your testing perhaps? I added two more test cases that should cover your use case and it worked for me. To test make sure you:

  1. Checkout the branch
  2. Rebuild mockgen with that source with Go 1.18: go install ./... . Code updates are required for the generator to parse these new ast types in 1.18
  3. Generate your mocks and test with by replacing the local branch with a version of mock recorded in your go.mod: go mod edit -replace github.com/golang/mock=/Path/To/Checked/Out/Code

Can you confirm that you rebuilt mockgen, I suspect that that is why you might be getting that error.

herlon214 commented 2 years ago

@codyoss here are the steps I did:

goroutine 1 [running]:
main.(*fileParser).parseGenericType(0x6f3aa0?, {0xc000018500?, 0xc0000a0ff0?}, {0x7ca958?, 0xc0000a0450?}, 0xc00020f3b0?)
        /home/herlon/dev/codyoss/mock/mockgen/generic_go118.go:38 +0x23f
main.(*fileParser).parseType(0xc00020fd10?, {0xc000018500?, 0x3a?}, {0x7ca958?, 0xc0000a0450?}, 0x8?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:545 +0xbd6
main.(*fileParser).parseType(0xc00020fd10, {0xc000018500, 0x3a}, {0x7cabc8?, 0xc00000c1b0?}, 0x8?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:532 +0x5c9
main.(*fileParser).parseFieldList(0xc00001c470?, {0xc000018500, 0x3a}, {0xc0000800d0, 0x2, 0x50?}, 0xc0000802d0?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:435 +0x13c
main.(*fileParser).parseFunc(0x6f6080?, {0xc000018500?, 0x3a?}, 0xc00008a700, 0x51ccbf?)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:412 +0x345
main.(*fileParser).parseInterface(0xc00020fd10, {0xc00001c194, 0x7}, {0xc000018500, 0x3a}, 0xc0000a0db0)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:300 +0x769
main.(*fileParser).parseFile(0xc00020fd10, {0xc000018500, 0x3a}, 0xc0000a2080)
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:232 +0x2d1
main.sourceMode({0x7ffea3a7ec05, 0x22})
        /home/herlon/dev/codyoss/mock/mockgen/parse.go:90 +0x40d
main.main()
        /home/herlon/dev/codyoss/mock/mockgen/mockgen.go:81 +0x2ea

Couldn't finish the step (3) you mentioned, once the mocks aren't generated.

Did I miss anything?

codyoss commented 2 years ago

@herlon214 What you did looks good, I think I see what the issue is now. I will try to get a patch out later today for this use case. Thank you for the report!

codyoss commented 2 years ago

@herlon214 I think the issue you were seeing now should be fixed. Had not accounted for generic structs that don't use type params from the interface.

herlon214 commented 2 years ago

@codyoss looks like it worked now! I'm gonna do more tests tomorrow and let you know, thanks in advance!!

herlon214 commented 2 years ago

Hey @codyoss I found another issue. I have a type which aliases one generic type:

type Persistence mongodb.Persistence[User]

The generated mocks for that are:

// Code generated by MockGen. DO NOT EDIT.
// Source: pkg/v1/domain/user/persistence.go

// Package mock is a generated GoMock package.
package mock
codyoss commented 2 years ago

@herlon214 Can you clarify what the issue is?

herlon214 commented 2 years ago

@codyoss the generated mocks are actually empty, that's the issue. mongodb.Persistence is:

type Persistence[T any] interface {
    List(ctx context.Context, filter bson.M) ([]T, error)
    Fetch(ctx context.Context, filter bson.M) (*T, error)
    Create(ctx context.Context, input T) error
    Update(ctx context.Context, filter bson.M, item T) error
    Delete(ctx context.Context, filter bson.M) error
}

So I would expect that using type Persistence mongodb.Persistence[User] would generate the mocks for it, but the result is one file with the package name and some comments:

// Code generated by MockGen. DO NOT EDIT.
// Source: pkg/v1/domain/user/persistence.go

// Package mock is a generated GoMock package.
package mock

Nothing else ^

codyoss commented 2 years ago

@herlon214 That is as intended, the type needs to be an interface. This is the same for non-generic types too.

codyoss commented 2 years ago

Support for source mode is now merged. I hope to get reflect done with week too and then I will publish a beta release to get feedback to make sure things are working as intended

herlon214 commented 2 years ago

@codyoss considering that mongodb.Persistence is an generic interface, shouldn't type Persistence mongodb.Persistence[User] become an interface as well? but applying one specific type to that generic interface.

codyoss commented 2 years ago

@herlon214 Maybe. But like I said, this does not work for non-generic types today either. If anything that should be a separate issue if people have use cases for this. The way mockgen treats aliases is already not great. Depending if you use reflect vs source mode the alias will be followed or not.

bradleygore commented 2 years ago

@herlon214 - would it work if you declared the type as interface and embed the typed generic interface?

type UserPersistence interface {
  mongodb.Persistence[User]
}
herlon214 commented 2 years ago

@bradleygore also doesn't work. But generating the mocks directly to mongodb.Persistence[T any] looks like it works fine.

Thanks again @codyoss, great job!

bradleygore commented 2 years ago

hmm, yeah this may be interesting for my use-case. Consider a repository setup:

type BaseRepo interface[T any] {
  Get(filters *Filters) (*PagedResults[T], error)
  GetByID(int64) (T, error)
  Create(T) error
  Update(T) error
  Delete(T) error
}

type UserRepo interface {
  BaseRepo[User]
  FindByEmail(string email) (*User, error)
}

@codyoss - any thoughts on this and if it should be supported, or should this actually be structured differently?

codyoss commented 2 years ago

@bradleygore I think embedded interfaces should be supported, sometimes these have required extra flags in the past though. I don't think I had a test for this in my first PR though. If this does not work on HEAD feel free to open a separate issue for that use-case. I think this could be iterated on in the future if needed once baseline reflect and source support are added.

bradleygore commented 2 years ago

Sounds good - will try to test soon. Thanks for all your efforts on this @codyoss 😃

bradleygore commented 2 years ago

I went ahead and created a new issue for this - there is still an issue with type annotations across packages. I have a fairly thorough issue repo set up at https://github.com/bradleygore/gomock-generics-issue and the issue I just wrote up is at https://github.com/golang/mock/issues/643 .

Again - thanks for all efforts on this front!

DanielSolomon commented 2 years ago

I also faced another issue with Generics, in case I use a concrete type:

type Pagination struct {
    Limit int64
    Skip  int64
}

//go:generate mockgen -source=$GOFILE -destination=mocks/autogen.$GOFILE -package=mocks
type IMongoDbClient interface {
    GetMany(ctx context.Context, collectionName string, filter bson.M, pagination optional.Optional[Pagination]) error
}

Optional is defined in other package as:

type Optional[T any] struct {
    value   T
    present bool
}

When running go generate ./..., the following error is raised:

2022/05/19 10:01:58 Loading input failed: failed parsing source file api.go: api.go:43:97: missing ',' in parameter list
lib/go/mongo/client/api.go:41: running "mockgen": exit status 1

Also tried using main branch (v1.6.1-0.20220512030613-73266f9366fc), still same error. Also tried using the workaround above:

type OptionalPagination = optional.Optional[Pagination]

Result in:

Loading input failed: failed parsing source file api.go: api.go:41:44: expected ';', found '['
lib/go/mongo/client/api.go:43: running "mockgen": exit status 1
0x4c6565 commented 2 years ago

The solution proposed by @powerman doesn't work for generics working with custom types from what I can see:

// generic thing in foo pkg
type Gen [T any] struct {
  Items []T
  // other fields
}

// package bar

import "path/to/foo"

type Thing struct {
  FirstName string
}

type GenericTypeGenThing = foo.Gen[*Thing]

// interface I want to generate a mock for
type Thinger interface {
  Get() GenericTypeGenThing
}

What I see happening is that the generated output is like this for the generic return parameter: foo.Gen[*path/to/bar.Thing]

It would appear that using gomock Reflect mode results in this too:

Source

pkg/connection/connection.go

package connection

type APIRequestParameters struct{}

type Paginated[T any] struct{}

pkg/model/model.go

package model

type TestStruct struct{}

pkg/testservice/sometestservice.go

package testservice

import (
    "github.com/someorg/testproject/pkg/connection"
    "github.com/someorg/testproject/pkg/model"
)

type SomeTestService interface {
    SomeTestMethod(parameters connection.APIRequestParameters) (*connection.Paginated[model.TestStruct], error)
}

Command

mockgen -package mocks -destination mock_sometestservice.go github.com/someorg/testproject/pkg/testservice SomeTestService

Output

2022/05/20 09:00:38 Failed to format generated source code: mock_sometestservice.go:37:106: missing ',' in parameter list (and 1 more errors)
// Code generated by MockGen. DO NOT EDIT.
// Source: github.com/someorg/testproject/pkg/testservice (interfaces: SomeTestService)

// Package mocks is a generated GoMock package.
package mocks

import (
        connection "github.com/someorg/testproject/pkg/connection"
        reflect "reflect"
        gomock "github.com/golang/mock/gomock"
)

// MockSomeTestService is a mock of SomeTestService interface.
type MockSomeTestService struct {
        ctrl     *gomock.Controller
        recorder *MockSomeTestServiceMockRecorder
}

// MockSomeTestServiceMockRecorder is the mock recorder for MockSomeTestService.
type MockSomeTestServiceMockRecorder struct {
        mock *MockSomeTestService
}

// NewMockSomeTestService creates a new mock instance.
func NewMockSomeTestService(ctrl *gomock.Controller) *MockSomeTestService {
        mock := &MockSomeTestService{ctrl: ctrl}
        mock.recorder = &MockSomeTestServiceMockRecorder{mock}
        return mock
}

// EXPECT returns an object that allows the caller to indicate expected use.
func (m *MockSomeTestService) EXPECT() *MockSomeTestServiceMockRecorder {
        return m.recorder
}

// SomeTestMethod mocks base method.
func (m *MockSomeTestService) SomeTestMethod(arg0 connection.APIRequestParameters) (*connection.Paginated[github.com/someorg/testproject/pkg/model.TestStruct], error) {
        m.ctrl.T.Helper()
        ret := m.ctrl.Call(m, "SomeTestMethod", arg0)
        ret0, _ := ret[0].(*connection.Paginated[github.com/someorg/testproject/pkg/model.TestStruct])
        ret1, _ := ret[1].(error)
        return ret0, ret1
}

// SomeTestMethod indicates an expected call of SomeTestMethod.
func (mr *MockSomeTestServiceMockRecorder) SomeTestMethod(arg0 interface{}) *gomock.Call {
        mr.mock.ctrl.T.Helper()
        return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SomeTestMethod", reflect.TypeOf((*MockSomeTestService)(nil).SomeTestMethod), arg0)
}

Update: I believe this is related to https://github.com/golang/mock/issues/643

codyoss commented 2 years ago

Looked more into reflect mode and I don't think implementing generics support for reflect mode will work. Reflect works with runtime types and by just looking at an interface with constraints there is no way to get these runtime types. At least for the initial release of support generics will only be supported in source mode. If I overlooked something here please feel free to correct me. For now I am closing out this issue as initial support has landed on HEAD. I plan on releasing a beta tag of this next week.

DanielSolomon commented 2 years ago

Hey @codyoss , I see you closed the issue, but when I update mockgen I still get the same error I attached above, should I create an issue on it? or am I using something incorrectly?

codyoss commented 2 years ago

@DanielSolomon Sorry I missed that, would you mind busting that out into a separate issue? I would like to treat any new requests for how things work here as bugs in the current impl. It will also help better track the discussion.

sodul commented 2 years ago

@codyoss What is the general status for Generics support? We are holding off on upgrading to Go 1.18 until an official mockgen release is available with full support for generics. We are also looking forward to #604 which has 2 potential PRs to address it.

Thank you!

codyoss commented 1 year ago

https://github.com/golang/mock/releases/tag/v1.7.0-rc.1

erkanzileli commented 1 year ago

Hi guys. It works great. Thanks for your effort.

When do you think there will be a release named v1.7.0?

pobochiigo commented 1 year ago

Hey, guys!

So where're we with support for embedded generic interfaces?

Tried mockgen v1.7.0-rc.1 - no luck. Got this "Loading input failed: don't know how to mock method of type *ast.IndexListExpr"

If anyone can help or point the right direction would appreciate it!

chenyanchen commented 1 year ago

@herlon214 Maybe. But like I said, this does not work for non-generic types today either. If anything that should be a separate issue if people have use cases for this. The way mockgen treats aliases is already not great. Depending if you use reflect vs source mode the alias will be followed or not.

Some standard struct or interface was very useful like:

type Storage[K comparable, V any] interface {
    Get(K) V
    Set(K, V)
    Del(K)
}

type node[V any] struct {
    left, right *node[V]
}

I think no more alias in needed.

fernandofleury commented 1 year ago

Even when trying with the 1.7.0-rc.1 I still get the same error message: Loading input failed: don't know how to mock method of type *ast.BinaryExpr

mgnsk commented 1 year ago

I got generics working by using the main branch and passing the -source parameter to mockgen.

rolandjitsu commented 1 year ago

Even when trying with the 1.7.0-rc.1 I still get the same error message: Loading input failed: don't know how to mock method of type *ast.BinaryExpr

Getting the same issue too. Tried the main branch as well.

The issue seems to be happening when I have a constraint declaration:

type Number interface {
  int64 | float64
}