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

Fails to correctly handle multi-alias imports. #45

Open
dave opened this issue May 12, 2020 · 2 comments
Open

Fails to correctly handle multi-alias imports. #45

dave opened this issue May 12, 2020 · 2 comments

Comments

@dave
Copy link
Owner

dave commented May 12, 2020

When dst automatically manages the imports block, the imports are internally keyed by path. This incorrectly assumes that only one import for each package path is possible. Although very rare, it is possible to add the same package multiple times with different aliases.

See: golang/go@3409ce3

In TestLoadStdLibAll we test that the entire standard library is able to be parsed and rebuilt including management of the imports block. This test fails for the two files server.go and request.go in net/http because of this.

In order to handle this gracefully would involve a major rewrite of the import management code which is the most complex part of dst. For now I've skipped the TestLoadStdLibAll for these two files.

Re: @dmitshur

@dave
Copy link
Owner Author

dave commented Aug 23, 2020

This has come up again in the Go 1.15 standard library. I will exclude src/crypto/x509/x509.go from this test because it now has a duplicate import. This has come up twice now so I really should fix it.

p.s. I wonder if it was intentional to add a duplicate crypto/sha1 import in golang/go@6f3a951 @katiehockman

@katiehockman
Copy link

@dave Nope that wasn't intentional. I think it should keep the import that golang/go@6f3a951 added and remove the blank identifier import. Thanks for pointing that out.

gopherbot pushed a commit to golang/go that referenced this issue Aug 25, 2020
Updates dave/dst#45.

Change-Id: I165e6b3d002407a33908bf90a66ad01f8003b260
Reviewed-on: https://go-review.googlesource.com/c/go/+/250497
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
bassosimone pushed a commit to ooni/oocrypto that referenced this issue May 21, 2022
Updates dave/dst#45.

Change-Id: I165e6b3d002407a33908bf90a66ad01f8003b260
Reviewed-on: https://go-review.googlesource.com/c/go/+/250497
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
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

No branches or pull requests

2 participants