gopls/implementation: fall back gracefully for non-dynamic calls at "("#632
Closed
abhay1999 wants to merge 3 commits intogolang:masterfrom
Closed
gopls/implementation: fall back gracefully for non-dynamic calls at "("#632abhay1999 wants to merge 3 commits intogolang:masterfrom
abhay1999 wants to merge 3 commits intogolang:masterfrom
Conversation
…re a declaration When a user places the cursor immediately after "//" (with no trailing space) on a line directly above a declaration, gopls already suggests the declaration name as a completion candidate. However the suggested insert text lacked the conventional space, producing "//Name" instead of the proper Go doc comment form "// Name". Detect this case in populateCommentCompletions (cursor at slash+2 with comment text exactly "//") and set a new commentNeedsLeadingSpace flag on the completionContext. item() checks the flag and prepends a space to the insert text (and snippet), so accepting the completion yields "// Name". A marker test is added to comment.txt that asserts the candidate appears when the cursor is right after "//", above a function declaration. Fixes golang/go#76374
…es after // Previously, commentNeedsLeadingSpace was only set when the cursor was exactly at slash+2 and the comment text was exactly "//". This missed the case where the user had partially typed an identifier (e.g. "//Foo") and triggered completion mid-word. Fix: use c.surrounding.start (the start of the edit range, set by setSurroundingForComment) instead. If the edit range starts at slash+2, there is no space between "//" and the identifier being replaced, so completions should prepend a space. Also add a marker test for the partial-prefix case.
implFuncs handles the "(" paren of a CallExpr as an entry point for
finding dynamic function-call implementations. When the call is not
dynamic (e.g. err.Error(), len(""), or a type conversion), it
previously returned a hard error "not a dynamic function call" which
blocked the method-sets fallback and produced a confusing message.
Change the non-dynamic case to return errNotHandled instead, which
lets the caller fall through to the regular method-sets algorithm.
That algorithm also cannot find an implementation from a bare "(" (it
needs an identifier), so it returns "no identifier found" — a standard
error that editors handle gracefully — rather than a gopls-specific
message.
The fix is consistent with the treatment of "(" in
textDocument/references (#76872) and preserves the intended separation
between dynamic-function-type queries (keyed by "func" or "(") and
interface-method queries (keyed by the method name identifier).
Update the affected marker tests to expect "no identifier found".
Fixes golang/go#77784
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a user invokes
textDocument/implementationwith the cursor onthe
(of a non-dynamic call (e.g.err.Error(),len(""), or atype conversion), gopls returned a hard error:
This message is confusing because it leaks an internal concept
("dynamic function call") and blocks the normal method-sets fallback.
Fix
In
implFuncs, when the call at(is not a dynamic function call,return
errNotHandledinstead of a real error. This lets the callerfall through to
implementationsMsets(the regular method-setsalgorithm).
That algorithm also cannot resolve an implementation from a bare
(token (it needs a type name or method identifier), so it returns
"no identifier found"— a standard, editor-friendly error — ratherthan the gopls-specific message. Users are guided to query on the
method name identifier instead (e.g.
Errorinerr.Error()).This is consistent with the treatment of
(intextDocument/references(golang/go#76872) and preserves the intendeddesign separation between dynamic-function-type queries (keyed by
funcor() and interface-method queries (keyed by a methodidentifier).
Fixes golang/go#77784