internal/test/marker: support multi-line locations

Address a long-standing TODO to permit multi-line locations in the
codeactionedit marker. This should unblock use of codeactionedit with
CL 610976.

Also use a 'converter' wrapper to add a bit more type safety in argument
conversion functions.

Change-Id: I851785c567bcde1a8df82a7921c2fba42def9085
Reviewed-on: https://go-review.googlesource.com/c/tools/+/612035
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

gopls/internal/test/marker/doc.go

@@ -107,10 +107,10 @@ The following markers are supported within marker tests:
     If titles are provided, they are used to filter the matching code
     action.
 
-    TODO(rfindley): consolidate with codeactionedit, via a @loc2 marker that
-    allows binding multi-line locations.
+    TODO(rfindley): now that 'location' supports multi-line matches, replace
+    uses of 'codeaction' with codeactionedit.
 
-  - codeactionedit(range, kind, golden, ...titles): a shorter form of
+  - codeactionedit(location, kind, golden, ...titles): a shorter form of
     codeaction. Invokes a code action of the given kind for the given
     in-line range, and compares the resulting formatted unified *edits*
     (notably, not the full file content) with the golden directory.
@@ -292,11 +292,15 @@ test function. Additional value conversions may occur for these argument ->
 parameter type pairs:
   - string->regexp: the argument is parsed as a regular expressions.
   - string->location: the argument is converted to the location of the first
-    instance of the argument in the partial line preceding the note.
+    instance of the argument in the file content starting from the beginning of
+    the line containing the note. Multi-line matches are permitted, but the
+    match must begin before the note.
   - regexp->location: the argument is converted to the location of the first
-    match for the argument in the partial line preceding the note. If the
-    regular expression contains exactly one subgroup, the position of the
-    subgroup is used rather than the position of the submatch.
+    match for the argument in the file content starting from the beginning of
+    the line containing the note. Multi-line matches are permitted, but the
+    match must begin before the note. If the regular expression contains
+    exactly one subgroup, the position of the subgroup is used rather than the
+    position of the submatch.
   - name->location: the argument is replaced by the named location.
   - name->Golden: the argument is used to look up golden content prefixed by
     @<argument>.
@@ -336,12 +340,12 @@ files, and sandboxed directory.
 
 Argument converters translate the "b" and "abc" arguments into locations by
 interpreting each one as a substring (or as a regular expression, if of the
-form re"a|b") and finding the location of its first occurrence on the preceding
-portion of the line, and the abc identifier into a the golden content contained
-in the file @abc. Then the hoverMarker method executes a textDocument/hover LSP
-request at the src position, and ensures the result spans "abc", with the
-markdown content from @abc. (Note that the markdown content includes the expect
-annotation as the doc comment.)
+form re"a|b") and finding the location of its first occurrence starting on the
+preceding portion of the line, and the abc identifier into a the golden content
+contained in the file @abc. Then the hoverMarker method executes a
+textDocument/hover LSP request at the src position, and ensures the result
+spans "abc", with the markdown content from @abc. (Note that the markdown
+content includes the expect annotation as the doc comment.)
 
 The next hover on the same line asserts the same result, but initiates the
 hover immediately after "abc" in the source. This tests that we find the

gopls/internal/test/marker/marker_test.go

