Skip to content

Commit 6381f0b

Browse files
findleyrgopherbot
authored andcommitted
gopls/internal/cache: refine bug reports
Refine a few bug reports related to incorrectly positioned type checker errors, by differentiating the 'fixed file' case from the case with no fixed files. Since we haven't made progress on these bugs, we need more information to guide our debugging. If the bugs only occur in the presence of AST fixes, we can probably downgrade them to not be a bug: if we're fixing the AST, we won't display type checker errors anyway. Nevertheless, leave the bug reports for now, so that we can collect data. For golang/go#65960 For golang/go#66765 For golang/go#66766 Change-Id: I2060c897d249cdd5cc3e7bb183d3f563987bec57 Reviewed-on: https://go-review.googlesource.com/c/tools/+/621876 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Robert Findley <[email protected]>
1 parent 63e4449 commit 6381f0b

File tree

2 files changed

+37
-8
lines changed

2 files changed

+37
-8
lines changed

gopls/internal/cache/check.go

+28-8
Original file line numberDiff line numberDiff line change
@@ -1877,7 +1877,11 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, inputs *typeCheckInputs, errs [
18771877
// over fixed syntax, which overflowed its file. So it's definitely
18781878
// possible that we get here (it's hard to reason about fixing up the
18791879
// AST). Nevertheless, it's a bug.
1880-
bug.Reportf("internal error: type checker error %q outside its Fset", e)
1880+
if pkg.hasFixedFiles() {
1881+
bug.Reportf("internal error: type checker error %q outside its Fset (fixed files)", e)
1882+
} else {
1883+
bug.Reportf("internal error: type checker error %q outside its Fset", e)
1884+
}
18811885
continue
18821886
}
18831887
pgf, err := pkg.File(protocol.URIFromPath(posn.Filename))
@@ -1889,12 +1893,16 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, inputs *typeCheckInputs, errs [
18891893
// package (the message would be rather confusing), but we do want to
18901894
// report an error in the current package (golang/go#59005).
18911895
if i == 0 {
1892-
bug.Reportf("internal error: could not locate file for primary type checker error %v: %v", e, err)
1896+
if pkg.hasFixedFiles() {
1897+
bug.Reportf("internal error: could not locate file for primary type checker error %v: %v (fixed files)", e, err)
1898+
} else {
1899+
bug.Reportf("internal error: could not locate file for primary type checker error %v: %v", e, err)
1900+
}
18931901
}
18941902
continue
18951903
}
18961904

1897-
// debugging #65960
1905+
// debugging golang/go#65960
18981906
//
18991907
// At this point, we know 'start' IsValid, and
19001908
// StartPosition(start) worked (with e.Fset).
@@ -1903,21 +1911,33 @@ func typeErrorsToDiagnostics(pkg *syntaxPackage, inputs *typeCheckInputs, errs [
19031911
// is also in range for pgf.Tok, which means
19041912
// the PosRange failure must be caused by 'end'.
19051913
if pgf.Tok != e.Fset.File(start) {
1906-
bug.Reportf("internal error: inconsistent token.Files for pos")
1914+
if pkg.hasFixedFiles() {
1915+
bug.Reportf("internal error: inconsistent token.Files for pos (fixed files)")
1916+
} else {
1917+
bug.Reportf("internal error: inconsistent token.Files for pos")
1918+
}
19071919
}
19081920

19091921
if end == start {
19101922
// Expand the end position to a more meaningful span.
19111923
end = analysisinternal.TypeErrorEndPos(e.Fset, pgf.Src, start)
19121924

1913-
// debugging #65960
1925+
// debugging golang/go#65960
19141926
if _, err := safetoken.Offset(pgf.Tok, end); err != nil {
1915-
bug.Reportf("TypeErrorEndPos returned invalid end: %v", err)
1927+
if pkg.hasFixedFiles() {
1928+
bug.Reportf("TypeErrorEndPos returned invalid end: %v (fixed files)", err)
1929+
} else {
1930+
bug.Reportf("TypeErrorEndPos returned invalid end: %v", err)
1931+
}
19161932
}
19171933
} else {
1918-
// debugging #65960
1934+
// debugging golang/go#65960
19191935
if _, err := safetoken.Offset(pgf.Tok, end); err != nil {
1920-
bug.Reportf("ReadGo116ErrorData returned invalid end: %v", err)
1936+
if pkg.hasFixedFiles() {
1937+
bug.Reportf("ReadGo116ErrorData returned invalid end: %v (fixed files)", err)
1938+
} else {
1939+
bug.Reportf("ReadGo116ErrorData returned invalid end: %v", err)
1940+
}
19211941
}
19221942
}
19231943

gopls/internal/cache/package.go

+9
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"go/scanner"
1111
"go/token"
1212
"go/types"
13+
"slices"
1314
"sync"
1415

1516
"golang.org/x/tools/gopls/internal/cache/metadata"
@@ -87,6 +88,14 @@ func (p *syntaxPackage) tests() *testfuncs.Index {
8788
return p._tests
8889
}
8990

91+
// hasFixedFiles reports whether there are any 'fixed' compiled go files in the
92+
// package.
93+
//
94+
// Intended to be used to refine bug reports.
95+
func (p *syntaxPackage) hasFixedFiles() bool {
96+
return slices.ContainsFunc(p.compiledGoFiles, (*parsego.File).Fixed)
97+
}
98+
9099
func (p *Package) String() string { return string(p.metadata.ID) }
91100

92101
func (p *Package) Metadata() *metadata.Package { return p.metadata }

0 commit comments

Comments
 (0)