mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: add missing bug reporting, and panic during marker tests 

Add additional bug reporting related to position invariants, and update
marker test runners to panic on encountered bugs.

This revealed a panic on older Go versions, where we try to create a
range for a missing package name span. Lift the check into the caller
for this case, and leave a big comment explaining that
checkForOrphanedFile probably shouldn't be so tolerant of invalid calls.

Change-Id: Ie16a07afba0f8a5682cca50ad8f04450bfa2da65
Reviewed-on: https://go-review.googlesource.com/c/tools/+/407885
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
This commit is contained in:
Robert Findley 2022-05-23 17:45:46 -04:00
parent 0e859afa53
commit 2bb78cf04e
7 changed files with 33 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gopls/test/gopls_test.go
View File

@ -9,6 +9,7 @@ import (
"testing"
"golang.org/x/tools/gopls/internal/hooks"
"golang.org/x/tools/internal/lsp/bug"
cmdtest "golang.org/x/tools/internal/lsp/cmd/test"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/lsp/tests"
@ -16,6 +17,7 @@ import (
)
func TestMain(m *testing.M) {
bug.PanicOnBugs = true
testenv.ExitIfSmallMachine()
os.Exit(m.Run())
}

8
internal/lsp/bug/bug.go
View File

@ -49,6 +49,14 @@ func Reportf(format string, args ...interface{}) {
Report(fmt.Sprintf(format, args...), nil)
}
// Errorf calls fmt.Errorf for the given arguments, and reports the resulting
// error message as a bug.
func Errorf(format string, args ...interface{}) error {
err := fmt.Errorf(format, args...)
Report(err.Error(), nil)
return err
}
// Report records a new bug encountered on the server.
// It uses reflection to report the position of the immediate caller.
func Report(description string, data Data) {

2
internal/lsp/cmd/cmd_test.go
View File

@ -8,12 +8,14 @@ import (
"os"
"testing"
"golang.org/x/tools/internal/lsp/bug"
cmdtest "golang.org/x/tools/internal/lsp/cmd/test"
"golang.org/x/tools/internal/lsp/tests"
"golang.org/x/tools/internal/testenv"
)
func TestMain(m *testing.M) {
bug.PanicOnBugs = true
testenv.ExitIfSmallMachine()
os.Exit(m.Run())
}

11
internal/lsp/diagnostics.go
View File

@ -439,6 +439,14 @@ func (s *Server) showCriticalErrorStatus(ctx context.Context, snapshot source.Sn
// If they cannot and the workspace is not otherwise unloaded, it also surfaces
// a warning, suggesting that the user check the file for build tags.
func (s *Server) checkForOrphanedFile(ctx context.Context, snapshot source.Snapshot, fh source.VersionedFileHandle) *source.Diagnostic {
// TODO(rfindley): this function may fail to produce a diagnostic for a
// variety of reasons, some of which should probably not be ignored. For
// example, should this function be tolerant of the case where fh does not
// exist, or does not have a package name?
//
// It would be better to panic or report a bug in several of the cases below,
// so that we can move toward guaranteeing we show the user a meaningful
// error whenever it makes sense.
if snapshot.View().FileKind(fh) != source.Go {
return nil
}
@ -454,6 +462,9 @@ func (s *Server) checkForOrphanedFile(ctx context.Context, snapshot source.Snaps
if err != nil {
return nil
}
if !pgf.File.Name.Pos().IsValid() {
return nil
}
spn, err := span.NewRange(snapshot.FileSet(), pgf.File.Name.Pos(), pgf.File.Name.End()).Span()
if err != nil {
return nil

2
internal/lsp/lsp_test.go
View File

@ -15,6 +15,7 @@ import (
"strings"
"testing"
"golang.org/x/tools/internal/lsp/bug"
"golang.org/x/tools/internal/lsp/cache"
"golang.org/x/tools/internal/lsp/command"
"golang.org/x/tools/internal/lsp/diff"
@ -27,6 +28,7 @@ import (
)
func TestMain(m *testing.M) {
bug.PanicOnBugs = true
testenv.ExitIfSmallMachine()
os.Exit(m.Run())
}

2
internal/lsp/source/source_test.go
View File

@ -15,6 +15,7 @@ import (
"strings"
"testing"
"golang.org/x/tools/internal/lsp/bug"
"golang.org/x/tools/internal/lsp/cache"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/diff/myers"
@ -28,6 +29,7 @@ import (
)
func TestMain(m *testing.M) {
bug.PanicOnBugs = true
testenv.ExitIfSmallMachine()
os.Exit(m.Run())
}

12
internal/span/token.go
View File

@ -8,6 +8,8 @@ import (
"errors"
"fmt"
"go/token"
"golang.org/x/tools/internal/lsp/bug"
)
// Range represents a source code range in token.Pos form.
@ -29,7 +31,7 @@ type TokenConverter struct {
func NewRange(fset *token.FileSet, start, end token.Pos) Range {
file := fset.File(start)
if file == nil {
// TODO: report a bug here via the bug reporting API, once it is available.
bug.Reportf("nil file")
}
return Range{
Start: start,
@ -42,7 +44,7 @@ func NewRange(fset *token.FileSet, start, end token.Pos) Range {
// token.File.
func NewTokenConverter(f *token.File) *TokenConverter {
if f == nil {
// TODO: report a bug here using the bug reporting API.
bug.Reportf("nil file")
}
return &TokenConverter{file: f}
}
@ -151,12 +153,10 @@ func (s Span) Range(converter *TokenConverter) (Range, error) {
// go/token will panic if the offset is larger than the file's size,
// so check here to avoid panicking.
if s.Start().Offset() > file.Size() {
// TODO: report a bug here via the future bug reporting API.
return Range{}, fmt.Errorf("start offset %v is past the end of the file %v", s.Start(), file.Size())
return Range{}, bug.Errorf("start offset %v is past the end of the file %v", s.Start(), file.Size())
}
if s.End().Offset() > file.Size() {
// TODO: report a bug here via the future bug reporting API.
return Range{}, fmt.Errorf("end offset %v is past the end of the file %v", s.End(), file.Size())
return Range{}, bug.Errorf("end offset %v is past the end of the file %v", s.End(), file.Size())
}
return Range{
Start: file.Pos(s.Start().Offset()),