@@ -1047,8 +1047,16 @@ var (
 //
 // Converters should return an error rather than calling marker.errorf().
 var customConverters = map[reflect.Type]func(marker, any) (any, error){
-	reflect.TypeOf(protocol.Location{}): convertLocation,
-	reflect.TypeOf(completionLabel("")): convertCompletionLabel,
+	reflect.TypeOf(protocol.Location{}): converter(convertLocation),
+	reflect.TypeOf(completionLabel("")): converter(convertCompletionLabel),
+}
+
+// converter transforms a typed argument conversion function to an untyped
+// conversion function.
+func converter[T any](f func(marker, any) (T, error)) func(marker, any) (any, error) {
+	return func(m marker, arg any) (any, error) {
+		return f(m, arg)
+	}
 }
 
 func convert(mark marker, arg any, paramType reflect.Type) (any, error) {
@@ -1086,26 +1094,64 @@ func convert(mark marker, arg any, paramType reflect.Type) (any, error) {
 // convertLocation converts a string or regexp argument into the protocol
 // location corresponding to the first position of the string (or first match
 // of the regexp) in the line preceding the note.
-func convertLocation(mark marker, arg any) (any, error) {
+func convertLocation(mark marker, arg any) (protocol.Location, error) {
+	// matchContent is used to match the given argument against the file content
+	// starting at the marker line.
+	var matchContent func([]byte) (int, int, error)
+
 	switch arg := arg.(type) {
 	case protocol.Location:
-		return arg, nil
+		return arg, nil // nothing to do
 	case string:
-		startOff, preceding, m, err := linePreceding(mark.run, mark.note.Pos)
-		if err != nil {
-			return protocol.Location{}, err
-		}
-		idx := bytes.Index(preceding, []byte(arg))
-		if idx < 0 {
-			return protocol.Location{}, fmt.Errorf("substring %q not found in %q", arg, preceding)
+		matchContent = func(content []byte) (int, int, error) {
+			idx := bytes.Index(content, []byte(arg))
+			if idx < 0 {
+				return 0, 0, fmt.Errorf("substring %q not found", arg)
+			}
+			return idx, idx + len(arg), nil
 		}
-		off := startOff + idx
-		return m.OffsetLocation(off, off+len(arg))
 	case *regexp.Regexp:
-		return findRegexpInLine(mark.run, mark.note.Pos, arg)
+		matchContent = func(content []byte) (int, int, error) {
+			matches := arg.FindSubmatchIndex(content)
+			if len(matches) == 0 {
+				return 0, 0, fmt.Errorf("no match for regexp %q", arg)
+			}
+			switch len(matches) {
+			case 2:
+				// no subgroups: return the range of the regexp expression
+				return matches[0], matches[1], nil
+			case 4:
+				// one subgroup: return its range
+				return matches[2], matches[3], nil
+			default:
+				return 0, 0, fmt.Errorf("invalid location regexp %q: expect either 0 or 1 subgroups, got %d", arg, len(matches)/2-1)
+			}
+		}
 	default:
 		return protocol.Location{}, fmt.Errorf("cannot convert argument type %T to location (must be a string or regexp to match the preceding line)", arg)
 	}
+
+	// Now use matchFunc to match a range starting on the marker line.
+
+	file := mark.run.test.fset.File(mark.note.Pos)
+	posn := safetoken.Position(file, mark.note.Pos)
+	lineStart := file.LineStart(posn.Line)
+	lineStartOff, lineEndOff, err := safetoken.Offsets(file, lineStart, mark.note.Pos)
+	if err != nil {
+		return protocol.Location{}, err
+	}
+	m := mark.mapper()
+	start, end, err := matchContent(m.Content[lineStartOff:])
+	if err != nil {
+		return protocol.Location{}, err
+	}
+	startOff, endOff := lineStartOff+start, lineStartOff+end
+	if startOff > lineEndOff {
+		// The start of the match must be between the start of the line and the
+		// marker position (inclusive).
+		return protocol.Location{}, fmt.Errorf("no matching range found starting on the current line")
+	}
+	return m.OffsetLocation(startOff, endOff)
 }
 
 // completionLabel is a special parameter type that may be converted from a
@@ -1122,7 +1168,7 @@ type completionLabel string
 //
 // This allows us to stage a migration of the "snippet" marker to a simpler
 // model where the completion label can just be listed explicitly.
-func convertCompletionLabel(mark marker, arg any) (any, error) {
+func convertCompletionLabel(mark marker, arg any) (completionLabel, error) {
 	switch arg := arg.(type) {
 	case string:
 		return completionLabel(arg), nil
@@ -1133,50 +1179,6 @@ func convertCompletionLabel(mark marker, arg any) (any, error) {
 	}
 }
 
-// findRegexpInLine searches the partial line preceding pos for a match for the
-// regular expression re, returning a location spanning the first match. If re
-// contains exactly one subgroup, the position of this subgroup match is
-// returned rather than the position of the full match.
-func findRegexpInLine(run *markerTestRun, pos token.Pos, re *regexp.Regexp) (protocol.Location, error) {
-	startOff, preceding, m, err := linePreceding(run, pos)
-	if err != nil {
-		return protocol.Location{}, err
-	}
-
-	matches := re.FindSubmatchIndex(preceding)
-	if len(matches) == 0 {
-		return protocol.Location{}, fmt.Errorf("no match for regexp %q found in %q", re, string(preceding))
-	}
-	var start, end int
-	switch len(matches) {
-	case 2:
-		// no subgroups: return the range of the regexp expression
-		start, end = matches[0], matches[1]
-	case 4:
-		// one subgroup: return its range
-		start, end = matches[2], matches[3]
-	default:
-		return protocol.Location{}, fmt.Errorf("invalid location regexp %q: expect either 0 or 1 subgroups, got %d", re, len(matches)/2-1)
-	}
-
-	return m.OffsetLocation(start+startOff, end+startOff)
-}
-
-func linePreceding(run *markerTestRun, pos token.Pos) (int, []byte, *protocol.Mapper, error) {
-	file := run.test.fset.File(pos)
-	posn := safetoken.Position(file, pos)
-	lineStart := file.LineStart(posn.Line)
-	startOff, endOff, err := safetoken.Offsets(file, lineStart, pos)
-	if err != nil {
-		return 0, nil, nil, err
-	}
-	m, err := run.env.Editor.Mapper(file.Name())
-	if err != nil {
-		return 0, nil, nil, err
-	}
-	return startOff, m.Content[startOff:endOff], m, nil
-}
-
 // convertStringMatcher converts a string, regexp, or identifier
 // argument into a stringMatcher. The string is a substring of the
 // expected error, the regexp is a pattern than matches the expected

gopls/internal/test/marker/testdata/codeaction/extract-variadic-63287.txt

@@ -13,13 +13,13 @@ package a
 
 func _() {
 	var logf func(string, ...any)
-	{ println(logf) } //@loc(block, re`{.*}`)
+	{ println(logf) } //@loc(block, re`{[^}]*}`)
 }
 
 -- @out/a/a.go --
 @@ -7 +7 @@
--	{ println(logf) } //@loc(block, re`{.*}`)
-+	{ newFunction(logf) } //@loc(block, re`{.*}`)
+-	{ println(logf) } //@loc(block, re`{[^}]*}`)
++	{ newFunction(logf) } //@loc(block, re`{[^}]*}`)
 @@ -10 +10,4 @@
 +func newFunction(logf func( string,  ...any)) {
 +	println(logf)

gopls/internal/test/marker/testdata/codeaction/extract_method.txt

@@ -30,7 +30,7 @@ func (a *A) XLessThanYP() bool {
 
 func (a *A) AddP() int {
 	sum := a.x + a.y //@loc(A_AddP1, re`sum.*a\.y`)
-	return sum       //@loc(A_AddP2, re`return.*sum`)
+	return sum       //@loc(A_AddP2, re`return.*?sum`)
 }
 
 func (a A) XLessThanY() bool {
@@ -39,7 +39,7 @@ func (a A) XLessThanY() bool {
 
 func (a A) Add() int {
 	sum := a.x + a.y //@loc(A_Add1, re`sum.*a\.y`)
-	return sum       //@loc(A_Add2, re`return.*sum`)
+	return sum       //@loc(A_Add2, re`return.*?sum`)
 }
 
 -- @func1/basic.go --
@@ -63,8 +63,8 @@ func (a A) Add() int {
 +
 -- @func3/basic.go --
 @@ -27 +27 @@
--	return sum       //@loc(A_AddP2, re`return.*sum`)
-+	return newFunction(sum)       //@loc(A_AddP2, re`return.*sum`)
+-	return sum       //@loc(A_AddP2, re`return.*?sum`)
++	return newFunction(sum)       //@loc(A_AddP2, re`return.*?sum`)
 @@ -30 +30,4 @@
 +func newFunction(sum int) int {
 +	return sum
@@ -91,8 +91,8 @@ func (a A) Add() int {
 +
 -- @func6/basic.go --
 @@ -36 +36 @@
--	return sum       //@loc(A_Add2, re`return.*sum`)
-+	return newFunction(sum)       //@loc(A_Add2, re`return.*sum`)
+-	return sum       //@loc(A_Add2, re`return.*?sum`)
++	return newFunction(sum)       //@loc(A_Add2, re`return.*?sum`)
 @@ -39 +39,4 @@
 +func newFunction(sum int) int {
 +	return sum
@@ -119,8 +119,8 @@ func (a A) Add() int {
 +
 -- @meth3/basic.go --
 @@ -27 +27 @@
--	return sum       //@loc(A_AddP2, re`return.*sum`)
-+	return a.newMethod(sum)       //@loc(A_AddP2, re`return.*sum`)
+-	return sum       //@loc(A_AddP2, re`return.*?sum`)
++	return a.newMethod(sum)       //@loc(A_AddP2, re`return.*?sum`)
 @@ -30 +30,4 @@
 +func (*A) newMethod(sum int) int {
 +	return sum
@@ -147,8 +147,8 @@ func (a A) Add() int {
 +
 -- @meth6/basic.go --
 @@ -36 +36 @@
--	return sum       //@loc(A_Add2, re`return.*sum`)
-+	return a.newMethod(sum)       //@loc(A_Add2, re`return.*sum`)
+-	return sum       //@loc(A_Add2, re`return.*?sum`)
++	return a.newMethod(sum)       //@loc(A_Add2, re`return.*?sum`)
 @@ -39 +39,4 @@
 +func (A) newMethod(sum int) int {
 +	return sum

gopls/internal/test/marker/testdata/completion/comment.txt

@@ -13,26 +13,26 @@ package comment_completion
 
 var p bool
 
-//@complete(re"$")
+//@complete(re"//()")
 
 func _() {
 	var a int
 
 	switch a {
 	case 1:
-		//@complete(re"$")
+		//@complete(re"//()")
 		_ = a
 	}
 
 	var b chan int
 	select {
 	case <-b:
-		//@complete(re"$")
+		//@complete(re"//()")
 		_ = b
 	}
 
 	var (
-		//@complete(re"$")
+		//@complete(re"//()")
 		_ = a
 	)
 }

gopls/internal/test/marker/testdata/definition/embed.txt

@@ -47,7 +47,7 @@ type J interface { //@loc(J, "J")
 -- b/b.go --
 package b
 
-import "mod.com/a" //@loc(AImport, re"\".*\"")
+import "mod.com/a" //@loc(AImport, re"\"[^\"]*\"")
 
 type embed struct {
 	F int //@loc(F, "F")

gopls/internal/test/marker/testdata/diagnostics/analyzers.txt

@@ -27,7 +27,7 @@ func _() {
 
 // printf
 func _() {
-	printfWrapper("%s") //@diag(re`printfWrapper\(.*\)`, re"example.com.printfWrapper format %s reads arg #1, but call has 0 args")
+	printfWrapper("%s") //@diag(re`printfWrapper\(.*?\)`, re"example.com.printfWrapper format %s reads arg #1, but call has 0 args")
 }
 
 func printfWrapper(format string, args ...interface{}) {