Skip to content

Commit 0dc10dc

Browse files
committed
gopls/internal/analysis/gofix: use cursor API
Use a cursor for Pass 2, to simplify the code. For golang/go#32816. Change-Id: Ib7ea08636d0cb2bb6274aee4767343fcc98361c7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/647299 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 2088703 commit 0dc10dc

File tree

1 file changed

+28
-32
lines changed

1 file changed

+28
-32
lines changed

gopls/internal/analysis/gofix/gofix.go

+28-32
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import (
1616
"golang.org/x/tools/go/analysis/passes/inspect"
1717
"golang.org/x/tools/go/ast/inspector"
1818
"golang.org/x/tools/go/types/typeutil"
19+
"golang.org/x/tools/gopls/internal/util/moreiters"
1920
"golang.org/x/tools/internal/analysisinternal"
21+
"golang.org/x/tools/internal/astutil/cursor"
22+
"golang.org/x/tools/internal/astutil/edge"
2023
"golang.org/x/tools/internal/diff"
2124
"golang.org/x/tools/internal/refactor/inline"
2225
"golang.org/x/tools/internal/typesinternal"
@@ -51,6 +54,12 @@ func run(pass *analysis.Pass) (any, error) {
5154
return content, nil
5255
}
5356

57+
// Return the unique ast.File for a cursor.
58+
currentFile := func(c cursor.Cursor) *ast.File {
59+
cf, _ := moreiters.First(c.Ancestors((*ast.File)(nil)))
60+
return cf.Node().(*ast.File)
61+
}
62+
5463
// Pass 1: find functions and constants annotated with an appropriate "//go:fix"
5564
// comment (the syntax proposed by #32816),
5665
// and export a fact for each one.
@@ -150,19 +159,8 @@ func run(pass *analysis.Pass) (any, error) {
150159
// and forward each reference to a forwardable constant.
151160
//
152161
// TODO(adonovan): handle multiple diffs that each add the same import.
153-
nodeFilter = []ast.Node{
154-
(*ast.File)(nil),
155-
(*ast.CallExpr)(nil),
156-
(*ast.SelectorExpr)(nil),
157-
(*ast.Ident)(nil),
158-
}
159-
var currentFile *ast.File
160-
var currentSel *ast.SelectorExpr
161-
inspect.Preorder(nodeFilter, func(n ast.Node) {
162-
if file, ok := n.(*ast.File); ok {
163-
currentFile = file
164-
return
165-
}
162+
for cur := range cursor.Root(inspect).Preorder((*ast.CallExpr)(nil), (*ast.Ident)(nil)) {
163+
n := cur.Node()
166164
switch n := n.(type) {
167165
case *ast.CallExpr:
168166
call := n
@@ -177,27 +175,28 @@ func run(pass *analysis.Pass) (any, error) {
177175
}
178176
}
179177
if callee == nil {
180-
return // nope
178+
continue // nope
181179
}
182180

183181
// Inline the call.
184182
content, err := readFile(call)
185183
if err != nil {
186184
pass.Reportf(call.Lparen, "invalid inlining candidate: cannot read source file: %v", err)
187-
return
185+
continue
188186
}
187+
curFile := currentFile(cur)
189188
caller := &inline.Caller{
190189
Fset: pass.Fset,
191190
Types: pass.Pkg,
192191
Info: pass.TypesInfo,
193-
File: currentFile,
192+
File: curFile,
194193
Call: call,
195194
Content: content,
196195
}
197196
res, err := inline.Inline(caller, callee, &inline.Options{Logf: discard})
198197
if err != nil {
199198
pass.Reportf(call.Lparen, "%v", err)
200-
return
199+
continue
201200
}
202201
if res.Literalized {
203202
// Users are not fond of inlinings that literalize
@@ -207,16 +206,16 @@ func run(pass *analysis.Pass) (any, error) {
207206
// and often literalizes when it cannot prove that
208207
// reducing the call is safe; the user of this tool
209208
// has no indication of what the problem is.)
210-
return
209+
continue
211210
}
212211
got := res.Content
213212

214213
// Suggest the "fix".
215214
var textEdits []analysis.TextEdit
216215
for _, edit := range diff.Bytes(content, got) {
217216
textEdits = append(textEdits, analysis.TextEdit{
218-
Pos: currentFile.FileStart + token.Pos(edit.Start),
219-
End: currentFile.FileStart + token.Pos(edit.End),
217+
Pos: curFile.FileStart + token.Pos(edit.Start),
218+
End: curFile.FileStart + token.Pos(edit.End),
220219
NewText: []byte(edit.New),
221220
})
222221
}
@@ -231,9 +230,6 @@ func run(pass *analysis.Pass) (any, error) {
231230
})
232231
}
233232

234-
case *ast.SelectorExpr:
235-
currentSel = n
236-
237233
case *ast.Ident:
238234
// If the identifier is a use of a forwardable constant, suggest forwarding it.
239235
if con, ok := pass.TypesInfo.Uses[n].(*types.Const); ok {
@@ -246,15 +242,15 @@ func run(pass *analysis.Pass) (any, error) {
246242
}
247243
}
248244
if fcon == nil {
249-
return // nope
245+
continue // nope
250246
}
251247

252248
// If n is qualified by a package identifier, we'll need the full selector expression.
253249
var sel *ast.SelectorExpr
254-
if currentSel != nil && n == currentSel.Sel {
255-
sel = currentSel
256-
currentSel = nil
250+
if e, _ := cur.Edge(); e == edge.SelectorExpr_Sel {
251+
sel = cur.Parent().Node().(*ast.SelectorExpr)
257252
}
253+
curFile := currentFile(cur)
258254

259255
// We have an identifier A here (n), possibly qualified by a package identifier (sel.X),
260256
// and a forwardable "const A = B" elsewhere (fcon).
@@ -267,16 +263,16 @@ func run(pass *analysis.Pass) (any, error) {
267263
// are in the current package.
268264
if pass.Pkg.Path() == fcon.RHSPkgPath {
269265
// fcon.rhsObj is the object referred to by B in the definition of A.
270-
scope := pass.TypesInfo.Scopes[currentFile].Innermost(n.Pos()) // n's scope
271-
_, obj := scope.LookupParent(fcon.RHSName, n.Pos()) // what "B" means in n's scope
266+
scope := pass.TypesInfo.Scopes[curFile].Innermost(n.Pos()) // n's scope
267+
_, obj := scope.LookupParent(fcon.RHSName, n.Pos()) // what "B" means in n's scope
272268
if obj == nil {
273269
// Should be impossible: if code at n can refer to the LHS,
274270
// it can refer to the RHS.
275271
panic(fmt.Sprintf("no object for forwardable const %s RHS %s", n.Name, fcon.RHSName))
276272
}
277273
if obj != fcon.rhsObj {
278274
// "B" means something different here than at the forwardable const's scope.
279-
return
275+
continue
280276
}
281277
}
282278
importPrefix := ""
@@ -285,7 +281,7 @@ func run(pass *analysis.Pass) (any, error) {
285281
// TODO(jba): fix AddImport so that it returns "." if an existing dot import will work.
286282
// We will need to tell AddImport the name of the identifier we want to qualify (fcon.RHSName here).
287283
importID, eds := analysisinternal.AddImport(
288-
pass.TypesInfo, currentFile, n.Pos(), fcon.RHSPkgPath, fcon.RHSPkgName)
284+
pass.TypesInfo, curFile, n.Pos(), fcon.RHSPkgPath, fcon.RHSPkgName)
289285
importPrefix = importID + "."
290286
edits = eds
291287
}
@@ -316,7 +312,7 @@ func run(pass *analysis.Pass) (any, error) {
316312
})
317313
}
318314
}
319-
})
315+
}
320316

321317
return nil, nil
322318
}

0 commit comments

Comments
 (0)