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: generate method stubs for a given interface #37537

Open
marwan-at-work opened this issue Feb 28, 2020 · 2 comments
Open

x/tools/gopls: generate method stubs for a given interface #37537

marwan-at-work opened this issue Feb 28, 2020 · 2 comments

Comments

@marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Feb 28, 2020

Proposal (TL;DR):

It would be great if Gopls implemented a way where a user:

  1. Moves the cursor to a type declaration
  2. Gets a list of all interface declarations within the workspace
  3. Picks an interface for gopls to generate method stubs for.

See this video for a demonstration: https://vimeo.com/394345925

Details

A common feature that Go and other typed languages have is to generate method stubs for an interface that a type wants to implement.

For most languages, the concrete type must declare what interface it intends to implement. This makes it easier for tools to generate those stubs.

For an example, take a look at this screenshot from TypeScript in VSCode:

Screen Shot 2020-02-27 at 1 59 07 PM

Notice that type X implements Greeter and therefore the dropdown knows which method it needs to stub out.

In Go, it is impossible to know that because interfaces are implicitly implemented.

Therefore, generating method stubs require a two step process:

  1. You pick the interface declaration that you'd like an interface to implement
  2. You pick the concrete type you'd like to add method-stubs to.

Therefore, I wrote a tool that that can do both of these features: https://github.com/marwan-at-work/impl

Here's a video of how I integrated the tool with VSCode locally to demonstrate the same feature working for Go: https://vimeo.com/394345925

However, the tool uses go/packages directly and is obviously a standalone that doesn't leverage gopls with its cache and environment. Therefore, I suggest we make this a feature within gopls.

If this is something worth including, I'd be very happy to port the work I did into gopls (with a little help).

Unfortunately, the LSP protocol does not have an explicit message for "generating method stubs", and so we a few options were brought up in slack that I want to highlight here:

  1. Use CodeAction -> refactor.extract / quickfix
  2. Use noneStandardRequest
  3. Hacking around Autocomplete

cc: @stamblerre @hyangah -- I'm not sure which of the above is the best way, but I'm happy to take on whichever you think is best 👍

PS. Note that this tool used to exist in GOPATH mode using https://github.com/josharian/impl but as far as I know, it is not modules aware and lacked a number of features such as "listing available interfaces" and "automatically adding import paths"

@gopherbot gopherbot added this to the Unreleased milestone Feb 28, 2020
@gopherbot gopherbot added the Tools label Feb 28, 2020
@ianthehat
Copy link

@ianthehat ianthehat commented Feb 28, 2020

Another possibility would be to add it as a quick fix to the

*MyImplementation does not implement MyInterface (missing MyMethod method)

type checking error.
You could intentionally trigger it with the

var _ MyInterface = (*MyImplementation)(nil)

stanza if needed.
Not saying this is the best way, just adding it to the list for consideration.

@marwan-at-work
Copy link
Contributor Author

@marwan-at-work marwan-at-work commented Feb 28, 2020

@ianthehat Would it be possible to expand it to wherever a concrete type is being casted as an interface? For example:

function GetWriter() io.Writer {
  return &myWriter{} // quick fix: implement io.Writer on *myWriter
}

function takeReader(r io.Reader) { ... }

takeReader(&myReader{}) // quick fix: implement io.Reader on *myReader

Similar to rename, it should only do it if the concrete type was defined within the workspace/module boundaries.

johnboyes added a commit to agilepathway/label-checker that referenced this issue Jul 5, 2020
It does not have support for go modules:
golang/go#37537
johnboyes added a commit to agilepathway/label-checker that referenced this issue Jul 5, 2020
It does not have support for go modules:
golang/go#37537
johnboyes added a commit to agilepathway/label-checker that referenced this issue Jul 5, 2020
It does not have support for go modules:
golang/go#37537
johnboyes added a commit to agilepathway/label-checker that referenced this issue Jul 5, 2020
* 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
@stamblerre stamblerre removed this from the Unreleased milestone Jul 23, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.