gopls/internal/template: use protocol.Mapper and simplify

This CL replaces all the complex bookkeeping logic with
the existing protocol.Mapper. This fixes the associated
bug.

Details:
- eliminate parsed.{nls,lastnl,check,nonASCII}.
- use UTF-16 or byte offsets, never runes.
- propagate all Mapper errors upwards and handle them properly.
- eliminate unnecessary "multiline" token distinction
  and alternative logic. Mapper works fine.
- remove tests that reduced to tests of Mapper.
- remove append to file.Handle.Content buffer (a data race).

The only behavior changes to tests are:
- the extent of a string token "foo" now includes its quote marks
- the length of an identifier or literal is in bytes, not runes

Also:
- use clearer variable names.
- mark existing comments as TODO where appropriate.
- move symAtPosition
- rename findWordAt -> wordAt

Fixes golang/go#74635

Change-Id: Ia25b7dcbe28e3bc472ae103bd85f71e3c09e3a30
Reviewed-on: https://go-review.googlesource.com/c/tools/+/688937
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

gopls/internal/protocol/semtok/semtok.go

@@ -9,8 +9,8 @@ import "sort"
 
 // A Token provides the extent and semantics of a token.
 type Token struct {
-	Line, Start uint32
-	Len         uint32
+	Line, Start uint32 // 0-based UTF-16 index
+	Len         uint32 // in UTF-16 codes
 	Type        Type
 	Modifiers   []Modifier
 }

gopls/internal/template/completion.go

