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: feature: code action to generate a stub function from a "no function f" type error #69692

Closed
xzbdmw opened this issue Sep 28, 2024 · 11 comments
Labels
FeatureRequest 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

@xzbdmw
Copy link

xzbdmw commented Sep 28, 2024

gopls version

golang.org/x/tools/gopls (devel)
golang.org/x/tools/gopls@(devel)
github.com/BurntSushi/[email protected] h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/[email protected] h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/[email protected] h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/[email protected] h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0=
golang.org/x/[email protected] h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
golang.org/x/[email protected] h1:nU8/tAV/21mkPrCjACUeSibjhynTovgRMXc32+Y1Aec=
golang.org/x/[email protected] h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc=
golang.org/x/[email protected] => ../
golang.org/x/[email protected] h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
honnef.co/go/[email protected] h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs=
mvdan.cc/[email protected] h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/[email protected] h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.22.4

go env

GO111MODULE='auto'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xzb/Library/Caches/go-build'
GOENV='/Users/xzb/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/xzb/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/xzb/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/xzb/Project/go/6.5840/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/gv/r110hgbx1gbgzp95kf_q71x40000gn/T/go-build504977365=/tmp/go-build -gno-record-gcc-switches -fno-common

What did you do?

The | stands for cursor position

type foo struct{}

func (f *foo) fn() {
	f.bar|("string")
}

I expect a code action that auto generate the missing methods, result in

type foo struct{}

func (f *foo) fn() {
	f.bar("string")
}

func (f *foo) bar(s string) {
	// todo
}

What did you see happen?

There are no similar action

What did you expect to see?

provide the action
Here is similar action in rust-analyzer:
image

Editor and settings

No response

Logs

No response

@xzbdmw xzbdmw added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Sep 28, 2024
@gopherbot gopherbot added this to the Unreleased milestone Sep 28, 2024
@adonovan
Copy link
Member

gopls already has a related feature, stubmethods, that is admittedly a little hard to find. (The same is unfortunately also true of its documentation.)

This seems like a reasonable feature request for a new type-error analyzer.

@adonovan adonovan changed the title x/tools/gopls: codeactions: generate function/method under cursor x/tools/gopls: feature: code action to generate a stub function from a "no function f" type error Sep 30, 2024
@xzbdmw
Copy link
Author

xzbdmw commented Sep 30, 2024

Nice, I'm looking at this feature, are there similar PRs that I can borrow from?

@adonovan
Copy link
Member

adonovan commented Sep 30, 2024

Nice, I'm looking at this feature, are there similar PRs that I can borrow from?

Search the codebase for stubmethods. It has two parts:

  • An analyzer, which analyzes a package and reports errors. Unlike most analyzers, which detect and report errors themselves, this one merely finds existing type errors of the right kind and reports a Diagnostic for each one. The diagnostic is allowed to suggest a fix. In this case the fix has SuggestedEdits.TextEdits=nil, meaning it is lazily computed by an ApplyFix("stubmethods") command.
  • An ApplyFix command handler for "stubmethods", that does the actual refactoring.

You should follow this structure. The stubmethods code may have nuggets you can steal/share.

@adonovan
Copy link
Member

Hmm, I notice the TODO comment not to do things this way. It might be simpler to avoid the analyzer altogether and just follow the code action plugin approach used by refactorRewriteRemoveUnusedParam, which also inspects pkg.TypeErrors().

@adonovan
Copy link
Member

adonovan commented Oct 1, 2024

@xzbdmw you may want to take a look at https://go.dev/cl/617035, which reorganizes stubmethods to fit directly into the CodeAction handler without using the go/analysis framework. Once this CL lands, I expect your change will follow the pattern used by stubmethods in the quickFix function in golang/codeaction.go, but instead of strings.Contains(msg, "missing method") it will look for "no method named...found" and produce a different code action.

@xzbdmw
Copy link
Author

xzbdmw commented Oct 3, 2024

Is it possible to get the error node's type by type inference, so the generated function can have correct return value?

use info.Types[n] where n is the missing methods ast.CallExpr yield nil typeinfo.

If the info can not be retrieved easily, we need to guess the type by looking up parent node, eg. variable assignment, parameter assignment, return statement etc.

@adonovan
Copy link
Member

adonovan commented Oct 4, 2024

Is it possible to get the error node's type by type inference, so the generated function can have correct return value?

use info.Types[n] where n is the missing methods ast.CallExpr yield nil typeinfo.

If the info can not be retrieved easily, we need to guess the type by looking up parent node, eg. variable assignment, parameter assignment, return statement etc.

The type checker records the type of each expression, but not the type of each "hole" that is filled by that expression. This would be very useful information for a wide range of analyses, but unfortunately it is not available. Gopl's completion engine goes to great lengths to infer it, but not in a way that is easily reused.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/617619 mentions this issue: gopls/internal: CodeAction: quickfix to generate missing method

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/621841 mentions this issue: gopls/internal: stubcalledfunction: improve logic of selecting insert position

gopherbot pushed a commit to golang/tools that referenced this issue Oct 28, 2024
… position

This CL adds the logic to insert generated method stubs
immediately after the enclosing method declaration when
the receiver types match. I often find myself move the
generated code to the position after current method immediately,
and this added logic is more consistent with undeclared.go's
generate missing function quickfix.

Related CL 617619
Updates golang/go#69692

Change-Id: Ia07422a0da7ee38ce648508dbb3e392f3dc8c93d
GitHub-Last-Rev: 0abde90
GitHub-Pull-Request: #537
Reviewed-on: https://go-review.googlesource.com/c/tools/+/621841
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
@findleyr findleyr modified the milestones: gopls/backlog, gopls/v0.17.0 Oct 30, 2024
@dennypenta
Copy link

@adonovan do you mind seeing a similar quick fix, but to declare a struct field? I couldn’t find it in the actual implementation.
If the idea is good, should I create another issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest 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

Successfully merging a pull request may close this issue.

7 participants