gopls/implementation: fall back gracefully for non-dynamic calls at "("

gopls/internal/golang/completion/completion.go

@@ -366,6 +366,13 @@ type completionContext struct {
 	// commentCompletion is true if we are completing a comment.
 	commentCompletion bool
 
+	// commentNeedsLeadingSpace is true when the completion edit range starts
+	// immediately after "//" with no intervening space. Completion items should
+	// prepend a space so that accepting a suggestion produces "// Name" rather
+	// than "//Name". This applies both when the cursor is right after "//" and
+	// when the user has partially typed an identifier (e.g. "//Fo").
+	commentNeedsLeadingSpace bool
+
 	// packageCompletion is true if we are completing a package name.
 	packageCompletion bool
 }
@@ -1049,6 +1056,20 @@ func (c *completer) populateCommentCompletions(comment *ast.CommentGroup) {
 	// comment is valid, set surrounding as word boundaries around cursor
 	c.setSurroundingForComment(comment)
 
+	// If the edit range (what's being replaced) starts immediately after "//"
+	// with no space, completions must prepend a space to produce proper Go doc
+	// comment style ("// Name" rather than "//Name"). This covers both the case
+	// where the cursor is right after "//" and where the user has already typed
+	// some characters (e.g. "//Fo" completing to "// FooBar").
+	if c.surrounding != nil {
+		for _, cmt := range comment.List {
+			if c.surrounding.start == cmt.Slash+2 {
+				c.completionContext.commentNeedsLeadingSpace = true
+				break
+			}
+		}
+	}
+
 	// Using the next line pos, grab and parse the exported symbol on that line
 	for _, n := range c.pgf.File.Decls {
 		declLine := safetoken.Line(file, n.Pos())

gopls/internal/golang/completion/format.go

@@ -229,6 +229,14 @@ Suffixes:
 	if cand.detail != "" {
 		detail = cand.detail
 	}
+
+	// When completing right after "//" (cursor at the slashes with no space),
+	// prepend a space so the result is "// Name" rather than "//Name".
+	if c.completionContext.commentNeedsLeadingSpace {
+		insert = " " + insert
+		snip.PrependText(" ")
+	}
+
 	item := CompletionItem{
 		Label:               label,
 		InsertText:          insert,

gopls/internal/golang/implementation.go

@@ -940,7 +940,10 @@ func implFuncs(pkg *cache.Package, curSel inspector.Cursor, start, end token.Pos
 			if inToken(n.Lparen, "(", start, end) {
 				t := dynamicFuncCallType(info, n)
 				if t == nil {
-					return nil, fmt.Errorf("not a dynamic function call")
+					// Not a dynamic function call (e.g. a regular method call
+					// like err.Error()). Fall back to the method-sets algorithm
+					// so the caller can find interface implementations normally.
+					return nil, errNotHandled
 				}
 				// Case 2b: dynamic call of function value.
 				// Report declarations of corresponding concrete functions.

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

@@ -72,6 +72,14 @@ func Function() int { //@item(function, "Function", "func() int", "func") //@com
 	return 0
 }
 
+// This tests that completing right after "//" (no space) above a declaration
+// suggests the identifier name, prepending a space for proper doc comment style.
+//@complete(re"//()", plainFunc)
+// This tests completing with a partial prefix after "//" (no space) also
+// prepends a space, covering the case where the user has typed "//Plain".
+//Plain//@complete(re"//Plain()", plainFunc)
+func PlainFunc() {} //@item(plainFunc, "PlainFunc", "func()", "func")
+
 // This tests multiline block comments and completion with prefix
 // Lorem Ipsum Multili//@complete(re"()Multi", multiline)
 // Lorem ipsum dolor sit ametom

gopls/internal/test/marker/testdata/implementation/issue74305.txt

@@ -8,4 +8,4 @@ go 1.18
 -- a/a.go --
 package a
 
-var _ = undefined() //@ diag("undefined", re"undefined"), implementation("(", err="not a dynamic function call")
+var _ = undefined() //@ diag("undefined", re"undefined"), implementation("(", err="no identifier found")

gopls/internal/test/marker/testdata/implementation/signature.txt

@@ -68,12 +68,15 @@ func _(
 
 	struct{x Nullary}{}.x() //@ loc(fieldCall, "("), implementation("(", nullaryFunc)
 
-	// Calls that are not dynamic function calls:
-	_ = len("")          //@ implementation("(", err="not a dynamic function call")
-	_ = int(0)           //@ implementation("(", err="not a dynamic function call")
-	_ = error.Error(nil) //@ implementation("(", err="not a dynamic function call")
-	_ = err.Error()      //@ implementation("(", err="not a dynamic function call")
-	_ = f4(0)            //@ implementation("(", err="not a dynamic function call"), loc(f4Call, "(")
+	// Calls that are not dynamic function calls: implFuncs returns
+	// errNotHandled so the method-sets fallback runs, but finds no
+	// identifier at "(" and returns "no identifier found".
+	// Users should query on the method name instead (e.g. "Error").
+	_ = len("")          //@ implementation("(", err="no identifier found")
+	_ = int(0)           //@ implementation("(", err="no identifier found")
+	_ = error.Error(nil) //@ implementation("(", err="no identifier found")
+	_ = err.Error()      //@ implementation("(", err="no identifier found")
+	_ = f4(0)            //@ implementation("(", err="no identifier found"), loc(f4Call, "(")
 }