diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go index 81c732001a..c5d8a2d789 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go +++ b/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 diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go index ecf00ecfc2..76c7ba685e 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/channels.go +++ b/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) { diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go index ab7b2ba5c1..73beace102 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/consecutive_params.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go index 341a9d2a45..5de9254112 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/error_param.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go index ab82463d00..c62174ec94 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/literals.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go index 9a543821ee..9396da4bd9 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/operation.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go index 9ed09a27f2..a4ed290d46 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/selector.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go index d741c68f68..5cde299add 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/slice.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go index 3148e8f4d4..9e91c59c25 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/tuple.go +++ b/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) { diff --git a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go b/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go index 98f77a43cd..5b4241425e 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/testdata/src/a/unique_params.go +++ b/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" } diff --git a/gopls/internal/lsp/analysis/undeclaredname/undeclared.go b/gopls/internal/lsp/analysis/undeclaredname/undeclared.go index faa14091ae..c06ae3538c 100644 --- a/gopls/internal/lsp/analysis/undeclaredname/undeclared.go +++ b/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() { diff --git a/gopls/internal/lsp/lsp_test.go b/gopls/internal/lsp/lsp_test.go index bdf8a6b499..269fdaf3fb 100644 --- a/gopls/internal/lsp/lsp_test.go +++ b/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 { diff --git a/gopls/internal/lsp/protocol/span.go b/gopls/internal/lsp/protocol/span.go index 23721ee703..58601a6b34 100644 --- a/gopls/internal/lsp/protocol/span.go +++ b/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) } diff --git a/gopls/internal/lsp/testdata/bad/bad1.go b/gopls/internal/lsp/testdata/bad/bad1.go index 512f2d9869..04472feae4 100644 --- a/gopls/internal/lsp/testdata/bad/bad1.go +++ b/gopls/internal/lsp/testdata/bad/bad1.go @@ -1,3 +1,4 @@ +//go:build go1.11 // +build go1.11 package bad @@ -5,7 +6,7 @@ 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) } diff --git a/gopls/internal/lsp/testdata/godef/a/a_x_test.go b/gopls/internal/lsp/testdata/godef/a/a_x_test.go index 4631eba2c0..f166f05508 100644 --- a/gopls/internal/lsp/testdata/godef/a/a_x_test.go +++ b/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") } diff --git a/gopls/internal/lsp/testdata/undeclared/var.go b/gopls/internal/lsp/testdata/undeclared/var.go index e27a733d94..3fda582ce1 100644 --- a/gopls/internal/lsp/testdata/undeclared/var.go +++ b/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 } diff --git a/gopls/internal/lsp/testdata/undeclared/var.go.golden b/gopls/internal/lsp/testdata/undeclared/var.go.golden index a266df7c0c..de5cbb42fb 100644 --- a/gopls/internal/lsp/testdata/undeclared/var.go.golden +++ b/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,15 +20,15 @@ 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 } @@ -36,16 +36,16 @@ func m() int { 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 } diff --git a/gopls/internal/lsp/tests/tests.go b/gopls/internal/lsp/tests/tests.go index 689fcca94f..253e28aaf9 100644 --- a/gopls/internal/lsp/tests/tests.go +++ b/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()) } diff --git a/gopls/internal/lsp/tests/util.go b/gopls/internal/lsp/tests/util.go index 8a08b044b5..6f2d01cd8d 100644 --- a/gopls/internal/lsp/tests/util.go +++ b/gopls/internal/lsp/tests/util.go @@ -86,21 +86,20 @@ func DiffDiagnostics(uri span.URI, want, got []*source.Diagnostic) string { } for i, w := range want { g := got[i] - 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) - - } else if !match { - return summarizeDiagnostics(i, uri, want, got, "got Message %q, want match for pattern %q", g.Message, w.Message) - } - if w.Severity != g.Severity { - return summarizeDiagnostics(i, uri, want, got, "got Severity %v, want %v", 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) } + if match, err := regexp.MatchString(w.Message, g.Message); err != nil { + 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, "%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, "%s: got Severity %v, want %v", g.Range.Start, g.Severity, w.Severity) + } + if w.Source != g.Source { + return summarizeDiagnostics(i, uri, want, got, "%s: got Source %v, want %v", g.Range.Start, g.Source, w.Source) + } } return "" }