gopls/internal/regtest: add expected ranges in completion tests

This change adds tests for package completion in multiline comments and package completion on a newline. It also adds expected ranges in completion tests since simply testing for returned item labels won't catch regression. For more info, see the discussion at https://go-review.googlesource.com/c/tools/+/253177.

Change-Id: Ia42e2645e26647617b32f676952767d2da90e427
Reviewed-on: https://go-review.googlesource.com/c/tools/+/255278
Run-TryBot: Danish Dua <danishdua@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Danish Dua <danishdua@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>

gopls/internal/regtest/completion_test.go

@@ -31,6 +31,10 @@ fun apple() int {
 -- fruits/testfile.go --
 // this is a comment
 
+/*
+ this is a multiline comment
+*/
+
 import "fmt"
 
 func test() {}
@@ -44,57 +48,78 @@ pac
 	var (
 		testfile4 = ""
 		testfile5 = "/*a comment*/ "
+		testfile6 = "/*a comment*/\n"
 	)
 	for _, tc := range []struct {
-		name      string
-		filename  string
-		content   *string
-		line, col int
-		want      []string
+		name          string
+		filename      string
+		content       *string
+		triggerRegexp string
+		want          []string
+		editRegexp    string
 	}{
 		{
-			name:     "package completion at valid position",
-			filename: "fruits/testfile.go",
-			line:     1, col: 0,
-			want: []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			name:          "package completion at valid position",
+			filename:      "fruits/testfile.go",
+			triggerRegexp: "\n()",
+			want:          []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			editRegexp:    "\n()",
 		},
 		{
-			name:     "package completion in a comment",
-			filename: "fruits/testfile.go",
-			line:     0, col: 5,
-			want: nil,
+			name:          "package completion in a comment",
+			filename:      "fruits/testfile.go",
+			triggerRegexp: "th(i)s",
+			want:          nil,
 		},
 		{
-			name:     "package completion at invalid position",
-			filename: "fruits/testfile.go",
-			line:     4, col: 0,
-			want: nil,
+			name:          "package completion in a multiline comment",
+			filename:      "fruits/testfile.go",
+			triggerRegexp: `\/\*\n()`,
+			want:          nil,
 		},
 		{
-			name:     "package completion after keyword 'package'",
-			filename: "fruits/testfile2.go",
-			line:     0, col: 7,
-			want: []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			name:          "package completion at invalid position",
+			filename:      "fruits/testfile.go",
+			triggerRegexp: "import \"fmt\"\n()",
+			want:          nil,
 		},
 		{
-			name:     "package completion with 'pac' prefix",
-			filename: "fruits/testfile3.go",
-			line:     0, col: 3,
-			want: []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			name:          "package completion after keyword 'package'",
+			filename:      "fruits/testfile2.go",
+			triggerRegexp: "package()",
+			want:          []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			editRegexp:    "package\n",
 		},
 		{
-			name:     "package completion at end of file",
-			filename: "fruits/testfile4.go",
-			line:     0, col: 0,
-			content: &testfile4,
-			want:    []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			name:          "package completion with 'pac' prefix",
+			filename:      "fruits/testfile3.go",
+			triggerRegexp: "pac()",
+			want:          []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			editRegexp:    "pac",
 		},
 		{
-			name:     "package completion without terminal newline",
-			filename: "fruits/testfile5.go",
-			line:     0, col: 14,
-			content: &testfile5,
-			want:    []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			name:          "package completion for empty file",
+			filename:      "fruits/testfile4.go",
+			triggerRegexp: "^$",
+			content:       &testfile4,
+			want:          []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			editRegexp:    "^$",
+		},
+		{
+			name:          "package completion without terminal newline",
+			filename:      "fruits/testfile5.go",
+			triggerRegexp: `\*\/ ()`,
+			content:       &testfile5,
+			want:          []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			editRegexp:    `\*\/ ()`,
+		},
+		{
+			name:          "package completion on terminal newline",
+			filename:      "fruits/testfile6.go",
+			triggerRegexp: `\*\/\n()`,
+			content:       &testfile6,
+			want:          []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
+			editRegexp:    `\*\/\n()`,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -106,10 +131,7 @@ pac
 					)
 				}
 				env.OpenFile(tc.filename)
-				completions, err := env.Editor.Completion(env.Ctx, tc.filename, fake.Pos{
-					Line:   tc.line,
-					Column: tc.col,
-				})
+				completions, err := env.Editor.Completion(env.Ctx, tc.filename, env.RegexpSearch(tc.filename, tc.triggerRegexp))
 				if err != nil {
 					t.Fatal(err)
 				}
@@ -124,6 +146,22 @@ pac
 						t.Fatalf("unexpected text edit range end line number: got %d, want less than %d", end, lineCount)
 					}
 				}
