Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: incorrect edit when completing test function name #57480

Closed
findleyr opened this issue Dec 27, 2022 · 3 comments
Closed

x/tools/gopls: incorrect edit when completing test function name #57480

findleyr opened this issue Dec 27, 2022 · 3 comments
Assignees
Labels
FrozenDueToAge gopls/completion Issues related to auto-completion in gopls. gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

Split off from #56852. When completing at _ in func Te_(t *testing.T) {} (i.e. modifying the test func name), the completion duplicates the test body:

func TestXxx(t *testing.T) {

}(t *testing.T) {

}

CC @pjweinb

@findleyr findleyr added NeedsFix The path to resolution is known, but the work has not been done. gopls/completion Issues related to auto-completion in gopls. labels Dec 27, 2022
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Dec 27, 2022
@gopherbot gopherbot added this to the Unreleased milestone Dec 27, 2022
@findleyr findleyr modified the milestones: Unreleased, gopls/later Dec 27, 2022
@pjweinb
Copy link

pjweinb commented Dec 28, 2022

This is definitely undesirable behavior but I'm not sure what to do about it. (In gopls' defence, Git Copilot also offers some completions that duplicate existing stuff, but that's undesirable too.)

Here's one possible way of handling this (where $ indicates either the end of the current line or the end of the editor buffer, _ is where the cursor is before and after the completion, Xx stands for any non-empty prefix of 'Test', Yy stands for any non-empty prefix of '(t *testing.T)')
'func Xx_$' completes to 'func Test_(t *testing.T) {$' (or maybe 'func Test_(t *testing.T) {}$')
'func Xx_Yy$' completes to 'func Test_Yy$'
and any other 'func Xx_' completes to 'func Test_'

I'm not sure this is all the cases, and I'm not sure how practical it is to implement. Thoughts welcome.

@muirdm
Copy link

muirdm commented Dec 28, 2022

completion/definition.go unconditionally adds the (t *testing.T) {} part. One fix would be to get the parent FuncDecl out of path and check if the recorded parens/curlies actually exist in the source code. For example, check if FuncDecl.Type.Params.Opening is valid and is actually a "(" in the source code.

If they don't exist, assume you have "func Te<>". If they do exist, assume you have "func Te<>(t *testing.T) {}" and don't add the boilerplate.

@gopherbot
Copy link

Change https://go.dev/cl/459559 mentions this issue: gopls/completion: Avoid duplicating text in test func completions

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls/completion Issues related to auto-completion in gopls. gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants