x/tools/gopls: support fill struct behavior #37576

frioux · 2020-02-28T23:30:02Z

What version of Go are you using (`go version`)?

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

n/a (feature request)

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/frew/.cache/go-build"
GOENV="/home/frew/.config/go/env"
GOEXE=""
GOFLAGS="-mod=readonly"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/frew/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org"
GOROOT="/home/frew/sdk/go1.14"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/frew/sdk/go1.14/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/run/shm/go-build445574914=/tmp/go-build -gno-record-gcc-switches"

What did you do?

n/a (feature request)

What did you expect to see?

n/a (feature request)

What did you see instead?

n/a (feature request)

Feature request

vim-go had a command called :GoFillStruct and it would transform (for example), this:

x := Person{}

into

x := Person{
	Name: "",
	Employed: false,
	Age: 0,
}

Basically it'd set all the fields to their zero value. In govim/govim#709 I was told that this needs to be added to gopls first, so here I am :)

The text was updated successfully, but these errors were encountered:

stamblerre · 2020-03-02T20:01:04Z

Thanks for the issue report! I agree that we should support this feature.

@muirdm: Do you think we could fit this behavior into completion?

muirdm · 2020-03-02T20:19:14Z

For my understanding, what is the use case for this?

stamblerre · 2020-03-02T20:22:35Z

I don't know if it's particularly useful for the Person case described above, but I would find it useful if the struct contained map values. (My classic example is go/types.Info, which I always find super annoying to create.) My ideal scenario when completing that struct is that, if I have a value of the correct type in scope, it should be used -- otherwise, we should make a map in the struct declaration.

muirdm · 2020-03-02T21:19:32Z

The zero value of map is "nil", so this proposal as written wouldn't help in the types.Info case.

In my experience having the literal completion that gives you a candidate for make(map[some.Long]annoying.Map) is already a major improvement. Having structs get automatically filled in with in-scope objects sounds hard to avoid false positives.

frioux · 2020-03-02T21:37:47Z

the main use case here is creating fresh struct with lots of fields. This is common when working with, for example, the aws sdk where completing methods is a very small help since you then need to pass in a struct with sometimes dozens of fields set.

…

On Mon, Mar 2, 2020, 1:19 PM Muir Manders ***@***.***> wrote: The zero value of map is "nil", so this proposal as written wouldn't help in the types.Info case. In my experience having the literal completion that gives you a candidate for make(map[some.Long]annoying.Map) is already a major improvement. Having structs get automatically filled in with in-scope objects sounds hard to avoid false positives. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#37576?email_source=notifications&email_token=AAAB7Y5OHXJKRI2XLFM4CA3RFQPGNA5CNFSM4K6CIMAKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENRAR5Q#issuecomment-593627382>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAB7YYIU35CO53T5KJSNV3RFQPGNANCNFSM4K6CIMAA> .

muirdm · 2020-03-03T00:05:39Z

I can see how that could be part of your workflow, but it also seems to overlap with existing facilities such as textDocument/hover which will show you all the struct fields or just a normal completion request in the struct literal, which also will list all the struct fields and their types.

To answer the original question of fitting this into completion, we could but I'm not sure what would trigger the completion or how it would show up clearly to the user. Maybe better as a code action?

frioux · 2020-03-03T06:26:35Z

I don't know what a code action is, but in the original setting it was something the user would explicitly request, rather than fitting into the same workflow as method completion. I personally think that's fine; this is something I'd do a few times a day, not constantly, like method completion.

frioux · 2020-03-03T06:26:37Z

I don't know what a code action is, but in the original setting it was something the user would explicitly request, rather than fitting into the same workflow as method completion. I personally think that's fine; this is something I'd do a few times a day, not constantly, like method completion.

leitzler · 2020-03-06T19:30:35Z

I support the codeAction suggestion, seems like it would be a better fit (maybe of kind refactor.rewrite.fillstruct).

As a feature this is something that, at least I, hear people ask for quite often. With the use case to get the field names quickly added when creating a new struct instance. Hover does show the same info, but it would be really helpful if gopls could add it for you so that you don't have to add them manually.

luciolas · 2020-04-06T09:44:50Z

Is anyone working on this? Mind if I take a look at implementing this feature?

stamblerre · 2020-04-06T14:16:03Z

I don't believe so, so please go ahead, @luciolas! As a starting point, I would suggest implementing this behavior as a command.

luciolas · 2020-04-07T08:29:13Z

Ahh, I went ahead and wrote some code in code_action.go. Is there a reason for implementing it as a command?

Secondly, I also tested if completions are viable for a struct fill.
(See GIF below for a small demo on using code action and completions)

Thirdly, a few things I have in mind:

Does Code Action support placeholders?
Is it possible to trigger completion while in a placeholder?

stamblerre · 2020-04-07T19:52:56Z

Ahh, I went ahead and wrote some code in code_action.go. Is there a reason for implementing it as a command?

I think either way probably works. I wasn't sure if it necessarily fit under "refactor.rewrite", but I think it works.

