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/internal/imports: redundant import name is not removed #33294

Open
nd opened this issue Jul 26, 2019 · 7 comments
Open

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

nd opened this issue Jul 26, 2019 · 7 comments
Labels
FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@nd
Copy link
Contributor

nd commented Jul 26, 2019

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 2e34cfc 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 9bea2ec: the imp.name != "" check prevents redundant import cleanup.

@gopherbot gopherbot added this to the Unreleased milestone Jul 26, 2019
@heschi
Copy link
Contributor

heschi commented Jul 26, 2019

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
Copy link
Contributor Author

nd commented Jul 26, 2019

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.

@katiehockman katiehockman added FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jul 29, 2019
@shawnshuang

This comment has been minimized.

@suzmue

This comment has been minimized.

@shawnshuang

This comment has been minimized.

@suzmue

This comment has been minimized.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@mitar
Copy link
Contributor

mitar commented Dec 14, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

7 participants