From 79f5729da29d44425eb9faf5247dfac3b4f1d433 Mon Sep 17 00:00:00 2001 From: Sean Hildebrand Date: Mon, 5 Apr 2021 16:31:12 -0400 Subject: [PATCH] internal/imports: remove duplicate unused imports 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 --- internal/imports/fix.go | 56 +++++++++++++++++++----------------- internal/imports/fix_test.go | 13 +++++++++ 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/internal/imports/fix.go b/internal/imports/fix.go index d859617b774..56388a997b0 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -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 @@ -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) @@ -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. @@ -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, + }) + } } } diff --git a/internal/imports/fix_test.go b/internal/imports/fix_test.go index 005bf96e7e6..02269361811 100644 --- a/internal/imports/fix_test.go +++ b/internal/imports/fix_test.go @@ -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 `, }, }