+
+				if tc.want != nil {
+					start, end := env.RegexpRange(tc.filename, tc.editRegexp)
+					expectedRng := protocol.Range{
+						Start: fake.Pos.ToProtocolPosition(start),
+						End:   fake.Pos.ToProtocolPosition(end),
+					}
+					for _, item := range completions.Items {
+						gotRng := item.TextEdit.Range
+						if expectedRng != gotRng {
+							t.Errorf("unexpected completion range for completion item %s: got %v, want %v",
+								item.Label, gotRng, expectedRng)
+						}
+					}
+				}
+
 				diff := compareCompletionResults(tc.want, completions.Items)
 				if diff != "" {
 					t.Error(diff)

gopls/internal/regtest/wrappers.go

@@ -98,6 +98,21 @@ func (e *Env) EditBuffer(name string, edits ...fake.Edit) {
 	}
 }
 
+// RegexpRange returns the range of the first match for re in the buffer
+// specified by name, calling t.Fatal on any error. It first searches for the
+// position in open buffers, then in workspace files.
+func (e *Env) RegexpRange(name, re string) (fake.Pos, fake.Pos) {
+	e.T.Helper()
+	start, end, err := e.Editor.RegexpRange(name, re)
+	if err == fake.ErrUnknownBuffer {
+		start, end, err = e.Sandbox.Workdir.RegexpRange(name, re)
+	}
+	if err != nil {
+		e.T.Fatalf("RegexpRange: %v, %v", name, err)
+	}
+	return start, end
+}
+
 // RegexpSearch returns the starting position of the first match for re in the
 // buffer specified by name, calling t.Fatal on any error. It first searches
 // for the position in open buffers, then in workspace files.

internal/lsp/fake/editor.go

@@ -426,7 +426,7 @@ func contentPosition(content string, offset int) (Pos, error) {
 		return Pos{}, errors.Errorf("scanning content: %w", err)
 	}
 	// Scan() will drop the last line if it is empty. Correct for this.
-	if strings.HasSuffix(content, "\n") && offset == start {
+	if (strings.HasSuffix(content, "\n") || content == "") && offset == start {
 		return Pos{Line: line, Column: 0}, nil
 	}
 	return Pos{}, fmt.Errorf("position %d out of bounds in %q (line = %d, start = %d)", offset, content, line, start)
@@ -471,6 +471,18 @@ func regexpRange(content, re string) (Pos, Pos, error) {
 	return startPos, endPos, nil
 }
 
+// RegexpRange returns the first range in the buffer bufName matching re. See
+// RegexpSearch for more information on matching.
+func (e *Editor) RegexpRange(bufName, re string) (Pos, Pos, error) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	buf, ok := e.buffers[bufName]
+	if !ok {
+		return Pos{}, Pos{}, ErrUnknownBuffer
+	}
+	return regexpRange(buf.text(), re)
+}
+
 // RegexpSearch returns the position of the first match for re in the buffer
 // bufName. For convenience, RegexpSearch supports the following two modes:
 //  1. If re has no subgroups, return the position of the match for re itself.
@@ -478,13 +490,7 @@ func regexpRange(content, re string) (Pos, Pos, error) {
 // It returns an error re is invalid, has more than one subgroup, or doesn't
 // match the buffer.
 func (e *Editor) RegexpSearch(bufName, re string) (Pos, error) {
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	buf, ok := e.buffers[bufName]
-	if !ok {
-		return Pos{}, ErrUnknownBuffer
-	}
-	start, _, err := regexpRange(buf.text(), re)
+	start, _, err := e.RegexpRange(bufName, re)
 	return start, err
 }
 

internal/lsp/fake/workdir.go

@@ -115,6 +115,14 @@ func (w *Workdir) ReadFile(path string) (string, error) {
 	return string(b), nil
 }
 
+func (w *Workdir) RegexpRange(path, re string) (Pos, Pos, error) {
+	content, err := w.ReadFile(path)
+	if err != nil {
+		return Pos{}, Pos{}, err
+	}
+	return regexpRange(content, re)
+}
+
 // RegexpSearch searches the file corresponding to path for the first position
 // matching re.
 func (w *Workdir) RegexpSearch(path string, re string) (Pos, error) {