Skip to content

Commit 1eb875f

Browse files
findleyrfindleyr
committed
all: merge master (7eebab3) into gopls-release-branch.0.17
Also add back the x/tools replace directive for gopls. For golang/go#70301 Merge List: + 2024-12-04 7eebab3 gopls/internal/golang: support extract variable at top level + 2024-12-04 e334696 gopls/internal/golang: ignore effects for change signature refactoring + 2024-12-04 3901733 internal/refactor/inline: substitute groups of dependent arguments + 2024-12-03 61b2408 gopls/internal/golang: add "Extract constant" counterpart + 2024-12-03 c01eead internal/gcimporter: require binary export data header + 2024-12-03 9a04136 gopls/internal/golang/stubmethods: refine crash into bug report + 2024-12-03 01e0b05 internal/refactor/inline: fix condition for splice assignment strategy + 2024-12-03 557f540 gopls/internal/golang: don't offer to move variadic parameters + 2024-12-03 399ee16 internal/gcimporter: update FindExportData to return a non-negative size + 2024-12-03 25b0003 internal/refactor/inline: more precise redundant conversion detection + 2024-12-03 eb46939 gopls/internal/test/marker: add reproducers for moveparam bug bash bugs + 2024-12-03 4296223 gopls/internal/analysis/yield: add comment about dataflow + 2024-12-03 7a4f3b0 internal/gcimporter: require archive files + 2024-12-02 2f73c61 gopls/internal/golang: avoid crash in lookupDocLinkSymbol + 2024-12-02 ef3d603 gopls/internal/golang/completion: fix crash with extra results + 2024-12-02 8ffeaba gopls/internal/settings: enable 'waitgroup' analyzer + 2024-12-02 4317959 go/analysis/passes/waitgroup: report WaitGroup.Add in goroutine + 2024-12-02 72fdfa6 gopls/internal/golang: Disable test generation for generic functions + 2024-12-02 b80f1ed gopls/internal/analysis/yield: peephole-optimize phi(false, x) + 2024-11-28 e7bd227 gopls/internal/golang: fix folding range for function calls + 2024-11-28 e71702b internal/versions: remove constraint.GoVersion wrapper + 2024-11-28 c99edec gopls/internal/golang/completion: add alias support for literals + 2024-11-27 bfe3046 go/packages: add a unit test for golang/go#70394 + 2024-11-27 df87831 gopls/internal/undeclaredname: add missing colon when appropriate + 2024-11-26 53efd30 gopls/internal/golang: simplify package name collection in add test Change-Id: I476e80493f61732701784befe2b130ca967b259e
2 parents c9ad642 + 7eebab3 commit 1eb875f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

72 files changed

+2482
-656
lines changed

Diff for: ‎go/analysis/passes/buildtag/buildtag.go

+6-19
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515

1616
"golang.org/x/tools/go/analysis"
1717
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
18-
"golang.org/x/tools/internal/versions"
1918
)
2019

