internal/analysisinternal: rationalize

This CL moves declarations as described below,
and updates comments. No logic was changed except
in merging the DocComment functions, and in
using safetoken from TypeErrorEndPos within gopls.

internal/astutil (pure syntax)
+ 	func Comments
+ 	func DocComment (merging various unexported copies)
+ 	func EnclosingFile
+ 	func Format
+ 	func IsChildOf

internal/typesinternal (typed syntax)
+ 	func EnclosingScope
+ 	func Imports
+ 	func IsFunctionNamed
+ 	func IsMethodNamed
+  	func IsPointerToNamed
+ 	func IsTypeNamed
- 	func IsZeroExpr (moved to fillreturns)

internal/analysisinternal (use of analysis framework)
- 	func AddImport
- 	func Comments
- 	func DeleteDecl
- 	func DeleteSpec
- 	func DeleteStmt
- 	func DeleteVar
- 	func EnclosingFile
- 	func EnclosingScope
- 	func Format
- 	func FreshName
- 	func Imports
- 	func IsChildOf
- 	func IsFunctionNamed
- 	func IsMethodNamed
- 	func IsPointerToNamed
- 	func IsTypeNamed
- 	func TypeErrorEndPos (moved to gopls/internal/cache)

internal/refactor: (computing text edits from typed syntax)
+ 	func AddImport
+ 	func DeleteDecl
+ 	func DeleteSpec
+ 	func DeleteStmt
+ 	func DeleteVar
+ 	func FreshName

Change-Id: I9fad550ee55efbeb217627570e3a2e9abfee2178
Reviewed-on: https://go-review.googlesource.com/c/tools/+/710295
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

Author: adonovan

go/analysis/passes/assign/assign.go

@@ -19,7 +19,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/inspector"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/astutil"
 )
 
 //go:embed doc.go
