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

Duplicate imports with different names in multiple files cause error #9

Open
nkovacs opened this issue May 29, 2017 · 4 comments
Open

Comments

@nkovacs
Copy link
Collaborator

nkovacs commented May 29, 2017

Assuming you have these two files:

package foo

import (
    pkg1 "github.com/user/pkg"
)

type FooStruct{}

func (f *FooStruct) One(pkg1.Foo) {
}
package foo

import (
    pkg2 "github.com/user/pkg"
)

func (f *FooStruct) Two(pkg2.Foo) {
}

If you pass both files to ifacemaker, it will generate:

import (
    pkg1 "github.com/user/pkg"
    pkg2 "github.com/user/pkg"
)

type FooInterface {
    One(pkg1.Foo)
    Two(pkg2.Foo)
}

which is not valid because of the duplicate imports.

Similarly problematic are pkg1 "github.com/user/pkg" + "github.com/user/pkg" and even pkg "github.com/user/pkg" + "github.com/user/pkg".

I'm not sure if it's worth fixing this. The hardest part is probably finding an alias that doesn't conflict, after that, aliasing everything should be easier.

@vburenin
Copy link
Owner

This is definitely an interesting issue that can point to the naming inconsistency in the original code. I think it is worth fixing, but it would be good to print a warning that two different names are used.

nkovacs added a commit to nkovacs/ifacemaker that referenced this issue Jun 29, 2017
Move all the business logic of parsing and generating code
into maker subpackage.

Instead of functions, the maker package exports a Maker struct,
which has two exported methods: ParseSource to add a file,
and MakeInterface to create the interface.

The struct now handles the data that was previously handled
by the main package between the invocation of the two functions.

Don't use token.Pos to access source file byte array,
this doesn't work when there are multiple files in the fileset.
Use go/printer or directly get the parsed strings from the ast nodes.

Ignore dot imports, assume they will not be needed by the
generated struct.

Throw errors when aliases conflict or a package is imported
with different aliases (vburenin#9).

Add a "Code generated by" comment per
https://golang.org/s/generatedcode

Add tests + travis.
nkovacs added a commit to nkovacs/ifacemaker that referenced this issue Jun 29, 2017
Move all the business logic of parsing and generating code
into maker subpackage.

Instead of functions, the maker package exports a Maker struct,
which has two exported methods: ParseSource to add a file,
and MakeInterface to create the interface.

The struct now handles the data that was previously handled
by the main package between the invocation of the two functions.

Don't use token.Pos to access source file byte array,
this doesn't work when there are multiple files in the fileset.
Use go/printer or directly get the parsed strings from the ast nodes.

Ignore dot imports, assume they will not be needed by the
generated struct.

Throw errors when aliases conflict or a package is imported
with different aliases (vburenin#9).

Add a "Code generated by" comment per
https://golang.org/s/generatedcode

Add tests + travis.
@nkovacs
Copy link
Collaborator Author

nkovacs commented Jun 29, 2017

I've implemented the warnings.

It's possible to fix this properly (i.e. automatically generate a non-conflicting alias), but it's quite complicated.

Since the package name doesn't have to be the same as the import path's last element, you have to find and parse imported packages to find out their actual package names, so that you can then find the SelectorExprs that need to be renamed. Actually I already did this in counterfeiter: maxbrunsfeld/counterfeiter#74, but counterfeiter already had code to find imported packages, because it already had to parse imports. It also already had code to rewrite the ast.
Functionality for rewriting the ast is missing from the standard library, so that also makes things more difficult: golang/go#17108

@vburenin
Copy link
Owner

That is definitely a lot of work. I would not go that far unless it is really needed.

nkovacs added a commit to nkovacs/ifacemaker that referenced this issue Jun 29, 2017
Move all the business logic of parsing and generating code
into maker subpackage.

Instead of functions, the maker package exports a Maker struct,
which has two exported methods: ParseSource to add a file,
and MakeInterface to create the interface.

The struct now handles the data that was previously handled
by the main package between the invocation of the two functions.

Don't use token.Pos to access source file byte slice,
this doesn't work when there are multiple files in the fileset.
Use go/printer or directly get the parsed strings from the ast nodes.

Ignore dot imports, assume they will not be needed by the
generated struct.

Throw errors when aliases conflict or a package is imported
with different aliases (vburenin#9).

Add a "Code generated by" comment per
https://golang.org/s/generatedcode

Add tests + travis.
@nkovacs nkovacs mentioned this issue Jun 29, 2017
nkovacs added a commit to nkovacs/ifacemaker that referenced this issue Jun 29, 2017
Also ignore files that don't contain exported methods
of the source struct, to avoid problems with imports (see vburenin#9).

Fixes vburenin#3
@nkovacs
Copy link
Collaborator Author

nkovacs commented Jun 29, 2017

I ran into this while fixing #3, but by just ignoring the files that don't contain methods, this error becomes much less likely to occur. I think it's reasonable to require those files to have consistent imports, and you get warnings if you make a mistake. That's a good enough solution for me.

nkovacs added a commit to nkovacs/ifacemaker that referenced this issue Jul 5, 2017
Also ignore files that don't contain exported methods
of the source struct, to avoid problems with imports (see vburenin#9).

Fixes vburenin#3
nkovacs added a commit to nkovacs/ifacemaker that referenced this issue Aug 8, 2017
Move all the business logic of parsing and generating code
into maker subpackage.

Instead of functions, the maker package exports a Maker struct,
which has two exported methods: ParseSource to add a file,
and MakeInterface to create the interface.

The struct now handles the data that was previously handled
by the main package between the invocation of the two functions.

Don't use token.Pos to access source file byte slice,
this doesn't work when there are multiple files in the fileset.
Use go/printer or directly get the parsed strings from the ast nodes.

Ignore dot imports, assume they will not be needed by the
generated struct.

Throw errors when aliases conflict or a package is imported
with different aliases (vburenin#9).

Add a "Code generated by" comment per
https://golang.org/s/generatedcode

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

No branches or pull requests

2 participants