Skip to content

Commit 99e8fee

Browse files
adonovangopherbot
authored andcommitted
x/tools: fset.File(file.Pos()) -> fset.File(file.FileStart)
This CL fixes a bug (#70149) in gopls/internal/golang/pkgdoc.go in which a call to fset.File(file.Pos()) would return nil because when file points to an empty ast.File, Pos() returns NoPos. Instead, we should use file.FileStart, which is (in principle) always valid even for an empty file. However, there is a separate bug in go1.23 (#70162) that means FileStart is invalid whenever Pos() is. So, this fix only works with go1.24, and there's no real workaround short of the additional logic this CL adds to parsego.Parse, which at least covers all of gopls. Also, we audit all of x/tools for similar faulty uses of Pos() and replace them with FileStart. In future, we should use File.Pos only for its specific meaning related to the package decl. Fixes golang/go#70149 Updates golang/go#70162 Change-Id: Ic8cedfe912e44a0b4eb6e5e6874a6266d4be9076 Reviewed-on: https://go-review.googlesource.com/c/tools/+/624437 Reviewed-by: Robert Findley <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 5cd08e2 commit 99e8fee

File tree

33 files changed

+78
-54
lines changed

33 files changed

+78
-54
lines changed

go/analysis/passes/cgocall/cgocall.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func typeCheckCgoSourceFiles(fset *token.FileSet, pkg *types.Package, files []*a
179179
for _, raw := range files {
180180
// If f is a cgo-generated file, Position reports
181181
// the original file, honoring //line directives.
182-
filename := fset.Position(raw.Pos()).Filename
182+
filename := fset.Position(raw.Pos()).Filename // sic: Pos, not FileStart
183183
f, err := parser.ParseFile(fset, filename, nil, parser.SkipObjectResolution)
184184
if err != nil {
185185
return nil, nil, fmt.Errorf("can't parse raw cgo file: %v", err)

go/analysis/passes/tests/tests.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ var acceptedFuzzTypes = []types.Type{
4848

4949
func run(pass *analysis.Pass) (interface{}, error) {
5050
for _, f := range pass.Files {
51-
if !strings.HasSuffix(pass.Fset.File(f.Pos()).Name(), "_test.go") {
51+
if !strings.HasSuffix(pass.Fset.File(f.FileStart).Name(), "_test.go") {
5252
continue
5353
}
5454
for _, decl := range f.Decls {

go/ast/inspector/inspector.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ func (in *Inspector) WithStack(types []ast.Node, f func(n ast.Node, push bool, s
180180
// traverse builds the table of events representing a traversal.
181181
func traverse(files []*ast.File) []event {
182182
// Preallocate approximate number of events
183-
// based on source file extent.
183+
// based on source file extent of the declarations.
184+
// (We use End-Pos not FileStart-FileEnd to neglect
185+
// the effect of long doc comments.)
184186
// This makes traverse faster by 4x (!).
185187
var extent int
186188
for _, f := range files {

go/callgraph/rta/rta_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func TestRTA(t *testing.T) {
100100
//
101101
// Functions are notated as if by ssa.Function.String.
102102
func check(t *testing.T, f *ast.File, pkg *ssa.Package, res *rta.Result) {
103-
tokFile := pkg.Prog.Fset.File(f.Pos())
103+
tokFile := pkg.Prog.Fset.File(f.FileStart)
104104

105105
// Find the WANT comment.
106106
expectation := func(f *ast.File) (string, int) {

go/loader/loader.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -340,13 +340,12 @@ func (conf *Config) addImport(path string, tests bool) {
340340
func (prog *Program) PathEnclosingInterval(start, end token.Pos) (pkg *PackageInfo, path []ast.Node, exact bool) {
341341
for _, info := range prog.AllPackages {
342342
for _, f := range info.Files {
343-
if f.Pos() == token.NoPos {
344-
// This can happen if the parser saw
345-
// too many errors and bailed out.
346-
// (Use parser.AllErrors to prevent that.)
343+
if f.FileStart == token.NoPos {
344+
// Workaround for #70162 (undefined FileStart).
345+
// TODO(adonovan): delete once go1.24 is assured.
347346
continue
348347
}
349-
if !tokenFileContainsPos(prog.Fset.File(f.Pos()), start) {
348+
if !tokenFileContainsPos(prog.Fset.File(f.FileStart), start) {
350349
continue
351350
}
352351
if path, exact := astutil.PathEnclosingInterval(f, start, end); path != nil {

go/packages/packages_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2684,7 +2684,7 @@ func testIssue48226(t *testing.T, exporter packagestest.Exporter) {
26842684
t.Fatalf("package has errors: %v", pkg.Errors)
26852685
}
26862686

2687-
fname := pkg.Fset.File(pkg.Syntax[0].Pos()).Name()
2687+
fname := pkg.Fset.File(pkg.Syntax[0].FileStart).Name()
26882688
if filepath.Base(fname) != "syntax.go" {
26892689
t.Errorf("expected the package declaration position "+
26902690
"to resolve to \"syntax.go\", got %q instead", fname)

go/types/internal/play/play.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func handleSelectJSON(w http.ResponseWriter, req *http.Request) {
113113

114114
fset := pkg.Fset
115115
file := pkg.Syntax[0]
116-
tokFile := fset.File(file.Pos())
116+
tokFile := fset.File(file.FileStart)
117117
startPos := tokFile.Pos(startOffset)
118118
endPos := tokFile.Pos(endOffset)
119119

godoc/index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ func (x *Indexer) addFile(f vfs.ReadSeekCloser, filename string, goFile bool) (f
649649
if goFile {
650650
// parse the file and in the process add it to the file set
651651
if ast, err = parser.ParseFile(x.fset, filename, src, parser.ParseComments); err == nil {
652-
file = x.fset.File(ast.Pos()) // ast.Pos() is inside the file
652+
file = x.fset.File(ast.FileStart) // ast.FileStart is inside the file
653653
return
654654
}
655655
// file has parse errors, and the AST may be incorrect -

gopls/doc/generate/generate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ func lowerFirst(x string) string {
533533
func fileForPos(pkg *packages.Package, pos token.Pos) (*ast.File, error) {
534534
fset := pkg.Fset
535535
for _, f := range pkg.Syntax {
536-
if safetoken.StartPosition(fset, f.Pos()).Filename == safetoken.StartPosition(fset, pos).Filename {
536+
if safetoken.StartPosition(fset, f.FileStart).Filename == safetoken.StartPosition(fset, pos).Filename {
537537
return f, nil
538538
}
539539
}

gopls/internal/analysis/fillreturns/fillreturns.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ outer:
4545
}
4646
var file *ast.File
4747
for _, f := range pass.Files {
48-
if f.Pos() <= typeErr.Pos && typeErr.Pos <= f.End() {
48+
if f.FileStart <= typeErr.Pos && typeErr.Pos <= f.FileEnd {
4949
file = f
5050
break
5151
}

0 commit comments

Comments
 (0)