x/tools/cmd/goimports: removes necessary imports with certain characters

[mattmoor]

Using goimports at release-branch.go1.10, I see it dropping necessary imports when they have - and are not aliased,

You can see it in this removal with a hyphen:
https://github.com/knative/eventing/pull/653/files#diff-abdff22f055e921908e64cdb584fa75cL25

... and this removal with a dot:
https://github.com/knative/docs/pull/611/files#diff-06a9a211281c9004b5638fc5207899d8L32

I can't use HEAD or 1.11 because of: #29041

[mattmoor]

Another with a dot: https://github.com/google/go-containerregistry/pull/313/files#diff-84d0b2b66c1e04aafeb35b922e2a399cL23

[agnivade]

Hi @mattmoor - Could you provide us with a self-contained code sample which reproduces this behavior ? I tried this myself but wasn't able to reproduce it.

I can't use HEAD or 1.11 because of: #29041

I believe that is a different issue than this, right ? Is there a reason you cannot use the tip goimports to test this bug ?

/cc @bradfitz

EDIT: I also have github.com/aws/aws-sdk-go throughout several codebases, and goimports work fine there. Trying to understand if there is a regression somewhere or this was fixed.

[mattmoor]

Weird, so I cannot reproduce this when github.com/knative/docs is on ${GOPATH}, but I can reproduce this if the repo is simply cloned into /workspace.

[mattmoor]

Steps:

1. go get golang.org/x/tools/cmd/goimports
2. Checkout release-branch.go1.10 (optional, I confirmed this reproduces @ HEAD)
3. git clone https://github.com/knative/docs /tmp/docs && cd /tmp/docs
4. go run golang.org/x/tools/cmd/goimports -w eventing/samples/iot-core/generator/main.go
5. git diff eventing/samples/iot-core/generator/main.go

[agnivade]

I just tried with HEAD (d0ca393) and I am still unable to reproduce it.

$git clone https://github.com/knative/docs /tmp/docs && cd /tmp/docs
Cloning into '/tmp/docs'...
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Compressing objects: 100% (7/7), done.
remote: Total 4597 (delta 0), reused 0 (delta 0), pack-reused 4590
Receiving objects: 100% (4597/4597), 13.36 MiB | 2.34 MiB/s, done.
Resolving deltas: 100% (2401/2401), done.
12:28:36-agniva-/tmp/docs$
12:31:41-agniva-/tmp/docs$go run golang.org/x/tools/cmd/goimports -w eventing/samples/iot-core/generator/main.go
12:31:46-agniva-/tmp/docs$git st
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
12:31:56-agniva-/tmp/docs$git diff eventing/samples/iot-core/generator/main.go
12:32:53-agniva-/tmp/docs$

Could you please attach the output of go env ?

[heschi]

Weird, so I cannot reproduce this when github.com/knative/docs is on ${GOPATH}, but I can reproduce this if the repo is simply cloned into /workspace.

Yeah. In general, if a package's name doesn't match its import path, goimports is going to have to load it. In GOPATH mode, that means having it on GOPATH. tip goimports now adds a name to the import in these situations, which prevents the problem.

$ GOPATH=/tmp/foo go get -d github.com/knative/docs
package github.com/knative/docs: no Go files in /tmp/foo/src/github.com/knative/docs
$ cd /tmp/foo/src/github.com/knative/docs 
$ goimports -w eventing/samples/iot-core/generator/main.go 
$ git diff
diff --git a/eventing/samples/iot-core/generator/main.go b/eventing/samples/iot-core/generator/main.go
index 813e2b0..018e9b0 100644
--- a/eventing/samples/iot-core/generator/main.go
+++ b/eventing/samples/iot-core/generator/main.go
@@ -27,9 +27,7 @@ import (
        "math/rand"
        "time"
 
-       "github.com/dgrijalva/jwt-go"
        MQTT "github.com/eclipse/paho.mqtt.golang"
-       "github.com/satori/go.uuid"
 )
 
 const (
$ git reset --hard
HEAD is now at 098b76e move eventing to 0.2.1 release (#629)
$ GOPATH=/tmp/foo goimports -w eventing/samples/iot-core/generator/main.go
$ git diff                                                                                 
diff --git a/eventing/samples/iot-core/generator/main.go b/eventing/samples/iot-core/generator/main.go
index 813e2b0..052ad2d 100644
--- a/eventing/samples/iot-core/generator/main.go
+++ b/eventing/samples/iot-core/generator/main.go
@@ -27,9 +27,9 @@ import (
        "math/rand"
        "time"
 
-       "github.com/dgrijalva/jwt-go"
+       jwt "github.com/dgrijalva/jwt-go"
        MQTT "github.com/eclipse/paho.mqtt.golang"
-       "github.com/satori/go.uuid"
+       uuid "github.com/satori/go.uuid"
 )
 
 const (
$ git commit -a -m foo  
[master f0aefbd] foo
 1 file changed, 2 insertions(+), 2 deletions(-)
$ goimports -w eventing/samples/iot-core/generator/main.go                
$ git diff
$ 

@agnivade: you probably had a copy of the package in your GOPATH already when you ran goimports.

I don't think there's anything we can do to improve the situation further, so I'm closing this. Comment if you disagree.

[mattmoor]

@heschik I worked around this by symlinking the repo onto GOPATH. I will try tip when I get a chance. thanks.