gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes

This analyzer detects failure to check the result of a call to
yield (which can cause a range loop to run beyond the sequence,
leading to a panic).

It is not always a mistake to ignore the
result of a call to yield; it depends on whether control can
reach another call to yield without checking that the first
call returned true. Consequently, this analyzer uses SSA
for control flow analysis.

We plan to add this analyzer to gopls before we promote it to vet.

+ test, relnote

Fixes golang/go#65795

Change-Id: I75fa272e2f546be0c2acb10a1978c82bc19db5bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/609617
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopls/internal/analysis/yield/testdata/src/a/a.go

@@ -83,6 +83,7 @@ func tricky(in io.ReadCloser) func(yield func(string, error) bool) {
 		}
 	}
 }
+<<<<<<< HEAD
 
 // Regression test for issue #70598.
 func shortCircuitAND(yield func(int) bool) {
@@ -118,3 +119,5 @@ func tricky3(yield func(int) bool) {
 		yield(3)
 	}
 }
+=======
+>>>>>>> 9b6e4f21d (gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes)

gopls/internal/analysis/yield/yield.go

@@ -20,7 +20,10 @@ import (
 	_ "embed"
 	"fmt"
 	"go/ast"
+<<<<<<< HEAD
 	"go/constant"
+=======
+>>>>>>> 9b6e4f21d (gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes)
 	"go/token"
 	"go/types"
 
@@ -120,6 +123,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 							// In that case visit only the "if !yield()" block.
 							cond := instr.Cond
 							t, f := b.Succs[0], b.Succs[1]
+<<<<<<< HEAD
 
 							// Strip off any NOT operator.
 							cond, t, f = unnegate(cond, t, f)
@@ -148,6 +152,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
 								}
 							}
 
+=======
+							if unop, ok := cond.(*ssa.UnOp); ok && unop.Op == token.NOT {
+								cond, t, f = unop.X, f, t
+							}
+>>>>>>> 9b6e4f21d (gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes)
 							if cond, ok := cond.(*ssa.Call); ok && ssaYieldCalls[cond] != nil {
 								// Skip the successor reached by "if yield() { ... }".
 							} else {
@@ -169,10 +178,13 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 	return nil, nil
 }
+<<<<<<< HEAD
 
 func unnegate(cond ssa.Value, t, f *ssa.BasicBlock) (_ ssa.Value, _, _ *ssa.BasicBlock) {
 	if unop, ok := cond.(*ssa.UnOp); ok && unop.Op == token.NOT {
 		return unop.X, f, t
 	}
 	return cond, t, f
 }
+=======
+>>>>>>> 9b6e4f21d (gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes)

gopls/internal/analysis/yield/yield_test.go

@@ -15,3 +15,4 @@ func Test(t *testing.T) {
 	testdata := analysistest.TestData()
 	analysistest.Run(t, testdata, yield.Analyzer, "a")
 }
+e

gopls/internal/doc/api.json

@@ -1304,12 +1304,15 @@
 			"Default": false
 		},
 		{
+<<<<<<< HEAD
 			"Name": "waitgroup",
 			"Doc": "check for misuses of sync.WaitGroup\n\nThis analyzer detects mistaken calls to the (*sync.WaitGroup).Add\nmethod from inside a new goroutine, causing Add to race with Wait:\n\n\t// WRONG\n\tvar wg sync.WaitGroup\n\tgo func() {\n\t        wg.Add(1) // \"WaitGroup.Add called from inside new goroutine\"\n\t        defer wg.Done()\n\t        ...\n\t}()\n\twg.Wait() // (may return prematurely before new goroutine starts)\n\nThe correct code calls Add before starting the goroutine:\n\n\t// RIGHT\n\tvar wg sync.WaitGroup\n\twg.Add(1)\n\tgo func() {\n\t\tdefer wg.Done()\n\t\t...\n\t}()\n\twg.Wait()",
 			"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/waitgroup",
 			"Default": true
 		},
 		{
+=======
+>>>>>>> 9b6e4f21d (gopls/internal/analysis/yield: analyzer for iterator (yield) mistakes)
 			"Name": "yield",
 			"Doc": "report calls to yield where the result is ignored\n\nAfter a yield function returns false, the caller should not call\nthe yield function again; generally the iterator should return\npromptly.\n\nThis example fails to check the result of the call to yield,\ncausing this analyzer to report a diagnostic:\n\n\tyield(1) // yield may be called again (on L2) after returning false\n\tyield(2)\n\nThe corrected code is either this:\n\n\tif yield(1) { yield(2) }\n\nor simply:\n\n\t_ = yield(1) \u0026\u0026 yield(2)\n\nIt is not always a mistake to ignore the result of yield.\nFor example, this is a valid single-element iterator:\n\n\tyield(1) // ok to ignore result\n\treturn\n\nIt is only a mistake when the yield call that returned false may be\nfollowed by another call.",
 			"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/yield",

gopls/internal/golang/assembly.go

@@ -1,4 +1,4 @@
-// Copyright 2024 The Go Authors. All rights reserved.
+ew// Copyright 2024 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.