Skip to content

Commit

Permalink
internal/imports: remove duplicate unused imports
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Deiz committed Apr 5, 2021
1 parent 8c34cc9 commit 79f5729
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 27 deletions.
56 changes: 29 additions & 27 deletions internal/imports/fix.go
Expand Up @@ -252,7 +252,7 @@ type pass struct {
otherFiles []*ast.File // sibling files.

// Intermediate state, generated by load.
existingImports map[string]*ImportInfo
existingImports map[string][]*ImportInfo
allRefs references
missingRefs references

Expand Down Expand Up @@ -317,7 +317,7 @@ func (p *pass) importIdentifier(imp *ImportInfo) string {
func (p *pass) load() ([]*ImportFix, bool) {
p.knownPackages = map[string]*packageInfo{}
p.missingRefs = references{}
p.existingImports = map[string]*ImportInfo{}
p.existingImports = map[string][]*ImportInfo{}

// Load basic information about the file in question.
p.allRefs = collectReferences(p.f)
Expand Down Expand Up @@ -348,7 +348,7 @@ func (p *pass) load() ([]*ImportFix, bool) {
}
}
for _, imp := range imports {
p.existingImports[p.importIdentifier(imp)] = imp
p.existingImports[p.importIdentifier(imp)] = append(p.existingImports[p.importIdentifier(imp)], imp)
}

// Find missing references.
Expand Down Expand Up @@ -387,31 +387,33 @@ func (p *pass) fix() ([]*ImportFix, bool) {

// Found everything, or giving up. Add the new imports and remove any unused.
var fixes []*ImportFix
for _, imp := range p.existingImports {
// We deliberately ignore globals here, because we can't be sure
// they're in the same package. People do things like put multiple
// main packages in the same directory, and we don't want to
// remove imports if they happen to have the same name as a var in
// a different package.
if _, ok := p.allRefs[p.importIdentifier(imp)]; !ok {
fixes = append(fixes, &ImportFix{
StmtInfo: *imp,
IdentName: p.importIdentifier(imp),
FixType: DeleteImport,
})
continue
}
for _, identifierImports := range p.existingImports {
for _, imp := range identifierImports {
// We deliberately ignore globals here, because we can't be sure
// they're in the same package. People do things like put multiple
// main packages in the same directory, and we don't want to
// remove imports if they happen to have the same name as a var in
// a different package.
if _, ok := p.allRefs[p.importIdentifier(imp)]; !ok {
fixes = append(fixes, &ImportFix{
StmtInfo: *imp,
IdentName: p.importIdentifier(imp),
FixType: DeleteImport,
})
continue
}

// An existing import may need to update its import name to be correct.
if name := p.importSpecName(imp); name != imp.Name {
fixes = append(fixes, &ImportFix{
StmtInfo: ImportInfo{
Name: name,
ImportPath: imp.ImportPath,
},
IdentName: p.importIdentifier(imp),
FixType: SetImportName,
})
// An existing import may need to update its import name to be correct.
if name := p.importSpecName(imp); name != imp.Name {
fixes = append(fixes, &ImportFix{
StmtInfo: ImportInfo{
Name: name,
ImportPath: imp.ImportPath,
},
IdentName: p.importIdentifier(imp),
FixType: SetImportName,
})
}
}
}

Expand Down
13 changes: 13 additions & 0 deletions internal/imports/fix_test.go
Expand Up @@ -1151,6 +1151,19 @@ var _, _ = rand.Read, rand.NewZipf
import "math/rand"
var _, _ = rand.Read, rand.NewZipf
`,
},
{
name: "unused_duplicate_imports_remove",
in: `package main
import (
"errors"
"github.com/pkg/errors"
)
`,
out: `package main
`,
},
}
Expand Down

0 comments on commit 79f5729

Please sign in to comment.