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

Incorrectly treating generic types as exported types #658

Open
alexandreLamarre opened this issue Jun 16, 2022 · 2 comments
Open

Incorrectly treating generic types as exported types #658

alexandreLamarre opened this issue Jun 16, 2022 · 2 comments
Milestone

Comments

@alexandreLamarre
Copy link

Consider a source file notifier.go we want to mock, with the following pattern :

type Clonable[T any] interface {
	Clone() T
}

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

type UpdateNotifier[T any] interface {

	NotifyC(ctx context.Context) <-chan []T

	Refresh(ctx context.Context)
}

Actual behavior A clear and concise description of what the bug is.

The three following functions are generated

// MockFinder is a mock of Finder interface.
type MockFinder[T notifier.Clonable[notifier.T]] struct {
	ctrl     *gomock.Controller
	recorder *MockFinderMockRecorder[T]
}

// MockFinderMockRecorder is the mock recorder for MockFinder.
type MockFinderMockRecorder[T notifier.Clonable[notifier.T]] struct {
	mock *MockFinder[T]
}

// NewMockFinder creates a new mock instance.
func NewMockFinder[T notifier.Clonable[notifier.T]](ctrl *gomock.Controller) *MockFinder[T] {
	mock := &MockFinder[T]{ctrl: ctrl}
	mock.recorder = &MockFinderMockRecorder[T]{mock}
	return mock
}

Note that in the above the generic type T is treated as an exported type from notifier


Expected behavior A clear and concise description of what you expected to
happen.

notifier.Clonable[notifier.T] should be notifier.Clonable[T] in the function signatures

// MockFinder is a mock of Finder interface.
type MockFinder[T notifier.Clonable[T]] struct {
	/* ... */
}

// MockFinderMockRecorder is the mock recorder for MockFinder.
type MockFinderMockRecorder[T notifier.Clonable[T]] struct {
	/* ... */
}

// NewMockFinder creates a new mock instance.
func NewMockFinder[T notifier.Clonable[T]](ctrl *gomock.Controller) *MockFinder[T] {
	/* ... */
}

To Reproduce Steps to reproduce the behavior

  1. mockgen -source notifier.go -output notifier_mock.go

Additional Information

  • gomock mode (reflect or source): source
  • gomock version or git ref: github.com/golang/mock v1.6.1-0.20220512030613-73266f9366fc
  • golang version: go version go1.18.2 darwin/arm64

Triage Notes for the Maintainers

Seems like a fun one

@alexandreLamarre alexandreLamarre changed the title Treating generic types as exported types Incorrectly treating generic types as exported types Jun 19, 2022
@alexandreLamarre
Copy link
Author

alexandreLamarre commented Jun 19, 2022

In mockgen.go GenerateMockInterface:

longTp, shortTp := g.formattedTypeParams(intf, outputPackagePath)

longTp is initialized as [T notifier.Clonable[notifier.T]] because :

for i, v := range it.TypeParams {
		if i != 0 {
			long.WriteString(", ")
			short.WriteString(", ")
		}
		long.WriteString(v.Name)
		short.WriteString(v.Name)
		long.WriteString(fmt.Sprintf(" %s", v.Type.String(g.packageMap, pkgOverride)))
	}

the nested T in [T notifier.Clonable[T]] is parsed into ast.NamedType in mockgen/generic_go118.go and Name() on NamedType always prefixes the NamedType with the package the NamedType is declared in.

Not very familiar with the codebase, but I would assume that to support nested generic interfaces, we would need to introduce another TypeParam implementation, e.g, something along the lines of PlaceholderType or GenericType. Let me know what you think, I'd be happy to work on this

alexandreLamarre added a commit to rancher/opni that referenced this issue Jun 20, 2022
@codyoss codyoss added this to the v1.7.0 milestone Jul 8, 2022
@tra4less
Copy link
Contributor

tra4less commented Aug 16, 2022

set tps before parse it.TypeParams[i].Type
src

tp, err := p.parseFieldList(pkg, it.typeParams, tps)
if err != nil {
    return nil, fmt.Errorf("unable to parse interface type parameters: %v", name)
}
iface.TypeParams = tp
for _, v := range tp {
    tps[v.Name] = true
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants