Skip to content

Commit b2d9e77

Browse files
adonovangopherbot
authored andcommitted
gopls/internal/analysis/yield: improve diagnostic
This change adds the location of the second (reached) yield call to the diagnostic in the form of RelatedInformation. Also, for both the primary and related positions, it now reports the start and end range of the enclosing yield CallExpr, instead of the single point of the SSA Call instruction, which is just the CallExpr.Lparen. (We should aim always to report meaningful ranges, not just points.) Tested interactively, since analysistest doesn't yet have a means of testing related information, nor do we generally assert column information; but see golang/go#75800. Change-Id: I70d54375c33ef3c85ce6dd7b53803b9f961e42dc Reviewed-on: https://go-review.googlesource.com/c/tools/+/712360 Reviewed-by: Markus Kusano <kusano@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Alan Donovan <adonovan@google.com>
1 parent e0917dc commit b2d9e77

File tree

1 file changed

+34
-2
lines changed

1 file changed

+34
-2
lines changed

gopls/internal/analysis/yield/yield.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"golang.org/x/tools/go/ssa"
3232
"golang.org/x/tools/gopls/internal/util/safetoken"
3333
"golang.org/x/tools/internal/analysisinternal"
34+
"golang.org/x/tools/internal/moreiters"
3435
)
3536

3637
//go:embed doc.go
@@ -121,14 +122,45 @@ func run(pass *analysis.Pass) (any, error) {
121122
for _, instr := range b.Instrs[start:] {
122123
switch instr := instr.(type) {
123124
case *ssa.Call:
125+
126+
// Precondition: v has a pos within a CallExpr.
127+
enclosingCall := func(v ssa.Value) ast.Node {
128+
pos := v.Pos()
129+
cur, ok := inspector.Root().FindByPos(pos, pos)
130+
if !ok {
131+
panic(fmt.Sprintf("can't find node at %v", safetoken.StartPosition(pass.Fset, pos)))
132+
}
133+
call, ok := moreiters.First(cur.Enclosing((*ast.CallExpr)(nil)))
134+
if !ok {
135+
panic(fmt.Sprintf("no call enclosing %v", safetoken.StartPosition(pass.Fset, pos)))
136+
}
137+
return call.Node()
138+
}
139+
124140
if !info.reported && ssaYieldCalls[instr] != nil {
125141
info.reported = true
126-
where := "" // "" => same yield call (a loop)
142+
var (
143+
where = "" // "" => same yield call (a loop)
144+
related []analysis.RelatedInformation
145+
)
146+
// Also report location of reached yield call, if distinct.
127147
if instr != call {
128148
otherLine := safetoken.StartPosition(pass.Fset, instr.Pos()).Line
129149
where = fmt.Sprintf("(on L%d) ", otherLine)
150+
otherCallExpr := enclosingCall(instr)
151+
related = []analysis.RelatedInformation{{
152+
Pos: otherCallExpr.Pos(),
153+
End: otherCallExpr.End(),
154+
Message: "other call here",
155+
}}
130156
}
131-
pass.Reportf(call.Pos(), "yield may be called again %safter returning false", where)
157+
callExpr := enclosingCall(call)
158+
pass.Report(analysis.Diagnostic{
159+
Pos: callExpr.Pos(),
160+
End: callExpr.End(),
161+
Message: fmt.Sprintf("yield may be called again %safter returning false", where),
162+
Related: related,
163+
})
132164
}
133165
case *ssa.If:
134166
// Visit both successors, unless cond is yield() or its negation.

0 commit comments

Comments
 (0)