gopls: convert some users of astutil to Cursors

This started as just a conversion of createUndeclared, but following
the transitive closure up and down the call path, I ended up converting
a significant portion of the code, focusing mainly on the areas where
TODOs had been left to update the code to cursor.

Change-Id: I8d30683f9963c7aa8895734cade37d59fd29fb3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/702355
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: DanielMorsing

gopls/internal/analysis/fillstruct/fillstruct.go

@@ -24,12 +24,12 @@ import (
 	"unicode"
 
 	"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/moreiters"
 	"golang.org/x/tools/internal/typeparams"
 	"golang.org/x/tools/internal/typesinternal"
 )
@@ -133,24 +133,18 @@ const FixCategory = "fillstruct" // recognized by gopls ApplyFix
 // diagnostics produced by the Analyzer above.
 func SuggestedFix(cpkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
 	var (
+		file = pgf.File
 		fset = cpkg.FileSet()
 		pkg  = cpkg.Types()
 		info = cpkg.TypesInfo()
 		pos  = start // don't use end
 	)
-	// TODO(adonovan): simplify, using Cursor.
-	file := pgf.Cursor.Node().(*ast.File)
-	path, _ := astutil.PathEnclosingInterval(file, pos, pos)
-	if len(path) == 0 {
+	cur, ok := pgf.Cursor.FindByPos(pos, pos)
+	if !ok {
 		return nil, nil, fmt.Errorf("no enclosing ast.Node")
 	}
-	var expr *ast.CompositeLit
-	for _, n := range path {
-		if node, ok := n.(*ast.CompositeLit); ok {
-			expr = node
-			break
-		}
-	}
+	curCompLit, _ := moreiters.First(cur.Enclosing((*ast.CompositeLit)(nil)))
+	expr := curCompLit.Node().(*ast.CompositeLit)
 
 	typ := info.TypeOf(expr)
 	if typ == nil {

gopls/internal/golang/codeaction.go

@@ -355,8 +355,8 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error {
 		// See [createUndeclared] for command implementation.
 		case strings.HasPrefix(msg, "undeclared name: "),
 			strings.HasPrefix(msg, "undefined: "):
-			path, _ := astutil.PathEnclosingInterval(req.pgf.File, start, end)
-			title := undeclaredFixTitle(path, msg)
+			cur, _ := req.pgf.Cursor.FindByPos(start, end)
+			title := undeclaredFixTitle(cur, msg)
 			if title != "" {
 				req.addApplyFixAction(title, fixCreateUndeclared, req.loc)
 			}

gopls/internal/golang/extract.go

@@ -293,6 +293,8 @@ Outer:
 //
 // `x` is a free variable defined in the IfStmt, we should not insert
 // the extracted expression outside the IfStmt scope, instead, return an error.
+//
+// TODO(dmo): make this function take a Cursor and simplify
 func stmtToInsertVarBefore(path []ast.Node, variables []*variable) (ast.Stmt, error) {
 	enclosingIndex := -1 // index in path of enclosing stmt
 	for i, p := range path {

gopls/internal/golang/invertifcondition.go

@@ -11,11 +11,11 @@ import (
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/go/ast/inspector"
 	"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/moreiters"
 )
 
 // invertIfCondition is a singleFileFixFunc that inverts an if/else statement
@@ -246,28 +246,22 @@ func invertAndOr(fset *token.FileSet, expr *ast.BinaryExpr, src []byte) ([]byte,
 // canInvertIfCondition reports whether we can do invert-if-condition on the
 // code in the given range.
 func canInvertIfCondition(curFile inspector.Cursor, start, end token.Pos) (*ast.IfStmt, bool, error) {
-	file := curFile.Node().(*ast.File)
-	// TODO(adonovan): simplify, using Cursor.
-	path, _ := astutil.PathEnclosingInterval(file, start, end)
-	for _, node := range path {
-		stmt, isIfStatement := node.(*ast.IfStmt)
-		if !isIfStatement {
-			continue
-		}
-
-		if stmt.Else == nil {
-			// Can't invert conditions without else clauses
-			return nil, false, fmt.Errorf("else clause required")
-		}
-
-		if _, hasElseIf := stmt.Else.(*ast.IfStmt); hasElseIf {
-			// Can't invert conditions with else-if clauses, unclear what that
-			// would look like
-			return nil, false, fmt.Errorf("else-if not supported")
-		}
+	curIf, _ := curFile.FindByPos(start, end)
+	curIf, ok := moreiters.First(curIf.Enclosing((*ast.IfStmt)(nil)))
+	if !ok {
+		return nil, false, fmt.Errorf("not an if statement")
+	}
+	stmt := curIf.Node().(*ast.IfStmt)
+	if stmt.Else == nil {
+		// Can't invert conditions without else clauses
+		return nil, false, fmt.Errorf("else clause required")
+	}
 
-		return stmt, true, nil
+	if _, hasElseIf := stmt.Else.(*ast.IfStmt); hasElseIf {
+		// Can't invert conditions with else-if clauses, unclear what that
+		// would look like
+		return nil, false, fmt.Errorf("else-if not supported")
 	}
 
-	return nil, false, fmt.Errorf("not an if statement")
+	return stmt, true, nil
 }

gopls/internal/golang/lines.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/go/ast/edge"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/gopls/internal/cache"
 	"golang.org/x/tools/gopls/internal/cache/parsego"
@@ -171,39 +171,34 @@ func processLines(fset *token.FileSet, items []ast.Node, comments []*ast.Comment
 
 // findSplitJoinTarget returns the first curly bracket/parens that encloses the current cursor.
 func findSplitJoinTarget(fset *token.FileSet, curFile inspector.Cursor, src []byte, start, end token.Pos) (itemType string, items []ast.Node, comments []*ast.CommentGroup, indent string, open, close token.Pos) {
-	isCursorInside := func(nodePos, nodeEnd token.Pos) bool {
-		return nodePos < start && end < nodeEnd
-	}
-
-	file := curFile.Node().(*ast.File)
-	// TODO(adonovan): simplify, using Cursor.
 
 	findTarget := func() (targetType string, target ast.Node, open, close token.Pos) {
-		path, _ := astutil.PathEnclosingInterval(file, start, end)
-		for _, node := range path {
-			switch node := node.(type) {
-			case *ast.FuncType:
-				// params or results of func signature
-				// Note:
-				// - each ast.Field (e.g. "x, y, z int") is considered a single item.
-				// - splitting Params and Results lists is not usually good style.
-				if p := node.Params; isCursorInside(p.Opening, p.Closing) {
-					return "parameters", p, p.Opening, p.Closing
-				}
-				if r := node.Results; r != nil && isCursorInside(r.Opening, r.Closing) {
-					return "results", r, r.Opening, r.Closing
-				}
-			case *ast.CallExpr: // f(a, b, c)
-				if isCursorInside(node.Lparen, node.Rparen) {
-					return "arguments", node, node.Lparen, node.Rparen
-				}
-			case *ast.CompositeLit: // T{a, b, c}
-				if isCursorInside(node.Lbrace, node.Rbrace) {
-					return "elements", node, node.Lbrace, node.Rbrace
+		cur, _ := curFile.FindByPos(start, end)
+		for cur := range cur.Enclosing() {
+			// TODO: do cur = enclosingUnparen(cur) first, once CL 701035 lands.
+			ek, _ := cur.ParentEdge()
+			switch ek {
+			// params or results of func signature
+			// Note:
+			// - each ast.Field (e.g. "x, y, z int") is considered a single item.
+			// - splitting Params and Results lists is not usually good style.
+			case edge.FuncType_Params:
+				p := cur.Node().(*ast.FieldList)
+				return "parameters", p, p.Opening, p.Closing
+			case edge.FuncType_Results:
+				r := cur.Node().(*ast.FieldList)
+				if !r.Opening.IsValid() {
+					continue
 				}
+				return "results", r, r.Opening, r.Closing
+			case edge.CallExpr_Args: // f(a, b, c)
+				node := cur.Parent().Node().(*ast.CallExpr)
+				return "arguments", node, node.Lparen, node.Rparen
+			case edge.CompositeLit_Elts: // T{a, b, c}
+				node := cur.Parent().Node().(*ast.CompositeLit)
+				return "elements", node, node.Lbrace, node.Rbrace
 			}
 		}
-
 		return "", nil, 0, 0
 	}
 
@@ -240,6 +235,7 @@ func findSplitJoinTarget(fset *token.FileSet, curFile inspector.Cursor, src []by
 	}
 
 	// preserve comments separately as it's not part of the targetNode AST.
+	file := curFile.Node().(*ast.File)
 	for _, cg := range file.Comments {
 		if open <= cg.Pos() && cg.Pos() < close {
 			comments = append(comments, cg)

gopls/internal/golang/stubmethods/stubcalledfunc.go

@@ -13,9 +13,10 @@ import (
 	"strings"
 	"unicode"
 
-	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/gopls/internal/cache/parsego"
 	"golang.org/x/tools/gopls/internal/util/typesutil"
+	"golang.org/x/tools/internal/moreiters"
 	"golang.org/x/tools/internal/typesinternal"
 )
 
@@ -31,71 +32,73 @@ type CallStubInfo struct {
 	After      types.Object // decl after which to insert the new decl
 	pointer    bool
 	info       *types.Info
-	path       []ast.Node // path enclosing the CallExpr
+	curCall    inspector.Cursor // cursor to the CallExpr
 }
 
 // GetCallStubInfo extracts necessary information to generate a method definition from
 // a CallExpr.
 func GetCallStubInfo(fset *token.FileSet, info *types.Info, pgf *parsego.File, start, end token.Pos) *CallStubInfo {
-	// TODO(adonovan): simplify, using pgf.Cursor.
-	path, _ := astutil.PathEnclosingInterval(pgf.File, start, end)
-	for i, n := range path {
-		switch n := n.(type) {
-		case *ast.CallExpr:
-			s, ok := n.Fun.(*ast.SelectorExpr)
-			// TODO: support generating stub functions in the same way.
-			if !ok {
-				return nil
-			}
+	callCur, _ := pgf.Cursor.FindByPos(start, end)
+	callCur, ok := moreiters.First(callCur.Enclosing((*ast.CallExpr)(nil)))
+	if !ok {
+		return nil
+	}
+	call := callCur.Node().(*ast.CallExpr)
+	s, ok := call.Fun.(*ast.SelectorExpr)
+	// TODO: support generating stub functions in the same way.
+	if !ok {
+		return nil
+	}
 
-			// If recvExpr is a package name, compiler error would be
-			// e.g., "undefined: http.bar", thus will not hit this code path.
-			recvExpr := s.X
-			recvType, pointer := concreteType(recvExpr, info)
+	// If recvExpr is a package name, compiler error would be
+	// e.g., "undefined: http.bar", thus will not hit this code path.
+	recvExpr := s.X
+	recvType, pointer := concreteType(recvExpr, info)
 
-			if recvType == nil || recvType.Obj().Pkg() == nil {
-				return nil
-			}
+	if recvType == nil || recvType.Obj().Pkg() == nil {
+		return nil
+	}
 
-			// A method of a function-local type cannot be stubbed
-			// since there's nowhere to put the methods.
-			recv := recvType.Obj()
-			if recv.Parent() != recv.Pkg().Scope() {
-				return nil
-			}
+	// A method of a function-local type cannot be stubbed
+	// since there's nowhere to put the methods.
+	recv := recvType.Obj()
+	if recv.Parent() != recv.Pkg().Scope() {
+		return nil
+	}
 
-			after := types.Object(recv)
-			// If the enclosing function declaration is a method declaration,
-			// and matches the receiver type of the diagnostic,
-			// insert after the enclosing method.
-			decl, ok := path[len(path)-2].(*ast.FuncDecl)
-			if ok && decl.Recv != nil {
-				if len(decl.Recv.List) != 1 {
-					return nil
-				}
-				mrt := info.TypeOf(decl.Recv.List[0].Type)
-				if mrt != nil && types.Identical(types.Unalias(typesinternal.Unpointer(mrt)), recv.Type()) {
-					after = info.ObjectOf(decl.Name)
-				}
-			}
-			return &CallStubInfo{
-				Fset:       fset,
-				Receiver:   recvType,
-				MethodName: s.Sel.Name,
-				After:      after,
-				pointer:    pointer,
-				path:       path[i:],
-				info:       info,
-			}
+	after := types.Object(recv)
+	// If the enclosing function declaration is a method declaration,
+	// and matches the receiver type of the diagnostic,
+	// insert after the enclosing method.
+	var decl *ast.FuncDecl
+	declCur, ok := moreiters.First(callCur.Enclosing((*ast.FuncDecl)(nil)))
+	if ok {
+		decl = declCur.Node().(*ast.FuncDecl)
+	}
+	if decl != nil && decl.Recv != nil {
+		if len(decl.Recv.List) != 1 {
+			return nil
+		}
+		mrt := info.TypeOf(decl.Recv.List[0].Type)
+		if mrt != nil && types.Identical(types.Unalias(typesinternal.Unpointer(mrt)), recv.Type()) {
+			after = info.ObjectOf(decl.Name)
 		}
 	}
-	return nil
+	return &CallStubInfo{
+		Fset:       fset,
+		Receiver:   recvType,
+		MethodName: s.Sel.Name,
+		After:      after,
+		pointer:    pointer,
+		curCall:    callCur,
+		info:       info,
+	}
 }
 
 // Emit writes to out the missing method based on type info of si.Receiver and CallExpr.
 func (si *CallStubInfo) Emit(out *bytes.Buffer, qual types.Qualifier) error {
 	params := si.collectParams()
-	rets := typesutil.TypesFromContext(si.info, si.path, si.path[0].Pos())
+	rets := typesutil.TypesFromContext(si.info, si.curCall)
 	recv := si.Receiver.Obj()
 	// Pointer receiver?
 	var star string
@@ -180,7 +183,7 @@ func (si *CallStubInfo) collectParams() []param {
 		params = append(params, p)
 	}
 
-	args := si.path[0].(*ast.CallExpr).Args
+	args := si.curCall.Node().(*ast.CallExpr).Args
 	for _, arg := range args {
 		t := si.info.TypeOf(arg)
 		switch t := t.(type) {