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/imports: Process does not remove all unused imports sharing a package name #45398

Closed
Deiz opened this issue Apr 5, 2021 · 2 comments · May be fixed by golang/tools#299
Closed

x/tools/imports: Process does not remove all unused imports sharing a package name #45398

Deiz opened this issue Apr 5, 2021 · 2 comments · May be fixed by golang/tools#299
Labels
NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@Deiz
Copy link

Deiz commented Apr 5, 2021

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

$ go version
go version go1.16 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

See https://play.golang.org/p/yv78HSm0AX6 for a full example, and https://play.golang.org/p/GmUF-tayOgI for an interactive example (hit "Format" with imports enabled).

What did you expect to see?

package main

import (
	"errors"

	"github.com/pkg/errors"
	"github.com/juju/errors"
)

func main() {
}

This file has three imported errors packages, none of them are referenced outside of the import block, so they should all be removed, resulting in this output:

package main

func main() {
}

What did you see instead?

Only one errors package is removed per iteration - seemingly always the last one to appear. Because we have three imported packages, we need to run imports.Process three times to prune all the unused imports.

Iteration 1:

package main

import (
    "errors"

    "github.com/pkg/errors"
)

func main() {
}

Iteration 2:

package main

import (
    "errors"
)

func main() {
}

Iteration 3:

package main

func main() {
}
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Apr 5, 2021
@gopherbot gopherbot added this to the Unreleased milestone Apr 5, 2021
Deiz added a commit to Deiz/tools that referenced this issue Apr 5, 2021
Track imports per identifier as a slice to accommodate duplicates. This
allows us to generate multiple import fixes per import identifier.

Fixes golang/go#45398
@gopherbot
Copy link

Change https://golang.org/cl/307449 mentions this issue: internal/imports: remove duplicate unused imports

@stevenh
Copy link
Contributor

stevenh commented Oct 14, 2021

I just hit this issue and came up with the same fix independently.

@heschi I know there's an open question on what happens for used identifiers but would be great if we could fix this case and look at that one later as currently goimports and all tools which use the x/tools/imports package is unstable due to this issue.

@dmitshur dmitshur changed the title x/tools/imports: imports.Process does not remove all unused imports sharing a package name x/tools/imports: Process does not remove all unused imports sharing a package name Oct 1, 2022
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants