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

x/tools/cmd/goimports: adds redundant aliases #30051

Closed
speedyhoon opened this issue Feb 1, 2019 · 4 comments
Closed

x/tools/cmd/goimports: adds redundant aliases #30051

speedyhoon opened this issue Feb 1, 2019 · 4 comments

Comments

@speedyhoon
Copy link

@speedyhoon speedyhoon commented Feb 1, 2019

What version of Go are you using?

go1.11.1 windows/amd64 (Windows 10)

Does this issue reproduce with the latest release?

Yes

What did you do?

run goimports in the current directory.
goimports .

What did you expect to see?

package main

import "github.com/speedyhoon/v8"

func main() {
        v8.Str(nil, "")
}

What did you see instead?

package main

import v8 "github.com/speedyhoon/v8"

func main() {
        v8.Str(nil, "")
}

I named this package "v8", abbreviated from "validation". (okay the name might be a little short)
The package name seems to be colliding with assumptions around goimports matching a versioning pattern like: ^v[0-9]+$
However the alias in this case is redundant because it matches the package name exactly.

@gopherbot gopherbot added this to the Unreleased milestone Feb 1, 2019
@mvdan
Copy link
Member

@mvdan mvdan commented Feb 1, 2019

@myitcv
Copy link
Member

@myitcv myitcv commented Feb 1, 2019

@speedyhoon - aside from any potential bug with goimports, you're going to need to use a different package name here:

#28435 (comment)

Because github.com/speedyhoon/v8 is also the module path as well as the path to the package named v8.

@heschik
Copy link
Contributor

@heschik heschik commented Feb 8, 2019

This is working as intended. goimports is recording the fact that this is a package named v8, rather than version 8 of the module github.com/speedyhoon. That allows it to avoid reading the package from disk in most cases, speeding up the common case where no dependencies are missing. I don't think there's a way to fix this without sacrificing performance in the much more common case that /vN is a version suffix.

@misterikkit
Copy link

@misterikkit misterikkit commented May 23, 2019

That allows it to avoid reading the package from disk in most cases,

Sorry to jump in on a closed issue, but how/why does this improve performance? We have a ton of packages in kubernetes that use /vN to denote versions, and the redundant import alias is still annoying and unwanted.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.