2120
const Doc = "check //go:build and // +build directives"
@@ -371,11 +370,6 @@ func (check *checker) finish() {
371370

372371
// tags reports issues in go versions in tags within the expression e.
373372
func (check *checker) tags(pos token.Pos, e constraint.Expr) {
374-
// Check that constraint.GoVersion is meaningful (>= go1.21).
375-
if versions.ConstraintGoVersion == nil {
376-
return
377-
}
378-
379373
// Use Eval to visit each tag.
380374
_ = e.Eval(func(tag string) bool {
381375
if malformedGoTag(tag) {
@@ -393,26 +387,19 @@ func malformedGoTag(tag string) bool {
393387
// Check for close misspellings of the "go1." prefix.
394388
for _, pre := range []string{"go.", "g1.", "go"} {
395389
suffix := strings.TrimPrefix(tag, pre)
396-
if suffix != tag {
397-
if valid, ok := validTag("go1." + suffix); ok && valid {
398-
return true
399-
}
390+
if suffix != tag && validGoVersion("go1."+suffix) {
391+
return true
400392
}
401393
}
402394
return false
403395
}
404396

405397
// The tag starts with "go1" so it is almost certainly a GoVersion.
406398
// Report it if it is not a valid build constraint.
407-
valid, ok := validTag(tag)
408-
return ok && !valid
399+
return !validGoVersion(tag)
409400
}
410401

411-
// validTag returns (valid, ok) where valid reports when a tag is valid,
412-
// and ok reports determining if the tag is valid succeeded.
413-
func validTag(tag string) (valid bool, ok bool) {
414-
if versions.ConstraintGoVersion != nil {
415-
return versions.ConstraintGoVersion(&constraint.TagExpr{Tag: tag}) != "", true
416-
}
417-
return false, false
402+
// validGoVersion reports when a tag is a valid go version.
403+
func validGoVersion(tag string) bool {
404+
return constraint.GoVersion(&constraint.TagExpr{Tag: tag}) != ""
418405
}

Diff for: ‎go/analysis/passes/waitgroup/doc.go

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package waitgroup defines an Analyzer that detects simple misuses
6+
// of sync.WaitGroup.
7+
//
8+
// # Analyzer waitgroup
9+
//
10+
// waitgroup: check for misuses of sync.WaitGroup
11+
//
12+
// This analyzer detects mistaken calls to the (*sync.WaitGroup).Add
13+
// method from inside a new goroutine, causing Add to race with Wait:
14+
//
15+
// // WRONG
16+
// var wg sync.WaitGroup
17+
// go func() {
18+
// wg.Add(1) // "WaitGroup.Add called from inside new goroutine"
19+
// defer wg.Done()
20+
// ...
21+
// }()
22+
// wg.Wait() // (may return prematurely before new goroutine starts)
23+
//
24+
// The correct code calls Add before starting the goroutine:
25+
//
26+
// // RIGHT
27+
// var wg sync.WaitGroup
28+
// wg.Add(1)
29+
// go func() {
30+
// defer wg.Done()
31+
// ...
32+
// }()
33+
// wg.Wait()
34+
package waitgroup

Diff for: ‎go/analysis/passes/waitgroup/main.go

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build ignore
6+
7+
// The waitgroup command applies the golang.org/x/tools/go/analysis/passes/waitgroup
8+
// analysis to the specified packages of Go source code.
9+
package main
10+
11+
import (
12+
"golang.org/x/tools/go/analysis/passes/waitgroup"
13+
"golang.org/x/tools/go/analysis/singlechecker"
14+
)
15+
16+
func main() { singlechecker.Main(waitgroup.Analyzer) }

Diff for: ‎go/analysis/passes/waitgroup/testdata/src/a/a.go

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package a
2+
3+
import "sync"
4+
5+
func f() {
6+
var wg sync.WaitGroup
7+
wg.Add(1) // ok
8+
go func() {
9+
wg.Add(1) // want "WaitGroup.Add called from inside new goroutine"
10+
// ...
11+
wg.Add(1) // ok
12+
}()
13+
wg.Add(1) // ok
14+
}

Diff for: ‎go/analysis/passes/waitgroup/waitgroup.go

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright 2023 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Package waitgroup defines an Analyzer that detects simple misuses
6+
// of sync.WaitGroup.
7+
package waitgroup
8+
9+
import (
10+
_ "embed"
11+
"go/ast"
12+
"go/types"
13+
"reflect"
14+
15+
"golang.org/x/tools/go/analysis"
16+
"golang.org/x/tools/go/analysis/passes/inspect"
17+
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
18+
"golang.org/x/tools/go/ast/inspector"
19+
"golang.org/x/tools/go/types/typeutil"
20+
"golang.org/x/tools/internal/typesinternal"
21+
)
22+
23+
//go:embed doc.go
24+
var doc string
25+
26+
var Analyzer = &analysis.Analyzer{
27+
Name: "waitgroup",
28+
Doc: analysisutil.MustExtractDoc(doc, "waitgroup"),
29+
URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/waitgroup",
30+
Requires: []*analysis.Analyzer{inspect.Analyzer},
31+
Run: run,
32+
}
33+
34+
func run(pass *analysis.Pass) (any, error) {
35+
if !analysisutil.Imports(pass.Pkg, "sync") {
36+
return nil, nil // doesn't directly import sync
37+
}
38+
39+
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
40+
nodeFilter := []ast.Node{
41+
(*ast.CallExpr)(nil),
42+
}
43+
44+
inspect.WithStack(nodeFilter, func(n ast.Node, push bool, stack []ast.Node) (proceed bool) {
45+
if push {
46+
call := n.(*ast.CallExpr)
47+
if fn, ok := typeutil.Callee(pass.TypesInfo, call).(*types.Func); ok &&
48+
isMethodNamed(fn, "sync", "WaitGroup", "Add") &&
49+
hasSuffix(stack, wantSuffix) &&
50+
backindex(stack, 1) == backindex(stack, 2).(*ast.BlockStmt).List[0] { // ExprStmt must be Block's first stmt
51+
52+
pass.Reportf(call.Lparen, "WaitGroup.Add called from inside new goroutine")
53+
}
54+
}
55+
return true
56+
})
57+
58+
return nil, nil
59+
}
60+
61+
// go func() {
62+
// wg.Add(1)
63+
// ...
64+
// }()
65+
var wantSuffix = []ast.Node{
66+
(*ast.GoStmt)(nil),
67+
(*ast.CallExpr)(nil),
68+
(*ast.FuncLit)(nil),
69+
(*ast.BlockStmt)(nil),
70+
(*ast.ExprStmt)(nil),
71+
(*ast.CallExpr)(nil),
72+
}
73+
74+
// hasSuffix reports whether stack has the matching suffix,
75+
// considering only node types.
76+
func hasSuffix(stack, suffix []ast.Node) bool {
77+
// TODO(adonovan): the inspector could implement this for us.
78+
if len(stack) < len(suffix) {
79+
return false
80+
}
81+
for i := range len(suffix) {
82+
if reflect.TypeOf(backindex(stack, i)) != reflect.TypeOf(backindex(suffix, i)) {
83+
return false
84+
}
85+
}
86+
return true
87+
}
88+
89+
// isMethodNamed reports whether f is a method with the specified
90+
// package, receiver type, and method names.
91+
func isMethodNamed(fn *types.Func, pkg, recv, name string) bool {
92+
if fn.Pkg() != nil && fn.Pkg().Path() == pkg && fn.Name() == name {
93+
if r := fn.Type().(*types.Signature).Recv(); r != nil {
94+
if _, gotRecv := typesinternal.ReceiverNamed(r); gotRecv != nil {
95+
return gotRecv.Obj().Name() == recv
96+
}
97+
}
98+
}
99+
return false
100+
}
101+
102+
// backindex is like [slices.Index] but from the back of the slice.
103+
func backindex[T any](slice []T, i int) T {
104+
return slice[len(slice)-1-i]
105+
}

Diff for: ‎go/analysis/passes/waitgroup/waitgroup_test.go

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package waitgroup_test
6+
7+
import (
8+
"testing"
9+
10+
"golang.org/x/tools/go/analysis/analysistest"
11+
"golang.org/x/tools/go/analysis/passes/waitgroup"
12+
)
13+
14+
func Test(t *testing.T) {
15+
analysistest.Run(t, analysistest.TestData(), waitgroup.Analyzer, "a")
16+
}

Diff for: ‎go/gcexportdata/gcexportdata.go

+8-14
Original file line numberDiff line numberDiff line change
@@ -106,24 +106,18 @@ func Find(importPath, srcDir string) (filename, path string) {
106106
// additional trailing data beyond the end of the export data.
107107
func NewReader(r io.Reader) (io.Reader, error) {
108108
buf := bufio.NewReader(r)
109-
_, size, err := gcimporter.FindExportData(buf)
109+
size, err := gcimporter.FindExportData(buf)
110110
if err != nil {
111111
return nil, err
112112
}
113113

114-
if size >= 0 {
115-
// We were given an archive and found the __.PKGDEF in it.
116-
// This tells us the size of the export data, and we don't
117-
// need to return the entire file.
118-
return &io.LimitedReader{
119-
R: buf,
120-
N: size,
121-
}, nil
122-
} else {
123-
// We were given an object file. As such, we don't know how large
124-
// the export data is and must return the entire file.
125-
return buf, nil
126-
}
114+
// We were given an archive and found the __.PKGDEF in it.
115+
// This tells us the size of the export data, and we don't
116+
// need to return the entire file.
117+
return &io.LimitedReader{
118+
R: buf,
119+
N: size,
120+
}, nil
127121
}
128122

129123
// readAll works the same way as io.ReadAll, but avoids allocations and copies

Diff for: ‎go/packages/packages_test.go

+45
Original file line numberDiff line numberDiff line change
@@ -3177,6 +3177,51 @@ func TestIssue69606b(t *testing.T) {
31773177
}
31783178
}
31793179

3180+
// TestIssue70394 tests materializing an alias type defined in a package (m/a)
3181+
// in another package (m/b) where the types for m/b are coming from the compiler,
3182+
// e.g. `go list -compiled=true ... m/b`.
3183+
func TestIssue70394(t *testing.T) {
3184+
// TODO(taking): backport https://go.dev/cl/604099 so that this works on 23.
3185+
testenv.NeedsGo1Point(t, 24)
3186+
testenv.NeedsTool(t, "go") // requires go list.
3187+
testenv.NeedsGoBuild(t) // requires the compiler for export data.
3188+
3189+
t.Setenv("GODEBUG", "gotypesalias=1")
3190+
3191+
dir := t.TempDir()
3192+
overlay := map[string][]byte{
3193+
filepath.Join(dir, "go.mod"): []byte("module m"), // go version of the module does not matter.
3194+
filepath.Join(dir, "a/a.go"): []byte(`package a; type A = int32`),
3195+
filepath.Join(dir, "b/b.go"): []byte(`package b; import "m/a"; var V a.A`),
3196+
}
3197+
cfg := &packages.Config{
3198+
Dir: dir,
3199+
Mode: packages.NeedTypes, // just NeedsTypes allows for loading export data.
3200+
Overlay: overlay,
3201+
Env: append(os.Environ(), "GOFLAGS=-mod=vendor", "GOWORK=off"),
3202+
}
3203+
pkgs, err := packages.Load(cfg, "m/b")
3204+
if err != nil {
3205+
t.Fatal(err)
3206+
}
3207+
if errs := packages.PrintErrors(pkgs); errs > 0 {
3208+
t.Fatalf("Got %d errors while loading packages.", errs)
3209+
}
3210+
if len(pkgs) != 1 {
3211+
t.Fatalf("Loaded %d packages. expected 1", len(pkgs))
3212+
}
3213+
3214+
pkg := pkgs[0]
3215+
scope := pkg.Types.Scope()
3216+
obj := scope.Lookup("V")
3217+
if obj == nil {
3218+
t.Fatalf("Failed to find object %q in package %q", "V", pkg)
3219+
}
3220+
if _, ok := obj.Type().(*types.Alias); !ok {
3221+
t.Errorf("Object %q has type %q. expected an alias", obj, obj.Type())
3222+
}
3223+
}
3224+
31803225
// TestNeedTypesInfoOnly tests when NeedTypesInfo was set and NeedSyntax & NeedTypes were not,
31813226
// Load should include the TypesInfo of packages properly
31823227
func TestLoadTypesInfoWithoutSyntaxOrTypes(t *testing.T) {

Diff for: ‎gopls/doc/analyzers.md

+31
Original file line numberDiff line numberDiff line change
@@ -976,6 +976,37 @@ Default: off. Enable by setting `"analyses": {"useany": true}`.
976976

977977
Package documentation: [useany](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/useany)
978978

979+
<a id='waitgroup'></a>
980+
## `waitgroup`: check for misuses of sync.WaitGroup
981+
982+
983+
This analyzer detects mistaken calls to the (*sync.WaitGroup).Add
984+
method from inside a new goroutine, causing Add to race with Wait:
985+
986+
// WRONG
987+
var wg sync.WaitGroup
988+
go func() {
989+
wg.Add(1) // "WaitGroup.Add called from inside new goroutine"
990+
defer wg.Done()
991+
...
992+
}()
993+
wg.Wait() // (may return prematurely before new goroutine starts)
994+
995+
The correct code calls Add before starting the goroutine:
996+
997+
// RIGHT
998+
var wg sync.WaitGroup
999+
wg.Add(1)
1000+
go func() {
1001+
defer wg.Done()
1002+
...
1003+
}()
1004+
wg.Wait()
1005+
1006+
Default: on.
1007+
1008+
Package documentation: [waitgroup](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/waitgroup)
1009+
9791010
<a id='yield'></a>
9801011
## `yield`: report calls to yield where the result is ignored
9811012

Diff for: ‎gopls/doc/features/diagnostics.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,13 @@ func main() {
213213
}
214214
```
215215

216-
The quick fix would be:
216+
The quick fix would insert a declaration with a default
217+
value inferring its type from the context:
217218

218219
```go
219220
func main() {
220221
x := 42
221-
y :=
222+
y := 0
222223
min(x, y)
223224
}
224225
```

0 commit comments

Comments
 (0)