bradleygore / gomock-generics-issue

reproduce reported issue in gomock with generics
0 stars 0 forks source link

quick hack #1

Open powerman opened 2 years ago

powerman commented 2 years ago

I'm too sleepy (it's 6AM here), so I didn't actually tried to understand the code, sorry. But this quick hack looks like works around code generation in ./workers:

diff --git a/workers/sized.go b/workers/sized.go
index bf10662..426b7f4 100644
--- a/workers/sized.go
+++ b/workers/sized.go
@@ -39,14 +39,19 @@ import (
    but also not how it behaves differently when the generic parameter passed-in is in same package as usage (see CustomWorker mock output)
    vs when the generic parameter passed-in is in a totally different package.
 */
+
+type WorkResultBigDetail iface.WorkResult[*workdetail.BigDetail]
+
 type BigWorker interface {
-   iface.Worker[*workdetail.BigDetail]
+   DoWork(...interface{}) WorkResultBigDetail
 }

 //go:generate mockgen -destination=./mocks/LittleWorker.go -package=mock_iface gomock-generics-issue/workers LittleWorker
 /*
    NOTE: This would have same issue as BigWorker interface above
 */
+type WorkResultLittleDetail iface.WorkResult[*workdetail.LittleDetail]
+
 type LittleWorker interface {
-   iface.Worker[*workdetail.LittleDetail]
+   DoWork(...interface{}) WorkResultLittleDetail
 }
bradleygore commented 2 years ago

Unfortunately, this kind of defeats the usefulness of having a base interface that I can embed into other interfaces. My specific situation is with a data access layer and repository pattern:

type Repository [T any] interface {
  Get (args....) ([]T, error)
  GetByID(id int64) (T, error)
  Create(T) error
  Update(T) error
  Delete(id int64) error
}

type RepositoryIDInt64 [T any] interface {
  GetByID(id int64) (T, error)
}

type RepositoryIDuuid [T any] interface {
  GetByUUID(uuid string) (T, error)
}

// then use it in specialized repos
type FruitsRepo interface {
  Repository[*models.Fruit]
  RepositoryIDInt64[*models.Fruit]
  EatFruit(id int64) error
}

I would have to duplicate all the embedded interface funcs if I go this route. I have dozens of repositories at present and would really love to not have to do this 😅

bradleygore commented 2 years ago

Just for ease of documentation, if anyone else runs across this repo, the catalyst of this is https://github.com/golang/mock/issues/621

powerman commented 2 years ago

Well… if we'll have to choose between some copy&paste of interfaces content or avoid using generics until gomock will support generics (no ETA at all AFAIK), or stop using gomock… I'll choose to copy&paste interfaces. This is harmless, it can be probably done in separate temporary files, it can be easily checked for errors at compile time by applying interface conversion for var _.

Also, I think you get the idea - probably there are some other ways to work around gomock limitation, experiment with it and maybe you'll find another one to avoid or minimize copy&paste.

bradleygore commented 2 years ago

@powerman - I tried this and it does not work:

type BigDetailWork = iface.WorkResult[*workdetail.BigDetail]

//go:generate mockgen -destination=./mocks/BigWorkerTwo.go -package=mock_iface gomock-generics-issue/workers BigWorkerTwo
type BigWorkerTwo interface {
    DoWork(...interface{}) (BigDetailWork, error)
}

Output in terminal (b/c it does not write to file on error):

...
// DoWork mocks base method.
func (m *MockBigWorkerTwo) DoWork(arg0 ...interface{}) (iface.WorkResult[*gomock-generics-issue/workdetail.BigDetail], error) {
        m.ctrl.T.Helper()
        varargs := []interface{}{}
        for _, a := range arg0 {
                varargs = append(varargs, a)
        }
        ret := m.ctrl.Call(m, "DoWork", varargs...)
        ret0, _ := ret[0].(iface.WorkResult[*gomock-generics-issue/workdetail.BigDetail])
        ret1, _ := ret[1].(error)
        return ret0, ret1
}
...

Notice that even though my interface references BigDetailWork type, the mock generation still extrapolates that to iface.WorkResult[*gomock-generics-issue/workdetail.BigDetail]

Sorry if I'm totally missing something, but I don't think this workaround will work with non-primitive values. If I change the definition of BigDetailWork to this: type BigDetailWork = iface.WorkResult[int] it works fine. Unfortunately, I'm not dealing with primitive values - I'm dealing with complex types 😢

powerman commented 2 years ago

As you can see in my diff above - I didn't used type alias this time. As long as non-aliased type is an interface there is no difference between alias or not - but without alias gomock code generation works.

bradleygore commented 2 years ago

I'm sorry - I looked back over the diff and I'm confused how this is not a type alias?

Screen Shot 2022-04-20 at 4 59 25 PM

WorkResultBigDetail is a type alias for the generic iface.WorkResult[T] where T is *workdetail.BigDetail. And you're using that alias in the BigWorker interface: DoWork(...interface{}) WorkResultBigDetail. Same concept applies to LittleWorker interface and its type.

I've tried the suggestion you posted above, per my earlier comment, and it just does not work then the value for T is non-primitive.

powerman commented 2 years ago

WorkResultBigDetail is a type alias for the generic iface.WorkResult[T]

No, it's not: there is no = in type definition syntax.

bradleygore commented 2 years ago

Oh - I see, I added an =, I missed that. I will try it without the = and let you know. Thanks for taking the time!

bradleygore commented 2 years ago

I tried it as a new type definition instead of a type alias, and that won't work in my case. In my case, I have common utility functions that rely on interface implementation. Consider:

func [T any] processWork(iface.Worker[T], someArgs ...interface{}) error {
  ....
}

If I change the type as type WorkResultBigDetail iface.WorkResult[*workdetail.BigDetail] then I cannot use the utility function as processWork [*workdetail.BigDetail](bigWorker, args...) b/c it doesn't recognize them as the same exact type.

Here's an example from the situation I'm working on:

cannot use r (variable of type *addrRepo) as query.PageGetter[*"pos-api/internal/types".Address] value in argument to getByID[*types.Address]: *addrRepo does not implement query.PageGetter[*"pos-api/internal/types".Address] (wrong type for method Get)
        have Get(opts *query.Opts) (PagedAddresses, error)
        want Get(opts *query.Opts) (*query.PagedResults[*"pos-api/internal/types".Address], error)

I guess I'll just have to wait until mockgen supports this a bit better.

powerman commented 2 years ago

I suppose PagedAddresses is not a type alias (there is no =), but as this type is concrete and not interface type you should use type alias - otherwise types will differ, as error tells.

powerman commented 2 years ago

Also you probably should (or at least can try to) define PagesAddresses type as an alias either to pointer type *query.PagedResults[…] or non-pointer type query.PagedResults[…].

bradleygore commented 2 years ago

Yeah, that's the problem I'm running into. If I use it as a new type, mockgen works but then I can't use my utility funcs. If I use it as a type alias, I can use my utility funcs but mockgen doesn't work. 🤷🏻‍♂️

powerman commented 2 years ago

From what I have found mockgen requires interface types to be non-aliases and usual types to be aliases.