Secondly, I also tested if completions are viable for a struct fill.
(See GIF below for a small demo on using code action and completions)

Looks great! Thanks for working on this. I think the code action is a good starting point, and we could possible extend this to be part of completions if we think of a good user experience for it.

Thirdly, a few things I have in mind:

Does Code Action support placeholders?

I don't believe so.

Is it possible to trigger completion while in a placeholder?

I think it should be possible, yes.

a-h · 2020-04-08T13:53:22Z

This looks great, I'd also like to see a code action to fill a struct from a variable (merge).

type A struct {
  ID string
  Name string
  Phone string
  Email string
}

type B struct {
  Name string
  Phone string
  Email string
  Fax string
}

In this case, the snippet would need to know the source and destination variable names (they should be in scope). Given variables a and b of type A and B respectively, the snippet would produce something like this.

var a A
// Not present on B.
// a.ID = b.ID
a.Name = b.Email
a.Phone = b.Email
a.Email = b.Email
// Not present on A.
// a.Fax = b.Fax

This would prevent a common mistake of forgetting to copy a field from one type to another, missing a field, or overwriting the wrong names (e.g. a.Fax = b.Email). These types of things pop up as the "last line effect", where developers switch off as they keep track of the fields they have or haven't mapped.

Runtime tools exist such as https://github.com/imdario/mergo but these must use reflection, resulting in poorer performance, and the use of a 3rd party library.

Not sure if this should be a completely different thing (merge?), but this feature looks like a good starting point.

leitzler · 2020-06-10T18:14:30Z

Can this be close now that https://golang.org/cl/227437 & golang/tools#220 is merged?

stamblerre · 2020-06-10T18:35:53Z

Yep. @joshbaum will be doing some more work on fillstruct to make it work through go/analysis like the rest of our code actions, but that should not change its behavior.

fillstruct fills a struct literal with default values[1] This functionality is now available in the gopls language server[2] [1] https://github.com/davidrjenni/reftools/tree/master/cmd/fillstruct [2] golang/go#37576 (comment)

* Enable Codespaces - add default Go files Codespaces[1] allows us to use a remote vscode environment embedded in the browser in GitHub[1] This commit adds the pre-built container configuration[2] for Go[3]. We can customise this configuration as we wish in future commits. [1] https://github.com/features/codespaces/ [2] https://docs.github.com/en/github/developing-online-with-codespaces/configuring-codespaces-for-your-project#using-a-pre-built-container-configuration [3] https://github.com/microsoft/vscode-dev-containers/tree/master/containers/go * Suppress/fix devcontainer Dockerfile warnings These were hadolint warnings, and there were three of them, all on .devcontainer/Dockerfile#L17: DL3003 Use WORKDIR to switch to a directory DL3008 Pin versions in apt get install. Instead of `apt-get install <package>` use `apt-get install <package>=<version>` DL3015 Avoid additional packages by specifying `--no-install-recommends` Suppressed the warnings apart from the DL3015 one, which was easy to fix. Not fixing the other warnings (for now anyway) as this Dockerfile is code that has been lifted and shifted from the `.devcontainer/Dockerfile` in: https://github.com/microsoft/vscode-dev-containers/tree/master/containers/go * Update module settings to "on" as we use modules See: https://dev.to/maelvls/why-is-go111module-everywhere-and-everything-about-go-modules-24k * Add shellcheck vscode extension to devcontainer https://www.shellcheck.net/ https://marketplace.visualstudio.com/items?itemName=timonwong.shellcheck * Use vscode go language server on devcontainer https://github.com/golang/vscode-go/blob/master/docs/gopls.md * Stop installing go dependencies on devcontainer There is no need to install them as we are using go modules - see: microsoft/vscode-go#2836 * Set vscode editor go tabsize to correct size of 8 See: microsoft/vscode-go#2479 (comment) * Use golangci-lint for vscode devcontainer linting https://github.com/golangci/golangci-lint * No longer install Guru on devcontainer It does not support Go modules: https://github.com/golang/vscode-go/blob/master/docs/tools.md#guru * Remove unnecessary gorename from devcontainer Not needed when using go modules and gopls language server: https://github.com/golang/vscode-go/blob/master/docs/tools.md#gorename * Remove unnecessary godoctor from devcontainer It is not needed when using go modules and the gopls language server: https://github.com/golang/vscode-go/blob/master/docs/tools.md#godoctor * Remove unnecessary goimports from devcontainer Imports are instead handled by the gopls language server: golang/go#33587 (comment) * Remove unnecessary golint from devcontainer Not needed as we are using golangci-lint * Remove unnecessary gotests from devcontainer gotests autogenerates table tests boilerplate code. We don't need this for this project. * Remove unnecessary goplay module from devcontainer We don't need this functionality: https://github.com/haya14busa/goplay/ * Remove unnecessary gometalinter from devcontainer Not needed as we are using golangci-lint for linting. * Remove unnecessary impl module from devcontainer It does not have support for go modules: golang/go#37537 * Remove unnecessary fillstruct from devcontainer fillstruct fills a struct literal with default values[1] This functionality is now available in the gopls language server[2] [1] https://github.com/davidrjenni/reftools/tree/master/cmd/fillstruct [2] golang/go#37576 (comment) * Add comment to explain why gopkgs is still needed It is still needed even with gopls: microsoft/vscode-go#3050 (comment) * Fix installation of golangci-lint on devcontainer Prior to this commit, vscode was saying "Analysis tools missing" and prompting to install it. Fix was to install golangci-lint via go get, as per: golangci/golangci-lint#1037 (comment) * Remove unnecessary gogetdoc from devcontainer Similar functionality is available in the gopls language server: fatih/vim-go#2808 (comment) * Remove unnecessary revive linter from devcontainer We are using golangci-lint for linting, so no need for revive. * Remove unnecessary go-tools from devcontainer go-tools primarily contains staticcheck, which we don't need as we are using golangci-lint for linting. * Output go version after building devcontainer Doing this just to provide assurance that the container has started successfully, after building. * Move settings which are not container-specific The guideline for`devcontainer.json` settings is that they should only contain settings which _must_ be be changed in a container (e.g. absolute paths)[1]. Moved the other settings to `.vscode/settings.json` so that they are more visible, and easier to use when working locally (i.e. not in a container). [1] microsoft/vscode-remote-release#874 (comment) * Up tests timeout as tests are slower on container The tests seem to run much slower (c. 100 seconds compared to c. 50 seconds) when running on a remote container, compared to locally. Will investigate to see if this is the same when running on Codespaces. If the tests are taking 100 seconds we may need to create an issue to speed them up. * Add recommended vscode go settings As per the recommendations at: https://github.com/golang/tools/blob/master/gopls/doc/vscode.md * Set dark colour theme for vscode in devcontainer * Set devcontainer vscode to autosave after 500ms Adding these settings to the devcontainer settings file rather than the vscode settings file, as we want them to apply at the machine level. https://code.visualstudio.com/docs/editor/codebasics#_save-auto-save

