Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

feat: support embedded generic interfaces #669

Closed
wants to merge 2 commits into from
Closed

feat: support embedded generic interfaces #669

wants to merge 2 commits into from

Conversation

tra4less
Copy link
Contributor

@tra4less tra4less commented Aug 8, 2022

fixes #668 #658 #700

@tra4less tra4less requested a review from codyoss as a code owner August 8, 2022 07:59
@google-cla
Copy link

google-cla bot commented Aug 8, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@tra4less tra4less changed the title support embedded generic interfaces feat: support embedded generic interfaces Aug 8, 2022
@codyoss
Copy link
Member

codyoss commented Aug 11, 2022

@n0trace Thanks for the PR! Would you be willing to expand support for this test case as well:

type EmbeddingIface interface {
	TwentyOne[other.Three]
	other.Twenty[StructType]
	Foo() error
}

If you don't have the time I can review this as is and this could be done in another PR.

@tra4less
Copy link
Contributor Author

tra4less commented Aug 12, 2022

@n0trace Thanks for the PR! Would you be willing to expand support for this test case as well:

type EmbeddingIface interface {
	TwentyOne[other.Three]
	other.Twenty[StructType]
	Foo() error
}

If you don't have the time I can review this as is and this could be done in another PR.

done @codyoss

@codyoss
Copy link
Member

codyoss commented Aug 12, 2022

@n0trace Sorry if I was not clear, I meant just copy paste that code literally into generics.go. I don't think this solution covers the case where the outer interface does not contain type parameters, but the inner impl does.

@tra4less
Copy link
Contributor Author

cases are complex than I have imagined, but done. please review. @codyoss

@tra4less
Copy link
Contributor Author

fixes #658

@codyoss
Copy link
Member

codyoss commented Aug 19, 2022

@n0trace I will take a look over the weekend, thank you!

@alexandreLamarre
Copy link

alexandreLamarre commented Sep 20, 2022

As a heads up I verified this also fixes the same issue we were seeing in #658

@codyoss
Copy link
Member

codyoss commented Oct 7, 2022

Reviewing this today, sorry. Had gotten pulled away from this project for awhile!

@codyoss
Copy link
Member

codyoss commented Oct 7, 2022

@n0trace I am still digging through the code, but would you mind adding some docs around the new types/fields/methods you added. I know parsing code is lacking this in general right now, but since this adds a bit of complexity it would be nice to have so details of what/how things are being used.

@lennycampino
Copy link

@n0trace @codyoss Is there any update on this? :)

@hirikarate
Copy link

I'm having the same issue and I'm hoping this fix could make it to official as soon as possible.
Thank you all for the great work.

@chkp-omris
Copy link

I'm also having the same issue, could you please review this?

@tofluid
Copy link

tofluid commented Jan 10, 2023

This is needed, please review. Thanks!

@tra4less
Copy link
Contributor Author

@codyoss we need a resolution for mock generics. please review.

@paprikati
Copy link

We are also blocked on this - would love if we could use this ❤️

@AyushG3112
Copy link

@n0trace @codyoss any update on this please?

@samvdb
Copy link

samvdb commented Mar 9, 2023

Thanks for the pull request. Support for generics would be most welcome! Any update?

@samvdb
Copy link

samvdb commented Mar 10, 2023

I tested this PR with go version go1.20.1 linux/amd64 and can confirm generics are working.

@tofluid
Copy link

tofluid commented Apr 7, 2023

@codyoss or anyone else that can approve this PR. My team is really getting hamstrung and we're now starting to consider a different mocking library for go but then that'll be two testing standards which I do not want.

Please get this approved!

@colinschoen
Copy link

@codyoss Can you please have a look at this

@tedkornish
Copy link

Also eagerly awaiting this.

@maestre3d
Copy link

maestre3d commented May 2, 2023

Just looked at Cody's LinkedIn profile. Looks like he got promoted and is no longer part of Google's Golang Developer Relations team.

I think we should ping another person to get this PR merged.

@ShiningRush
Copy link

ShiningRush commented May 4, 2023

@n0trace hi, I get a error when mock the interface which embedded a another package's pointer and generic types (mockgen is build from your branch):
Loading input failed: unable to parse interface type parameters: Repo

test demo like below:

package a

type ChildRepo interface {
	b.Repo[*b.SomeStruct]
}
package b

type Repo[T any] interface {
}

type SomeStruct struct {
}

It will be ok when I use struct to instead of pointer

type ChildRepo interface {
	b.Repo[b.SomeStruct]
}

@tra4less
Copy link
Contributor Author

tra4less commented Jun 25, 2023

@ShiningRush fixed it

@codyoss I've refactored my implementation, and now it's easy to understand and maintain. I'd appreciate it if you could take some time to review it

@tra4less tra4less closed this by deleting the head repository Jan 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mockgen] Errors creating mocks for embedded generic interfaces