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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: typeDefinition should support jumping to type of function/method's single return value? #38589

Closed
myitcv opened this issue Apr 22, 2020 · 5 comments
Closed

x/tools/gopls: typeDefinition should support jumping to type of function/method's single return value? #38589

myitcv opened this issue Apr 22, 2020 · 5 comments
Labels
FeatureRequest 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
gopls/v0.7.0

Comments

@myitcv
Copy link
Member

myitcv commented Apr 22, 2020

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

$ go version
go version devel +801cd7c84d Thu Apr 2 09:00:44 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200408132156-9ee5ef7a2c0d
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20200408132156-9ee5ef7a2c0d

Does this issue reproduce with the latest release?

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/.vim/plugged/govim/go.mod"
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=/tmp/go-build031253818=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Consider the following example:

-- go.mod --
module mod.com

go 1.12
-- main.go --
package main

import "fmt"

type X int

func GetX() X {
	return 5
}

func main() {
	fmt.Println(GetX())
}

With the cursor within the GetX() call inside the call to fmt.Println(), a jump to definition correctly takes us to the definition of GetX().

However, a goto type definition results in an error:

[Trace - 11:30:11.115 AM] Sending request 'textDocument/typeDefinition - (3)'.
Params: {"textDocument":{"uri":"file:///home/myitcv/gostuff/src/github.com/myitcv/playground/main.go"},"position":{"line":11,"character":14}}


[Error - 11:30:11.116 AM] Received #3 start pos is not valid

Given GetX() returns a single value in this instance this could be made to work. Indeed having go to type definition work in the case a function/method returning a single value seems useful.

Thoughts?

What did you expect to see?

Go to type definition from the cursor position described above to jump to the definition of X.

What did you see instead?

An error.

cc @stamblerre

FYI @leitzler
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Apr 22, 2020
@gopherbot gopherbot added this to the Unreleased milestone Apr 22, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Apr 22, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 22, 2020
@stamblerre
Copy link
Contributor

That seems reasonable to me. Thanks for reporting.

@muirdm
Copy link

muirdm commented Apr 28, 2020

Consider the case:

type myFunc func(int) string

var foo myFunc

bar := foo()

If you run go-to-type-definition on "foo()" it should take you to the myFunc definition, not the "string" return value.

Most go-to-thingy commands operate on an identifier, not an expression, so this behavior would be inconsistent.

@stamblerre stamblerre added help wanted FeatureRequest and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 24, 2020
@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 24, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre stamblerre mentioned this issue Nov 13, 2020
Closed
@stamblerre
Copy link
Contributor

From @pjweinb on #42585:

Perhaps the user's intent is unambiguous, and gopls should respond to that?
Failing that, perhaps a more user-friendly error message?

@stamblerre stamblerre mentioned this issue Mar 15, 2021
Closed
@ShoshinNikita
Copy link

I think it's a good idea to support functions/methods with two return values when the second one is either error or bool. These patterns

func GetX() (x X, err error)
// or
func GetX() (x X, ok bool)

are more common than this one

func GetX() X

The implementation won't become much more difficult. What do you think?

ShoshinNikita added a commit to ShoshinNikita/tools that referenced this issue Apr 25, 2021
@ShoshinNikita
internal/lsp/source: support typeDefinition for function/method's ret…
8fa17f2 
…urn values

Support typeDefinition for functions with a single return value and
double return values if the second one is either bool or error:

* func foo() X
* func foo() (X, bool)
* func foo() (X, error)

Fixes golang/go#38589
@ShoshinNikita ShoshinNikita mentioned this issue Apr 25, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/313093 mentions this issue: internal/lsp/source: support typeDefinition for function/method's return values

@golang golang locked and limited conversation to collaborators May 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FeatureRequest 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
Milestone
gopls/v0.7.0
Development

Successfully merging a pull request may close this issue.

internal/lsp/source: support typeDefinition for function/method's return values ShoshinNikita/tools
5 participants
@myitcv @stamblerre @gopherbot @muirdm @ShoshinNikita