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 <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Peter Weinberger <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>

Author: pjweinbgo

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

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"}},
+		{`<table class="chroma" data-new-comment-url="{{if $.PageIsPullFiles}}{{$.Issue.HTMLURL}}/files/reviews/new_comment{{else}}{{$.CommitHTML}}/new_comment^{{end}}">`, 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{