Skip to content

Commit 1f054fd

Browse files
committed
x/tools: downgrade token.FileSet parameters to token.File
Various functions are inherently concerned with exactly one file. By passing a token.File instead of a whole FileSet as a parameter, this is more obvious at a glance; and a callee can't modify a FileSet it doesn't have. This CL does this for a number of internal functions. (What's the downside? Formatting an ast.Node does require a FileSet, even when the tree contains nodes only from one file, as usual. However, we can construct an ephemeral singleton FileSet using AddExistingFiles should the need arise.) Change-Id: If8a38af220f105f581ffb4fd7d554f795b55847c Reviewed-on: https://go-review.googlesource.com/c/tools/+/710255 Reviewed-by: Robert Findley <[email protected]> Commit-Queue: Alan Donovan <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 44e71e5 commit 1f054fd

File tree

18 files changed

+75
-72
lines changed

18 files changed

+75
-72
lines changed

go/analysis/internal/checker/fix_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ var markerAnalyzer = &analysis.Analyzer{
431431
if err != nil {
432432
return nil, err
433433
}
434-
notes, err := expect.ExtractGo(pass.Fset, file)
434+
notes, err := expect.ExtractGo(tokFile, file)
435435
if err != nil {
436436
return nil, err
437437
}

go/analysis/passes/modernize/errorsastype.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func errorsastype(pass *analysis.Pass) (any, error) {
148148
Message: fmt.Sprintf("Replace errors.As with AsType[%s]", errtype),
149149
TextEdits: append(
150150
// delete "var myerr *MyErr"
151-
analysisinternal.DeleteStmt(pass.Fset, curDeclStmt),
151+
analysisinternal.DeleteStmt(pass.Fset.File(call.Fun.Pos()), curDeclStmt),
152152
// if errors.As (err, &myerr) { ... }
153153
// ------------- -------------- -------- ----
154154
// if myerr, ok := errors.AsType[*MyErr](err ); ok { ... }

go/analysis/passes/modernize/forvar.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func forvar(pass *analysis.Pass) (any, error) {
7171
isLoopVarRedecl(assign) {
7272

7373
curStmt, _ := curLoop.FindNode(stmt)
74-
edits := analysisinternal.DeleteStmt(pass.Fset, curStmt)
74+
edits := analysisinternal.DeleteStmt(pass.Fset.File(stmt.Pos()), curStmt)
7575
if len(edits) > 0 {
7676
pass.Report(analysis.Diagnostic{
7777
Pos: stmt.Pos(),

go/analysis/passes/modernize/waitgroup.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ func waitgroup(pass *analysis.Pass) (any, error) {
130130
if !fileUses(info, file, "go1.25") {
131131
continue
132132
}
133+
tokFile := pass.Fset.File(file.Pos())
133134

134135
var addCallRecvText bytes.Buffer
135136
err := printer.Fprint(&addCallRecvText, pass.Fset, addCallRecv)
@@ -145,9 +146,9 @@ func waitgroup(pass *analysis.Pass) (any, error) {
145146
Message: "Simplify by using WaitGroup.Go",
146147
TextEdits: slices.Concat(
147148
// delete "wg.Add(1)"
148-
analysisinternal.DeleteStmt(pass.Fset, curAddStmt),
149+
analysisinternal.DeleteStmt(tokFile, curAddStmt),
149150
// delete "wg.Done()" or "defer wg.Done()"
150-
analysisinternal.DeleteStmt(pass.Fset, curDoneStmt),
151+
analysisinternal.DeleteStmt(tokFile, curDoneStmt),
151152
[]analysis.TextEdit{
152153
// go func()
153154
// ------

go/ssa/builder_generic_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,10 @@ func TestGenericBodies(t *testing.T) {
521521
t.Parallel()
522522
ssapkg, ppkg := buildPackage(t, content, ssa.SanityCheckFunctions)
523523
fset := ssapkg.Prog.Fset
524+
file := ppkg.Syntax[0]
524525

525526
// Collect all notes in f, i.e. comments starting with "//@ types".
526-
notes, err := expect.ExtractGo(fset, ppkg.Syntax[0])
527+
notes, err := expect.ExtractGo(fset.File(file.Pos()), file)
527528
if err != nil {
528529
t.Errorf("expect.ExtractGo: %v", err)
529530
}
@@ -759,7 +760,7 @@ func TestInstructionString(t *testing.T) {
759760
p.Build()
760761

761762
// Collect all notes in f, i.e. comments starting with "//@ instr".
762-
notes, err := expect.ExtractGo(prog.Fset, f)
763+
notes, err := expect.ExtractGo(prog.Fset.File(f.Pos()), f)
763764
if err != nil {
764765
t.Errorf("expect.ExtractGo: %v", err)
765766
}

go/ssa/builder_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,7 @@ func TestRangeOverInt(t *testing.T) {
13681368
}
13691369

13701370
// Collect all notes in f, i.e. comments starting with "//@ types".
1371-
notes, err := expect.ExtractGo(fset, f)
1371+
notes, err := expect.ExtractGo(fset.File(f.Pos()), f)
13721372
if err != nil {
13731373
t.Fatal(err)
13741374
}

go/ssa/source_test.go

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,12 @@ func TestObjValueLookup(t *testing.T) {
3333
}
3434
readFile := func(filename string) ([]byte, error) { return src, nil }
3535

36-
mode := ssa.GlobalDebug /*|ssa.PrintFunctions*/
37-
mainPkg, ppkg := buildPackage(t, string(src), mode)
38-
fset := ppkg.Fset
36+
var (
37+
mode = ssa.GlobalDebug /*|ssa.PrintFunctions*/
38+
mainPkg, ppkg = buildPackage(t, string(src), mode)
39+
f = ppkg.Syntax[0]
40+
tokFile = ppkg.Fset.File(f.Pos())
41+
)
3942

4043
// Maps each var Ident (represented "name:linenum") to the
4144
// kind of ssa.Value we expect (represented "Constant", "&Alloc").
@@ -44,35 +47,35 @@ func TestObjValueLookup(t *testing.T) {
4447
// Each note of the form @ssa(x, "BinOp") in testdata/objlookup.go
4548
// specifies an expectation that an object named x declared on the
4649
// same line is associated with an ssa.Value of type *ssa.BinOp.
47-
notes, err := expect.ExtractGo(fset, ppkg.Syntax[0])
50+
notes, err := expect.ExtractGo(tokFile, f)
4851
if err != nil {
4952
t.Fatal(err)
5053
}
5154
for _, n := range notes {
5255
if n.Name != "ssa" {
53-
t.Errorf("%v: unexpected note type %q, want \"ssa\"", fset.Position(n.Pos), n.Name)
56+
t.Errorf("%v: unexpected note type %q, want \"ssa\"", tokFile.Position(n.Pos), n.Name)
5457
continue
5558
}
5659
if len(n.Args) != 2 {
57-
t.Errorf("%v: ssa has %d args, want 2", fset.Position(n.Pos), len(n.Args))
60+
t.Errorf("%v: ssa has %d args, want 2", tokFile.Position(n.Pos), len(n.Args))
5861
continue
5962
}
6063
ident, ok := n.Args[0].(expect.Identifier)
6164
if !ok {
62-
t.Errorf("%v: got %v for arg 1, want identifier", fset.Position(n.Pos), n.Args[0])
65+
t.Errorf("%v: got %v for arg 1, want identifier", tokFile.Position(n.Pos), n.Args[0])
6366
continue
6467
}
6568
exp, ok := n.Args[1].(string)
6669
if !ok {
67-
t.Errorf("%v: got %v for arg 2, want string", fset.Position(n.Pos), n.Args[1])
70+
t.Errorf("%v: got %v for arg 2, want string", tokFile.Position(n.Pos), n.Args[1])
6871
continue
6972
}
70-
p, _, err := expect.MatchBefore(fset, readFile, n.Pos, string(ident))
73+
p, _, err := expect.MatchBefore(tokFile, readFile, n.Pos, string(ident))
7174
if err != nil {
7275
t.Error(err)
7376
continue
7477
}
75-
pos := fset.Position(p)
78+
pos := tokFile.Position(p)
7679
key := fmt.Sprintf("%s:%d", ident, pos.Line)
7780
expectations[key] = exp
7881
}
@@ -107,8 +110,8 @@ func TestObjValueLookup(t *testing.T) {
107110
// The result varies based on the specific Ident.
108111
for i, id := range varIds {
109112
obj := varObjs[i]
110-
ref, _ := astutil.PathEnclosingInterval(ppkg.Syntax[0], id.Pos(), id.Pos())
111-
pos := fset.Position(id.Pos())
113+
ref, _ := astutil.PathEnclosingInterval(f, id.Pos(), id.Pos())
114+
pos := tokFile.Position(id.Pos())
112115
exp := expectations[fmt.Sprintf("%s:%d", id.Name, pos.Line)]
113116
if exp == "" {
114117
t.Errorf("%s: no expectation for var ident %s ", pos, id.Name)
@@ -222,9 +225,12 @@ func testValueForExpr(t *testing.T, testfile string) {
222225
t.Fatal(err)
223226
}
224227

225-
mode := ssa.GlobalDebug /*|ssa.PrintFunctions*/
226-
mainPkg, ppkg := buildPackage(t, string(src), mode)
227-
fset, file := ppkg.Fset, ppkg.Syntax[0]
228+
var (
229+
mode = ssa.GlobalDebug /*|ssa.PrintFunctions*/
230+
mainPkg, ppkg = buildPackage(t, string(src), mode)
231+
file = ppkg.Syntax[0]
232+
tokFile = ppkg.Fset.File(file.Pos())
233+
)
228234

229235
if false {
230236
// debugging
@@ -245,7 +251,7 @@ func testValueForExpr(t *testing.T, testfile string) {
245251
return true
246252
})
247253

248-
notes, err := expect.ExtractGo(fset, file)
254+
notes, err := expect.ExtractGo(tokFile, file)
249255
if err != nil {
250256
t.Fatal(err)
251257
}
@@ -254,7 +260,7 @@ func testValueForExpr(t *testing.T, testfile string) {
254260
if want == "nil" {
255261
want = "<nil>"
256262
}
257-
position := fset.Position(n.Pos)
263+
position := tokFile.Position(n.Pos)
258264
var e ast.Expr
259265
for _, paren := range parenExprs {
260266
if paren.Pos() > n.Pos {

gopls/internal/analysis/unusedvariable/unusedvariable.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ func run(pass *analysis.Pass) (any, error) {
5555
continue // not the right identifier
5656
}
5757

58-
edits := analysisinternal.DeleteVar(pass.Fset, pass.TypesInfo, curId)
58+
tokFile := pass.Fset.File(ident.Pos())
59+
edits := analysisinternal.DeleteVar(tokFile, pass.TypesInfo, curId)
5960
if len(edits) > 0 {
6061
pass.Report(analysis.Diagnostic{
6162
Pos: ident.Pos(),

gopls/internal/cache/check.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2061,7 +2061,7 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, inputs *typeCheckInputs, errs [
20612061
//
20622062
// TODO(adonovan): It is the type checker's responsibility
20632063
// to ensure that (start, end) are meaningful; see #71803.
2064-
end = analysisinternal.TypeErrorEndPos(e.Fset, pgf.Src, start)
2064+
end = analysisinternal.TypeErrorEndPos(pgf.Tok, pgf.Src, start)
20652065

20662066
// debugging golang/go#65960
20672067
if _, err := safetoken.Offset(pgf.Tok, end); err != nil {

internal/analysisinternal/analysis.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,9 @@ import (
2929

3030
// Deprecated: this heuristic is ill-defined.
3131
// TODO(adonovan): move to sole use in gopls/internal/cache.
32-
func TypeErrorEndPos(fset *token.FileSet, src []byte, start token.Pos) token.Pos {
32+
func TypeErrorEndPos(tokFile *token.File, src []byte, start token.Pos) token.Pos {
3333
// Get the end position for the type error.
34-
file := fset.File(start)
35-
if file == nil {
36-
return start
37-
}
38-
if offset := file.PositionFor(start, false).Offset; offset > len(src) {
34+
if offset := tokFile.PositionFor(start, false).Offset; offset > len(src) {
3935
return start
4036
} else {
4137
src = src[offset:]
@@ -61,7 +57,7 @@ func TypeErrorEndPos(fset *token.FileSet, src []byte, start token.Pos) token.Pos
6157
s.Init(f, src, nil /* no error handler */, scanner.ScanComments)
6258
pos, tok, lit := s.Scan()
6359
if tok != token.SEMICOLON && token.Pos(f.Base()) <= pos && pos <= token.Pos(f.Base()+f.Size()) {
64-
off := file.Offset(pos) + len(lit)
60+
off := tokFile.Offset(pos) + len(lit)
6561
src = src[off:]
6662
end += token.Pos(off)
6763
}
@@ -527,17 +523,14 @@ func CanImport(from, to string) bool {
527523
// DeleteStmt returns the edits to remove the [ast.Stmt] identified by
528524
// curStmt, if it is contained within a BlockStmt, CaseClause,
529525
// CommClause, or is the STMT in switch STMT; ... {...}. It returns nil otherwise.
530-
//
531-
// TODO(adonovan): downgrade first param to a *token.File?
532-
func DeleteStmt(fset *token.FileSet, curStmt inspector.Cursor) []analysis.TextEdit {
526+
func DeleteStmt(tokFile *token.File, curStmt inspector.Cursor) []analysis.TextEdit {
533527
stmt := curStmt.Node().(ast.Stmt)
534528
// if the stmt is on a line by itself delete the whole line
535529
// otherwise just delete the statement.
536530

537531
// this logic would be a lot simpler with the file contents, and somewhat simpler
538532
// if the cursors included the comments.
539533

540-
tokFile := fset.File(stmt.Pos())
541534
lineOf := tokFile.Line
542535
stmtStartLine, stmtEndLine := lineOf(stmt.Pos()), lineOf(stmt.End())
543536

0 commit comments

Comments
 (0)