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
Closed
Assignees
Milestone

Comments

@rileykarson
Copy link

@rileykarson rileykarson commented Dec 12, 2018

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

@rileykarson rileykarson commented Dec 12, 2018

Possibly related to #23001

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

@heschik heschik commented Dec 12, 2018

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

@chrisst
Copy link

@chrisst 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

@heschik
Copy link
Contributor

@heschik heschik 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

@gopherbot gopherbot commented Dec 12, 2018

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

@rileykarson
Copy link
Author

@rileykarson rileykarson commented Dec 12, 2018

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

@heschik
Copy link
Contributor

@heschik heschik commented Dec 12, 2018

Quite welcome, sorry for the glitch.

@golang golang locked and limited conversation to collaborators Dec 12, 2019
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
4 participants
You can’t perform that action at this time.