@@ -66,8 +66,8 @@ func run(pass *analysis.Pass) (any, error) {
 				!isMapIndex(pass.TypesInfo, lhs) &&
 				reflect.TypeOf(lhs) == reflect.TypeOf(rhs) { // short-circuit the heavy-weight gofmt check
 
-				le = analysisinternal.Format(pass.Fset, lhs)
-				re := analysisinternal.Format(pass.Fset, rhs)
+				le = astutil.Format(pass.Fset, lhs)
+				re := astutil.Format(pass.Fset, rhs)
 				if le == re {
 					isSelfAssign = true
 				}

go/analysis/passes/atomic/atomic.go

@@ -14,7 +14,8 @@ import (
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/astutil"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 //go:embed doc.go
@@ -30,7 +31,7 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (any, error) {
-	if !analysisinternal.Imports(pass.Pkg, "sync/atomic") {
+	if !typesinternal.Imports(pass.Pkg, "sync/atomic") {
 		return nil, nil // doesn't directly import sync/atomic
 	}
 
@@ -54,7 +55,7 @@ func run(pass *analysis.Pass) (any, error) {
 				continue
 			}
 			obj := typeutil.Callee(pass.TypesInfo, call)
-			if analysisinternal.IsFunctionNamed(obj, "sync/atomic", "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr") {
+			if typesinternal.IsFunctionNamed(obj, "sync/atomic", "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr") {
 				checkAtomicAddAssignment(pass, n.Lhs[i], call)
 			}
 		}
@@ -72,7 +73,7 @@ func checkAtomicAddAssignment(pass *analysis.Pass, left ast.Expr, call *ast.Call
 	arg := call.Args[0]
 	broken := false
 
-	gofmt := func(e ast.Expr) string { return analysisinternal.Format(pass.Fset, e) }
+	gofmt := func(e ast.Expr) string { return astutil.Format(pass.Fset, e) }
 
 	if uarg, ok := arg.(*ast.UnaryExpr); ok && uarg.Op == token.AND {
 		broken = gofmt(left) == gofmt(uarg.X)

go/analysis/passes/atomicalign/atomicalign.go

@@ -18,7 +18,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 const Doc = "check for non-64-bits-aligned arguments to sync/atomic functions"
@@ -35,7 +35,7 @@ func run(pass *analysis.Pass) (any, error) {
 	if 8*pass.TypesSizes.Sizeof(types.Typ[types.Uintptr]) == 64 {
 		return nil, nil // 64-bit platform
 	}
-	if !analysisinternal.Imports(pass.Pkg, "sync/atomic") {
+	if !typesinternal.Imports(pass.Pkg, "sync/atomic") {
 		return nil, nil // doesn't directly import sync/atomic
 	}
 
@@ -54,7 +54,7 @@ func run(pass *analysis.Pass) (any, error) {
 	inspect.Preorder(nodeFilter, func(node ast.Node) {
 		call := node.(*ast.CallExpr)
 		obj := typeutil.Callee(pass.TypesInfo, call)
-		if analysisinternal.IsFunctionNamed(obj, "sync/atomic", funcNames...) {
+		if typesinternal.IsFunctionNamed(obj, "sync/atomic", funcNames...) {
 			// For all the listed functions, the expression to check is always the first function argument.
 			check64BitAlignment(pass, obj.Name(), call.Args[0])
 		}

go/analysis/passes/bools/bools.go

@@ -15,7 +15,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/inspector"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/astutil"
 )
 
 const Doc = "check for common mistakes involving boolean operators"
@@ -104,7 +104,7 @@ func (op boolOp) commutativeSets(info *types.Info, e *ast.BinaryExpr, seen map[*
 func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) {
 	seen := make(map[string]bool)
 	for _, e := range exprs {
-		efmt := analysisinternal.Format(pass.Fset, e)
+		efmt := astutil.Format(pass.Fset, e)
 		if seen[efmt] {
 			pass.ReportRangef(e, "redundant %s: %s %s %s", op.name, efmt, op.tok, efmt)
 		} else {
@@ -150,8 +150,8 @@ func (op boolOp) checkSuspect(pass *analysis.Pass, exprs []ast.Expr) {
 		}
 
 		// e is of the form 'x != c' or 'x == c'.
-		xfmt := analysisinternal.Format(pass.Fset, x)
-		efmt := analysisinternal.Format(pass.Fset, e)
+		xfmt := astutil.Format(pass.Fset, x)
+		efmt := astutil.Format(pass.Fset, e)
 		if prev, found := seen[xfmt]; found {
 			// checkRedundant handles the case in which efmt == prev.
 			if efmt != prev {

go/analysis/passes/cgocall/cgocall.go

@@ -18,7 +18,7 @@ import (
 	"strconv"
 
 	"golang.org/x/tools/go/analysis"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 const debug = false
@@ -41,7 +41,7 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (any, error) {
-	if !analysisinternal.Imports(pass.Pkg, "runtime/cgo") {
+	if !typesinternal.Imports(pass.Pkg, "runtime/cgo") {
 		return nil, nil // doesn't use cgo
 	}
 

go/analysis/passes/copylock/copylock.go

@@ -16,8 +16,9 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/astutil"
 	"golang.org/x/tools/internal/typeparams"
+	"golang.org/x/tools/internal/typesinternal"
 	"golang.org/x/tools/internal/versions"
 )
 
@@ -86,7 +87,7 @@ func checkCopyLocksAssign(pass *analysis.Pass, assign *ast.AssignStmt, goversion
 	lhs := assign.Lhs
 	for i, x := range assign.Rhs {
 		if path := lockPathRhs(pass, x); path != nil {
-			pass.ReportRangef(x, "assignment copies lock value to %v: %v", analysisinternal.Format(pass.Fset, assign.Lhs[i]), path)
+			pass.ReportRangef(x, "assignment copies lock value to %v: %v", astutil.Format(pass.Fset, assign.Lhs[i]), path)
 			lhs = nil // An lhs has been reported. We prefer the assignment warning and do not report twice.
 		}
 	}
@@ -100,7 +101,7 @@ func checkCopyLocksAssign(pass *analysis.Pass, assign *ast.AssignStmt, goversion
 				if id, ok := l.(*ast.Ident); ok && id.Name != "_" {
 					if obj := pass.TypesInfo.Defs[id]; obj != nil && obj.Type() != nil {
 						if path := lockPath(pass.Pkg, obj.Type(), nil); path != nil {
-							pass.ReportRangef(l, "for loop iteration copies lock value to %v: %v", analysisinternal.Format(pass.Fset, l), path)
+							pass.ReportRangef(l, "for loop iteration copies lock value to %v: %v", astutil.Format(pass.Fset, l), path)
 						}
 					}
 				}
@@ -132,7 +133,7 @@ func checkCopyLocksCompositeLit(pass *analysis.Pass, cl *ast.CompositeLit) {
 			x = node.Value
 		}
 		if path := lockPathRhs(pass, x); path != nil {
-			pass.ReportRangef(x, "literal copies lock value from %v: %v", analysisinternal.Format(pass.Fset, x), path)
+			pass.ReportRangef(x, "literal copies lock value from %v: %v", astutil.Format(pass.Fset, x), path)
 		}
 	}
 }
@@ -166,7 +167,7 @@ func checkCopyLocksCallExpr(pass *analysis.Pass, ce *ast.CallExpr) {
 	}
 	for _, x := range ce.Args {
 		if path := lockPathRhs(pass, x); path != nil {
-			pass.ReportRangef(x, "call of %s copies lock value: %v", analysisinternal.Format(pass.Fset, ce.Fun), path)
+			pass.ReportRangef(x, "call of %s copies lock value: %v", astutil.Format(pass.Fset, ce.Fun), path)
 		}
 	}
 }
@@ -233,7 +234,7 @@ func checkCopyLocksRangeVar(pass *analysis.Pass, rtok token.Token, e ast.Expr) {
 		return
 	}
 	if path := lockPath(pass.Pkg, typ, nil); path != nil {
-		pass.Reportf(e.Pos(), "range var %s copies lock: %v", analysisinternal.Format(pass.Fset, e), path)
+		pass.Reportf(e.Pos(), "range var %s copies lock: %v", astutil.Format(pass.Fset, e), path)
 	}
 }
 
@@ -353,7 +354,7 @@ func lockPath(tpkg *types.Package, typ types.Type, seen map[types.Type]bool) typ
 	// In go1.10, sync.noCopy did not implement Locker.
 	// (The Unlock method was added only in CL 121876.)
 	// TODO(adonovan): remove workaround when we drop go1.10.
-	if analysisinternal.IsTypeNamed(typ, "sync", "noCopy") {
+	if typesinternal.IsTypeNamed(typ, "sync", "noCopy") {
 		return []string{typ.String()}
 	}
 

go/analysis/passes/deepequalerrors/deepequalerrors.go

@@ -14,7 +14,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 const Doc = `check for calls of reflect.DeepEqual on error values
@@ -35,7 +35,7 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (any, error) {
-	if !analysisinternal.Imports(pass.Pkg, "reflect") {
+	if !typesinternal.Imports(pass.Pkg, "reflect") {
 		return nil, nil // doesn't directly import reflect
 	}
 
@@ -47,7 +47,7 @@ func run(pass *analysis.Pass) (any, error) {
 	inspect.Preorder(nodeFilter, func(n ast.Node) {
 		call := n.(*ast.CallExpr)
 		obj := typeutil.Callee(pass.TypesInfo, call)
-		if analysisinternal.IsFunctionNamed(obj, "reflect", "DeepEqual") && hasError(pass, call.Args[0]) && hasError(pass, call.Args[1]) {
+		if typesinternal.IsFunctionNamed(obj, "reflect", "DeepEqual") && hasError(pass, call.Args[0]) && hasError(pass, call.Args[1]) {
 			pass.ReportRangef(call, "avoid using reflect.DeepEqual with errors")
 		}
 	})

go/analysis/passes/defers/defers.go

@@ -13,7 +13,7 @@ import (
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
-	"golang.org/x/tools/internal/analysisinternal"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 //go:embed doc.go
@@ -29,14 +29,14 @@ var Analyzer = &analysis.Analyzer{
 }
 
 func run(pass *analysis.Pass) (any, error) {
-	if !analysisinternal.Imports(pass.Pkg, "time") {
+	if !typesinternal.Imports(pass.Pkg, "time") {
 		return nil, nil
 	}
 
 	checkDeferCall := func(node ast.Node) bool {
 		switch v := node.(type) {
 		case *ast.CallExpr:
-			if analysisinternal.IsFunctionNamed(typeutil.Callee(pass.TypesInfo, v), "time", "Since") {
+			if typesinternal.IsFunctionNamed(typeutil.Callee(pass.TypesInfo, v), "time", "Since") {
 				pass.Reportf(v.Pos(), "call to time.Since is not deferred")
 			}
 		case *ast.FuncLit:

go/analysis/passes/httpmux/httpmux.go

@@ -17,7 +17,6 @@ import (
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/types/typeutil"
-	"golang.org/x/tools/internal/analysisinternal"
 	"golang.org/x/tools/internal/typesinternal"
 )
 
@@ -46,7 +45,7 @@ func run(pass *analysis.Pass) (any, error) {
 			return nil, nil
 		}
 	}
-	if !analysisinternal.Imports(pass.Pkg, "net/http") {
+	if !typesinternal.Imports(pass.Pkg, "net/http") {
 		return nil, nil
 	}
 	// Look for calls to ServeMux.Handle or ServeMux.HandleFunc.
@@ -79,15 +78,15 @@ func isServeMuxRegisterCall(pass *analysis.Pass, call *ast.CallExpr) bool {
 	if fn == nil {
 		return false
 	}
-	if analysisinternal.IsFunctionNamed(fn, "net/http", "Handle", "HandleFunc") {
+	if typesinternal.IsFunctionNamed(fn, "net/http", "Handle", "HandleFunc") {
 		return true
 	}
 	if !isMethodNamed(fn, "net/http", "Handle", "HandleFunc") {
 		return false
 	}
 	recv := fn.Type().(*types.Signature).Recv() // isMethodNamed() -> non-nil
 	isPtr, named := typesinternal.ReceiverNamed(recv)
-	return isPtr && analysisinternal.IsTypeNamed(named, "net/http", "ServeMux")
+	return isPtr && typesinternal.IsTypeNamed(named, "net/http", "ServeMux")
 }
 
 // isMethodNamed reports when a function f is a method,

go/analysis/passes/httpresponse/httpresponse.go

@@ -13,7 +13,6 @@ import (
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
-	"golang.org/x/tools/internal/analysisinternal"
 	"golang.org/x/tools/internal/typesinternal"
 )
 
@@ -46,7 +45,7 @@ func run(pass *analysis.Pass) (any, error) {
 
 	// Fast path: if the package doesn't import net/http,
 	// skip the traversal.
-	if !analysisinternal.Imports(pass.Pkg, "net/http") {
+	if !typesinternal.Imports(pass.Pkg, "net/http") {
 		return nil, nil
 	}
 
@@ -118,7 +117,7 @@ func isHTTPFuncOrMethodOnClient(info *types.Info, expr *ast.CallExpr) bool {
 		return false // the function called does not return two values.
 	}
 	isPtr, named := typesinternal.ReceiverNamed(res.At(0))
-	if !isPtr || named == nil || !analysisinternal.IsTypeNamed(named, "net/http", "Response") {
+	if !isPtr || named == nil || !typesinternal.IsTypeNamed(named, "net/http", "Response") {
 		return false // the first return type is not *http.Response.
 	}
 
@@ -133,11 +132,11 @@ func isHTTPFuncOrMethodOnClient(info *types.Info, expr *ast.CallExpr) bool {
 		return ok && id.Name == "http" // function in net/http package.
 	}
 
-	if analysisinternal.IsTypeNamed(typ, "net/http", "Client") {
+	if typesinternal.IsTypeNamed(typ, "net/http", "Client") {
 		return true // method on http.Client.
 	}
 	ptr, ok := types.Unalias(typ).(*types.Pointer)
-	return ok && analysisinternal.IsTypeNamed(ptr.Elem(), "net/http", "Client") // method on *http.Client.
+	return ok && typesinternal.IsTypeNamed(ptr.Elem(), "net/http", "Client") // method on *http.Client.
 }
 
 // restOfBlock, given a traversal stack, finds the innermost containing