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>

go/packages/packagestest/export.go

@@ -217,7 +217,10 @@ func Copy(source string) Writer {
 // 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
 		}

internal/lsp/diagnostics.go

@@ -63,23 +63,27 @@ func (v *view) diagnostics(uri protocol.DocumentURI) (map[string][]protocol.Diag
 }
 
 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
 }

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)
+		}
+	}
+}

internal/lsp/server.go

@@ -16,9 +16,7 @@ import (
 // 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 @@ func (s *server) Initialize(ctx context.Context, params *protocol.InitializePara
 	if s.initialized {
 		return nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidRequest, "server already initialized")
 	}
+	s.view = newView()
 	s.initialized = true
 	return &protocol.InitializeResult{
 		Capabilities: protocol.ServerCapabilities{

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")
+	}
+}

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
+}

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()
+}

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()
+}

internal/lsp/testdata/diagnostics/foo/foo.go

@@ -0,0 +1,3 @@
+package foo
+
+func Foo() {}

internal/lsp/testdata/diagnostics/good/good.go

@@ -0,0 +1,6 @@
+package good
+
+func stuff() {
+	x := 5
+	random2(x)
+}

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
+}

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 '.'")

internal/lsp/view.go

@@ -13,14 +13,21 @@ import (
 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 @@ func (v *view) clearActiveFile(uri protocol.DocumentURI) {
 
 // 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]