internal/imports: FixImports should be cancellable

Historically, FixImports did not have access to a context, and performed
its imports scan with context.Background. Since then, FixImports is
called by gopls with the CodeAction Context, yet cancelling this context
does not abort the scan. Fix this by using the correct context, and
checking context cancellation before parsing.

It's a little hard to see that context cancellation doesn't leave the
process environent in a broken state, but we can infer that this is OK
because other scans (such as that used by unimported completion) do
cancel their context.

Additionally, remove a 'fixImportsDefault' extensibility seam that is
apparently unused after six years.

For golang/go#67289

Change-Id: I32261b1bfb38af32880e981cd2423414069b32a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/589975
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

internal/imports/fix.go

@@ -104,19 +104,25 @@ type packageInfo struct {
 
 // parseOtherFiles parses all the Go files in srcDir except filename, including
 // test files if filename looks like a test.
-func parseOtherFiles(fset *token.FileSet, srcDir, filename string) []*ast.File {
+//
+// It returns an error only if ctx is cancelled. Files with parse errors are
+// ignored.
+func parseOtherFiles(ctx context.Context, fset *token.FileSet, srcDir, filename string) ([]*ast.File, error) {
 	// This could use go/packages but it doesn't buy much, and it fails
 	// with https://golang.org/issue/26296 in LoadFiles mode in some cases.
 	considerTests := strings.HasSuffix(filename, "_test.go")
 
 	fileBase := filepath.Base(filename)
 	packageFileInfos, err := os.ReadDir(srcDir)
 	if err != nil {
-		return nil
+		return nil, ctx.Err()
 	}
 
 	var files []*ast.File
 	for _, fi := range packageFileInfos {
+		if ctx.Err() != nil {
+			return nil, ctx.Err()
+		}
 		if fi.Name() == fileBase || !strings.HasSuffix(fi.Name(), ".go") {
 			continue
 		}
@@ -132,7 +138,7 @@ func parseOtherFiles(fset *token.FileSet, srcDir, filename string) []*ast.File {
 		files = append(files, f)
 	}
 
-	return files
+	return files, ctx.Err()
 }
 
 // addGlobals puts the names of package vars into the provided map.
@@ -557,12 +563,7 @@ func (p *pass) addCandidate(imp *ImportInfo, pkg *packageInfo) {
 
 // fixImports adds and removes imports from f so that all its references are
 // satisfied and there are no unused imports.
-//
-// This is declared as a variable rather than a function so goimports can
-// easily be extended by adding a file with an init function.
-var fixImports = fixImportsDefault
-
-func fixImportsDefault(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv) error {
+func fixImports(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv) error {
 	fixes, err := getFixes(context.Background(), fset, f, filename, env)
 	if err != nil {
 		return err
@@ -592,7 +593,10 @@ func getFixes(ctx context.Context, fset *token.FileSet, f *ast.File, filename st
 		return fixes, nil
 	}
 
-	otherFiles := parseOtherFiles(fset, srcDir, filename)
+	otherFiles, err := parseOtherFiles(ctx, fset, srcDir, filename)
+	if err != nil {
+		return nil, err
+	}
 
 	// Second pass: add information from other files in the same package,
 	// like their package vars and imports.
@@ -1192,7 +1196,7 @@ func addExternalCandidates(ctx context.Context, pass *pass, refs references, fil
 	if err != nil {
 		return err
 	}
-	if err = resolver.scan(context.Background(), callback); err != nil {
+	if err = resolver.scan(ctx, callback); err != nil {
 		return err
 	}
 
@@ -1203,7 +1207,7 @@ func addExternalCandidates(ctx context.Context, pass *pass, refs references, fil
 	}
 	results := make(chan result, len(refs))
 
-	ctx, cancel := context.WithCancel(context.TODO())
+	ctx, cancel := context.WithCancel(ctx)
 	var wg sync.WaitGroup
 	defer func() {
 		cancel()