From a61892390376707b967ab0cbf0598db2c64566f3 Mon Sep 17 00:00:00 2001 From: pjw Date: Mon, 22 Nov 2021 11:56:01 -0500 Subject: [PATCH] internal/lsp/template: fix error that causes crashes When LSP asks for a completion it sends a position, which is the position of the first character after the cursor. The old code mistakenly thought it was the position of the last character seen, and sometimes produced an impossible slice to parse. The example at line 29 of completion_test.go used to cause a panic. Fixes: golang/go#49600 Change-Id: I772fe38a1a12d3876ef866819e6a878b297bb92f Reviewed-on: https://go-review.googlesource.com/c/tools/+/366036 Run-TryBot: Peter Weinberger gopls-CI: kokoro TryBot-Result: Go Bot Trust: Peter Weinberger Reviewed-by: Hyang-Ah Hana Kim --- internal/lsp/template/completion.go | 6 ++- internal/lsp/template/completion_test.go | 61 +++++++++++++----------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/internal/lsp/template/completion.go b/internal/lsp/template/completion.go index 4ec7623ba9a..55b59f82152 100644 --- a/internal/lsp/template/completion.go +++ b/internal/lsp/template/completion.go @@ -78,13 +78,15 @@ func filterSyms(syms map[string]symbol, ns []symbol) { // return the starting position of the enclosing token, or -1 if none func inTemplate(fc *Parsed, pos protocol.Position) int { + // pos is the pos-th character. if the cursor is at the beginning + // of the file, pos is 0. That is, we've only seen characters before pos // 1. pos might be in a Token, return tk.Start // 2. pos might be after an elided but before a Token, return elided // 3. return -1 for false offset := fc.FromPosition(pos) // this could be a binary search, as the tokens are ordered for _, tk := range fc.tokens { - if tk.Start <= offset && offset < tk.End { + if tk.Start < offset && offset <= tk.End { return tk.Start } } @@ -244,7 +246,7 @@ func scan(buf []byte) []string { ans[len(ans)-1] = "." + lit } else if tok == token.IDENT && len(ans) > 0 && ans[len(ans)-1] == "$" { ans[len(ans)-1] = "$" + lit - } else { + } else if lit != "" { ans = append(ans, lit) } } diff --git a/internal/lsp/template/completion_test.go b/internal/lsp/template/completion_test.go index 7d17ab1ebab..b7ef89fbeb9 100644 --- a/internal/lsp/template/completion_test.go +++ b/internal/lsp/template/completion_test.go @@ -23,40 +23,41 @@ type tparse struct { } // Test completions in templates that parse enough (if completion needs symbols) +// Seen characters up to the ^ func TestParsed(t *testing.T) { var tests = []tparse{ - {"{{^if}}", []string{"index", "if"}}, - {"{{if .}}{{^e {{end}}", []string{"eq", "end}}", "else", "end"}}, - {"{{foo}}{{^f", []string{"foo"}}, - {"{{^$}}", []string{"$"}}, - {"{{$x:=4}}{{^$", []string{"$x"}}, - {"{{$x:=4}}{{$^ ", []string{}}, - {"{{len .Modified}}{{^.Mo", []string{"Modified"}}, - {"{{len .Modified}}{{.m^f", []string{"Modified"}}, - {"{{^$ }}", []string{"$"}}, - {"{{$a =3}}{{^$", []string{"$a"}}, + {``, nil}, + {"{{i^f}}", []string{"index", "if"}}, + {"{{if .}}{{e^ {{end}}", []string{"eq", "end}}", "else", "end"}}, + {"{{foo}}{{f^", []string{"foo"}}, + {"{{$^}}", []string{"$"}}, + {"{{$x:=4}}{{$^", []string{"$x"}}, + {"{{$x:=4}}{{$ ^ ", []string{}}, + {"{{len .Modified}}{{.^Mo", []string{"Modified"}}, + {"{{len .Modified}}{{.mf^", []string{"Modified"}}, + {"{{$^ }}", []string{"$"}}, + {"{{$a =3}}{{$^", []string{"$a"}}, // .two is not good here: fix someday - {`{{.Modified}}{{^.{{if $.one.two}}xxx{{end}}`, []string{"Modified", "one", "two"}}, - {`{{.Modified}}{{.^o{{if $.one.two}}xxx{{end}}`, []string{"one"}}, - {"{{.Modiifed}}{{.one.^t{{if $.one.two}}xxx{{end}}", []string{"two"}}, - {`{{block "foo" .}}{{^i`, []string{"index", "if"}}, - {"{{i^n{{Internal}}", []string{"index", "Internal", "if"}}, + {`{{.Modified}}{{.^{{if $.one.two}}xxx{{end}}`, []string{"Modified", "one", "two"}}, + {`{{.Modified}}{{.o^{{if $.one.two}}xxx{{end}}`, []string{"one"}}, + {"{{.Modiifed}}{{.one.t^{{if $.one.two}}xxx{{end}}", []string{"two"}}, + {`{{block "foo" .}}{{i^`, []string{"index", "if"}}, + {"{{in^{{Internal}}", []string{"index", "Internal", "if"}}, // simple number has no completions {"{{4^e", []string{}}, // simple string has no completions - {"{{`^e", []string{}}, - {"{{`No ^i", []string{}}, // example of why go/scanner is used - {"{{xavier}}{{12. ^x", []string{"xavier"}}, + {"{{`e^", []string{}}, + {"{{`No i^", []string{}}, // example of why go/scanner is used + {"{{xavier}}{{12. x^", []string{"xavier"}}, } for _, tx := range tests { c := testCompleter(t, tx) - ans, err := c.complete() - if err != nil { - t.Fatal(err) - } var v []string - for _, a := range ans.Items { - v = append(v, a.Label) + if c != nil { + ans, _ := c.complete() + for _, a := range ans.Items { + v = append(v, a.Label) + } } if len(v) != len(tx.wanted) { t.Errorf("%q: got %v, wanted %v", tx.marked, v, tx.wanted) @@ -75,16 +76,18 @@ func TestParsed(t *testing.T) { func testCompleter(t *testing.T, tx tparse) *completer { t.Helper() - col := strings.Index(tx.marked, "^") + 1 - offset := strings.LastIndex(tx.marked[:col], string(Left)) - if offset < 0 { - t.Fatalf("no {{ before ^: %q", tx.marked) - } + // seen chars up to ^ + col := strings.Index(tx.marked, "^") buf := strings.Replace(tx.marked, "^", "", 1) p := parseBuffer([]byte(buf)) + pos := protocol.Position{Line: 0, Character: uint32(col)} if p.ParseErr != nil { log.Printf("%q: %v", tx.marked, p.ParseErr) } + offset := inTemplate(p, pos) + if offset == -1 { + return nil + } syms := make(map[string]symbol) filterSyms(syms, p.symbols) c := &completer{