Skip to content

x/tools/gopls: suggest testing.F if function name starts with Fuzz* #46896

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

Closed
hyangah opened this issue Jun 23, 2021 · 9 comments
Closed

x/tools/gopls: suggest testing.F if function name starts with Fuzz* #46896

hyangah opened this issue Jun 23, 2021 · 9 comments
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Jun 23, 2021

$ go version -m ~/go/bin/gopls
/Users/hakim/go/bin/gopls: devel go1.17-3fa42437b5 Wed Jun 23 16:05:25 2021 +0000
        path    golang.org/x/tools/gopls
        mod     golang.org/x/tools/gopls        v0.7.0  h1:JQBHW81Gsyim6iDjUwGoPeSpXrSqwen3isPJLfDfaYU=
        dep     github.com/BurntSushi/toml      v0.3.1  h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
        dep     github.com/google/go-cmp        v0.5.5  h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
        dep     github.com/sergi/go-diff        v1.1.0  h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
        dep     golang.org/x/mod        v0.4.2  h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
        dep     golang.org/x/sync       v0.0.0-20210220032951-036812b2e83c      h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
        dep     golang.org/x/sys        v0.0.0-20210510120138-977fb7262007      h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
        dep     golang.org/x/tools      v0.1.3-0.20210608163600-9ed039809d4c    h1:Pv9gNyJFYVdpUAVZYJ1BDSU4eGgXQ+0f3DIGAdolO5s=
        dep     golang.org/x/xerrors    v0.0.0-20200804184101-5ec99f83aff1      h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
        dep     honnef.co/go/tools      v0.2.0  h1:ws8AfbgTX3oIczLPNPCu5166oBg9ST2vNs0rcht+mDE=
        dep     mvdan.cc/gofumpt        v0.1.1  h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
        dep     mvdan.cc/xurls/v2       v2.2.0  h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=

Screen Shot 2021-06-23 at 1 23 00 PM

@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 Jun 23, 2021
@gopherbot gopherbot added this to the Unreleased milestone Jun 23, 2021
@suzmue suzmue added the FeatureRequest Issues asking for a new feature that does not need a proposal. label Jun 24, 2021
@suzmue suzmue modified the milestones: Unreleased, gopls/unplanned Jun 24, 2021
@danishprakash
Copy link

Would like to work on this cc/ @hyangah

@hyangah
Copy link
Contributor Author

hyangah commented Dec 15, 2021

And I wonder if gopls can handle autocomplete specially for f.Fuzz since its function signature itself is not very informative, but it is meant to take only function type arg of a special form. And analyzer on this param would be nice. @findleyr do you want to keep track of this as a separate feature request?

Screen Shot 2021-12-15 at 3 43 10 PM

cc @katiehockman

@findleyr
Copy link
Member

And analyzer on this param would be nice. @findleyr do you want to keep track of this as a separate feature request?

That is a very good idea, and easy to implement! Please open a separate issue, in gopls/on-deck. It can be help-wanted too as this should be straightforward to add.

@danishprakash
Copy link

I completely forgot about this issue, will work on this over the weekend, thanks for the ping :)

@danishprakash
Copy link

Sorry for the huge delay on this.

@hyangah while looking into this, I realized we don't show/order completions based on the function name, please correct me If I'm wrong here.

Based on the above, I tried a very brittle implementation which essentially compares the enclosing function with all the shortlisted candidates and orders them accordingly. The output looks something like this:

Bench* Fuzz*
oie_pTwnCKaT8L3G oie_i7ZE2HmjKucs

It does the same for Test* too but again, the heuristic is relatively straightforward. I can polish this up and raise a CL for the team to review for further discussion.

danishprakash/tools@bc8097d

cc: @findleyr

@findleyr findleyr modified the milestones: gopls/unplanned, gopls/v0.8.0 Feb 8, 2022
@suzmue suzmue modified the milestones: gopls/v0.8.0, gopls/v0.8.1 Feb 11, 2022
@pjweinb
Copy link

pjweinb commented Feb 12, 2022

I'm unclear on what's being requested/proposed in this issue. In a test file, after the '.', there's only one legal completion in
'TestFoo(t *testing.)', 'TestMain(t *testing.)', 'BenchmarkFoo(t *testing.', or 'FuzzFoo(t *testing.'. So the top gopls suggestion then should be 'T', or 'M', or 'B' or 'F'. This seems doable, but doesn't save the user much.

Best would be to offer to complete the whole signature after the user types the '(', but gopls does not generate completions when it sees '('.

Intermediate would be to offer ' *testing.T' when the user types the formal parameter in 'func TestFoo(t' (and ' *testing.F' for 'FuzzFoo(f' etc).

@hyangah
Copy link
Contributor Author

hyangah commented Feb 12, 2022

Throughout typing, I've never seen testing.F suggested as top candidates. Ideally, it would be nice if testing.F was presented as soon as I start typing inside () or after t *. (gopls v0.8.0-pre.1 with go1.18beta2)

Screen Shot 2022-02-12 at 10 08 40 AM
Screen Shot 2022-02-12 at 10 08 50 AM
Screen Shot 2022-02-12 at 10 09 00 AM

Finally -
Screen Shot 2022-02-12 at 10 09 24 AM

As @danishprakash found out, there is no specialization for test/bench functions either so same problem exists for testing.T. testing.B may be just lucky one because it's one of the first candidates in alphabetical order.

@pjweinb pjweinb self-assigned this Feb 15, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/385974 mentions this issue: internal/lsp: Provide completions for test function definitions

@zikaeroh
Copy link
Contributor

I would definitely like to see this completion list prioritized to put the "right" struct type at the top; I suggested using snippet completion on above CL (to complete the full signature with an empty placeholder or something), but for those that don't use snippet completions, the sorting would be a marked improvement. For me, it's definitely more useful than "doesn't save the user much" would imply.

@findleyr findleyr modified the milestones: gopls/v0.8.1, gopls/v0.8.0 Feb 28, 2022
@rsc rsc unassigned pjweinb Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

7 participants