gopls: update to handle 'undefined:' versus 'undeclared' in type errors

Update gopls to handle the new form of "undeclared name: ..." error
messages, "undefined: ...", and update tests to be tolerant of both.

Also do some minor cleanup of test error messages related to mismatching
diagnostics.

With this change, TryBots should succeed at CL 432556.

Updates golang/go#54845

Change-Id: I9214d00c59110cd34470845b72d3e2f5e73291c1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/434636
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go

@@ -6,22 +6,22 @@ package undeclared
 
 func x() int {
 	var z int
-	z = y // want "undeclared name: y"
+	z = y // want "(undeclared name|undefined): y"
 
-	if z == m { // want "undeclared name: m"
+	if z == m { // want "(undeclared name|undefined): m"
 		z = 1
 	}
 
 	if z == 1 {
 		z = 1
-	} else if z == n+1 { // want "undeclared name: n"
+	} else if z == n+1 { // want "(undeclared name|undefined): n"
 		z = 1
 	}
 
 	switch z {
 	case 10:
 		z = 1
-	case a: // want "undeclared name: a"
+	case a: // want "(undeclared name|undefined): a"
 		z = 1
 	}
 	return z

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go

@@ -5,7 +5,7 @@
 package undeclared
 
 func channels(s string) {
-	undefinedChannels(c()) // want "undeclared name: undefinedChannels"
+	undefinedChannels(c()) // want "(undeclared name|undefined): undefinedChannels"
 }
 
 func c() (<-chan string, chan string) {

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go

@@ -6,5 +6,5 @@ package undeclared
 
 func consecutiveParams() {
 	var s string
-	undefinedConsecutiveParams(s, s) // want "undeclared name: undefinedConsecutiveParams"
+	undefinedConsecutiveParams(s, s) // want "(undeclared name|undefined): undefinedConsecutiveParams"
 }

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go

@@ -6,5 +6,5 @@ package undeclared
 
 func errorParam() {
 	var err error
-	undefinedErrorParam(err) // want "undeclared name: undefinedErrorParam"
+	undefinedErrorParam(err) // want "(undeclared name|undefined): undefinedErrorParam"
 }

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go

@@ -7,5 +7,5 @@ package undeclared
 type T struct{}
 
 func literals() {
-	undefinedLiterals("hey compiler", T{}, &T{}) // want "undeclared name: undefinedLiterals"
+	undefinedLiterals("hey compiler", T{}, &T{}) // want "(undeclared name|undefined): undefinedLiterals"
 }

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go

@@ -7,5 +7,5 @@ package undeclared
 import "time"
 
 func operation() {
-	undefinedOperation(10 * time.Second) // want "undeclared name: undefinedOperation"
+	undefinedOperation(10 * time.Second) // want "(undeclared name|undefined): undefinedOperation"
 }

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go

@@ -6,5 +6,5 @@ package undeclared
 
 func selector() {
 	m := map[int]bool{}
-	undefinedSelector(m[1]) // want "undeclared name: undefinedSelector"
+	undefinedSelector(m[1]) // want "(undeclared name|undefined): undefinedSelector"
 }

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go

@@ -5,5 +5,5 @@
 package undeclared
 
 func slice() {
-	undefinedSlice([]int{1, 2}) // want "undeclared name: undefinedSlice"
+	undefinedSlice([]int{1, 2}) // want "(undeclared name|undefined): undefinedSlice"
 }

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go

@@ -5,7 +5,7 @@
 package undeclared
 
 func tuple() {
-	undefinedTuple(b()) // want "undeclared name: undefinedTuple"
+	undefinedTuple(b()) // want "(undeclared name|undefined): undefinedTuple"
 }
 
 func b() (string, error) {

gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go

@@ -7,5 +7,5 @@ package undeclared
 func uniqueArguments() {
 	var s string
 	var i int
-	undefinedUniqueArguments(s, i, s) // want "undeclared name: undefinedUniqueArguments"
+	undefinedUniqueArguments(s, i, s) // want "(undeclared name|undefined): undefinedUniqueArguments"
 }

gopls/internal/lsp/analysis/undeclaredname/undeclared.go

@@ -45,7 +45,7 @@ var Analyzer = &analysis.Analyzer{
 	RunDespiteErrors: true,
 }
 
-const undeclaredNamePrefix = "undeclared name: "
+var undeclaredNamePrefixes = []string{"undeclared name: ", "undefined: "}
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	for _, err := range analysisinternal.GetTypeErrors(pass) {
@@ -55,10 +55,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
 }
 
 func runForError(pass *analysis.Pass, err types.Error) {
-	if !strings.HasPrefix(err.Msg, undeclaredNamePrefix) {
+	var name string
+	for _, prefix := range undeclaredNamePrefixes {
+		if !strings.HasPrefix(err.Msg, prefix) {
+			continue
+		}
+		name = strings.TrimPrefix(err.Msg, prefix)
+	}
+	if name == "" {
 		return
 	}
-	name := strings.TrimPrefix(err.Msg, undeclaredNamePrefix)
 	var file *ast.File
 	for _, f := range pass.Files {
 		if f.Pos() <= err.Pos && err.Pos < f.End() {

gopls/internal/lsp/lsp_test.go

@@ -513,7 +513,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []tests.S
 			break
 		}
 	}
-	codeActionKinds := []protocol.CodeActionKind{}
+	var codeActionKinds []protocol.CodeActionKind
 	for _, k := range actionKinds {
 		codeActionKinds = append(codeActionKinds, protocol.CodeActionKind(k.ActionKind))
 	}
@@ -541,12 +541,11 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []tests.S
 
 	}
 	if len(actions) != expectedActions {
-		// Hack: We assume that we only get one code action per range.
-		var cmds []string
+		var summaries []string
 		for _, a := range actions {
-			cmds = append(cmds, fmt.Sprintf("%s (%s)", a.Command, a.Title))
+			summaries = append(summaries, fmt.Sprintf("%q (%s)", a.Title, a.Kind))
 		}
-		t.Fatalf("unexpected number of code actions, want %d, got %d: %v", expectedActions, len(actions), cmds)
+		t.Fatalf("CodeAction(...): got %d code actions (%v), want %d", len(actions), summaries, expectedActions)
 	}
 	action := actions[0]
 	var match bool
@@ -557,7 +556,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []tests.S
 		}
 	}
 	if !match {
-		t.Fatalf("unexpected kind for code action %s, expected one of %v, got %v", action.Title, codeActionKinds, action.Kind)
+		t.Fatalf("unexpected kind for code action %s, got %v, want one of %v", action.Title, action.Kind, codeActionKinds)
 	}
 	var res map[span.URI]string
 	if cmd := action.Command; cmd != nil {

gopls/internal/lsp/protocol/span.go

@@ -186,6 +186,18 @@ func Intersect(a, b Range) bool {
 		(a.End.Line == b.Start.Line) && a.End.Character < b.Start.Character)
 }
 
+// Format implements fmt.Formatter.
+//
+// Note: Formatter is implemented instead of Stringer (presumably) for
+// performance reasons, though it is not clear that it matters in practice.
 func (r Range) Format(f fmt.State, _ rune) {
-	fmt.Fprintf(f, "%v:%v-%v:%v", r.Start.Line, r.Start.Character, r.End.Line, r.End.Character)
+	fmt.Fprintf(f, "%v-%v", r.Start, r.End)
+}
+
+// Format implements fmt.Formatter.
+//
+// See Range.Format for discussion of why the Formatter interface is
+// implemented rather than Stringer.
+func (p Position) Format(f fmt.State, _ rune) {
+	fmt.Fprintf(f, "%v:%v", p.Line, p.Character)
 }

gopls/internal/lsp/testdata/bad/bad1.go

@@ -1,11 +1,12 @@
+//go:build go1.11
 // +build go1.11
 
 package bad
 
 // See #36637
 type stateFunc func() stateFunc //@item(stateFunc, "stateFunc", "func() stateFunc", "type")
 
-var a unknown //@item(global_a, "a", "unknown", "var"),diag("unknown", "compiler", "undeclared name: unknown", "error")
+var a unknown //@item(global_a, "a", "unknown", "var"),diag("unknown", "compiler", "(undeclared name|undefined): unknown", "error")
 
 func random() int { //@item(random, "random", "func() int", "func")
 	//@complete("", global_a, bob, random, random2, random3, stateFunc, stuff)
@@ -14,8 +15,8 @@ func random() int { //@item(random, "random", "func() int", "func")
 
 func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func"),item(bad_y_param, "y", "int", "var")
 	x := 6       //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared but not used", "error")
-	var q blah   //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "compiler", "undeclared name: blah", "error")
-	var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "compiler", "undeclared name: blob", "error")
+	var q blah   //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "compiler", "(undeclared name|undefined): blah", "error")
+	var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "compiler", "(undeclared name|undefined): blob", "error")
 	//@complete("", q, t, x, bad_y_param, global_a, bob, random, random2, random3, stateFunc, stuff)
 
 	return y
@@ -24,10 +25,10 @@ func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func")
 func random3(y ...int) { //@item(random3, "random3", "func(y ...int)", "func"),item(y_variadic_param, "y", "[]int", "var")
 	//@complete("", y_variadic_param, global_a, bob, random, random2, random3, stateFunc, stuff)
 
-	var ch chan (favType1)   //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "compiler", "undeclared name: favType1", "error")
-	var m map[keyType]int    //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "compiler", "undeclared name: keyType", "error")
-	var arr []favType2       //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "compiler", "undeclared name: favType2", "error")
-	var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "compiler", "undeclared name: badResult", "error")
-	var fn2 func(badParam)   //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "compiler", "undeclared name: badParam", "error")
+	var ch chan (favType1)   //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "compiler", "(undeclared name|undefined): favType1", "error")
+	var m map[keyType]int    //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "compiler", "(undeclared name|undefined): keyType", "error")
+	var arr []favType2       //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "compiler", "(undeclared name|undefined): favType2", "error")
+	var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "compiler", "(undeclared name|undefined): badResult", "error")
+	var fn2 func(badParam)   //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "compiler", "(undeclared name|undefined): badParam", "error")
 	//@complete("", arr, ch, fn1, fn2, m, y_variadic_param, global_a, bob, random, random2, random3, stateFunc, stuff)
 }

gopls/internal/lsp/testdata/godef/a/a_x_test.go

@@ -5,5 +5,5 @@ import (
 )
 
 func TestA2(t *testing.T) { //@TestA2,godef(TestA2, TestA2)
-	Nonexistant() //@diag("Nonexistant", "compiler", "undeclared name: Nonexistant", "error")
+	Nonexistant() //@diag("Nonexistant", "compiler", "(undeclared name|undefined): Nonexistant", "error")
 }

gopls/internal/lsp/testdata/undeclared/var.go

@@ -1,14 +1,14 @@
 package undeclared
 
 func m() int {
-	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix", "")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "(undeclared name|undefined): y", "error"),suggestedfix("y", "quickfix", "")
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix", "")
+	} else if 100 > n+2 { //@diag("n", "compiler", "(undeclared name|undefined): n", "error"),suggestedfix("n", "quickfix", "")
 		z = 4
 	}
-	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix", "")
+	for i < 200 { //@diag("i", "compiler", "(undeclared name|undefined): i", "error"),suggestedfix("i", "quickfix", "")
 	}
-	r() //@diag("r", "compiler", "undeclared name: r", "error")
+	r() //@diag("r", "compiler", "(undeclared name|undefined): r", "error")
 	return z
 }

gopls/internal/lsp/testdata/undeclared/var.go.golden

@@ -2,16 +2,16 @@
 package undeclared
 
 func m() int {
-	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix", "")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "(undeclared name|undefined): y", "error"),suggestedfix("y", "quickfix", "")
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix", "")
+	} else if 100 > n+2 { //@diag("n", "compiler", "(undeclared name|undefined): n", "error"),suggestedfix("n", "quickfix", "")
 		z = 4
 	}
 	i := 
-	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix", "")
+	for i < 200 { //@diag("i", "compiler", "(undeclared name|undefined): i", "error"),suggestedfix("i", "quickfix", "")
 	}
-	r() //@diag("r", "compiler", "undeclared name: r", "error")
+	r() //@diag("r", "compiler", "(undeclared name|undefined): r", "error")
 	return z
 }
 
@@ -20,32 +20,32 @@ package undeclared
 
 func m() int {
 	y := 
-	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix", "")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "(undeclared name|undefined): y", "error"),suggestedfix("y", "quickfix", "")
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix", "")
+	} else if 100 > n+2 { //@diag("n", "compiler", "(undeclared name|undefined): n", "error"),suggestedfix("n", "quickfix", "")
 		z = 4
 	}
-	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix", "")
+	for i < 200 { //@diag("i", "compiler", "(undeclared name|undefined): i", "error"),suggestedfix("i", "quickfix", "")
 	}
