internal/lsp: use packagestest markers to test diagnostics
Add some basic tests for diagnostics using the new
go/packages/packagestest framework.
Change-Id: I6a7bfba6c392928a9eb123ab71ceb73785c12600
Reviewed-on: https://go-review.googlesource.com/c/145697
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
diff --git a/go/packages/packagestest/export.go b/go/packages/packagestest/export.go
index 2086b1c..c59f7c0 100644
--- a/go/packages/packagestest/export.go
+++ b/go/packages/packagestest/export.go
@@ -217,7 +217,10 @@
// This will panic if there is any kind of error trying to walk the file tree.
func MustCopyFileTree(root string) map[string]interface{} {
result := map[string]interface{}{}
- if err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+ if err := filepath.Walk(filepath.FromSlash(root), func(path string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
if info.IsDir() {
return nil
}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 1a38d73..2ca622b 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -63,23 +63,27 @@
}
func parseErrorPos(pkgErr packages.Error) (pos token.Position) {
- split := strings.Split(pkgErr.Pos, ":")
- if len(split) <= 1 {
- return pos
- }
- pos.Filename = split[0]
- line, err := strconv.ParseInt(split[1], 10, 64)
- if err != nil {
- return pos
- }
- pos.Line = int(line)
- if len(split) == 3 {
- col, err := strconv.ParseInt(split[2], 10, 64)
- if err != nil {
- return pos
- }
- pos.Column = int(col)
+ remainder1, first, hasLine := chop(pkgErr.Pos)
+ remainder2, second, hasColumn := chop(remainder1)
+ if hasLine && hasColumn {
+ pos.Filename = remainder2
+ pos.Line = second
+ pos.Column = first
+ } else if hasLine {
+ pos.Filename = remainder1
+ pos.Line = first
}
return pos
+}
+func chop(text string) (remainder string, value int, ok bool) {
+ i := strings.LastIndex(text, ":")
+ if i < 0 {
+ return text, 0, false
+ }
+ v, err := strconv.ParseInt(text[i+1:], 10, 64)
+ if err != nil {
+ return text, 0, false
+ }
+ return text[:i], int(v), true
}
diff --git a/internal/lsp/diagnostics_test.go b/internal/lsp/diagnostics_test.go
new file mode 100644
index 0000000..9f58584
--- /dev/null
+++ b/internal/lsp/diagnostics_test.go
@@ -0,0 +1,87 @@
+package lsp
+
+import (
+ "go/token"
+ "reflect"
+ "sort"
+ "strings"
+ "testing"
+
+ "golang.org/x/tools/go/packages"
+ "golang.org/x/tools/go/packages/packagestest"
+ "golang.org/x/tools/internal/lsp/protocol"
+)
+
+func TestDiagnostics(t *testing.T) {
+ packagestest.TestAll(t, testDiagnostics)
+}
+
+func testDiagnostics(t *testing.T, exporter packagestest.Exporter) {
+ files := packagestest.MustCopyFileTree("testdata/diagnostics")
+ // TODO(rstambler): Stop hardcoding this if we have more files that don't parse.
+ files["noparse/noparse.go"] = packagestest.Copy("testdata/diagnostics/noparse/noparse.go.in")
+ modules := []packagestest.Module{
+ {
+ Name: "golang.org/x/tools/internal/lsp",
+ Files: files,
+ },
+ }
+ exported := packagestest.Export(t, exporter, modules)
+ defer exported.Cleanup()
+
+ wants := make(map[string][]protocol.Diagnostic)
+ for _, module := range modules {
+ for fragment := range module.Files {
+ if !strings.HasSuffix(fragment, ".go") {
+ continue
+ }
+ filename := exporter.Filename(exported, module.Name, fragment)
+ wants[filename] = []protocol.Diagnostic{}
+ }
+ }
+ err := exported.Expect(map[string]interface{}{
+ "diag": func(pos token.Position, msg string) {
+ line := float64(pos.Line - 1)
+ col := float64(pos.Column - 1)
+ want := protocol.Diagnostic{
+ Range: protocol.Range{
+ Start: protocol.Position{
+ Line: line,
+ Character: col,
+ },
+ End: protocol.Position{
+ Line: line,
+ Character: col,
+ },
+ },
+ Severity: protocol.SeverityError,
+ Source: "LSP: Go compiler",
+ Message: msg,
+ }
+ if _, ok := wants[pos.Filename]; ok {
+ wants[pos.Filename] = append(wants[pos.Filename], want)
+ } else {
+ t.Errorf("unexpected filename: %v", pos.Filename)
+ }
+ },
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ v := newView()
+ v.config = exported.Config
+ v.config.Mode = packages.LoadSyntax
+ for filename, want := range wants {
+ diagnostics, err := v.diagnostics(filenameToURI(filename))
+ if err != nil {
+ t.Fatal(err)
+ }
+ got := diagnostics[filename]
+ sort.Slice(got, func(i int, j int) bool {
+ return got[i].Range.Start.Line < got[j].Range.Start.Line
+ })
+ if equal := reflect.DeepEqual(want, got); !equal {
+ t.Errorf("diagnostics failed for %s: (expected: %v), (got: %v)", filename, want, got)
+ }
+ }
+}
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index c246270..58cecde 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -16,9 +16,7 @@
// RunServer starts an LSP server on the supplied stream, and waits until the
// stream is closed.
func RunServer(ctx context.Context, stream jsonrpc2.Stream, opts ...interface{}) error {
- s := &server{
- view: newView(),
- }
+ s := &server{}
conn, client := protocol.RunServer(ctx, stream, s, opts...)
s.client = client
return conn.Wait(ctx)
@@ -39,6 +37,7 @@
if s.initialized {
return nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "server already initialized")
}
+ s.view = newView()
s.initialized = true
return &protocol.InitializeResult{
Capabilities: protocol.ServerCapabilities{
diff --git a/internal/lsp/testdata/diagnostics/bad/bad.go b/internal/lsp/testdata/diagnostics/bad/bad.go
new file mode 100644
index 0000000..abbf57e
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/bad/bad.go
@@ -0,0 +1,19 @@
+package bad
+
+func stuff() {
+ x := "heeeeyyyy"
+ random2(x) //@diag("x", "cannot use x (variable of type string) as int value in argument to random2")
+ random2(1)
+ y := 3 //@diag("y", "y declared but not used")
+}
+
+type bob struct {
+ x int
+}
+
+func _() {
+ var q int
+ _ = &bob{
+ f: q, //@diag("f", "unknown field f in struct literal")
+ }
+}
diff --git a/internal/lsp/testdata/diagnostics/bad/bad_util.go b/internal/lsp/testdata/diagnostics/bad/bad_util.go
new file mode 100644
index 0000000..9ab8a73
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/bad/bad_util.go
@@ -0,0 +1,6 @@
+package bad
+
+func random2(y int) int {
+ x := 6 //@diag("x", "x declared but not used")
+ return y
+}
diff --git a/internal/lsp/testdata/diagnostics/bar/bar.go b/internal/lsp/testdata/diagnostics/bar/bar.go
new file mode 100644
index 0000000..f9f99af
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/bar/bar.go
@@ -0,0 +1,7 @@
+package bar
+
+import "golang.org/x/tools/internal/lsp/foo"
+
+func Bar() {
+ foo.Foo()
+}
diff --git a/internal/lsp/testdata/diagnostics/baz/baz.go b/internal/lsp/testdata/diagnostics/baz/baz.go
new file mode 100644
index 0000000..3b95ee2
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/baz/baz.go
@@ -0,0 +1,7 @@
+package baz
+
+import "golang.org/x/tools/internal/lsp/bar"
+
+func Baz() {
+ bar.Bar()
+}
diff --git a/internal/lsp/testdata/diagnostics/foo/foo.go b/internal/lsp/testdata/diagnostics/foo/foo.go
new file mode 100644
index 0000000..18be5d3
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/foo/foo.go
@@ -0,0 +1,3 @@
+package foo
+
+func Foo() {}
diff --git a/internal/lsp/testdata/diagnostics/good/good.go b/internal/lsp/testdata/diagnostics/good/good.go
new file mode 100644
index 0000000..14b41e6
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/good/good.go
@@ -0,0 +1,6 @@
+package good
+
+func stuff() {
+ x := 5
+ random2(x)
+}
diff --git a/internal/lsp/testdata/diagnostics/good/good_util.go b/internal/lsp/testdata/diagnostics/good/good_util.go
new file mode 100644
index 0000000..ae71ad4
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/good/good_util.go
@@ -0,0 +1,10 @@
+package good
+
+func random() int {
+ y := 6 + 7
+ return y
+}
+
+func random2(y int) int {
+ return y
+}
diff --git a/internal/lsp/testdata/diagnostics/noparse/noparse.go.in b/internal/lsp/testdata/diagnostics/noparse/noparse.go.in
new file mode 100644
index 0000000..9dede9c
--- /dev/null
+++ b/internal/lsp/testdata/diagnostics/noparse/noparse.go.in
@@ -0,0 +1,11 @@
+package noparse
+
+func bye(x int) {
+ hi()
+}
+
+func stuff() {
+ x := 5
+}
+
+func .() {} //@diag(".", "expected 'IDENT', found '.'")
diff --git a/internal/lsp/view.go b/internal/lsp/view.go
index 825981f..19400d6 100644
--- a/internal/lsp/view.go
+++ b/internal/lsp/view.go
@@ -13,14 +13,21 @@
type view struct {
activeFilesMu sync.Mutex
activeFiles map[protocol.DocumentURI][]byte
+ config *packages.Config
fset *token.FileSet
}
func newView() *view {
+ fset := token.NewFileSet()
return &view{
+ config: &packages.Config{
+ Mode: packages.LoadSyntax,
+ Fset: fset,
+ Tests: true,
+ },
activeFiles: make(map[protocol.DocumentURI][]byte),
- fset: token.NewFileSet(),
+ fset: fset,
}
}
@@ -55,14 +62,12 @@
// typeCheck type-checks the package for the given package path.
func (v *view) typeCheck(uri protocol.DocumentURI) (*packages.Package, error) {
- cfg := &packages.Config{
- Mode: packages.LoadSyntax,
- Fset: v.fset,
- Overlay: v.overlay(),
- Tests: true,
- }
- pkgs, err := packages.Load(cfg, fmt.Sprintf("file=%s", uriToFilename(uri)))
+ v.config.Overlay = v.overlay()
+ pkgs, err := packages.Load(v.config, fmt.Sprintf("file=%s", uriToFilename(uri)))
if len(pkgs) == 0 {
+ if err == nil {
+ err = fmt.Errorf("no packages found for %s", uri)
+ }
return nil, err
}
pkg := pkgs[0]
