internal/lsp/source: fix completion prefix for comment completion

This CL addresses completion prefix not being overwritten for completion
in comments for exported variables/functions/types etc. Instead of
setting the surrounding range as cursor position, we expand out from
cursor instead to replace the word we're currently on.

Fixes golang/go#39262

Change-Id: I90c28562e3ef285ce6848598f8d7bd7545d5c957
Reviewed-on: https://go-review.googlesource.com/c/tools/+/246237
Run-TryBot: Danish Dua <danishdua@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

internal/lsp/source/completion.go

@@ -18,6 +18,7 @@ import (
 	"strings"
 	"sync"
 	"time"
+	"unicode"
 
 	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/internal/event"
@@ -703,12 +704,12 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast
 
 	// Using the comment position find the line after
 	fset := c.snapshot.View().Session().Cache().FileSet()
-	file := fset.File(comment.Pos())
+	file := fset.File(comment.End())
 	if file == nil {
 		return
 	}
 
-	line := file.Line(comment.Pos())
+	line := file.Line(comment.End())
 	if file.LineCount() < line+1 {
 		return
 	}
@@ -718,6 +719,9 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast
 		return
 	}
 
+	// comment is valid, set surrounding as word boundaries around cursor
+	c.setSurroundingForComment(comment)
+
 	// Using the next line pos, grab and parse the exported symbol on that line
 	for _, n := range c.file.Decls {
 		if n.Pos() != nextLinePos {
@@ -771,6 +775,45 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast
 	}
 }
 
+// sets word boundaries surrounding a cursor for a comment
+func (c *completer) setSurroundingForComment(comments *ast.CommentGroup) {
+	var cursorComment *ast.Comment
+	for _, comment := range comments.List {
+		if c.pos >= comment.Pos() && c.pos <= comment.End() {
+			cursorComment = comment
+			break
+		}
+	}
+	// if cursor isn't in the comment
+	if cursorComment == nil {
+		return
+	}
+
+	// index of cursor in comment text
+	cursorOffset := int(c.pos - cursorComment.Pos())
+	start, end := cursorOffset, cursorOffset
+	for start > 0 && isValidIdentifierChar(cursorComment.Text[start-1]) {
+		start--
+	}
+	for end < len(cursorComment.Text) && isValidIdentifierChar(cursorComment.Text[end]) {
+		end++
+	}
+
+	c.surrounding = &Selection{
+		content: cursorComment.Text[start:end],
+		cursor:  c.pos,
+		mappedRange: newMappedRange(c.snapshot.View().Session().Cache().FileSet(), c.mapper,
+			token.Pos(int(cursorComment.Slash)+start), token.Pos(int(cursorComment.Slash)+end)),
+	}
+}
+
+// isValidIdentifierChar returns true if a byte is a valid go identifier character
+// i.e unicode letter or digit or undescore
+func isValidIdentifierChar(char byte) bool {
+	charRune := rune(char)
+	return unicode.In(charRune, unicode.Letter, unicode.Digit) || char == '_'
+}
+
 func (c *completer) wantStructFieldCompletions() bool {
 	clInfo := c.enclosingCompositeLiteral
 	if clInfo == nil {

internal/lsp/testdata/lsp/primarymod/complit/complit.go.in

@@ -17,6 +17,13 @@ func ExportedFunc() int { //@item(exportedFunc, "ExportedFunc", "func() int", "f
 	return 0
 }
 
+// This tests multiline block comments and completion with prefix
+// Lorem Ipsum Multi//@complete(" ", multilineWithPrefix)
+// Lorem ipsum dolor sit ametom
+func MultilineWithPrefix() int { //@item(multilineWithPrefix, "MultilineWithPrefix", "func() int", "func")
+	return 0
+}
+
 // general completions
 
 type position struct { //@item(structPosition, "position", "struct{...}", "struct")
@@ -25,7 +32,7 @@ type position struct { //@item(structPosition, "position", "struct{...}", "struc
 
 func _() {
 	_ = position{
-		//@complete("", fieldX, fieldY, exportedFunc, structPosition, cVar, exportedConst, exportedType)
+		//@complete("", fieldX, fieldY, exportedFunc, multilineWithPrefix, structPosition, cVar, exportedConst, exportedType)
 	}
 	_ = position{
 		X: 1,
@@ -37,7 +44,7 @@ func _() {
 	}
 	_ = []*position{
         {
-            //@complete("", fieldX, fieldY, exportedFunc, structPosition, cVar, exportedConst, exportedType)
+            //@complete("", fieldX, fieldY, exportedFunc, multilineWithPrefix, structPosition, cVar, exportedConst, exportedType)
         },
 	}
 }
@@ -53,7 +60,7 @@ func _() {
 	}
 
 	_ = map[int]int{
-		//@complete("", abVar, exportedFunc, aaVar, structPosition, cVar, exportedConst, exportedType)
+		//@complete("", abVar, exportedFunc, multilineWithPrefix, aaVar, structPosition, cVar, exportedConst, exportedType)
 	}
 
 	_ = []string{a: ""} //@complete(":", abVar, aaVar)
@@ -61,7 +68,7 @@ func _() {
 
 	_ = position{X: a}   //@complete("}", abVar, aaVar)
 	_ = position{a}      //@complete("}", abVar, aaVar)
-	_ = position{a, }      //@complete("}", abVar, exportedFunc, aaVar, structPosition, cVar, exportedConst, exportedType)
+	_ = position{a, }      //@complete("}", abVar, exportedFunc, multilineWithPrefix, aaVar, structPosition, cVar, exportedConst, exportedType)
 
 	_ = []int{a}  //@complete("}", abVar, aaVar)
 	_ = [1]int{a} //@complete("}", abVar, aaVar)
@@ -89,7 +96,7 @@ func _() {
 
 func _() {
 	_ := position{
-		X: 1, //@complete("X", fieldX),complete(" 1", exportedFunc, structPosition, cVar, exportedConst, exportedType)
-		Y: ,  //@complete(":", fieldY),complete(" ,", exportedFunc, structPosition, cVar, exportedConst, exportedType)
+		X: 1, //@complete("X", fieldX),complete(" 1", exportedFunc, multilineWithPrefix, structPosition, cVar, exportedConst, exportedType)
+		Y: ,  //@complete(":", fieldY),complete(" ,", exportedFunc, multilineWithPrefix, structPosition, cVar, exportedConst, exportedType)
 	}
 }

internal/lsp/testdata/lsp/summary.txt.golden

@@ -1,6 +1,6 @@
 -- summary --
 CodeLensCount = 4
-CompletionsCount = 244
+CompletionsCount = 245
 CompletionSnippetCount = 80
 UnimportedCompletionsCount = 6
 DeepCompletionsCount = 5