-	r() //@diag("r", "compiler", "undeclared name: r", "error")
+	r() //@diag("r", "compiler", "(undeclared name|undefined): r", "error")
 	return z
 }
 
 -- suggestedfix_var_7_18 --
 package undeclared
 
 func m() int {
-	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix", "")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "(undeclared name|undefined): y", "error"),suggestedfix("y", "quickfix", "")
 	n := 
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix", "")
+	} else if 100 > n+2 { //@diag("n", "compiler", "(undeclared name|undefined): n", "error"),suggestedfix("n", "quickfix", "")
 		z = 4
 	}
-	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix", "")
+	for i < 200 { //@diag("i", "compiler", "(undeclared name|undefined): i", "error"),suggestedfix("i", "quickfix", "")
 	}
-	r() //@diag("r", "compiler", "undeclared name: r", "error")
+	r() //@diag("r", "compiler", "(undeclared name|undefined): r", "error")
 	return z
 }
 

gopls/internal/lsp/tests/tests.go

@@ -1404,6 +1404,8 @@ func uriName(uri span.URI) string {
 	return filepath.Base(strings.TrimSuffix(uri.Filename(), ".go"))
 }
 
+// TODO(golang/go#54845): improve the formatting here to match standard
+// line:column position formatting.
 func SpanName(spn span.Span) string {
 	return fmt.Sprintf("%v_%v_%v", uriName(spn.URI()), spn.Start().Line(), spn.Start().Column())
 }

