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

v0.5.0 mock generated do not compile due to name clash #218

Closed
HassanCehef opened this issue Oct 22, 2024 · 2 comments · Fixed by #219
Closed

v0.5.0 mock generated do not compile due to name clash #218

HassanCehef opened this issue Oct 22, 2024 · 2 comments · Fixed by #219
Assignees

Comments

@HassanCehef
Copy link

HassanCehef commented Oct 22, 2024

Hello. I tried to use this project in v0.5.0 from v0.4.0, and this produced code that doesn't compile anymore. I produced a smaller example here than my real project. I can push a private simplified repo if needs be for full repro.

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

generating mocks finishes without error, and produces code that does not compile. Instead, when running tests, I'll get a packagename.WhateverExported is not a type

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

generating mocks finishes without error, and produces code that compiles

To Reproduce Steps to reproduce the behavior

$ tree
.
├── go.mod
├── go.sum
├── main.go
├── p2
│   ├── mocks
│   │   └── mocks.go
│   └── thing.go
├── packagename
│   └── foo.go
├── tools.go
└── vendor
   snip
    ├── go.uber.org
    │   └── mock
    │       ├── AUTHORS
    │       ├── LICENSE
    │       └── mockgen
    │           ├── deprecated.go
    │           ├── generic.go
    │           ├── gob.go
    │           ├── mockgen.go
    │           ├── model
    │           │   └── model.go
    │           ├── package_mode.go
    │           ├── parse.go
    │           └── version.go
    └── modules.txt

45 directories, 113 files

// go.mod
module mockgenImportProblem

go 1.23.0

require go.uber.org/mock v0.5.0

require (
	golang.org/x/mod v0.18.0 // indirect
	golang.org/x/sync v0.7.0 // indirect
	golang.org/x/tools v0.22.0 // indirect
)


// packagename/foo.go

package packagename

type WhateverExported struct {
}

// p2/thing.go

//go:generate mockgen -destination=mocks/mocks.go -package=mocks mockgenImportProblem/p2 Mything
package p2

import (
	"mockgenImportProblem/packagename"
)

type Mything interface {
	// issue here, is that the variable has the same name as an imported package.
	DoThat(packagename int) packagename.WhateverExported
}

With v0.4.0, we get

// DoThat mocks base method.
func (m *MockMything) DoThat(arg0 int) packagename.WhateverExported {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "DoThat", arg0)
	ret0, _ := ret[0].(packagename.WhateverExported)
	return ret0
}

with v0.5.0 we have

// DoThat mocks base method.
func (m *MockMything) DoThat(packagename int) packagename.WhateverExported {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "DoThat", packagename)
	ret0, _ := ret[0].(packagename.WhateverExported) // there is the problem. because the 'packagename' refers to the integer passed as parameter, and not to the import. This was not an issue in v0.4 because few packages are named (arg0, arg1, ...) but the potential for conflict between variable names and import packages is more important in v0.5 since both are provided by the end developer.
	return ret0
}

The solution for my work project is to wait for a fix upstream. A workaround is to rename the variable DoThat(packagename int) packagename.WhateverExported => DoThat(packagename2 int) packagename.WhateverExported, but it's annoying.
A full fix will be to introduce some deterministic aliases to the package imports, making sure that the name is not reused by a parameter name.

Additional Information

  • gomock mode (reflect or source): SOURCE, with 0.4 and 0.5
  • golang version: 1.23

Triage Notes for the Maintainers

Thanks for maintaining this project.

@bstncartwright
Copy link
Contributor

Also encountering this. For now I just edit the generated code manually but like @HassanCehef said, it is inconvenient and makes it so we cannot generate as part of continuous integration.

@bstncartwright
Copy link
Contributor

I've opened a pull request with a fix for this in #219

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

Successfully merging a pull request may close this issue.

3 participants