x/tools/internal/imports: redundant import name is not removed

[nd]

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

$ go version
go version go1.12.6 linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/nd/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/nd/go"
GOPROXY=""
GORACE=""
GOROOT="/home/nd/go/go1.12.6"
GOTMPDIR=""
GOTOOLDIR="/home/nd/go/go1.12.6/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/nd/p/golang-tools/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build074254564=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I've ran goimports built on revision 2e34cfcb95cb3d24b197d58fe6d25046b8f25c86 on the following file:

package main

import opentracing "github.com/opentracing/opentracing-go"

func main() {
    println(opentracing.Binary)
}

What did you expect to see?

The redundant import alias is removed.

What did you see instead?

Nothing is changed.

Looks like introduced by 9bea2ecb9504b47b0689751119be8b92a371a771: the imp.name != "" check prevents redundant import cleanup.

[heschi]

cc @suzmue, but I think that commit just moved around existing code.

I'm not sure goimports has ever been able to do this, so it's more of a feature request than a bug, but I think it's a reasonable one. Just to confirm: if you remove the import name manually, it stays gone?

[nd]

Hmm, I was under impression that goimports removed redundant aliases, but it seems I was wrogn. Yes, if I remove the alias it is not added back.

[shawnshuang]

I am also experiencing this. Previously, redundant names would not be added, but now they are.

How I tested this:

(1) I use VSCode with the Go extension (https://github.com/Microsoft/vscode-go, v0.11.4), which provides the ability to change the formatting tool (i.e. gofmt, goimports, goreturns).

If I have the formatting tool set to gofmt or goreturns, the redundant aliases are not added.

If I have the formatting tool set to goimports, the redundant aliases are added. The redundant aliases are even added back when I remove them.

(2) Some of our team members use VSCode and others GoLand. GoLand doesn’t have extensions, but they have file watches, for which you can set one up with goimports.

When I enable the gofmt file watcher and disable the goimports one, the redundant aliases are not added.

Likewise with VSCode, when I disable the gofmt file watcher and enable the goimports one, the redundant aliases are added, and removing them has them added back.

(3) I used goimports on a file on the command line, and doing so also adds the redundant alias.

[suzmue]

@shawnshuang could you provide an example that will have redundant aliases added by goimports? Thanks!

[shawnshuang]

@suzmue github.com/go-ozzo/ozzo-dbx is one of the libraries that our team uses. If you:

(1) Run go get github.com/go-ozzo/ozzo-dbx (2) Create a file with the following somewhere in your GOPATH:

package main

import (
    "fmt"

         // Intentionally leaving out "github.com/go-ozzo/ozzo-dbx"
)

func main() {
        // Does nothing, just code to access something from the dbx package
    params := dbx.Params{}

    fmt.Println(params)
}

(3) Run goimports -w path/to/the/above/file (4) You should see that dbx "github.com/go-ozzo/ozzo-dbx" is added to the file, which is redundant since the package name is already dbx.

[suzmue]

@shawnshuang Thanks for the repro!

This is intended behavior from goimports, which makes sure that an existing import with mismatched path/name has its name added (See #28428).

If you disagree with this behavior, please file a separate issue.

[mitar]

So this is the function which extracts "perceived" name from the import path. It has some exceptions. So for github.com/go-ozzo/ozzo-dbx it extracts ozzo (and because this does not match dbx, dbx alias is added). And for github.com/opentracing/opentracing-go it extracts opentracing. This is why it does NOT add opentracing alias, because the extracted "perceived" name matches the package name.

What this issue is above is a feature request that unnecessary aliases (those which do match extracted "perceived" name and the package name) should be removed. goimports does not remove aliases if you have:

import (
        opentracing "github.com/opentracing/opentracing-go"
        http "net/http"
        fmt "fmt"
)

And this issue is that these aliases should be removed. In all these cases. I agree.