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: Extract Function refactor doesn't put the context first #60738

Closed
vikstrous2 opened this issue Jun 12, 2023 · 2 comments
Closed

x/tools/gopls: Extract Function refactor doesn't put the context first #60738

vikstrous2 opened this issue Jun 12, 2023 · 2 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.13.0

Comments

@vikstrous2
Copy link

vikstrous2 commented Jun 12, 2023 •
edited

gopls version

gopls -v version
Build info
----------
golang.org/x/tools/gopls v0.12.1
    golang.org/x/tools/gopls@v0.12.2 h1:s4mznBqCHVHeuTYjxGYJhsW3Wp4FYon8YAqiJlwlpTY=
    github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
    github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/exp@v0.0.0-20220722155223-a9213eeb770e h1:+WEEuIdZHnUeJJmEUjyYC2gfUMj69yZXw17EnHg/otA=
    golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
    golang.org/x/mod@v0.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk=
    golang.org/x/sync@v0.2.0 h1:PUR+T4wwASmuSTYdKjYHI5TD22Wy5ogLU5qZCOLxBrI=
    golang.org/x/sys@v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
    golang.org/x/text@v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE=
    golang.org/x/tools@v0.9.4-0.20230601215739-41a7dc76dd8f h1:rZCNEwRXLwl4xnXOEYY7jTuTh3+xNnsq8IfB2idOhD0=
    golang.org/x/vuln@v0.0.0-20230110180137-6ad3e3d07815 h1:A9kONVi4+AnuOr1dopsibH6hLi1Huy54cbeJxnq4vmU=
    honnef.co/go/tools@v0.4.2 h1:6qXr+R5w+ktL5UkwEbPp+fEvfyoMPche6GkOpGHZcLc=
    mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
    mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.20.4

go env

probably not relevant

What did you do?

I'm trying to use the extract function refactoring feature, but it doesn't order the parameters the way I would expect. It doesn't put the context first. Here's a one file example:

package main

import (
	"context"
	"net/http"
)

func main() {
	ctx := context.Background()
	aUrl := "exmaple.com"
	ptrUrl := &aUrl

	// Refactor start
	derefUrl := *ptrUrl
	r, err := http.NewRequestWithContext(ctx, http.MethodGet, derefUrl, nil)
	// Refactor end

	if err != nil {
		panic(err)
	}
	_ = r
}

Edit: the pointer stuff is not relevant. It turns out that the arguments are in the order in which they appear in the body of the function.

What did you expect to see?

package main

import (
	"context"
	"net/http"
)

func main() {
	ctx := context.Background()
	aUrl := "exmaple.com"
	ptrUrl := &aUrl
	r, err := newFunction(ctx, ptrUrl)
	if err != nil {
		panic(err)
	}
	_ = r
}

func newFunction( ctx context.Context, ptrUrl *string) (*http.Request, error) {
	derefUrl := *ptrUrl
	r, err := http.NewRequestWithContext(ctx, http.MethodGet, derefUrl, nil)
	return r, err
}

What did you see instead?

package main

import (
	"context"
	"net/http"
)

func main() {
	ctx := context.Background()
	aUrl := "exmaple.com"
	ptrUrl := &aUrl
	r, err := newFunction(ptrUrl, ctx)
	if err != nil {
		panic(err)
	}
	_ = r
}

func newFunction(ptrUrl *string, ctx context.Context) (*http.Request, error) {
	derefUrl := *ptrUrl
	r, err := http.NewRequestWithContext(ctx, http.MethodGet, derefUrl, nil)
	return r, err
}

Editor and settings

coder/code-server (fork of vs code) https://github.com/coder/code-server version 1.77.3

Logs

Probably not relevant, but I see these logs:

[Error - 1:38:39 PM] Request textDocument/semanticTokens/range failed.
  Message: semantictokens are disabled
  Code: 0 
[Error - 1:38:39 PM] Request textDocument/semanticTokens/full failed.
  Message: semantictokens are disabled
  Code: 0
@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 12, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jun 12, 2023
@vikstrous2 vikstrous2 mentioned this issue Jun 14, 2023
Closed
@findleyr
Copy link
Contributor

Thanks for taking this on! Left some comments on your CL, but generally this looks like a good change!

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.13.0 Jun 15, 2023
vikstrous2 added a commit to vikstrous2/tools that referenced this issue Jun 17, 2023
@vikstrous2
gopls/internal/lsp/source: put context first in extracted functions
a979900 
Put the context first when extracting functions/methods

Fixes golang/go#60738
@gopherbot
Copy link

Change https://go.dev/cl/503295 mentions this issue: gopls/internal/lsp/source: put context first in extracted functions

@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.13.0 Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.13.0
Development

Successfully merging a pull request may close this issue.

gopls/internal/lsp/source: put context first in extracted functions vikstrous2/tools
3 participants
@gopherbot @vikstrous2 @findleyr