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.

Sign up for GitHub

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

Jump to bottom

x/tools/cmd/goimports: doesn't handle packages named "v1", etc #29041

Closed
mattmoor opened this issue Nov 30, 2018 · 10 comments
Closed

x/tools/cmd/goimports: doesn't handle packages named "v1", etc #29041

mattmoor opened this issue Nov 30, 2018 · 10 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@mattmoor
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.11 linux/amd64

I'm seeing a bad format with goimports at commit: 1c3d964

You can see the bad formatting turned into a PR here: knative/pkg#176

The comment run is what's in the PR description (minus the -s, it's WIP).

cc @bradfitz
@gopherbot gopherbot added this to the Unreleased milestone Nov 30, 2018
@mattmoor
Copy link
Author

I see this with the head of release-branch.go1.11 as well.

@bradfitz
Copy link
Contributor

Don't name your packages v1, I guess?

@bradfitz bradfitz changed the title x/tools/goimports: strips necessary import x/tools/cmd/goimports: doesn't handle packages named "v1", etc Nov 30, 2018
@mattmoor
Copy link
Author

Yeah this blows up on anything touching K8s :P

@bradfitz
Copy link
Contributor

Was this caused by https://go-review.googlesource.com/122616? (golang/tools@893c2b1)

Curious whether/how people think we should fix this.

@rsc @heschik @bcmills @dmitshur @kevinburke @Gnouc @josharian @mvdan @fraenkel @haya14busa

@bcmills
Copy link
Contributor

bcmills commented Nov 30, 2018

See also #28435.

@bcmills
Copy link
Contributor

bcmills commented Nov 30, 2018

Curious whether/how people think we should fix this.

If v1 is used as the Expr in a SelectorExpr, then goimports should not remove packages that end with the suffix v1. (It's fine if it doesn't add them, though: v1 is a terrible name for a package.)

We can compute the set of “names that might be package names” in an O(N) scan over the source file.

@bcmills
Copy link
Contributor

bcmills commented Nov 30, 2018

As a workaround, what happens if you rename the package explicitly on import?

That is, change

import (
	"k8s.io/api/core/v1"
)

to

import (
	v1 "k8s.io/api/core/v1"
)

?

@heschi
Copy link
Contributor

heschi commented Nov 30, 2018

I haven't looked at this closely but I believe Bryan's suggestion will fix it, and my fix for #28428 will add the v1 automatically.

@heschi heschi self-assigned this Nov 30, 2018
@mattmoor mattmoor mentioned this issue Nov 30, 2018
Closed
@heschi
Copy link
Contributor

heschi commented Nov 30, 2018

Give https://golang.org/cl/152000 a try?

@heschi
Copy link
Contributor

heschi commented Dec 5, 2018

Confirmed that tip goimports now adds the v1 name to the import rather than deleting it.

@heschi heschi closed this as completed Dec 5, 2018
@heschi heschi mentioned this issue Dec 12, 2018
Open
negz added a commit to negz/crossplane that referenced this issue Mar 6, 2019
@negz
Import versioned ('v1' etc) style names explicitly
dd1f963 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane that referenced this issue Mar 6, 2019
@negz
Import versioned ('v1' etc) style names explicitly
15feeff 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane that referenced this issue Mar 6, 2019
@negz
Import versioned ('v1' etc) style names explicitly
59a4349 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane that referenced this issue Mar 6, 2019
@negz
Import versioned ('v1' etc) style names explicitly
9b37047 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane that referenced this issue Mar 6, 2019
@negz
Import versioned ('v1' etc) style names explicitly
a2dd915 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
negz added a commit to negz/crossplane that referenced this issue Mar 6, 2019
@negz
Import versioned ('v1' etc) style names explicitly
928dbec 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
jbw976 pushed a commit to jbw976/gcp-filter-test that referenced this issue Aug 19, 2019
@negz
Import versioned ('v1' etc) style names explicitly
acadf1f 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
jbw976 pushed a commit to jbw976/crossplane that referenced this issue Aug 20, 2019
@negz
Import versioned ('v1' etc) style names explicitly
65b39a2 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
jbw976 pushed a commit to jbw976/crossplane that referenced this issue Aug 20, 2019
@negz
Import versioned ('v1' etc) style names explicitly
fedd7fd 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
jbw976 pushed a commit to jbw976/crossplane that referenced this issue Aug 20, 2019
@negz
Import versioned ('v1' etc) style names explicitly
d1de6b1 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
hasheddan pushed a commit to hasheddan/crossplane-runtime-temp that referenced this issue Aug 20, 2019
@negz
Import versioned ('v1' etc) style names explicitly
ec20c78 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
raghavendra-talur added a commit to raghavendra-talur/heketi that referenced this issue Aug 23, 2019
@raghavendra-talur
all: fix goimports errors
29faa83 
There are 3 types of errors fixed:
a. one empty line is required before where third-party pkgs imports
b. packages can have dashes in import path but won't have in name[1],
explicitly name them on import.
c. packages that are just version numbers like "v1" need explicit naming
too[2].

[1] golang/go#17546
[2] golang/go#29041

Signed-off-by: Raghavendra Talur <rtalur@redhat.com>
@raghavendra-talur raghavendra-talur mentioned this issue Aug 23, 2019
Merged
phlogistonjohn pushed a commit to heketi/heketi that referenced this issue Aug 23, 2019
@raghavendra-talur @phlogistonjohn
all: fix goimports errors
0af21d9 
There are 3 types of errors fixed:
a. one empty line is required before where third-party pkgs imports
b. packages can have dashes in import path but won't have in name[1],
explicitly name them on import.
c. packages that are just version numbers like "v1" need explicit naming
too[2].

[1] golang/go#17546
[2] golang/go#29041

Signed-off-by: Raghavendra Talur <rtalur@redhat.com>
jbw976 pushed a commit to crossplane-contrib/provider-azure that referenced this issue Aug 30, 2019
@negz
Import versioned ('v1' etc) style names explicitly
cb8d710 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
jbw976 pushed a commit to crossplane-contrib/provider-aws that referenced this issue Sep 1, 2019
@negz
Import versioned ('v1' etc) style names explicitly
5cacac2 
golang/go#29041

This is a slightly counterintuitive workaround for the above bug. The
alternative to this commit would be to disable goimports linting until the bug
is fixed. I think it's worth the workaround for the time being in exchange for
the value goimports linting brings.

Signed-off-by: Nic Cope <negz@rk0n.org>
@shu-mutou shu-mutou mentioned this issue Oct 7, 2019
Merged
@golang golang locked and limited conversation to collaborators Dec 5, 2019
@rsc rsc unassigned heschi Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bradfitz @mattmoor @bcmills @gopherbot @heschi