@@ -31,15 +31,17 @@ func Completion(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p
 	var start int // the beginning of the Token (completed or not)
 	syms := make(map[string]symbol)
 	var p *parsed
-	for fn, fc := range all.files {
+	for uri, fc := range all.files {
 		// collect symbols from all template files
 		filterSyms(syms, fc.symbols)
-		if fn.Path() != fh.URI().Path() {
+		if uri.Path() != fh.URI().Path() {
 			continue
 		}
-		if start = inTemplate(fc, pos); start == -1 {
-			return nil, nil
+		offset, err := enclosingTokenStart(fc, pos)
+		if err != nil {
+			return nil, err
 		}
+		start = offset
 		p = fc
 	}
 	if p == nil {
@@ -74,20 +76,26 @@ 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 {
+// enclosingTokenStart returns the start offset of the enclosing token.
+// A (-1, non-nil) result indicates "no enclosing token".
+func enclosingTokenStart(fc *parsed, pos protocol.Position) (int, error) {
 	// 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
+	offset, err := fc.mapper.PositionOffset(pos)
+	if err != nil {
+		return 0, err
+	}
+
+	// TODO: opt: this could be a binary search, as the tokens are ordered
 	for _, tk := range fc.tokens {
 		if tk.start+len(lbraces) <= offset && offset+len(rbraces) <= tk.end {
-			return tk.start
+			return tk.start, nil
 		}
 	}
+
 	for _, x := range fc.elided {
 		if x+len(lbraces) > offset {
 			// fc.elided is sorted, and x is the position where a '{{' was replaced
@@ -98,10 +106,10 @@ func inTemplate(fc *parsed, pos protocol.Position) int {
 		// If the interval [x,offset] does not contain Left or Right
 		// then provide completions. (do we need the test for Right?)
 		if !bytes.Contains(fc.buf[x:offset], lbraces) && !bytes.Contains(fc.buf[x:offset], rbraces) {
-			return x
+			return x, nil
 		}
 	}
-	return -1
+	return -1, fmt.Errorf("no token enclosing %d", pos)
 }
 
 var (
@@ -115,7 +123,10 @@ var (
 // The error return is always nil.
 func (c *completer) complete() (*protocol.CompletionList, error) {
 	ans := &protocol.CompletionList{IsIncomplete: true, Items: []protocol.CompletionItem{}}
-	start := c.p.fromPosition(c.pos)
+	start, err := c.p.mapper.PositionOffset(c.pos)
+	if err != nil {
+		return ans, err
+	}
 	sofar := c.p.buf[c.offset:start]
 	if len(sofar) == 0 || sofar[len(sofar)-1] == ' ' || sofar[len(sofar)-1] == '\t' {
 		return ans, nil

gopls/internal/template/completion_test.go

@@ -19,13 +19,13 @@ func init() {
 
 type tparse struct {
 	marked string   // ^ shows where to ask for completions. (The user just typed the following character.)
-	wanted []string // expected completions
+	wanted []string // expected completions; nil => no enclosing token
 }
 
 // Test completions in templates that parse enough (if completion needs symbols)
 // Seen characters up to the ^
 func TestParsed(t *testing.T) {
-	var tests = []tparse{
+	for _, test := range []tparse{
 		{"{{x}}{{12. xx^", nil}, // https://github.com/golang/go/issues/50430
 		{`<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"}},
@@ -50,53 +50,56 @@ func TestParsed(t *testing.T) {
 		{"{{`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)
-		var v []string
-		if c != nil {
-			ans, _ := c.complete()
-			for _, a := range ans.Items {
-				v = append(v, a.Label)
+	} {
+		t.Run("", func(t *testing.T) {
+			var got []string
+			if c := testCompleter(t, test); c != nil {
+				ans, _ := c.complete()
+				for _, a := range ans.Items {
+					got = append(got, a.Label)
+				}
 			}
-		}
-		if len(v) != len(tx.wanted) {
-			t.Errorf("%q: got %q, wanted %q %d,%d", tx.marked, v, tx.wanted, len(v), len(tx.wanted))
-			continue
-		}
-		sort.Strings(tx.wanted)
-		sort.Strings(v)
-		for i := 0; i < len(v); i++ {
-			if tx.wanted[i] != v[i] {
-				t.Errorf("%q at %d: got %v, wanted %v", tx.marked, i, v, tx.wanted)
-				break
+			if len(got) != len(test.wanted) {
+				t.Fatalf("%q: got %q, wanted %q %d,%d", test.marked, got, test.wanted, len(got), len(test.wanted))
 			}
-		}
+			sort.Strings(test.wanted)
+			sort.Strings(got)
+			for i := 0; i < len(got); i++ {
+				if test.wanted[i] != got[i] {
+					t.Fatalf("%q at %d: got %v, wanted %v", test.marked, i, got, test.wanted)
+				}
+			}
+		})
 	}
 }
 
 func testCompleter(t *testing.T, tx tparse) *completer {
-	t.Helper()
 	// seen chars up to ^
-	col := strings.Index(tx.marked, "^")
+	offset := strings.Index(tx.marked, "^")
 	buf := strings.Replace(tx.marked, "^", "", 1)
-	p := parseBuffer([]byte(buf))
-	pos := protocol.Position{Line: 0, Character: uint32(col)}
+	p := parseBuffer("", []byte(buf))
 	if p.parseErr != nil {
-		log.Printf("%q: %v", tx.marked, p.parseErr)
+		t.Logf("%q: %v", tx.marked, p.parseErr)
+	}
+	pos, err := p.mapper.OffsetPosition(offset)
+	if err != nil {
+		t.Fatal(err)
 	}
-	offset := inTemplate(p, pos)
-	if offset == -1 {
-		return nil
+
+	start, err := enclosingTokenStart(p, pos)
+	if err != nil {
+		if start == -1 {
+			return nil // no enclosing token
+		}
+		t.Fatal(err)
 	}
 	syms := make(map[string]symbol)
 	filterSyms(syms, p.symbols)
-	c := &completer{
+	return &completer{
 		p:      p,
-		pos:    protocol.Position{Line: 0, Character: uint32(col)},
-		offset: offset + len(lbraces),
+		pos:    pos,
+		offset: start + len(lbraces),
 		ctx:    protocol.CompletionContext{TriggerKind: protocol.Invoked},
 		syms:   syms,
 	}
-	return c
 }

gopls/internal/template/highlight.go

@@ -19,29 +19,34 @@ func Highlight(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, lo
 	if err != nil {
 		return nil, err
 	}
-	p := parseBuffer(buf)
-	pos := p.fromPosition(loc)
-	var ans []protocol.DocumentHighlight
+	p := parseBuffer(fh.URI(), buf)
+	pos, err := p.mapper.PositionOffset(loc)
+	if err != nil {
+		return nil, err
+	}
+
 	if p.parseErr == nil {
 		for _, s := range p.symbols {
-			if s.start <= pos && pos < s.start+s.length {
+			if s.start <= pos && pos < s.start+s.len {
 				return markSymbols(p, s)
 			}
 		}
 	}
+
 	// these tokens exist whether or not there was a parse error
 	// (symbols require a successful parse)
 	for _, tok := range p.tokens {
 		if tok.start <= pos && pos < tok.end {
-			wordAt := findWordAt(p, pos)
+			wordAt := wordAt(p.buf, pos)
 			if len(wordAt) > 0 {
 				return markWordInToken(p, wordAt)
 			}
 		}
 	}
-	// find the 'word' at pos, etc: someday
+
+	// TODO: find the 'word' at pos, etc: someday
 	// until then we get the default action, which doesn't respect word boundaries
-	return ans, nil
+	return nil, nil
 }
 
 func markSymbols(p *parsed, sym symbol) ([]protocol.DocumentHighlight, error) {
@@ -52,8 +57,12 @@ func markSymbols(p *parsed, sym symbol) ([]protocol.DocumentHighlight, error) {
 			if s.vardef {
 				kind = protocol.Write
 			}
+			rng, err := p.mapper.OffsetRange(s.offsets())
+			if err != nil {
+				return nil, err
+			}
 			ans = append(ans, protocol.DocumentHighlight{
-				Range: p._range(s.start, s.length),
+				Range: rng,
 				Kind:  kind,
 			})
 		}
@@ -69,29 +78,35 @@ func markWordInToken(p *parsed, wordAt string) ([]protocol.DocumentHighlight, er
 		return nil, fmt.Errorf("%q: unmatchable word (%v)", wordAt, err)
 	}
 	for _, tok := range p.tokens {
-		got := pat.FindAllIndex(p.buf[tok.start:tok.end], -1)
-		for i := range got {
+		matches := pat.FindAllIndex(p.buf[tok.start:tok.end], -1)
+		for _, match := range matches {
+			rng, err := p.mapper.OffsetRange(match[0], match[1])
+			if err != nil {
+				return nil, err
+			}
 			ans = append(ans, protocol.DocumentHighlight{
-				Range: p._range(got[i][0], got[i][1]-got[i][0]),
+				Range: rng,
 				Kind:  protocol.Text,
 			})
 		}
 	}
 	return ans, nil
 }
 
-var wordRe = regexp.MustCompile(`[$]?\w+$`)
-var moreRe = regexp.MustCompile(`^[$]?\w+`)
-
-// findWordAt finds the word the cursor is in (meaning in or just before)
-func findWordAt(p *parsed, pos int) string {
-	if pos >= len(p.buf) {
-		return "" // can't happen, as we are called with pos < tok.End
+// wordAt returns the word the cursor is in (meaning in or just before)
+func wordAt(buf []byte, pos int) string {
+	if pos >= len(buf) {
+		return ""
 	}
-	after := moreRe.Find(p.buf[pos:])
+	after := moreRe.Find(buf[pos:])
 	if len(after) == 0 {
 		return "" // end of the word
 	}
-	got := wordRe.Find(p.buf[:pos+len(after)])
+	got := wordRe.Find(buf[:pos+len(after)])
 	return string(got)
 }
+
+var (
+	wordRe = regexp.MustCompile(`[$]?\w+$`)
+	moreRe = regexp.MustCompile(`^[$]?\w+`)
+)