uber-go / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
2.21k stars 115 forks source link

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

Open HassanCehef opened 3 days ago

HassanCehef commented 3 days ago

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

Triage Notes for the Maintainers

Thanks for maintaining this project.

bstncartwright commented 3 days ago

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 commented 3 days ago

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