Golang x/tools/gopls: Extract variable from type switch produces invalid code

gopls version

Build info

golang.org/x/tools/gopls v0.15.3 golang.org/x/tools/gopls@v0.15.3 h1:zbdOidFrPTc8Bx0YrN5QKgJ0zCjyGi0L27sKQ/bDG5o= github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y= golang.org/x/mod@v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= golang.org/x/sync@v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= golang.org/x/sys@v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= golang.org/x/telemetry@v0.0.0-20240209200032-7b892fcb8a78 h1:vcVnuftN4J4UKLRcgetjzfU9FjjgXUUYUc3JhFplgV4= golang.org/x/text@v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/tools@v0.18.1-0.20240412183611-d92ae0781217 h1:uH9jJYgeLCvblH0S+03kFO0qUDxRkbLRLFiKVVDl7ak= golang.org/x/vuln@v1.0.1 h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU= honnef.co/go/tools@v0.4.6 h1:oFEHCKeID7to/3autwsWfnuv69j3NsfcXbvJKuIcep8= mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo= mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8= go: go1.22.3 X:rangefunc

go env

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Chris\AppData\Local\go-build
set GOENV=C:\Users\Chris\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Chris\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Chris\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.3
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=C:\Users\Chris\proj\gopls-testing\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Chris\AppData\Local\Temp\go-build2552415688=/tmp/go-build -gno-record-gcc-switches

What did you do?

Create a module with the following code in VSCode and follow the instructions in the comment.

package main

import (
    "bytes"
    "fmt"
    "io"
    "strings"
)

func r() io.Reader {
    return &bytes.Buffer{}
}

func main() {
    switch r := r().(type) { // Highlight 'r()' and perform extract variable refactor
    case *bytes.Buffer:
        fmt.Println("buf", r.Available())
    case *strings.Reader:
        fmt.Println("str", r.Size())
    default:
        fmt.Println("ior")
    }
}

What did you see happen?

After the refactor the code changes to this:

package main

import (
    "bytes"
    "fmt"
    "io"
    "strings"
)

func r() io.Reader {
    return &bytes.Buffer{}
}

func main() {
    switch x := r()
    switch r := x.(type) { // Highlight 'r()' and perform extract variable refactor
    case *bytes.Buffer:
        fmt.Println("buf", r.Available())
    case *strings.Reader:
        fmt.Println("str", r.Size())
    default:
        fmt.Println("ior")
    }
}

What did you expect to see?

package main

import (
    "bytes"
    "fmt"
    "io"
    "strings"
)

func r() io.Reader {
    return &bytes.Buffer{}
}

func main() {
    x := r()
    switch r := x.(type) { // Highlight 'r()' and perform extract variable refactor
    case *bytes.Buffer:
        fmt.Println("buf", r.Available())
    case *strings.Reader:
        fmt.Println("str", r.Size())
    default:
        fmt.Println("ior")
    }
}

Editor and settings

VSCode

Logs

No response

Comment From: gabyhelp

Similar Issues

• https://github.com/golang/go/issues/57384
• https://github.com/golang/go/issues/52425
• https://github.com/golang/go/issues/59554
• https://github.com/golang/go/issues/60016
• https://github.com/golang/go/issues/56257

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

Comment From: ssd71

This might be because the enclosing statement of the Assignment statement is returned as the assignment statement itself instead of the Switch statement enclosing the actual Assignment, see here:

https://github.com/golang/tools/blob/03419175878954be2b792c470f49819e7e0f616e/internal/analysisinternal/analysis.go#L263-L273

Maybe something like an additional case would solve this issue:

--- a/internal/analysisinternal/analysis.go
+++ b/internal/analysisinternal/analysis.go
@@ -269,6 +269,8 @@ func StmtToInsertVarBefore(path []ast.Node) ast.Stmt {
                if expr.Init == enclosingStmt || expr.Post == enclosingStmt {
                        return expr
                }
+       case *ast.SwitchStmt, *ast.TypeSwitchStmt:
+               return expr.(ast.Stmt)
        }
        return enclosingStmt.(ast.Stmt)
 }

I can work on this if no one else is working on it

Comment From: gopherbot

Change https://go.dev/cl/591496 mentions this issue: gopls: fix StmtToInsertVarBefore for switch stmts