gopls/internal/golang: make singleFileFixer like fixer

In the past the singleFileFixer has two roles: to simplify
the signature for fixers of a single file, and to allow them
to be expressed only in terms of go/{ast,types} data types,
allowing fixers to be more loosely coupled to gopls. But that
latter role seems unimportant now, so this CL simplifies the
two functions to make them more alike.

Change-Id: I42ee9fd275e344fafee0b27b9861f8f599f89e3e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/650641
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/analysis/fillstruct/fillstruct.go

@@ -25,10 +25,11 @@ import (
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/gopls/internal/cache"
+	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/fuzzy"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/analysisinternal"
-	"golang.org/x/tools/internal/astutil/cursor"
 	"golang.org/x/tools/internal/typeparams"
 	"golang.org/x/tools/internal/typesinternal"
 )
@@ -130,15 +131,15 @@ const FixCategory = "fillstruct" // recognized by gopls ApplyFix
 
 // SuggestedFix computes the suggested fix for the kinds of
 // diagnostics produced by the Analyzer above.
-func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, curFile cursor.Cursor, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	if info == nil {
-		return nil, nil, fmt.Errorf("nil types.Info")
-	}
-
-	pos := start // don't use the end
-
+func SuggestedFix(cpkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	var (
+		fset = cpkg.FileSet()
+		pkg  = cpkg.Types()
+		info = cpkg.TypesInfo()
+		pos  = start // don't use end
+	)
 	// TODO(adonovan): simplify, using Cursor.
-	file := curFile.Node().(*ast.File)
+	file := pgf.Cursor.Node().(*ast.File)
 	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
 	if len(path) == 0 {
 		return nil, nil, fmt.Errorf("no enclosing ast.Node")
@@ -235,7 +236,7 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, cur
 	}
 
 	// Find the line on which the composite literal is declared.
-	split := bytes.Split(content, []byte("\n"))
+	split := bytes.Split(pgf.Src, []byte("\n"))
 	lineNumber := safetoken.StartPosition(fset, expr.Lbrace).Line
 	firstLine := split[lineNumber-1] // lines are 1-indexed
 

gopls/internal/golang/extract.go

@@ -20,6 +20,8 @@ import (
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/gopls/internal/cache"
+	"golang.org/x/tools/gopls/internal/cache/parsego"
 	goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
 	"golang.org/x/tools/gopls/internal/util/bug"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
@@ -29,13 +31,13 @@ import (
 )
 
 // extractVariable implements the refactor.extract.{variable,constant} CodeAction command.
-func extractVariable(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, _ *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	return extractExprs(fset, start, end, src, curFile, info, false)
+func extractVariable(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	return extractExprs(pkg, pgf, start, end, false)
 }
 
 // extractVariableAll implements the refactor.extract.{variable,constant}-all CodeAction command.
-func extractVariableAll(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, _ *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	return extractExprs(fset, start, end, src, curFile, info, true)
+func extractVariableAll(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	return extractExprs(pkg, pgf, start, end, true)
 }
 
 // extractExprs replaces occurrence(s) of a specified expression within the same function
@@ -44,11 +46,15 @@ func extractVariableAll(fset *token.FileSet, start, end token.Pos, src []byte, c
 //
 // The new variable/constant is declared as close as possible to the first found expression
 // within the deepest common scope accessible to all candidate occurrences.
-func extractExprs(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, info *types.Info, all bool) (*token.FileSet, *analysis.SuggestedFix, error) {
-	file := curFile.Node().(*ast.File)
+func extractExprs(pkg *cache.Package, pgf *parsego.File, start, end token.Pos, all bool) (*token.FileSet, *analysis.SuggestedFix, error) {
+	var (
+		fset = pkg.FileSet()
+		info = pkg.TypesInfo()
+		file = pgf.File
+	)
 	// TODO(adonovan): simplify, using Cursor.
 	tokFile := fset.File(file.FileStart)
-	exprs, err := canExtractVariable(info, curFile, start, end, all)
+	exprs, err := canExtractVariable(info, pgf.Cursor, start, end, all)
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot extract: %v", err)
 	}
@@ -157,7 +163,7 @@ Outer:
 			return nil, nil, fmt.Errorf("cannot find location to insert extraction: %v", err)
 		}
 		// Within function: compute appropriate statement indentation.
-		indent, err := calculateIndentation(src, tokFile, before)
+		indent, err := calculateIndentation(pgf.Src, tokFile, before)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -576,13 +582,13 @@ type returnVariable struct {
 }
 
 // extractMethod refactors the selected block of code into a new method.
-func extractMethod(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	return extractFunctionMethod(fset, start, end, src, curFile, pkg, info, true)
+func extractMethod(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	return extractFunctionMethod(pkg, pgf, start, end, true)
 }
 
 // extractFunction refactors the selected block of code into a new function.
-func extractFunction(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	return extractFunctionMethod(fset, start, end, src, curFile, pkg, info, false)
+func extractFunction(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	return extractFunctionMethod(pkg, pgf, start, end, false)
 }
 
 // extractFunctionMethod refactors the selected block of code into a new function/method.
@@ -593,19 +599,26 @@ func extractFunction(fset *token.FileSet, start, end token.Pos, src []byte, curF
 // and return values of the extracted function/method. Lastly, we construct the call
 // of the function/method and insert this call as well as the extracted function/method into
 // their proper locations.
-func extractFunctionMethod(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, pkg *types.Package, info *types.Info, isMethod bool) (*token.FileSet, *analysis.SuggestedFix, error) {
+func extractFunctionMethod(cpkg *cache.Package, pgf *parsego.File, start, end token.Pos, isMethod bool) (*token.FileSet, *analysis.SuggestedFix, error) {
+	var (
+		fset = cpkg.FileSet()
+		pkg  = cpkg.Types()
+		info = cpkg.TypesInfo()
+		src  = pgf.Src
+	)
+
 	errorPrefix := "extractFunction"
 	if isMethod {
 		errorPrefix = "extractMethod"
 	}
 
-	file := curFile.Node().(*ast.File)
+	file := pgf.Cursor.Node().(*ast.File)
 	// TODO(adonovan): simplify, using Cursor.
 	tok := fset.File(file.FileStart)
 	if tok == nil {
 		return nil, nil, bug.Errorf("no file for position")
 	}
-	p, ok, methodOk, err := canExtractFunction(tok, start, end, src, curFile)
+	p, ok, methodOk, err := canExtractFunction(tok, start, end, src, pgf.Cursor)
 	if (!ok && !isMethod) || (!methodOk && isMethod) {
 		return nil, nil, fmt.Errorf("%s: cannot extract %s: %v", errorPrefix,
 			safetoken.StartPosition(fset, start), err)

gopls/internal/golang/fix.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"fmt"
 	"go/token"
-	"go/types"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/gopls/internal/analysis/embeddirective"
@@ -19,7 +18,6 @@ import (
 	"golang.org/x/tools/gopls/internal/file"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/util/bug"
-	"golang.org/x/tools/internal/astutil/cursor"
 	"golang.org/x/tools/internal/imports"
 )
 
@@ -41,18 +39,14 @@ import (
 // A fixer may return (nil, nil) if no fix is available.
 type fixer func(ctx context.Context, s *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error)
 
-// A singleFileFixer is a Fixer that inspects only a single file,
-// and does not depend on data types from the cache package.
-//
-// TODO(adonovan): move fillstruct and undeclaredname into this
-// package, so we can remove the import restriction and push
-// the singleFile wrapper down into each singleFileFixer?
-type singleFileFixer func(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error)
+// A singleFileFixer is a [fixer] that inspects only a single file.
+type singleFileFixer func(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error)
 
-// singleFile adapts a single-file fixer to a Fixer.
+// singleFile adapts a [singleFileFixer] to a [fixer]
+// by discarding the snapshot and the context it needs.
 func singleFile(fixer1 singleFileFixer) fixer {
-	return func(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
-		return fixer1(pkg.FileSet(), start, end, pgf.Src, pgf.Cursor, pkg.Types(), pkg.TypesInfo())
+	return func(_ context.Context, _ *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+		return fixer1(pkg, pgf, start, end)
 	}
 }
 

gopls/internal/golang/invertifcondition.go

@@ -8,18 +8,24 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"go/types"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/gopls/internal/cache"
+	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/astutil/cursor"
 )
 
 // invertIfCondition is a singleFileFixFunc that inverts an if/else statement
-func invertIfCondition(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, _ *types.Package, _ *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	ifStatement, _, err := canInvertIfCondition(curFile, start, end)
+func invertIfCondition(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	var (
+		fset = pkg.FileSet()
+		src  = pgf.Src
+	)
+
+	ifStatement, _, err := canInvertIfCondition(pgf.Cursor, start, end)
 	if err != nil {
 		return nil, nil, err
 	}

gopls/internal/golang/lines.go

@@ -12,13 +12,14 @@ import (
 	"bytes"
 	"go/ast"
 	"go/token"
-	"go/types"
 	"slices"
 	"sort"
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/gopls/internal/cache"
+	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	"golang.org/x/tools/internal/astutil/cursor"
 )
@@ -85,23 +86,25 @@ func canSplitJoinLines(items []ast.Node, comments []*ast.CommentGroup) bool {
 }
 
 // splitLines is a singleFile fixer.
-func splitLines(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, _ *types.Package, _ *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	itemType, items, comments, indent, braceOpen, braceClose := findSplitJoinTarget(fset, curFile, src, start, end)
+func splitLines(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	fset := pkg.FileSet()
+	itemType, items, comments, indent, braceOpen, braceClose := findSplitJoinTarget(fset, pgf.Cursor, pgf.Src, start, end)
 	if itemType == "" {
 		return nil, nil, nil // no fix available
 	}
 
-	return fset, processLines(fset, items, comments, src, braceOpen, braceClose, ",\n", "\n", ",\n"+indent, indent+"\t"), nil
+	return fset, processLines(fset, items, comments, pgf.Src, braceOpen, braceClose, ",\n", "\n", ",\n"+indent, indent+"\t"), nil
 }
 
 // joinLines is a singleFile fixer.
-func joinLines(fset *token.FileSet, start, end token.Pos, src []byte, curFile cursor.Cursor, _ *types.Package, _ *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	itemType, items, comments, _, braceOpen, braceClose := findSplitJoinTarget(fset, curFile, src, start, end)
+func joinLines(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	fset := pkg.FileSet()
+	itemType, items, comments, _, braceOpen, braceClose := findSplitJoinTarget(fset, pgf.Cursor, pgf.Src, start, end)
 	if itemType == "" {
 		return nil, nil, nil // no fix available
 	}
 
-	return fset, processLines(fset, items, comments, src, braceOpen, braceClose, ", ", "", "", ""), nil
+	return fset, processLines(fset, items, comments, pgf.Src, braceOpen, braceClose, ", ", "", "", ""), nil
 }
 
 // processLines is the common operation for both split and join lines because this split/join operation is

gopls/internal/golang/undeclared.go

@@ -16,8 +16,9 @@ import (
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/gopls/internal/cache"
+	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/util/typesutil"
-	"golang.org/x/tools/internal/astutil/cursor"
 	"golang.org/x/tools/internal/typesinternal"
 )
 
@@ -70,9 +71,13 @@ func undeclaredFixTitle(path []ast.Node, errMsg string) string {
 }
 
 // createUndeclared generates a suggested declaration for an undeclared variable or function.
-func createUndeclared(fset *token.FileSet, start, end token.Pos, content []byte, curFile cursor.Cursor, pkg *types.Package, info *types.Info) (*token.FileSet, *analysis.SuggestedFix, error) {
-	pos := start // don't use the end
-	file := curFile.Node().(*ast.File)
+func createUndeclared(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
+	var (
+		fset = pkg.FileSet()
+		info = pkg.TypesInfo()
+		file = pgf.File
+		pos  = start // don't use end
+	)
 	// TODO(adonovan): simplify, using Cursor.
 	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
 	if len(path) < 2 {
@@ -86,7 +91,7 @@ func createUndeclared(fset *token.FileSet, start, end token.Pos, content []byte,
 	// Check for a possible call expression, in which case we should add a
 	// new function declaration.
 	if isCallPosition(path) {
-		return newFunctionDeclaration(path, file, pkg, info, fset)
+		return newFunctionDeclaration(path, file, pkg.Types(), info, fset)
 	}
 	var (
 		firstRef     *ast.Ident // We should insert the new declaration before the first occurrence of the undefined ident.
@@ -132,7 +137,7 @@ func createUndeclared(fset *token.FileSet, start, end token.Pos, content []byte,
 	if err != nil {
 		return nil, nil, fmt.Errorf("could not locate insertion point: %v", err)
 	}
-	indent, err := calculateIndentation(content, fset.File(file.FileStart), insertBeforeStmt)
+	indent, err := calculateIndentation(pgf.Src, fset.File(file.FileStart), insertBeforeStmt)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -141,7 +146,7 @@ func createUndeclared(fset *token.FileSet, start, end token.Pos, content []byte,
 		// Default to 0.
 		typs = []types.Type{types.Typ[types.Int]}
 	}
-	expr, _ := typesinternal.ZeroExpr(typs[0], typesinternal.FileQualifier(file, pkg))
+	expr, _ := typesinternal.ZeroExpr(typs[0], typesinternal.FileQualifier(file, pkg.Types()))
 	assignStmt := &ast.AssignStmt{
 		Lhs: []ast.Expr{ast.NewIdent(ident.Name)},
 		Tok: token.DEFINE,