gopherbot added this to the Unreleased milestone Feb 28, 2020

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 Feb 28, 2020

stamblerre changed the title ~~x/tools/gopls: feature Request: fill struct~~ x/tools/gopls: support fill struct behavior Mar 2, 2020

stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Mar 2, 2020

golang deleted a comment from gopherbot Mar 2, 2020

stamblerre added the FeatureRequest label Mar 2, 2020

myitcv mentioned this issue Mar 5, 2020

cmd/govim: feature Request: fill struct govim/govim#709

Closed

stamblerre modified the milestones: gopls/v1.0.0, Unreleased Mar 12, 2020

stamblerre added the help wanted label Mar 12, 2020

luciolas mentioned this issue Apr 22, 2020

internal/lsp: added a fill struct code action golang/tools#220

Closed

stamblerre closed this as completed Jun 10, 2020

ubombi mentioned this issue Jun 28, 2020

FillStruct using gopls (feature request) fatih/vim-go#2940

Closed

golang locked and limited conversation to collaborators Jun 10, 2021

gopherbot added the FrozenDueToAge label Jun 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/gopls: support fill struct behavior #37576

x/tools/gopls: support fill struct behavior #37576

frioux commented Feb 28, 2020

stamblerre commented Mar 2, 2020 •

edited

muirdm commented Mar 2, 2020 •

edited

stamblerre commented Mar 2, 2020

muirdm commented Mar 2, 2020

frioux commented Mar 2, 2020 via email

muirdm commented Mar 3, 2020

frioux commented Mar 3, 2020

frioux commented Mar 3, 2020

leitzler commented Mar 6, 2020

luciolas commented Apr 6, 2020

stamblerre commented Apr 6, 2020

luciolas commented Apr 7, 2020

stamblerre commented Apr 7, 2020

a-h commented Apr 8, 2020

leitzler commented Jun 10, 2020

stamblerre commented Jun 10, 2020

x/tools/gopls: support fill struct behavior #37576

x/tools/gopls: support fill struct behavior #37576

Comments

frioux commented Feb 28, 2020

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

Feature request

stamblerre commented Mar 2, 2020 • edited

muirdm commented Mar 2, 2020 • edited

stamblerre commented Mar 2, 2020

muirdm commented Mar 2, 2020

frioux commented Mar 2, 2020 via email

muirdm commented Mar 3, 2020

frioux commented Mar 3, 2020

frioux commented Mar 3, 2020

leitzler commented Mar 6, 2020

luciolas commented Apr 6, 2020

stamblerre commented Apr 6, 2020

luciolas commented Apr 7, 2020

stamblerre commented Apr 7, 2020

a-h commented Apr 8, 2020

leitzler commented Jun 10, 2020

stamblerre commented Jun 10, 2020

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

stamblerre commented Mar 2, 2020 •

edited

muirdm commented Mar 2, 2020 •

edited