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

[vikstrous2]

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 

[findleyr]

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

[gopherbot]

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