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: Unexpected goimports behaviour depending on prior imports #29180

Closed
rileykarson opened this issue Dec 12, 2018 · 7 comments

Comments

@rileykarson
Copy link

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

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

darwin (MacOs), amd64

What did you do?

I work on a code generator that generates a Terraform provider for Google Cloud Platform; we generate code without imports and use goimports to add them.

I updated my local copy of goimports today, and ran into some very unexpected changes. There was a large diff, but most importantly goimports isn't generating correct imports for at least 1 file anymore.

These examples aren't runnable; they're variants of this file, the correct output: https://github.com/terraform-providers/terraform-provider-google/blob/2.0.0/google/resource_compute_subnetwork.go

Given this file: https://play.golang.org/p/Ku1KKO3urCd

when I run goimports -v -w google/resource_compute_subnetwork.go

What did you expect to see?

I expect to see https://github.com/terraform-providers/terraform-provider-google/blob/2.0.0/google/resource_compute_subnetwork.go

If I run goimports in a separate directory with no other files, I see the correct import path. When I run it on an unformatted copy of the file in the provider repo, I get an incorrect output with the bad import path.

What did you see instead?

-compute "google.golang.org/api/compute/v1"
+computeBeta "google.golang.org/api/compute/v0.beta"

Full file: https://play.golang.org/p/wKgEyMWdZto

@gopherbot gopherbot added this to the Unreleased milestone Dec 12, 2018
@rileykarson
Copy link
Author

Possibly related to #23001

@heschi heschi self-assigned this Dec 12, 2018
@heschi
Copy link
Contributor

heschi commented Dec 12, 2018

This is probably related to my recent changes. I'll take a look tomorrow.

@chrisst
Copy link

chrisst commented Dec 12, 2018

Adding a few more details:
I removed the entire imports block and ran goimports and found the output was as @rileykarson posted above with the reference to the compute beta. Running goimports a second time on the same file (without any other changes) resulted in something more correct looking:

 -   compute "google.golang.org/api/compute/v1"
 +   "google.golang.org/api/compute/v1"

The reference to computeBeta doesn't build, but after running goimports a second time it does result in compilable code.

The behavior appears to have changed as of https://go.googlesource.com/tools/+/ee45598d2ff288037f53f9e13ae0b1a6e2165ad5

@heschi
Copy link
Contributor

heschi commented Dec 12, 2018

This is a variant of a bug I failed to fix correctly in https://golang.org/cl/153440. CL shortly.

@gopherbot
Copy link

Change https://golang.org/cl/153859 mentions this issue: imports: fix renamed sibling imports more

@rileykarson
Copy link
Author

For completeness, I wanted to confirm that this is working again. Thanks for the quick turnaround!

@heschi
Copy link
Contributor

heschi commented Dec 12, 2018

Quite welcome, sorry for the glitch.

@golang golang locked and limited conversation to collaborators Dec 12, 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.
Projects
None yet
Development

No branches or pull requests

4 participants