all: merge master (9b63f3d) into gopls-release-branch.0.14
Also reinstate the x/tools replace directive.
For golang/go#63220
Conflicts:
- gopls/go.sum
Merge List:
+ 2023-10-11 9b63f3d1d gopls: upgrade x/telemetry dependency
+ 2023-10-11 f38ff07b8 internal/refactor/inline: T{} is duplicable for struct/array
+ 2023-10-11 ecbfa885b go/analysis/passes/timeformat: simplify isTimeDotFormat
+ 2023-10-11 187911b87 internal/refactor/inline: more precise SelectorExpr effects
+ 2023-10-10 dbf6f4218 go/analysis/passes/httpmux: add command
+ 2023-10-10 7e7568c76 go/analysis/passes/httpmux: check for enhanced ServeMux patterns
+ 2023-10-10 b26815635 gopls: allow all drive letters in cache/filemap_test.go
+ 2023-10-10 be4e4d62f go/analysis/passes/internal/analysisutil: account for nil Func.Pkg
+ 2023-10-10 0e4fc907c internal/refactor/inline: add missing spread context (return)
+ 2023-10-10 8954aa7a5 go/types/internal/play: fix slice OOB when *ast.File is selected
+ 2023-10-10 fda3fe3aa gopls/internal/lsp: use the correct options for semantic tokens legend
+ 2023-10-10 f6d8589ec cmd/compilebench: pass linker flags to prebuild
+ 2023-10-09 5874869c2 go/analysis/passes/internal/analysisutil: add IsFunctionNamed
+ 2023-10-09 0b06fd80c cmd/gonew: skip Test if exec is unsupported
+ 2023-10-09 f5fd4c914 go/analysis/passes/bools: use astutil.Unparen
+ 2023-10-09 59ac17fcd go/analysis/passes/internal/analysisutil: remove Unparen
+ 2023-10-08 a3b5082fb go/analysis/passes/appends: improve check for append builtin
+ 2023-10-08 395d39326 go/analysis/passes/internal/analysisutil: add IsNamedType
+ 2023-10-08 22abcd608 oogo/analysis/passes/bools: remove duplicate functions
+ 2023-10-06 3f4194ee2 go.mod: update golang.org/x dependencies
+ 2023-10-05 1e4ce7c30 internal/refactor/inline: yet more tweaks to everything test
Change-Id: Iaf86891ab5af9ee05ada507c75234e750633e0ef
diff --git a/cmd/compilebench/main.go b/cmd/compilebench/main.go
index 681ffd3..15323c2 100644
--- a/cmd/compilebench/main.go
+++ b/cmd/compilebench/main.go
@@ -350,7 +350,7 @@
return err
}
- importcfg, err := genImportcfgFile(c.dir, false)
+ importcfg, err := genImportcfgFile(c.dir, "", false) // TODO: pass compiler flags?
if err != nil {
return err
}
@@ -418,12 +418,19 @@
}
// Build dependencies.
- out, err := exec.Command(*flagGoCmd, "build", "-o", "/dev/null", r.dir).CombinedOutput()
+ ldflags := *flagLinkerFlags
+ if r.flags != "" {
+ if ldflags != "" {
+ ldflags += " "
+ }
+ ldflags += r.flags
+ }
+ out, err := exec.Command(*flagGoCmd, "build", "-o", "/dev/null", "-ldflags="+ldflags, r.dir).CombinedOutput()
if err != nil {
return fmt.Errorf("go build -a %s: %v\n%s", r.dir, err, out)
}
- importcfg, err := genImportcfgFile(r.dir, true)
+ importcfg, err := genImportcfgFile(r.dir, "-ldflags="+ldflags, true)
if err != nil {
return err
}
@@ -643,15 +650,19 @@
// genImportcfgFile generates an importcfg file for building package
// dir. Returns the generated importcfg file path (or empty string
// if the package has no dependency).
-func genImportcfgFile(dir string, full bool) (string, error) {
+func genImportcfgFile(dir string, flags string, full bool) (string, error) {
need := "{{.Imports}}"
if full {
// for linking, we need transitive dependencies
need = "{{.Deps}}"
}
+ if flags == "" {
+ flags = "--" // passing "" to go list, it will match to the current directory
+ }
+
// find imported/dependent packages
- cmd := exec.Command(*flagGoCmd, "list", "-f", need, dir)
+ cmd := exec.Command(*flagGoCmd, "list", "-f", need, flags, dir)
cmd.Stderr = os.Stderr
out, err := cmd.Output()
if err != nil {
@@ -667,7 +678,7 @@
}
// build importcfg for imported packages
- cmd = exec.Command(*flagGoCmd, "list", "-export", "-f", "{{if .Export}}packagefile {{.ImportPath}}={{.Export}}{{end}}")
+ cmd = exec.Command(*flagGoCmd, "list", "-export", "-f", "{{if .Export}}packagefile {{.ImportPath}}={{.Export}}{{end}}", flags)
cmd.Args = append(cmd.Args, strings.Fields(string(out))...)
cmd.Stderr = os.Stderr
out, err = cmd.Output()
diff --git a/cmd/godoc/godoc_test.go b/cmd/godoc/godoc_test.go
index 42582c4..b7b0e1b 100644
--- a/cmd/godoc/godoc_test.go
+++ b/cmd/godoc/godoc_test.go
@@ -47,7 +47,7 @@
func godocPath(t *testing.T) string {
if !testenv.HasExec() {
- t.Skipf("skipping test that requires exec")
+ t.Skipf("skipping test: exec not supported on %s/%s", runtime.GOOS, runtime.GOARCH)
}
exe.once.Do(func() {
diff --git a/cmd/gonew/main_test.go b/cmd/gonew/main_test.go
index 590bda0..142788b 100644
--- a/cmd/gonew/main_test.go
+++ b/cmd/gonew/main_test.go
@@ -17,6 +17,7 @@
"testing"
"golang.org/x/tools/internal/diffp"
+ "golang.org/x/tools/internal/testenv"
"golang.org/x/tools/txtar"
)
@@ -28,6 +29,9 @@
}
func Test(t *testing.T) {
+ if !testenv.HasExec() {
+ t.Skipf("skipping test: exec not supported on %s/%s", runtime.GOOS, runtime.GOARCH)
+ }
exe, err := os.Executable()
if err != nil {
t.Fatal(err)
diff --git a/go.mod b/go.mod
index 4a01af3..9688b9d 100644
--- a/go.mod
+++ b/go.mod
@@ -4,9 +4,9 @@
require (
github.com/yuin/goldmark v1.4.13
- golang.org/x/mod v0.12.0
- golang.org/x/net v0.15.0
- golang.org/x/sys v0.12.0
+ golang.org/x/mod v0.13.0
+ golang.org/x/net v0.16.0
+ golang.org/x/sys v0.13.0
)
-require golang.org/x/sync v0.3.0
+require golang.org/x/sync v0.4.0
diff --git a/go.sum b/go.sum
index 3f717ed..78a350f 100644
--- a/go.sum
+++ b/go.sum
@@ -1,10 +1,10 @@
github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
-golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc=
-golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
-golang.org/x/net v0.15.0 h1:ugBLEUaxABaB5AJqW9enI0ACdci2RUd4eP51NTBvuJ8=
-golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk=
-golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E=
-golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
-golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
-golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY=
+golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
+golang.org/x/net v0.16.0 h1:7eBu7KsSvFDtSXUIDbh3aqlK4DPsZ1rByC8PFfBThos=
+golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
+golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
+golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
+golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
+golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
diff --git a/go/analysis/passes/appends/appends.go b/go/analysis/passes/appends/appends.go
index f0b90a4..6976f0d 100644
--- a/go/analysis/passes/appends/appends.go
+++ b/go/analysis/passes/appends/appends.go
@@ -15,6 +15,7 @@
"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/go/types/typeutil"
)
//go:embed doc.go
@@ -36,12 +37,9 @@
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
call := n.(*ast.CallExpr)
- if ident, ok := call.Fun.(*ast.Ident); ok && ident.Name == "append" {
- if _, ok := pass.TypesInfo.Uses[ident].(*types.Builtin); ok {
- if len(call.Args) == 1 {
- pass.ReportRangef(call, "append with no values")
- }
- }
+ b, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Builtin)
+ if ok && b.Name() == "append" && len(call.Args) == 1 {
+ pass.ReportRangef(call, "append with no values")
}
})
diff --git a/go/analysis/passes/appends/testdata/src/b/b.go b/go/analysis/passes/appends/testdata/src/b/b.go
index 87a04c4..b4e99d4 100644
--- a/go/analysis/passes/appends/testdata/src/b/b.go
+++ b/go/analysis/passes/appends/testdata/src/b/b.go
@@ -16,3 +16,9 @@
sli = append(sli, 4, 5, 6)
sli = append(sli)
}
+
+func localvar() {
+ append := func(int) int { return 0 }
+ a := append(1)
+ _ = a
+}
diff --git a/go/analysis/passes/assign/assign.go b/go/analysis/passes/assign/assign.go
index 10489be..3bfd501 100644
--- a/go/analysis/passes/assign/assign.go
+++ b/go/analysis/passes/assign/assign.go
@@ -18,6 +18,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/ast/inspector"
)
@@ -77,7 +78,7 @@
// isMapIndex returns true if e is a map index expression.
func isMapIndex(info *types.Info, e ast.Expr) bool {
- if idx, ok := analysisutil.Unparen(e).(*ast.IndexExpr); ok {
+ if idx, ok := astutil.Unparen(e).(*ast.IndexExpr); ok {
if typ := info.Types[idx.X].Type; typ != nil {
_, ok := typ.Underlying().(*types.Map)
return ok
diff --git a/go/analysis/passes/atomic/atomic.go b/go/analysis/passes/atomic/atomic.go
index b40e081..931f9ca 100644
--- a/go/analysis/passes/atomic/atomic.go
+++ b/go/analysis/passes/atomic/atomic.go
@@ -8,12 +8,12 @@
_ "embed"
"go/ast"
"go/token"
- "go/types"
"golang.org/x/tools/go/analysis"
"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/go/types/typeutil"
)
//go:embed doc.go
@@ -52,18 +52,8 @@
if !ok {
continue
}
- sel, ok := call.Fun.(*ast.SelectorExpr)
- if !ok {
- continue
- }
- pkgIdent, _ := sel.X.(*ast.Ident)
- pkgName, ok := pass.TypesInfo.Uses[pkgIdent].(*types.PkgName)
- if !ok || pkgName.Imported().Path() != "sync/atomic" {
- continue
- }
-
- switch sel.Sel.Name {
- case "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr":
+ fn := typeutil.StaticCallee(pass.TypesInfo, call)
+ if analysisutil.IsFunctionNamed(fn, "sync/atomic", "AddInt32", "AddInt64", "AddUint32", "AddUint64", "AddUintptr") {
checkAtomicAddAssignment(pass, n.Lhs[i], call)
}
}
diff --git a/go/analysis/passes/atomic/testdata/src/a/a.go b/go/analysis/passes/atomic/testdata/src/a/a.go
index dc12bd0..e784605 100644
--- a/go/analysis/passes/atomic/testdata/src/a/a.go
+++ b/go/analysis/passes/atomic/testdata/src/a/a.go
@@ -14,9 +14,10 @@
func AtomicTests() {
x := uint64(1)
- x = atomic.AddUint64(&x, 1) // want "direct assignment to atomic value"
- _, x = 10, atomic.AddUint64(&x, 1) // want "direct assignment to atomic value"
- x, _ = atomic.AddUint64(&x, 1), 10 // want "direct assignment to atomic value"
+ x = atomic.AddUint64(&x, 1) // want "direct assignment to atomic value"
+ _, x = 10, atomic.AddUint64(&x, 1) // want "direct assignment to atomic value"
+ x, _ = atomic.AddUint64(&x, 1), 10 // want "direct assignment to atomic value"
+ x, _ = (atomic.AddUint64)(&x, 1), 10 // want "direct assignment to atomic value"
y := &x
*y = atomic.AddUint64(y, 1) // want "direct assignment to atomic value"
diff --git a/go/analysis/passes/atomicalign/atomicalign.go b/go/analysis/passes/atomicalign/atomicalign.go
index 01683e4..aff6d25 100644
--- a/go/analysis/passes/atomicalign/atomicalign.go
+++ b/go/analysis/passes/atomicalign/atomicalign.go
@@ -18,6 +18,7 @@
"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/go/types/typeutil"
)
const Doc = "check for non-64-bits-aligned arguments to sync/atomic functions"
@@ -42,31 +43,20 @@
nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
}
+ funcNames := []string{
+ "AddInt64", "AddUint64",
+ "LoadInt64", "LoadUint64",
+ "StoreInt64", "StoreUint64",
+ "SwapInt64", "SwapUint64",
+ "CompareAndSwapInt64", "CompareAndSwapUint64",
+ }
inspect.Preorder(nodeFilter, func(node ast.Node) {
call := node.(*ast.CallExpr)
- sel, ok := call.Fun.(*ast.SelectorExpr)
- if !ok {
- return
- }
- pkgIdent, ok := sel.X.(*ast.Ident)
- if !ok {
- return
- }
- pkgName, ok := pass.TypesInfo.Uses[pkgIdent].(*types.PkgName)
- if !ok || pkgName.Imported().Path() != "sync/atomic" {
- return
- }
-
- switch sel.Sel.Name {
- case "AddInt64", "AddUint64",
- "LoadInt64", "LoadUint64",
- "StoreInt64", "StoreUint64",
- "SwapInt64", "SwapUint64",
- "CompareAndSwapInt64", "CompareAndSwapUint64":
-
+ fn := typeutil.StaticCallee(pass.TypesInfo, call)
+ if analysisutil.IsFunctionNamed(fn, "sync/atomic", funcNames...) {
// For all the listed functions, the expression to check is always the first function argument.
- check64BitAlignment(pass, sel.Sel.Name, call.Args[0])
+ check64BitAlignment(pass, fn.Name(), call.Args[0])
}
})
diff --git a/go/analysis/passes/atomicalign/testdata/src/a/a.go b/go/analysis/passes/atomicalign/testdata/src/a/a.go
index 45dd73d..deebc30 100644
--- a/go/analysis/passes/atomicalign/testdata/src/a/a.go
+++ b/go/analysis/passes/atomicalign/testdata/src/a/a.go
@@ -4,6 +4,7 @@
// This file contains tests for the atomic alignment checker.
+//go:build arm || 386
// +build arm 386
package testdata
@@ -102,7 +103,8 @@
atomic.LoadInt64(&a.b) // want "address of non 64-bit aligned field .b passed to atomic.LoadInt64"
atomic.LoadInt64(&a.c)
- atomic.LoadUint64(&a.e) // want "address of non 64-bit aligned field .e passed to atomic.LoadUint64"
+ atomic.LoadUint64(&a.e) // want "address of non 64-bit aligned field .e passed to atomic.LoadUint64"
+ (atomic.LoadUint64)(&a.e) // want "address of non 64-bit aligned field .e passed to atomic.LoadUint64"
}
func anonymousFieldAlignment() {
diff --git a/go/analysis/passes/bools/bools.go b/go/analysis/passes/bools/bools.go
index 4219f08..5643297 100644
--- a/go/analysis/passes/bools/bools.go
+++ b/go/analysis/passes/bools/bools.go
@@ -14,6 +14,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/ast/inspector"
)
@@ -83,7 +84,7 @@
i := 0
var sets [][]ast.Expr
for j := 0; j <= len(exprs); j++ {
- if j == len(exprs) || hasSideEffects(info, exprs[j]) {
+ if j == len(exprs) || analysisutil.HasSideEffects(info, exprs[j]) {
if i < j {
sets = append(sets, exprs[i:j])
}
@@ -162,46 +163,13 @@
}
}
-// hasSideEffects reports whether evaluation of e has side effects.
-func hasSideEffects(info *types.Info, e ast.Expr) bool {
- safe := true
- ast.Inspect(e, func(node ast.Node) bool {
- switch n := node.(type) {
- case *ast.CallExpr:
- typVal := info.Types[n.Fun]
- switch {
- case typVal.IsType():
- // Type conversion, which is safe.
- case typVal.IsBuiltin():
- // Builtin func, conservatively assumed to not
- // be safe for now.
- safe = false
- return false
- default:
- // A non-builtin func or method call.
- // Conservatively assume that all of them have
- // side effects for now.
- safe = false
- return false
- }
- case *ast.UnaryExpr:
- if n.Op == token.ARROW {
- safe = false
- return false
- }
- }
- return true
- })
- return !safe
-}
-
// split returns a slice of all subexpressions in e that are connected by op.
// For example, given 'a || (b || c) || d' with the or op,
// split returns []{d, c, b, a}.
// seen[e] is already true; any newly processed exprs are added to seen.
func (op boolOp) split(e ast.Expr, seen map[*ast.BinaryExpr]bool) (exprs []ast.Expr) {
for {
- e = unparen(e)
+ e = astutil.Unparen(e)
if b, ok := e.(*ast.BinaryExpr); ok && b.Op == op.tok {
seen[b] = true
exprs = append(exprs, op.split(b.Y, seen)...)
@@ -213,14 +181,3 @@
}
return
}
-
-// unparen returns e with any enclosing parentheses stripped.
-func unparen(e ast.Expr) ast.Expr {
- for {
- p, ok := e.(*ast.ParenExpr)
- if !ok {
- return e
- }
- e = p.X
- }
-}
diff --git a/go/analysis/passes/cgocall/cgocall.go b/go/analysis/passes/cgocall/cgocall.go
index 98d9a77..4e86439 100644
--- a/go/analysis/passes/cgocall/cgocall.go
+++ b/go/analysis/passes/cgocall/cgocall.go
@@ -19,6 +19,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/astutil"
)
const debug = false
@@ -64,7 +65,7 @@
// Is this a C.f() call?
var name string
- if sel, ok := analysisutil.Unparen(call.Fun).(*ast.SelectorExpr); ok {
+ if sel, ok := astutil.Unparen(call.Fun).(*ast.SelectorExpr); ok {
if id, ok := sel.X.(*ast.Ident); ok && id.Name == "C" {
name = sel.Sel.Name
}
diff --git a/go/analysis/passes/copylock/copylock.go b/go/analysis/passes/copylock/copylock.go
index ec7727d..2eeb0a3 100644
--- a/go/analysis/passes/copylock/copylock.go
+++ b/go/analysis/passes/copylock/copylock.go
@@ -16,6 +16,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/ast/inspector"
"golang.org/x/tools/internal/typeparams"
)
@@ -223,7 +224,7 @@
}
func lockPathRhs(pass *analysis.Pass, x ast.Expr) typePath {
- x = analysisutil.Unparen(x) // ignore parens on rhs
+ x = astutil.Unparen(x) // ignore parens on rhs
if _, ok := x.(*ast.CompositeLit); ok {
return nil
@@ -233,7 +234,7 @@
return nil
}
if star, ok := x.(*ast.StarExpr); ok {
- if _, ok := analysisutil.Unparen(star.X).(*ast.CallExpr); ok {
+ if _, ok := astutil.Unparen(star.X).(*ast.CallExpr); ok {
// A call may return a pointer to a zero value.
return nil
}
@@ -319,9 +320,7 @@
// 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 named, ok := typ.(*types.Named); ok &&
- named.Obj().Name() == "noCopy" &&
- named.Obj().Pkg().Path() == "sync" {
+ if analysisutil.IsNamedType(typ, "sync", "noCopy") {
return []string{typ.String()}
}
diff --git a/go/analysis/passes/deepequalerrors/deepequalerrors.go b/go/analysis/passes/deepequalerrors/deepequalerrors.go
index 3a18187..1a83bdd 100644
--- a/go/analysis/passes/deepequalerrors/deepequalerrors.go
+++ b/go/analysis/passes/deepequalerrors/deepequalerrors.go
@@ -46,11 +46,8 @@
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
call := n.(*ast.CallExpr)
- fn, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Func)
- if !ok {
- return
- }
- if fn.FullName() == "reflect.DeepEqual" && hasError(pass, call.Args[0]) && hasError(pass, call.Args[1]) {
+ fn, _ := typeutil.Callee(pass.TypesInfo, call).(*types.Func)
+ if analysisutil.IsFunctionNamed(fn, "reflect", "DeepEqual") && hasError(pass, call.Args[0]) && hasError(pass, call.Args[1]) {
pass.ReportRangef(call, "avoid using reflect.DeepEqual with errors")
}
})
diff --git a/go/analysis/passes/defers/defers.go b/go/analysis/passes/defers/defers.go
index ed2a122..5e8e80a 100644
--- a/go/analysis/passes/defers/defers.go
+++ b/go/analysis/passes/defers/defers.go
@@ -7,7 +7,6 @@
import (
_ "embed"
"go/ast"
- "go/types"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
@@ -36,8 +35,7 @@
checkDeferCall := func(node ast.Node) bool {
switch v := node.(type) {
case *ast.CallExpr:
- fn, ok := typeutil.Callee(pass.TypesInfo, v).(*types.Func)
- if ok && fn.Name() == "Since" && fn.Pkg().Path() == "time" {
+ if analysisutil.IsFunctionNamed(typeutil.StaticCallee(pass.TypesInfo, v), "time", "Since") {
pass.Reportf(v.Pos(), "call to time.Since is not deferred")
}
case *ast.FuncLit:
diff --git a/go/analysis/passes/defers/testdata/src/a/a.go b/go/analysis/passes/defers/testdata/src/a/a.go
index e8bc8cd..6f0118a 100644
--- a/go/analysis/passes/defers/testdata/src/a/a.go
+++ b/go/analysis/passes/defers/testdata/src/a/a.go
@@ -26,8 +26,8 @@
defer fmt.Println(evalBefore)
do := func(f func()) {}
defer do(func() { time.Since(now) })
- defer fmt.Println(Since()) // OK because Since function is not in module time
-
+ defer fmt.Println(Since()) // OK because Since function is not in module time
+ defer copy([]int(nil), []int{1}) // check that a builtin doesn't cause a panic
}
type y struct{}
diff --git a/go/analysis/passes/errorsas/errorsas.go b/go/analysis/passes/errorsas/errorsas.go
index 2fcbdfa..43996b8 100644
--- a/go/analysis/passes/errorsas/errorsas.go
+++ b/go/analysis/passes/errorsas/errorsas.go
@@ -51,15 +51,12 @@
inspect.Preorder(nodeFilter, func(n ast.Node) {
call := n.(*ast.CallExpr)
fn := typeutil.StaticCallee(pass.TypesInfo, call)
- if fn == nil {
- return // not a static call
+ if !analysisutil.IsFunctionNamed(fn, "errors", "As") {
+ return
}
if len(call.Args) < 2 {
return // not enough arguments, e.g. called with return values of another function
}
- if fn.FullName() != "errors.As" {
- return
- }
if err := checkAsTarget(pass, call.Args[1]); err != nil {
pass.ReportRangef(call, "%v", err)
}
diff --git a/go/analysis/passes/httpmux/cmd/httpmux/main.go b/go/analysis/passes/httpmux/cmd/httpmux/main.go
new file mode 100644
index 0000000..e8a6311
--- /dev/null
+++ b/go/analysis/passes/httpmux/cmd/httpmux/main.go
@@ -0,0 +1,13 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// The httpmux command runs the httpmux analyzer.
+package main
+
+import (
+ "golang.org/x/tools/go/analysis/passes/httpmux"
+ "golang.org/x/tools/go/analysis/singlechecker"
+)
+
+func main() { singlechecker.Main(httpmux.Analyzer) }
diff --git a/go/analysis/passes/httpmux/httpmux.go b/go/analysis/passes/httpmux/httpmux.go
new file mode 100644
index 0000000..fa99296
--- /dev/null
+++ b/go/analysis/passes/httpmux/httpmux.go
@@ -0,0 +1,186 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package httpmux
+
+import (
+ "go/ast"
+ "go/constant"
+ "go/types"
+ "regexp"
+ "strings"
+
+ "golang.org/x/mod/semver"
+ "golang.org/x/tools/go/analysis"
+ "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/go/types/typeutil"
+)
+
+const Doc = `report using Go 1.22 enhanced ServeMux patterns in older Go versions
+
+The httpmux analysis is active for Go modules configured to run with Go 1.21 or
+earlier versions. It reports calls to net/http.ServeMux.Handle and HandleFunc
+methods whose patterns use features added in Go 1.22, like HTTP methods (such as
+"GET") and wildcards. (See https://pkg.go.dev/net/http#ServeMux for details.)
+Such patterns can be registered in older versions of Go, but will not behave as expected.`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "httpmux",
+ Doc: Doc,
+ URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/httpmux",
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+}
+
+var inTest bool // So Go version checks can be skipped during testing.
+
+func run(pass *analysis.Pass) (any, error) {
+ if !inTest {
+ // Check that Go version is 1.21 or earlier.
+ if goVersionAfter121(goVersion(pass.Pkg)) {
+ return nil, nil
+ }
+ }
+ if !analysisutil.Imports(pass.Pkg, "net/http") {
+ return nil, nil
+ }
+ // Look for calls to ServeMux.Handle or ServeMux.HandleFunc.
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+ nodeFilter := []ast.Node{
+ (*ast.CallExpr)(nil),
+ }
+
+ inspect.Preorder(nodeFilter, func(n ast.Node) {
+ call := n.(*ast.CallExpr)
+ if isServeMuxRegisterCall(pass, call) {
+ pat, ok := stringConstantExpr(pass, call.Args[0])
+ if ok && likelyEnhancedPattern(pat) {
+ pass.ReportRangef(call.Args[0], "possible enhanced ServeMux pattern used with Go version before 1.22 (update go.mod file?)")
+ }
+ }
+ })
+ return nil, nil
+}
+
+// isServeMuxRegisterCall reports whether call is a static call to one of:
+// - net/http.Handle
+// - net/http.HandleFunc
+// - net/http.ServeMux.Handle
+// - net/http.ServeMux.HandleFunc
+// TODO(jba): consider expanding this to accommodate wrappers around these functions.
+func isServeMuxRegisterCall(pass *analysis.Pass, call *ast.CallExpr) bool {
+ fn := typeutil.StaticCallee(pass.TypesInfo, call)
+ if fn == nil {
+ return false
+ }
+ if analysisutil.IsFunctionNamed(fn, "net/http", "Handle", "HandleFunc") {
+ return true
+ }
+ if !isMethodNamed(fn, "net/http", "Handle", "HandleFunc") {
+ return false
+ }
+ t, ok := fn.Type().(*types.Signature).Recv().Type().(*types.Pointer)
+ if !ok {
+ return false
+ }
+ return analysisutil.IsNamedType(t.Elem(), "net/http", "ServeMux")
+}
+
+func isMethodNamed(f *types.Func, pkgPath string, names ...string) bool {
+ if f == nil {
+ return false
+ }
+ if f.Pkg() == nil || f.Pkg().Path() != pkgPath {
+ return false
+ }
+ if f.Type().(*types.Signature).Recv() == nil {
+ return false
+ }
+ for _, n := range names {
+ if f.Name() == n {
+ return true
+ }
+ }
+ return false
+}
+
+// stringConstantExpr returns expression's string constant value.
+//
+// ("", false) is returned if expression isn't a string
+// constant.
+func stringConstantExpr(pass *analysis.Pass, expr ast.Expr) (string, bool) {
+ lit := pass.TypesInfo.Types[expr].Value
+ if lit != nil && lit.Kind() == constant.String {
+ return constant.StringVal(lit), true
+ }
+ return "", false
+}
+
+// A valid wildcard must start a segment, and its name must be valid Go
+// identifier.
+var wildcardRegexp = regexp.MustCompile(`/\{[_\pL][_\pL\p{Nd}]*(\.\.\.)?\}`)
+
+// likelyEnhancedPattern reports whether the ServeMux pattern pat probably
+// contains either an HTTP method name or a wildcard, extensions added in Go 1.22.
+func likelyEnhancedPattern(pat string) bool {
+ if strings.Contains(pat, " ") {
+ // A space in the pattern suggests that it begins with an HTTP method.
+ return true
+ }
+ return wildcardRegexp.MatchString(pat)
+}
+
+func goVersionAfter121(goVersion string) bool {
+ if goVersion == "" { // Maybe the stdlib?
+ return true
+ }
+ version := versionFromGoVersion(goVersion)
+ return semver.Compare(version, "v1.21") > 0
+}
+
+func goVersion(pkg *types.Package) string {
+ // types.Package.GoVersion did not exist before Go 1.21.
+ if p, ok := any(pkg).(interface{ GoVersion() string }); ok {
+ return p.GoVersion()
+ }
+ return ""
+}
+
+var (
+ // Regexp for matching go tags. The groups are:
+ // 1 the major.minor version
+ // 2 the patch version, or empty if none
+ // 3 the entire prerelease, if present
+ // 4 the prerelease type ("beta" or "rc")
+ // 5 the prerelease number
+ tagRegexp = regexp.MustCompile(`^go(\d+\.\d+)(\.\d+|)((beta|rc)(\d+))?$`)
+)
+
+// Copied from pkgsite/internal/stdlib.VersionForTag.
+func versionFromGoVersion(goVersion string) string {
+ // Special cases for go1.
+ if goVersion == "go1" {
+ return "v1.0.0"
+ }
+ if goVersion == "go1.0" {
+ return ""
+ }
+ m := tagRegexp.FindStringSubmatch(goVersion)
+ if m == nil {
+ return ""
+ }
+ version := "v" + m[1]
+ if m[2] != "" {
+ version += m[2]
+ } else {
+ version += ".0"
+ }
+ if m[3] != "" {
+ version += "-" + m[4] + "." + m[5]
+ }
+ return version
+}
diff --git a/go/analysis/passes/httpmux/httpmux_test.go b/go/analysis/passes/httpmux/httpmux_test.go
new file mode 100644
index 0000000..f2cb9c7
--- /dev/null
+++ b/go/analysis/passes/httpmux/httpmux_test.go
@@ -0,0 +1,37 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package httpmux
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+)
+
+func Test(t *testing.T) {
+ testdata := analysistest.TestData()
+ tests := []string{"a"}
+ inTest = true
+ analysistest.Run(t, testdata, Analyzer, tests...)
+}
+
+func TestGoVersion(t *testing.T) {
+ for _, test := range []struct {
+ in string
+ want bool
+ }{
+ {"", true},
+ {"go1", false},
+ {"go1.21", false},
+ {"go1.21rc3", false},
+ {"go1.22", true},
+ {"go1.22rc1", true},
+ } {
+ got := goVersionAfter121(test.in)
+ if got != test.want {
+ t.Errorf("%q: got %t, want %t", test.in, got, test.want)
+ }
+ }
+}
diff --git a/go/analysis/passes/httpmux/testdata/src/a/a.go b/go/analysis/passes/httpmux/testdata/src/a/a.go
new file mode 100644
index 0000000..ad5b3ba
--- /dev/null
+++ b/go/analysis/passes/httpmux/testdata/src/a/a.go
@@ -0,0 +1,47 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This file contains tests for the httpmux checker.
+
+package a
+
+import "net/http"
+
+func _() {
+ http.HandleFunc("GET /x", nil) // want "enhanced ServeMux pattern"
+ http.HandleFunc("/{a}/b/", nil) // want "enhanced ServeMux pattern"
+ mux := http.NewServeMux()
+ mux.Handle("example.com/c/{d}", nil) // want "enhanced ServeMux pattern"
+ mux.HandleFunc("/{x...}", nil) // want "enhanced ServeMux pattern"
+
+ // Should not match.
+
+ // not an enhanced pattern
+ http.Handle("/", nil)
+
+ // invalid wildcard; will panic in 1.22
+ http.HandleFunc("/{/a/}", nil)
+ mux.Handle("/{1}", nil)
+ mux.Handle("/x{a}", nil)
+
+ // right package, wrong method
+ http.ParseTime("GET /")
+
+ // right function name, wrong package
+ Handle("GET /", nil)
+ HandleFunc("GET /", nil)
+
+ // right method name, wrong type
+ var sm ServeMux
+ sm.Handle("example.com/c/{d}", nil)
+ sm.HandleFunc("method /{x...}", nil)
+}
+
+func Handle(pat string, x any) {}
+func HandleFunc(pat string, x any) {}
+
+type ServeMux struct{}
+
+func (*ServeMux) Handle(pat string, x any) {}
+func (*ServeMux) HandleFunc(pat string, x any) {}
diff --git a/go/analysis/passes/httpresponse/httpresponse.go b/go/analysis/passes/httpresponse/httpresponse.go
index 61c3b76..c6b6c81 100644
--- a/go/analysis/passes/httpresponse/httpresponse.go
+++ b/go/analysis/passes/httpresponse/httpresponse.go
@@ -116,7 +116,7 @@
if res.Len() != 2 {
return false // the function called does not return two values.
}
- if ptr, ok := res.At(0).Type().(*types.Pointer); !ok || !isNamedType(ptr.Elem(), "net/http", "Response") {
+ if ptr, ok := res.At(0).Type().(*types.Pointer); !ok || !analysisutil.IsNamedType(ptr.Elem(), "net/http", "Response") {
return false // the first return type is not *http.Response.
}
@@ -131,11 +131,11 @@
return ok && id.Name == "http" // function in net/http package.
}
- if isNamedType(typ, "net/http", "Client") {
+ if analysisutil.IsNamedType(typ, "net/http", "Client") {
return true // method on http.Client.
}
ptr, ok := typ.(*types.Pointer)
- return ok && isNamedType(ptr.Elem(), "net/http", "Client") // method on *http.Client.
+ return ok && analysisutil.IsNamedType(ptr.Elem(), "net/http", "Client") // method on *http.Client.
}
// restOfBlock, given a traversal stack, finds the innermost containing
@@ -171,13 +171,3 @@
return nil
}
}
-
-// isNamedType reports whether t is the named type path.name.
-func isNamedType(t types.Type, path, name string) bool {
- n, ok := t.(*types.Named)
- if !ok {
- return false
- }
- obj := n.Obj()
- return obj.Name() == name && obj.Pkg() != nil && obj.Pkg().Path() == path
-}
diff --git a/go/analysis/passes/internal/analysisutil/util.go b/go/analysis/passes/internal/analysisutil/util.go
index a8d8403..c006075 100644
--- a/go/analysis/passes/internal/analysisutil/util.go
+++ b/go/analysis/passes/internal/analysisutil/util.go
@@ -55,17 +55,6 @@
return !safe
}
-// Unparen returns e with any enclosing parentheses stripped.
-func Unparen(e ast.Expr) ast.Expr {
- for {
- p, ok := e.(*ast.ParenExpr)
- if !ok {
- return e
- }
- e = p.X
- }
-}
-
// ReadFile reads a file and adds it to the FileSet
// so that we can report errors against it using lineStart.
func ReadFile(fset *token.FileSet, filename string) ([]byte, *token.File, error) {
@@ -118,3 +107,46 @@
}
return false
}
+
+// IsNamedType reports whether t is the named type with the given package path
+// and one of the given names.
+// This function avoids allocating the concatenation of "pkg.Name",
+// which is important for the performance of syntax matching.
+func IsNamedType(t types.Type, pkgPath string, names ...string) bool {
+ n, ok := t.(*types.Named)
+ if !ok {
+ return false
+ }
+ obj := n.Obj()
+ if obj == nil || obj.Pkg() == nil || obj.Pkg().Path() != pkgPath {
+ return false
+ }
+ name := obj.Name()
+ for _, n := range names {
+ if name == n {
+ return true
+ }
+ }
+ return false
+}
+
+// IsFunctionNamed reports whether f is a top-level function defined in the
+// given package and has one of the given names.
+// It returns false if f is nil or a method.
+func IsFunctionNamed(f *types.Func, pkgPath string, names ...string) bool {
+ if f == nil {
+ return false
+ }
+ if f.Pkg() == nil || f.Pkg().Path() != pkgPath {
+ return false
+ }
+ if f.Type().(*types.Signature).Recv() != nil {
+ return false
+ }
+ for _, n := range names {
+ if f.Name() == n {
+ return true
+ }
+ }
+ return false
+}
diff --git a/go/analysis/passes/loopclosure/loopclosure.go b/go/analysis/passes/loopclosure/loopclosure.go
index 5620c35..fbcdc22 100644
--- a/go/analysis/passes/loopclosure/loopclosure.go
+++ b/go/analysis/passes/loopclosure/loopclosure.go
@@ -359,20 +359,5 @@
if ptr, ok := recv.Type().(*types.Pointer); ok {
rtype = ptr.Elem()
}
- named, ok := rtype.(*types.Named)
- if !ok {
- return false
- }
- if named.Obj().Name() != typeName {
- return false
- }
- pkg := f.Pkg()
- if pkg == nil {
- return false
- }
- if pkg.Path() != pkgPath {
- return false
- }
-
- return true
+ return analysisutil.IsNamedType(rtype, pkgPath, typeName)
}
diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go
index b2b8c67..070654f 100644
--- a/go/analysis/passes/printf/printf.go
+++ b/go/analysis/passes/printf/printf.go
@@ -511,15 +511,10 @@
sig := fn.Type().(*types.Signature)
return sig.Params().Len() == 2 &&
sig.Results().Len() == 0 &&
- isNamed(sig.Params().At(0).Type(), "fmt", "State") &&
+ analysisutil.IsNamedType(sig.Params().At(0).Type(), "fmt", "State") &&
types.Identical(sig.Params().At(1).Type(), types.Typ[types.Rune])
}
-func isNamed(T types.Type, pkgpath, name string) bool {
- named, ok := T.(*types.Named)
- return ok && named.Obj().Pkg().Path() == pkgpath && named.Obj().Name() == name
-}
-
// formatState holds the parsed representation of a printf directive such as "%3.*[4]d".
// It is constructed by parsePrintfVerb.
type formatState struct {
diff --git a/go/analysis/passes/reflectvaluecompare/reflectvaluecompare.go b/go/analysis/passes/reflectvaluecompare/reflectvaluecompare.go
index 2767713..6789d73 100644
--- a/go/analysis/passes/reflectvaluecompare/reflectvaluecompare.go
+++ b/go/analysis/passes/reflectvaluecompare/reflectvaluecompare.go
@@ -49,11 +49,8 @@
}
}
case *ast.CallExpr:
- fn, ok := typeutil.Callee(pass.TypesInfo, n).(*types.Func)
- if !ok {
- return
- }
- if fn.FullName() == "reflect.DeepEqual" && (isReflectValue(pass, n.Args[0]) || isReflectValue(pass, n.Args[1])) {
+ fn, _ := typeutil.Callee(pass.TypesInfo, n).(*types.Func)
+ if analysisutil.IsFunctionNamed(fn, "reflect", "DeepEqual") && (isReflectValue(pass, n.Args[0]) || isReflectValue(pass, n.Args[1])) {
pass.ReportRangef(n, "avoid using reflect.DeepEqual with reflect.Value")
}
}
@@ -68,11 +65,7 @@
return false
}
// See if the type is reflect.Value
- named, ok := tv.Type.(*types.Named)
- if !ok {
- return false
- }
- if obj := named.Obj(); obj == nil || obj.Pkg() == nil || obj.Pkg().Path() != "reflect" || obj.Name() != "Value" {
+ if !analysisutil.IsNamedType(tv.Type, "reflect", "Value") {
return false
}
if _, ok := e.(*ast.CompositeLit); ok {
diff --git a/go/analysis/passes/slog/slog.go b/go/analysis/passes/slog/slog.go
index 92c1da8..a1323c3 100644
--- a/go/analysis/passes/slog/slog.go
+++ b/go/analysis/passes/slog/slog.go
@@ -139,7 +139,7 @@
}
func isAttr(t types.Type) bool {
- return isNamed(t, "log/slog", "Attr")
+ return analysisutil.IsNamedType(t, "log/slog", "Attr")
}
// shortName returns a name for the function that is shorter than FullName.
@@ -232,12 +232,3 @@
sel := info.Selections[s]
return sel != nil && sel.Kind() == types.MethodExpr
}
-
-// isNamed reports whether t is exactly a named type in a package with a given path.
-func isNamed(t types.Type, path, name string) bool {
- if n, ok := t.(*types.Named); ok {
- obj := n.Obj()
- return obj.Pkg() != nil && obj.Pkg().Path() == path && obj.Name() == name
- }
- return false
-}
diff --git a/go/analysis/passes/sortslice/analyzer.go b/go/analysis/passes/sortslice/analyzer.go
index 1fe206b..6c151a0 100644
--- a/go/analysis/passes/sortslice/analyzer.go
+++ b/go/analysis/passes/sortslice/analyzer.go
@@ -47,12 +47,7 @@
inspect.Preorder(nodeFilter, func(n ast.Node) {
call := n.(*ast.CallExpr)
fn, _ := typeutil.Callee(pass.TypesInfo, call).(*types.Func)
- if fn == nil {
- return
- }
-
- fnName := fn.FullName()
- if fnName != "sort.Slice" && fnName != "sort.SliceStable" && fnName != "sort.SliceIsSorted" {
+ if !analysisutil.IsFunctionNamed(fn, "sort", "Slice", "SliceStable", "SliceIsSorted") {
return
}
@@ -131,7 +126,7 @@
pass.Report(analysis.Diagnostic{
Pos: call.Pos(),
End: call.End(),
- Message: fmt.Sprintf("%s's argument must be a slice; is called with %s", fnName, typ.String()),
+ Message: fmt.Sprintf("%s's argument must be a slice; is called with %s", fn.FullName(), typ.String()),
SuggestedFixes: fixes,
})
})
diff --git a/go/analysis/passes/tests/tests.go b/go/analysis/passes/tests/tests.go
index 9589a46..d0b0ebb 100644
--- a/go/analysis/passes/tests/tests.go
+++ b/go/analysis/passes/tests/tests.go
@@ -257,13 +257,7 @@
if !ok {
return false
}
- named, ok := ptr.Elem().(*types.Named)
- if !ok {
- return false
- }
- obj := named.Obj()
- // obj.Pkg is nil for the error type.
- return obj != nil && obj.Pkg() != nil && obj.Pkg().Path() == "testing" && obj.Name() == testingType
+ return analysisutil.IsNamedType(ptr.Elem(), "testing", testingType)
}
// Validate that fuzz target function's arguments are of accepted types.
diff --git a/go/analysis/passes/timeformat/timeformat.go b/go/analysis/passes/timeformat/timeformat.go
index c45b9fa..eb84502 100644
--- a/go/analysis/passes/timeformat/timeformat.go
+++ b/go/analysis/passes/timeformat/timeformat.go
@@ -88,29 +88,16 @@
}
func isTimeDotFormat(f *types.Func) bool {
- if f.Name() != "Format" || f.Pkg().Path() != "time" {
- return false
- }
- sig, ok := f.Type().(*types.Signature)
- if !ok {
+ if f.Name() != "Format" || f.Pkg() == nil || f.Pkg().Path() != "time" {
return false
}
// Verify that the receiver is time.Time.
- recv := sig.Recv()
- if recv == nil {
- return false
- }
- named, ok := recv.Type().(*types.Named)
- return ok && named.Obj().Name() == "Time"
+ recv := f.Type().(*types.Signature).Recv()
+ return recv != nil && analysisutil.IsNamedType(recv.Type(), "time", "Time")
}
func isTimeDotParse(f *types.Func) bool {
- if f.Name() != "Parse" || f.Pkg().Path() != "time" {
- return false
- }
- // Verify that there is no receiver.
- sig, ok := f.Type().(*types.Signature)
- return ok && sig.Recv() == nil
+ return analysisutil.IsFunctionNamed(f, "time", "Parse")
}
// badFormatAt return the start of a bad format in e or -1 if no bad format is found.
diff --git a/go/analysis/passes/unsafeptr/unsafeptr.go b/go/analysis/passes/unsafeptr/unsafeptr.go
index e43ac20..32e71ef 100644
--- a/go/analysis/passes/unsafeptr/unsafeptr.go
+++ b/go/analysis/passes/unsafeptr/unsafeptr.go
@@ -15,6 +15,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/ast/inspector"
)
@@ -68,7 +69,7 @@
// Check unsafe.Pointer safety rules according to
// https://golang.org/pkg/unsafe/#Pointer.
- switch x := analysisutil.Unparen(x).(type) {
+ switch x := astutil.Unparen(x).(type) {
case *ast.SelectorExpr:
// "(6) Conversion of a reflect.SliceHeader or
// reflect.StringHeader Data field to or from Pointer."
@@ -104,8 +105,7 @@
}
switch sel.Sel.Name {
case "Pointer", "UnsafeAddr":
- t, ok := info.Types[sel.X].Type.(*types.Named)
- if ok && t.Obj().Pkg().Path() == "reflect" && t.Obj().Name() == "Value" {
+ if analysisutil.IsNamedType(info.Types[sel.X].Type, "reflect", "Value") {
return true
}
}
@@ -118,7 +118,7 @@
// isSafeArith reports whether x is a pointer arithmetic expression that is safe
// to convert to unsafe.Pointer.
func isSafeArith(info *types.Info, x ast.Expr) bool {
- switch x := analysisutil.Unparen(x).(type) {
+ switch x := astutil.Unparen(x).(type) {
case *ast.CallExpr:
// Base case: initial conversion from unsafe.Pointer to uintptr.
return len(x.Args) == 1 &&
@@ -153,13 +153,5 @@
// isReflectHeader reports whether t is reflect.SliceHeader or reflect.StringHeader.
func isReflectHeader(t types.Type) bool {
- if named, ok := t.(*types.Named); ok {
- if obj := named.Obj(); obj.Pkg() != nil && obj.Pkg().Path() == "reflect" {
- switch obj.Name() {
- case "SliceHeader", "StringHeader":
- return true
- }
- }
- }
- return false
+ return analysisutil.IsNamedType(t, "reflect", "SliceHeader", "StringHeader")
}
diff --git a/go/analysis/passes/unusedresult/unusedresult.go b/go/analysis/passes/unusedresult/unusedresult.go
index cb487a2..7f79b4a 100644
--- a/go/analysis/passes/unusedresult/unusedresult.go
+++ b/go/analysis/passes/unusedresult/unusedresult.go
@@ -24,6 +24,7 @@
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
+ "golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/ast/inspector"
"golang.org/x/tools/go/types/typeutil"
)
@@ -82,7 +83,7 @@
(*ast.ExprStmt)(nil),
}
inspect.Preorder(nodeFilter, func(n ast.Node) {
- call, ok := analysisutil.Unparen(n.(*ast.ExprStmt).X).(*ast.CallExpr)
+ call, ok := astutil.Unparen(n.(*ast.ExprStmt).X).(*ast.CallExpr)
if !ok {
return // not a call statement
}
@@ -92,7 +93,6 @@
if !ok {
return // e.g. var or builtin
}
-
if sig := fn.Type().(*types.Signature); sig.Recv() != nil {
// method (e.g. foo.String())
if types.Identical(sig, sigNoArgsStringResult) {
diff --git a/go/types/internal/play/play.go b/go/types/internal/play/play.go
index 79cdc5a..7a760e2 100644
--- a/go/types/internal/play/play.go
+++ b/go/types/internal/play/play.go
@@ -174,7 +174,7 @@
}
// selection x.f information (if cursor is over .f)
- for _, n := range path[:2] {
+ for _, n := range path[:min(2, len(path))] {
if sel, ok := n.(*ast.SelectorExpr); ok {
seln, ok := pkg.TypesInfo.Selections[sel]
if ok {
@@ -334,3 +334,12 @@
body { color: gray; }
div#out { font-family: monospace; font-size: 80%; }
`
+
+// TODO(adonovan): use go1.21 built-in.
+func min(x, y int) int {
+ if x < y {
+ return x
+ } else {
+ return y
+ }
+}
diff --git a/gopls/go.mod b/gopls/go.mod
index 80ce516..b4f027b 100644
--- a/gopls/go.mod
+++ b/gopls/go.mod
@@ -7,10 +7,10 @@
github.com/jba/printsrc v0.2.2
github.com/jba/templatecheck v0.6.0
github.com/sergi/go-diff v1.1.0
- golang.org/x/mod v0.12.0
- golang.org/x/sync v0.3.0
- golang.org/x/sys v0.12.0
- golang.org/x/telemetry v0.0.0-20231003223302-0168ef4ebbd3
+ golang.org/x/mod v0.13.0
+ golang.org/x/sync v0.4.0
+ golang.org/x/sys v0.13.0
+ golang.org/x/telemetry v0.0.0-20231011160506-788d5629a052
golang.org/x/text v0.13.0
golang.org/x/tools v0.13.1-0.20231005170247-117a486e6b4a
golang.org/x/vuln v1.0.1
@@ -26,3 +26,5 @@
golang.org/x/exp/typeparams v0.0.0-20221212164502-fae10dda9338 // indirect
)
+
+replace golang.org/x/tools => ../
diff --git a/gopls/go.sum b/gopls/go.sum
index dbf0717..ea1d12e 100644
--- a/gopls/go.sum
+++ b/gopls/go.sum
@@ -30,15 +30,15 @@
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
golang.org/x/exp/typeparams v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/exp/typeparams v0.0.0-20221212164502-fae10dda9338/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
-golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc=
-golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/mod v0.13.0 h1:I/DsJXRlw/8l/0c24sM9yb0T4z9liZTduXvdAWYiysY=
+golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E=
-golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
-golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
-golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/telemetry v0.0.0-20231003223302-0168ef4ebbd3 h1:vxxQvncMbcRAtqHV5HsHGJkbya+BIOYIY+y6cdPZhzk=
-golang.org/x/telemetry v0.0.0-20231003223302-0168ef4ebbd3/go.mod h1:ppZ76JTkRgJC2GQEgtVY3fiuJR+N8FU2MAlp+gfN1E4=
+golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
+golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
+golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
+golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/telemetry v0.0.0-20231011160506-788d5629a052 h1:1baVNneD/IRxmu8JQdBuki78zUqBtZxq8smZXQj0X2Y=
+golang.org/x/telemetry v0.0.0-20231011160506-788d5629a052/go.mod h1:6p4ScoNeC2dhpQ1nSSMmkZ7mEj5JQUSCyc0uExBp5T4=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
diff --git a/gopls/internal/lsp/cache/filemap_test.go b/gopls/internal/lsp/cache/filemap_test.go
index 3d5bab4..a1d10af 100644
--- a/gopls/internal/lsp/cache/filemap_test.go
+++ b/gopls/internal/lsp/cache/filemap_test.go
@@ -7,7 +7,6 @@
import (
"path/filepath"
"sort"
- "strings"
"testing"
"github.com/google/go-cmp/cmp"
@@ -56,7 +55,12 @@
// Normalize paths for windows compatibility.
normalize := func(path string) string {
- return strings.TrimPrefix(filepath.ToSlash(path), "C:") // the span packages adds 'C:'
+ y := filepath.ToSlash(path)
+ // Windows paths may start with a drive letter
+ if len(y) > 2 && y[1] == ':' && y[0] >= 'A' && y[0] <= 'Z' {
+ y = y[2:]
+ }
+ return y
}
for _, test := range tests {
diff --git a/gopls/internal/lsp/fake/editor.go b/gopls/internal/lsp/fake/editor.go
index ccee51e..b8e6901 100644
--- a/gopls/internal/lsp/fake/editor.go
+++ b/gopls/internal/lsp/fake/editor.go
@@ -44,7 +44,8 @@
config EditorConfig // editor configuration
buffers map[string]buffer // open buffers (relative path -> buffer content)
serverCapabilities protocol.ServerCapabilities // capabilities / options
- watchPatterns []*glob.Glob // glob patterns to watch
+ semTokOpts protocol.SemanticTokensOptions
+ watchPatterns []*glob.Glob // glob patterns to watch
// Call metrics for the purpose of expectations. This is done in an ad-hoc
// manner for now. Perhaps in the future we should do something more
@@ -306,8 +307,13 @@
if err != nil {
return fmt.Errorf("initialize: %w", err)
}
+ semTokOpts, err := marshalUnmarshal[protocol.SemanticTokensOptions](resp.Capabilities.SemanticTokensProvider)
+ if err != nil {
+ return fmt.Errorf("unmarshalling semantic tokens options: %v", err)
+ }
e.mu.Lock()
e.serverCapabilities = resp.Capabilities
+ e.semTokOpts = semTokOpts
e.mu.Unlock()
if err := e.Server.Initialized(ctx, &protocol.InitializedParams{}); err != nil {
@@ -318,6 +324,19 @@
return nil
}
+// marshalUnmarshal is a helper to json Marshal and then Unmarshal as a
+// different type. Used to work around cases where our protocol types are not
+// specific.
+func marshalUnmarshal[T any](v any) (T, error) {
+ var t T
+ data, err := json.Marshal(v)
+ if err != nil {
+ return t, err
+ }
+ err = json.Unmarshal(data, &t)
+ return t, err
+}
+
// HasCommand reports whether the connected server supports the command with the given ID.
func (e *Editor) HasCommand(id string) bool {
for _, command := range e.serverCapabilities.ExecuteCommandProvider.Commands {
@@ -1492,3 +1511,61 @@
return e.Server.DocumentHighlight(ctx, params)
}
+
+// SemanticTokens invokes textDocument/semanticTokens/full, and interprets its
+// result.
+func (e *Editor) SemanticTokens(ctx context.Context, path string) ([]SemanticToken, error) {
+ p := &protocol.SemanticTokensParams{
+ TextDocument: protocol.TextDocumentIdentifier{
+ URI: e.sandbox.Workdir.URI(path),
+ },
+ }
+ resp, err := e.Server.SemanticTokensFull(ctx, p)
+ if err != nil {
+ return nil, err
+ }
+ content, ok := e.BufferText(path)
+ if !ok {
+ return nil, fmt.Errorf("buffer %s is not open", path)
+ }
+ return e.interpretTokens(resp.Data, content), nil
+}
+
+// A SemanticToken is an interpreted semantic token value.
+type SemanticToken struct {
+ Token string
+ TokenType string
+ Mod string
+}
+
+// Note: previously this function elided comment, string, and number tokens.
+// Instead, filtering of token types should be done by the caller.
+func (e *Editor) interpretTokens(x []uint32, contents string) []SemanticToken {
+ e.mu.Lock()
+ legend := e.semTokOpts.Legend
+ e.mu.Unlock()
+ lines := strings.Split(contents, "\n")
+ ans := []SemanticToken{}
+ line, col := 1, 1
+ for i := 0; i < len(x); i += 5 {
+ line += int(x[i])
+ col += int(x[i+1])
+ if x[i] != 0 { // new line
+ col = int(x[i+1]) + 1 // 1-based column numbers
+ }
+ sz := x[i+2]
+ t := legend.TokenTypes[x[i+3]]
+ l := x[i+4]
+ var mods []string
+ for i, mod := range legend.TokenModifiers {
+ if l&(1<<i) != 0 {
+ mods = append(mods, mod)
+ }
+ }
+ // Preexisting note: "col is a utf-8 offset"
+ // TODO(rfindley): is that true? Or is it UTF-16, like other columns in the LSP?
+ tok := lines[line-1][col-1 : col-1+int(sz)]
+ ans = append(ans, SemanticToken{tok, t, strings.Join(mods, " ")})
+ }
+ return ans
+}
diff --git a/gopls/internal/lsp/general.go b/gopls/internal/lsp/general.go
index 7d9ed35..02e9f70 100644
--- a/gopls/internal/lsp/general.go
+++ b/gopls/internal/lsp/general.go
@@ -172,8 +172,8 @@
Range: &protocol.Or_SemanticTokensOptions_range{Value: true},
Full: &protocol.Or_SemanticTokensOptions_full{Value: true},
Legend: protocol.SemanticTokensLegend{
- TokenTypes: nonNilSliceString(s.Options().SemanticTypes),
- TokenModifiers: nonNilSliceString(s.Options().SemanticMods),
+ TokenTypes: nonNilSliceString(options.SemanticTypes),
+ TokenModifiers: nonNilSliceString(options.SemanticMods),
},
},
SignatureHelpProvider: &protocol.SignatureHelpOptions{
@@ -217,7 +217,6 @@
}
s.notifications = nil
- options := s.Options()
if err := s.addFolders(ctx, s.pendingFolders); err != nil {
return err
}
@@ -225,6 +224,7 @@
s.checkViewGoVersions()
var registrations []protocol.Registration
+ options := s.Options()
if options.ConfigurationSupported && options.DynamicConfigurationSupported {
registrations = append(registrations, protocol.Registration{
ID: "workspace/didChangeConfiguration",
diff --git a/gopls/internal/lsp/regtest/regtest.go b/gopls/internal/lsp/regtest/regtest.go
index 7def1d7..6e14b91 100644
--- a/gopls/internal/lsp/regtest/regtest.go
+++ b/gopls/internal/lsp/regtest/regtest.go
@@ -104,7 +104,7 @@
}
if !testenv.HasExec() {
- fmt.Printf("skipping all tests: exec not supported on %s\n", runtime.GOOS)
+ fmt.Printf("skipping all tests: exec not supported on %s/%s\n", runtime.GOOS, runtime.GOARCH)
os.Exit(0)
}
testenv.ExitIfSmallMachine()
diff --git a/gopls/internal/regtest/misc/semantictokens_test.go b/gopls/internal/regtest/misc/semantictokens_test.go
index a96024b..d50ceb0 100644
--- a/gopls/internal/regtest/misc/semantictokens_test.go
+++ b/gopls/internal/regtest/misc/semantictokens_test.go
@@ -5,11 +5,10 @@
package misc
import (
- "strings"
"testing"
"github.com/google/go-cmp/cmp"
- "golang.org/x/tools/gopls/internal/lsp"
+ "golang.org/x/tools/gopls/internal/lsp/fake"
"golang.org/x/tools/gopls/internal/lsp/protocol"
. "golang.org/x/tools/gopls/internal/lsp/regtest"
"golang.org/x/tools/internal/typeparams"
@@ -48,31 +47,30 @@
// fix bug involving type parameters and regular parameters
// (golang/vscode-go#2527)
func TestSemantic_2527(t *testing.T) {
- if !typeparams.Enabled {
- t.Skip("type parameters are needed for this test")
- }
// these are the expected types of identifiers in text order
- want := []result{
- {"package", "keyword", ""},
- {"foo", "namespace", ""},
- {"func", "keyword", ""},
- {"Add", "function", "definition deprecated"},
- {"T", "typeParameter", "definition"},
- {"int", "type", "defaultLibrary"},
- {"target", "parameter", "definition"},
- {"T", "typeParameter", ""},
- {"l", "parameter", "definition"},
- {"T", "typeParameter", ""},
- {"T", "typeParameter", ""},
- {"return", "keyword", ""},
- {"append", "function", "defaultLibrary"},
- {"l", "parameter", ""},
- {"target", "parameter", ""},
- {"for", "keyword", ""},
- {"range", "keyword", ""},
- {"l", "parameter", ""},
- {"return", "keyword", ""},
- {"nil", "variable", "readonly defaultLibrary"},
+ want := []fake.SemanticToken{
+ {Token: "package", TokenType: "keyword"},
+ {Token: "foo", TokenType: "namespace"},
+ {Token: "// Deprecated (for testing)", TokenType: "comment"},
+ {Token: "func", TokenType: "keyword"},
+ {Token: "Add", TokenType: "function", Mod: "definition deprecated"},
+ {Token: "T", TokenType: "typeParameter", Mod: "definition"},
+ {Token: "int", TokenType: "type", Mod: "defaultLibrary"},
+ {Token: "target", TokenType: "parameter", Mod: "definition"},
+ {Token: "T", TokenType: "typeParameter"},
+ {Token: "l", TokenType: "parameter", Mod: "definition"},
+ {Token: "T", TokenType: "typeParameter"},
+ {Token: "T", TokenType: "typeParameter"},
+ {Token: "return", TokenType: "keyword"},
+ {Token: "append", TokenType: "function", Mod: "defaultLibrary"},
+ {Token: "l", TokenType: "parameter"},
+ {Token: "target", TokenType: "parameter"},
+ {Token: "for", TokenType: "keyword"},
+ {Token: "range", TokenType: "keyword"},
+ {Token: "l", TokenType: "parameter"},
+ {Token: "// test coverage", TokenType: "comment"},
+ {Token: "return", TokenType: "keyword"},
+ {Token: "nil", TokenType: "variable", Mod: "readonly defaultLibrary"},
}
src := `
-- go.mod --
@@ -96,16 +94,10 @@
env.AfterChange(
Diagnostics(env.AtRegexp("main.go", "for range")),
)
- p := &protocol.SemanticTokensParams{
- TextDocument: protocol.TextDocumentIdentifier{
- URI: env.Sandbox.Workdir.URI("main.go"),
- },
- }
- v, err := env.Editor.Server.SemanticTokensFull(env.Ctx, p)
+ seen, err := env.Editor.SemanticTokens(env.Ctx, "main.go")
if err != nil {
t.Fatal(err)
}
- seen := interpret(v.Data, env.BufferText("main.go"))
if x := cmp.Diff(want, seen); x != "" {
t.Errorf("Semantic tokens do not match (-want +got):\n%s", x)
}
@@ -142,16 +134,10 @@
Settings{"semanticTokens": true},
).Run(t, src, func(t *testing.T, env *Env) {
env.OpenFile("main.go")
- p := &protocol.SemanticTokensParams{
- TextDocument: protocol.TextDocumentIdentifier{
- URI: env.Sandbox.Workdir.URI("main.go"),
- },
- }
- v, err := env.Editor.Server.SemanticTokensFull(env.Ctx, p)
+ seen, err := env.Editor.SemanticTokens(env.Ctx, "main.go")
if err != nil {
t.Fatal(err)
}
- seen := interpret(v.Data, env.BufferText("main.go"))
for i, s := range seen {
if (s.Token == "K" || s.Token == "V") && s.TokenType != "typeParameter" {
t.Errorf("%d: expected K and V to be type parameters, but got %v", i, s)
@@ -159,46 +145,3 @@
}
})
}
-
-type result struct {
- Token string
- TokenType string
- Mod string
-}
-
-// human-readable version of the semantic tokens
-// comment, string, number are elided
-// (and in the future, maybe elide other things, like operators)
-func interpret(x []uint32, contents string) []result {
- lines := strings.Split(contents, "\n")
- ans := []result{}
- line, col := 1, 1
- for i := 0; i < len(x); i += 5 {
- line += int(x[i])
- col += int(x[i+1])
- if x[i] != 0 { // new line
- col = int(x[i+1]) + 1 // 1-based column numbers
- }
- sz := x[i+2]
- t := semanticTypes[x[i+3]]
- if t == "comment" || t == "string" || t == "number" {
- continue
- }
- l := x[i+4]
- var mods []string
- for i, mod := range semanticModifiers {
- if l&(1<<i) != 0 {
- mods = append(mods, mod)
- }
- }
- // col is a utf-8 offset
- tok := lines[line-1][col-1 : col-1+int(sz)]
- ans = append(ans, result{tok, t, strings.Join(mods, " ")})
- }
- return ans
-}
-
-var (
- semanticTypes = lsp.SemanticTypes()
- semanticModifiers = lsp.SemanticModifiers()
-)
diff --git a/internal/refactor/inline/calleefx.go b/internal/refactor/inline/calleefx.go
index 6e3dc79..11246e5 100644
--- a/internal/refactor/inline/calleefx.go
+++ b/internal/refactor/inline/calleefx.go
@@ -116,10 +116,23 @@
visitExpr(n.X)
case *ast.SelectorExpr:
- if sel, ok := info.Selections[n]; ok {
+ if seln, ok := info.Selections[n]; ok {
visitExpr(n.X)
- if sel.Indirect() {
- effect(rinf) // indirect read x.f of heap variable
+
+ // See types.SelectionKind for background.
+ switch seln.Kind() {
+ case types.MethodExpr:
+ // A method expression T.f acts like a
+ // reference to a func decl,
+ // so it doesn't read x until called.
+
+ case types.MethodVal, types.FieldVal:
+ // A field or method value selection x.f
+ // reads x if the selection indirects a pointer.
+
+ if indirectSelection(seln) {
+ effect(rinf)
+ }
}
} else {
// qualified identifier: treat like unqualified
diff --git a/internal/refactor/inline/escape.go b/internal/refactor/inline/escape.go
index d05d2b9..795ad4f 100644
--- a/internal/refactor/inline/escape.go
+++ b/internal/refactor/inline/escape.go
@@ -72,9 +72,11 @@
if sel, ok := n.Fun.(*ast.SelectorExpr); ok {
if seln, ok := info.Selections[sel]; ok &&
seln.Kind() == types.MethodVal &&
- !seln.Indirect() &&
- is[*types.Pointer](seln.Obj().Type().(*types.Signature).Recv().Type()) {
- lvalue(sel.X, true) // &x.f
+ isPointer(seln.Obj().Type().(*types.Signature).Recv().Type()) {
+ tArg, indirect := effectiveReceiver(seln)
+ if !indirect && !isPointer(tArg) {
+ lvalue(sel.X, true) // &x.f
+ }
}
}
diff --git a/internal/refactor/inline/everything_test.go b/internal/refactor/inline/everything_test.go
index a1c5448..f611149 100644
--- a/internal/refactor/inline/everything_test.go
+++ b/internal/refactor/inline/everything_test.go
@@ -10,6 +10,7 @@
"go/ast"
"go/parser"
"go/types"
+ "log"
"os"
"path/filepath"
"strings"
@@ -37,7 +38,7 @@
//
// And these commands to inline everything in the kubernetes repository:
//
-// $ go build -c -o /tmp/everything ./internal/refactor/inline/
+// $ go test -c -o /tmp/everything ./internal/refactor/inline/
// $ (cd kubernetes && /tmp/everything -test.run=Everything -packages=./...)
//
// TODO(adonovan):
@@ -226,7 +227,7 @@
noMutCheck()
}
}
- t.Errorf("Analyzed %d packages", len(pkgs))
+ log.Printf("Analyzed %d packages", len(pkgs))
}
type importerFunc func(path string) (*types.Package, error)
diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go
index 9615ab4..618b179 100644
--- a/internal/refactor/inline/inline.go
+++ b/internal/refactor/inline/inline.go
@@ -844,23 +844,30 @@
// x, y = f()
// or the sole argument to a spread call:
// printf(f())
+ // or spread return statement:
+ // return f()
res.old = context
switch context := context.(type) {
case *ast.AssignStmt:
- // Inv: the call is in Rhs[0], not Lhs.
+ // Inv: the call must be in Rhs[0], not Lhs.
assign := shallowCopy(context)
assign.Rhs = results
res.new = assign
case *ast.ValueSpec:
- // Inv: the call is in Values[0], not Names.
+ // Inv: the call must be in Values[0], not Names.
spec := shallowCopy(context)
spec.Values = results
res.new = spec
case *ast.CallExpr:
- // Inv: the Call is Args[0], not Fun.
+ // Inv: the call must be in Args[0], not Fun.
call := shallowCopy(context)
call.Args = results
res.new = call
+ case *ast.ReturnStmt:
+ // Inv: the call must be Results[0].
+ ret := shallowCopy(context)
+ ret.Results = results
+ res.new = ret
default:
return nil, fmt.Errorf("internal error: unexpected context %T for spread call", context)
}
@@ -1086,7 +1093,7 @@
debugFormatNode(caller.Fset, caller.Call.Fun),
fld.Name())
}
- if is[*types.Pointer](arg.typ.Underlying()) {
+ if isPointer(arg.typ) {
arg.pure = false // implicit *ptr operation => impure
}
arg.expr = &ast.SelectorExpr{
@@ -1098,8 +1105,8 @@
}
// Make * or & explicit.
- argIsPtr := arg.typ != deref(arg.typ)
- paramIsPtr := is[*types.Pointer](seln.Obj().Type().(*types.Signature).Recv().Type())
+ argIsPtr := isPointer(arg.typ)
+ paramIsPtr := isPointer(seln.Obj().Type().(*types.Signature).Recv().Type())
if !argIsPtr && paramIsPtr {
// &recv
arg.expr = &ast.UnaryExpr{Op: token.AND, X: arg.expr}
@@ -1903,27 +1910,23 @@
return true
case *ast.SelectorExpr:
- if sel, ok := info.Selections[e]; ok {
- switch sel.Kind() {
+ if seln, ok := info.Selections[e]; ok {
+
+ // See types.SelectionKind for background.
+ switch seln.Kind() {
case types.MethodExpr:
// A method expression T.f acts like a
// reference to a func decl, so it is pure.
return true
- case types.MethodVal:
- // A method value x.f acts like a
- // closure around a T.f(x, ...) call,
- // so it is as pure as x.
- return pure(e.X)
-
- case types.FieldVal:
- // A field selection x.f is pure if
- // x is pure and the selection does
+ case types.MethodVal, types.FieldVal:
+ // A field or method selection x.f is pure
+ // if x is pure and the selection does
// not indirect a pointer.
- return !sel.Indirect() && pure(e.X)
+ return !indirectSelection(seln) && pure(e.X)
default:
- panic(sel)
+ panic(seln)
}
} else {
// A qualified identifier is
@@ -1971,7 +1974,7 @@
// integer literals, unary negation, and selectors x.f where x is not
// a pointer. But we would not wish to duplicate expressions that:
// - have side effects (e.g. nearly all calls),
-// - are not referentially transparent (e.g. &T{}, ptr.field), or
+// - are not referentially transparent (e.g. &T{}, ptr.field, *ptr), or
// - are long (e.g. "huge string literal").
func duplicable(info *types.Info, e ast.Expr) bool {
switch e := e.(type) {
@@ -1995,17 +1998,35 @@
case *ast.UnaryExpr: // e.g. +1, -1
return (e.Op == token.ADD || e.Op == token.SUB) && duplicable(info, e.X)
+ case *ast.CompositeLit:
+ // Empty struct or array literals T{} are duplicable.
+ // (Non-empty literals are too verbose, and slice/map
+ // literals allocate indirect variables.)
+ if len(e.Elts) == 0 {
+ switch info.TypeOf(e).Underlying().(type) {
+ case *types.Struct, *types.Array:
+ return true
+ }
+ }
+ return false
+
case *ast.CallExpr:
// Don't treat a conversion T(x) as duplicable even
// if x is duplicable because it could duplicate
- // allocations. There may be cases to tease apart here.
+ // allocations.
+ //
+ // TODO(adonovan): there are cases to tease apart here:
+ // duplicating string([]byte) conversions increases
+ // allocation but doesn't change behavior, but the
+ // reverse, []byte(string), allocates a distinct array,
+ // which is observable
return false
case *ast.SelectorExpr:
- if sel, ok := info.Selections[e]; ok {
+ if seln, ok := info.Selections[e]; ok {
// A field or method selection x.f is referentially
// transparent if it does not indirect a pointer.
- return !sel.Indirect()
+ return !indirectSelection(seln)
}
// A qualified identifier pkg.Name is referentially transparent.
return true
diff --git a/internal/refactor/inline/inline_test.go b/internal/refactor/inline/inline_test.go
index 8362e44..189ac3f 100644
--- a/internal/refactor/inline/inline_test.go
+++ b/internal/refactor/inline/inline_test.go
@@ -391,6 +391,56 @@
})
}
+func TestDuplicable(t *testing.T) {
+ runTests(t, []testcase{
+ {
+ "Empty strings are duplicable.",
+ `func f(s string) { print(s, s) }`,
+ `func _() { f("") }`,
+ `func _() { print("", "") }`,
+ },
+ {
+ "Non-empty string literals are not duplicable.",
+ `func f(s string) { print(s, s) }`,
+ `func _() { f("hi") }`,
+ `func _() {
+ var s string = "hi"
+ print(s, s)
+}`,
+ },
+ {
+ "Empty array literals are duplicable.",
+ `func f(a [2]int) { print(a, a) }`,
+ `func _() { f([2]int{}) }`,
+ `func _() { print([2]int{}, [2]int{}) }`,
+ },
+ {
+ "Non-empty array literals are not duplicable.",
+ `func f(a [2]int) { print(a, a) }`,
+ `func _() { f([2]int{1, 2}) }`,
+ `func _() {
+ var a [2]int = [2]int{1, 2}
+ print(a, a)
+}`,
+ },
+ {
+ "Empty struct literals are duplicable.",
+ `func f(s S) { print(s, s) }; type S struct { x int }`,
+ `func _() { f(S{}) }`,
+ `func _() { print(S{}, S{}) }`,
+ },
+ {
+ "Non-empty struct literals are not duplicable.",
+ `func f(s S) { print(s, s) }; type S struct { x int }`,
+ `func _() { f(S{x: 1}) }`,
+ `func _() {
+ var s S = S{x: 1}
+ print(s, s)
+}`,
+ },
+ })
+}
+
func TestExprStmtReduction(t *testing.T) {
runTests(t, []testcase{
{
@@ -617,6 +667,12 @@
)
}`,
},
+ {
+ "Spread call in return (#63398).",
+ `func f() (int, error) { return 0, nil }`,
+ `func _() (int, error) { return f() }`,
+ `func _() (int, error) { return 0, nil }`,
+ },
})
}
@@ -715,6 +771,21 @@
`func _() { f(g(1), g(2), g(3)) }`,
`func _() { func(int, y any, z int) { defer g(0); println(int, y, z) }(g(1), g(2), g(3)) }`,
},
+ {
+ "An indirect method selection (*x).g acts as a read.",
+ `func f(x *T, y any) any { return x.g(y) }; type T struct{}; func (T) g(x any) any { return x }`,
+ `func _(x *T) { f(x, recover()) }`,
+ `func _(x *T) {
+ var y any = recover()
+ x.g(y)
+}`,
+ },
+ {
+ "A direct method selection x.g is pure.",
+ `func f(x *T, y any) any { return x.g(y) }; type T struct{}; func (*T) g(x any) any { return x }`,
+ `func _(x *T) { f(x, recover()) }`,
+ `func _(x *T) { x.g(recover()) }`,
+ },
})
}
diff --git a/internal/refactor/inline/util.go b/internal/refactor/inline/util.go
index 98d654e..267ef74 100644
--- a/internal/refactor/inline/util.go
+++ b/internal/refactor/inline/util.go
@@ -12,6 +12,8 @@
"go/types"
"reflect"
"strings"
+
+ "golang.org/x/tools/internal/typeparams"
)
func is[T any](x any) bool {
@@ -117,3 +119,42 @@
Args: []ast.Expr{x},
}
}
+
+// isPointer reports whether t is a pointer type.
+func isPointer(t types.Type) bool { return t != deref(t) }
+
+// indirectSelection is like seln.Indirect() without bug #8353.
+func indirectSelection(seln *types.Selection) bool {
+ // Work around bug #8353 in Selection.Indirect when Kind=MethodVal.
+ if seln.Kind() == types.MethodVal {
+ tArg, indirect := effectiveReceiver(seln)
+ if indirect {
+ return true
+ }
+
+ tParam := seln.Obj().Type().(*types.Signature).Recv().Type()
+ return isPointer(tArg) && !isPointer(tParam) // implicit *
+ }
+
+ return seln.Indirect()
+}
+
+// effectiveReceiver returns the effective type of the method
+// receiver after all implicit field selections (but not implicit * or
+// & operations) have been applied.
+//
+// The boolean indicates whether any implicit field selection was indirect.
+func effectiveReceiver(seln *types.Selection) (types.Type, bool) {
+ assert(seln.Kind() == types.MethodVal, "not MethodVal")
+ t := seln.Recv()
+ indices := seln.Index()
+ indirect := false
+ for _, index := range indices[:len(indices)-1] {
+ if tElem := deref(t); tElem != t {
+ indirect = true
+ t = tElem
+ }
+ t = typeparams.CoreType(t).(*types.Struct).Field(index).Type()
+ }
+ return t, indirect
+}
