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

Ignore anonymous imports when resolving import aliases #150

Merged
merged 2 commits into from
Jul 4, 2021
Merged

Ignore anonymous imports when resolving import aliases #150

merged 2 commits into from
Jul 4, 2021

Conversation

maneac
Copy link
Contributor

@maneac maneac commented Jul 1, 2021

Problem

Given the following two files in a package:

file1.go:

package foo

import (
    bar "some.repo/import/bar"
)

type Barrier interface {
    Baz(bar.Qux)
}

...

file2.go:

package foo

import (
    _ "some.repo/import/bar"
)

...

, suppose you want to generate a mock of Barrier.

Today, running moq -out barrier.mock.go . Barrier would produce the following:

barrier.mock.go

package foo

import (
    _ "some.repo/import/bar"
    "sync"
)

...

type BarrierMock struct {
    // BazFunc mocks the Baz method.
    BazFunc(quxer _.Qux)

...

, which is invalid Go 😢 .

Why

This is caused by a specific interaction between two steps:

  • When parsing the import statements of each file in the requested package to construct the alias map, they are processed in alphabetical filename order.
  • When adding an alias to a map, it overwrites any existing alias for that import path.

Combined, the above means the valid bar alias is overwritten by the invalid _ that gets processed later, and applied to the imported types in the generated functions.

Solution

Ignore anonymous imports when constructing the alias map. This means a valid alias will never be overwritten, and as anonymous imports will always be unused by a mock, they will always be safe to ignore.

@sudo-suhas
Copy link
Collaborator

Thanks for explaining the problem and proposing the solution. Could we please also add a test case for the same?

@sudo-suhas sudo-suhas merged commit b4465d5 into matryer:master Jul 4, 2021
@sudo-suhas
Copy link
Collaborator

Thank you @maneac for the PR 🎉

Change has been tagged and released in v0.2.2.

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 this pull request may close these issues.

None yet

2 participants