Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support embedded generic interfaces #13

Merged
merged 3 commits into from
Jul 11, 2023
Merged

support embedded generic interfaces #13

merged 3 commits into from
Jul 11, 2023

Conversation

tra4less
Copy link
Contributor

@tra4less
Copy link
Contributor Author

cc @sywhang

@sywhang sywhang self-assigned this Jun 29, 2023
@sywhang sywhang added the enhancement New feature or request label Jun 29, 2023
@sywhang
Copy link
Contributor

sywhang commented Jun 29, 2023

Thanks @n0trace , I'll get to this shortly.

@bradleygore
Copy link

Hey @n0trace and @sywhang - I have a similar PR in the origin repo: golang/mock#663
One thing in that PR is that there is some (hopefully good) cleanup around the parse.go file (https://github.com/golang/mock/pull/663/files#diff-eaed3d31482bbbe3987808f6f3e8cf43426d098cd55d0c37699d691fb4da8d85) that I'd love to see make it in if nothing else. I know they just recently flagged the origin repo as archived and such, so I don't want to inundate with PRs especially since the PR in this thread is to solve the same issue 😄

TIA for your consideration, and for taking over this great tool!

}

type Finder[T Clonable[T]] interface {
Find(ctx context.Context) ([]T, error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One case I think we should cover is variadic type parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Twenty(S, R) (T, Z)
}

type TwentyThree[U, V any] interface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming: Could the more sophisticated types be named by their function instead of a number? It took some time for me to organize which cases were tested while reviewing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, you are right, i redesigned the case, including the additions under this file

Copy link
Contributor

@r-hang r-hang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me after these comment! Thanks for adding the Variadic case and clearing up the function of the examples.

mockgen/internal/tests/generics/external.go Outdated Show resolved Hide resolved
mockgen/internal/tests/generics/source/assert_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@sywhang sywhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you for this work!

@sywhang sywhang merged commit 0181604 into uber-go:main Jul 11, 2023
@tra4less tra4less deleted the issues-668 branch July 12, 2023 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mockgen] Errors creating mocks for embedded generic interfaces
4 participants