Refactor links tests to resuse diff logic

internal/lsp/cmd/test/links.go

@@ -5,44 +5,32 @@
 package cmdtest
 
 import (
-	"reflect"
-	"sort"
-	"strings"
+	"encoding/json"
 	"testing"
 
 	"golang.org/x/tools/internal/lsp/cmd"
+	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/tests"
 	"golang.org/x/tools/internal/span"
 	"golang.org/x/tools/internal/tool"
 )
 
 func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) {
-	filename := uri.Filename()
-	args := []string{"links", filename}
+	m, err := r.data.Mapper(uri)
+	if err != nil {
+		t.Fatal(err)
+	}
+	args := []string{"links", "-json", uri.Filename()}
 	app := cmd.New("gopls-test", r.data.Config.Dir, r.data.Exported.Config.Env, r.options)
-	got := CaptureStdOut(t, func() {
+	out := CaptureStdOut(t, func() {
 		_ = tool.Run(r.ctx, app, args)
 	})
-	got = strings.Trim(got, "\n") // remove extra new line
-	gotStrings := strings.Split(got, "\n")
-	// The files that are checked include `expect.Note`'s which also include expected links.
-	// For cmd testing we cannot ignore these comments so we get duplicates. So hence we select only the uniques.
-	uniques := make(map[string]struct{})
-	for _, v := range gotStrings {
-		uniques[v] = struct{}{}
-	}
-	var result []string
-	for k := range uniques {
-		result = append(result, k)
-	}
-	sort.Strings(result)
-
-	var wantStrings []string
-	for _, v := range wantLinks {
-		wantStrings = append(wantStrings, v.Target)
+	var got []protocol.DocumentLink
+	err = json.Unmarshal([]byte(out), &got)
+	if err != nil {
+		t.Fatal(err)
 	}
-	sort.Strings(wantStrings)
-	if !reflect.DeepEqual(result, wantStrings) {
-		t.Errorf("links not equal for %s, expected:\n%v\ngot:\n%v", filename, wantStrings, result)
+	if diff := tests.DiffLinks(m, wantLinks, got); diff != "" {
+		t.Error(diff)
 	}
 }

internal/lsp/lsp_test.go

@@ -759,49 +759,16 @@ func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	gotLinks, err := r.server.DocumentLink(r.ctx, &protocol.DocumentLinkParams{
+	got, err := r.server.DocumentLink(r.ctx, &protocol.DocumentLinkParams{
 		TextDocument: protocol.TextDocumentIdentifier{
 			URI: protocol.NewURI(uri),
 		},
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	var notePositions []token.Position
-	links := make(map[span.Span]string, len(wantLinks))
-	for _, link := range wantLinks {
-		links[link.Src] = link.Target
-		notePositions = append(notePositions, link.NotePosition)
-	}
-
-	for _, link := range gotLinks {
-		spn, err := m.RangeSpan(link.Range)
-		if err != nil {
-			t.Fatal(err)
-		}
-		linkInNote := false
-		for _, notePosition := range notePositions {
-			// Drop the links found inside expectation notes arguments as this links are not collected by expect package
-			if notePosition.Line == spn.Start().Line() &&
-				notePosition.Column <= spn.Start().Column() {
-				delete(links, spn)
-				linkInNote = true
-			}
-		}
-		if linkInNote {
-			continue
-		}
-		if target, ok := links[spn]; ok {
-			delete(links, spn)
-			if target != link.Target {
-				t.Errorf("for %v want %v, got %v\n", spn, link.Target, target)
-			}
-		} else {
-			t.Errorf("unexpected link %v:%v\n", spn, link.Target)
-		}
-	}
-	for spn, target := range links {
-		t.Errorf("missing link %v:%v\n", spn, target)
+	if diff := tests.DiffLinks(m, wantLinks, got); diff != "" {
+		t.Error(diff)
 	}
 }
 

internal/lsp/tests/links.go

@@ -0,0 +1,55 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package tests
+
+import (
+	"fmt"
+	"go/token"
+
+	"golang.org/x/tools/internal/lsp/protocol"
+	"golang.org/x/tools/internal/span"
+)
+
+// DiffLinks takes the links we got and checks if they are located within the source or a Note.
+// If the link is within a Note, the link is removed.
+// Returns an diff comment if there are differences and empty string if no diffs
+func DiffLinks(mapper *protocol.ColumnMapper, wantLinks []Link, gotLinks []protocol.DocumentLink) string {
+	var notePositions []token.Position
+	links := make(map[span.Span]string, len(wantLinks))
+	for _, link := range wantLinks {
+		links[link.Src] = link.Target
+		notePositions = append(notePositions, link.NotePosition)
+	}
+	for _, link := range gotLinks {
+		spn, err := mapper.RangeSpan(link.Range)
+		if err != nil {
+			return fmt.Sprintf("%v", err)
+		}
+		linkInNote := false
+		for _, notePosition := range notePositions {
+			// Drop the links found inside expectation notes arguments as this links are not collected by expect package
+			if notePosition.Line == spn.Start().Line() &&
+				notePosition.Column <= spn.Start().Column() {
+				delete(links, spn)
+				linkInNote = true
+			}
+		}
+		if linkInNote {
+			continue
+		}
+		if target, ok := links[spn]; ok {
+			delete(links, spn)
+			if target != link.Target {
+				return fmt.Sprintf("for %v want %v, got %v\n", spn, link.Target, target)
+			}
+		} else {
+			return fmt.Sprintf("unexpected link %v:%v\n", spn, link.Target)
+		}
+	}
+	for spn, target := range links {
+		return fmt.Sprintf("missing link %v:%v\n", spn, target)
+	}
+	return ""
+}