gopls/internal/lsp/tests/util.go

@@ -86,20 +86,19 @@ func DiffDiagnostics(uri span.URI, want, got []*source.Diagnostic) string {
 	}
 	for i, w := range want {
 		g := got[i]
+		if !rangeOverlaps(g.Range, w.Range) {
+			return summarizeDiagnostics(i, uri, want, got, "got Range %v, want overlap with %v", g.Range, w.Range)
+		}
 		if match, err := regexp.MatchString(w.Message, g.Message); err != nil {
-			return summarizeDiagnostics(i, uri, want, got, "invalid regular expression %q: %v", w.Message, err)
-
+			return summarizeDiagnostics(i, uri, want, got, "%s: invalid regular expression %q: %v", w.Range.Start, w.Message, err)
 		} else if !match {
-			return summarizeDiagnostics(i, uri, want, got, "got Message %q, want match for pattern %q", g.Message, w.Message)
+			return summarizeDiagnostics(i, uri, want, got, "%s: got Message %q, want match for pattern %q", g.Range.Start, g.Message, w.Message)
 		}
 		if w.Severity != g.Severity {
-			return summarizeDiagnostics(i, uri, want, got, "got Severity %v, want %v", g.Severity, w.Severity)
+			return summarizeDiagnostics(i, uri, want, got, "%s: got Severity %v, want %v", g.Range.Start, g.Severity, w.Severity)
 		}
 		if w.Source != g.Source {
-			return summarizeDiagnostics(i, uri, want, got, "got Source %v, want %v", g.Source, w.Source)
-		}
-		if !rangeOverlaps(g.Range, w.Range) {
-			return summarizeDiagnostics(i, uri, want, got, "got Range %v, want overlap with %v", g.Range, w.Range)
+			return summarizeDiagnostics(i, uri, want, got, "%s: got Source %v, want %v", g.Range.Start, g.Source, w.Source)
 		}
 	}
 	return ""