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

Jump to bottom

x/tools/gopls: "Extract Method" leaves comments behind #50851

Open
rothskeller opened this issue Jan 26, 2022 · 4 comments
Open

x/tools/gopls: "Extract Method" leaves comments behind #50851

rothskeller opened this issue Jan 26, 2022 · 4 comments
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/backlog

Comments

@rothskeller
Copy link

What version of Go, VS Code & VS Code Go extension are you using?

Version Information

  • Run go version to get version of Go from the VS Code integrated terminal.

    • go version go1.17 darwin/amd64

  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.

    • golang.org/x/tools/gopls v0.7.5

  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.

    • 1.63.2

  • Check your installed extensions to get the version of the VS Code Go extension

    • 0.30.0

  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.

    • Checking configured tools....
      GOBIN: undefined
      toolsGopath:
      gopath: /Users/stever/go
      GOROOT: /usr/local/go
      PATH: /usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Library/Apple/usr/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/Users/stever/go/bin

    go: /usr/local/go/bin/go: go version go1.17 darwin/amd64

    gopkgs: /Users/stever/go/bin/gopkgs: go1.17
    path github.com/uudashr/gopkgs/v2/cmd/gopkgs
    mod github.com/uudashr/gopkgs/v2 v2.1.2 h1:A0+QH6wqNRHORJnxmqfeuBEsK4nYQ7pgcOHhqpqcrpo=
    dep github.com/karrick/godirwalk v1.12.0 h1:nkS4xxsjiZMvVlazd0mFyiwD4BR9f3m6LXGhM2TUx3Y=
    dep github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=

    go-outline: /Users/stever/go/bin/go-outline: go1.17
    path github.com/ramya-rao-a/go-outline
    mod github.com/ramya-rao-a/go-outline v0.0.0-20210608161538-9736a4bde949 h1:iaD+iVf9xGfajsJp+zYrg9Lrk6gMJ6/hZHO4cYq5D5o=
    dep golang.org/x/tools v0.1.1 h1:wGiQel/hW0NnEkJUk8lbzkX2gFJU6PFxf1v5OlCfuOs=

    gotests: /Users/stever/go/bin/gotests: go1.17
    path github.com/cweill/gotests/gotests
    mod github.com/cweill/gotests v1.6.0 h1:KJx+/p4EweijYzqPb4Y/8umDCip1Cv6hEVyOx0mE9W8=
    dep golang.org/x/tools v0.0.0-20191109212701-97ad0ed33101 h1:LCmXVkvpQCDj724eX6irUTPCJP5GelFHxqGSWL2D1R0=

    gomodifytags: /Users/stever/go/bin/gomodifytags: go1.17
    path github.com/fatih/gomodifytags
    mod github.com/fatih/gomodifytags v1.13.0 h1:fmhwoecjZ5c34Q2chjRB9cL8Rgag+1TOSMy+grissMc=
    dep github.com/fatih/camelcase v1.0.0 h1:hxNvNX/xYBp0ovncs8WyWZrOrpBNub/JfaMvbURyft8=
    dep github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4=
    dep golang.org/x/tools v0.0.0-20180824175216-6c1c5e93cdc1 h1:EAPsk8kfGCjxQagrkWjzXlUWe2p3gj5MknO+z2o9GKc=

    impl: /Users/stever/go/bin/impl: go1.17
    path github.com/josharian/impl
    mod github.com/josharian/impl v1.1.0 h1:gafhg1OFVMq46ifdkBa8wp4hlGogjktjjA5h/2j4+2k=
    dep golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
    dep golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375 h1:SjQ2+AKWgZLc1xej6WSzL+Dfs5Uyd5xcZH1mGC411IA=
    dep golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=

    goplay: /Users/stever/go/bin/goplay: go1.17
    path github.com/haya14busa/goplay/cmd/goplay
    mod github.com/haya14busa/goplay v1.0.0 h1:ED4BMrGQ3WH7H3YXrcnWMVzj1xeSepaYTkLh1DtFi/4=
    dep github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 h1:JIAuq3EEf9cgbU6AtGPK4CTG3Zf6CKMNqf0MHTggAUA=

    dlv: /Users/stever/go/bin/dlv: go1.17
    path github.com/go-delve/delve/cmd/dlv
    mod github.com/go-delve/delve v1.7.0 h1:MaWAD3LtvjE/LL98urSHPjaMT+OubpQ2sqF3R2Uj1rc=
    dep github.com/cosiner/argv v0.1.0 h1:BVDiEL32lwHukgJKP87btEPenzrrHUjajs/8yzaqcXg=
    dep github.com/cpuguy83/go-md2man v1.0.10 h1:BSKMNlYxDvnunlTymqtgONjNnaRV1sTpcovwwjF22jk=
    dep github.com/google/go-dap v0.5.0 h1:RMHAVn5xeunBakYk65ggHXttk6qjZVdbmi+xhAoL2wY=
    dep github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
    dep github.com/mattn/go-isatty v0.0.3 h1:ns/ykhmWi7G9O+8a448SecJU3nSMBXJfqQkl0upE1jI=
    dep github.com/peterh/liner v0.0.0-20170317030525-88609521dc4b h1:8uaXtUkxiy+T/zdLWuxa/PG4so0TPZDZfafFNNSaptE=
    dep github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNueLj0oo=
    dep github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I=
    dep github.com/spf13/cobra v0.0.0-20170417170307-b6cb39589372 h1:eRfW1vRS4th8IX2iQeyqQ8cOUNOySvAYJ0IUvTXGoYA=
    dep github.com/spf13/pflag v0.0.0-20170417173400-9e4c21054fa1 h1:7bozMfSdo41n2NOc0GsVTTVUiA+Ncaj6pXNpm4UHKys=
    dep go.starlark.net v0.0.0-20200821142938-949cc6f4b097 h1:YiRMXXgG+Pg26t1fjq+iAjaauKWMC9cmGFrtOEuwDDg=
    dep golang.org/x/arch v0.0.0-20190927153633-4e8777c89be4 h1:QlVATYS7JBoZMVaf+cNjb90WD/beKVHnIxFKT4QaHVI=
    dep golang.org/x/sys v0.0.0-20200625212154-ddb9806d33ae h1:Ih9Yo4hSPImZOpfGuA4bR/ORKTAbhZo2AbWNRCnevdo=
    dep gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE=

    dlv-dap: /Users/stever/go/bin/dlv-dap: go1.17
    path github.com/go-delve/delve/cmd/dlv
    mod github.com/go-delve/delve v1.7.3-0.20211026171155-b48ceec161d5 h1:YBUOq0F2HHjVo6aeY9Y5PB69C0tSVFxxex3Ea4aTQms=
    dep github.com/cosiner/argv v0.1.0 h1:BVDiEL32lwHukgJKP87btEPenzrrHUjajs/8yzaqcXg=
    dep github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng4PGlyM=
    dep github.com/derekparker/trie v0.0.0-20200317170641-1fdf38b7b0e9 h1:G765iDCq7bP5opdrPkXk+4V3yfkgV9iGFuheWZ/X/zY=
    dep github.com/google/go-dap v0.6.0 h1:Y1RHGUtv3R8y6sXq2dtGRMYrFB2hSqyFVws7jucrzX4=
    dep github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
    dep github.com/mattn/go-isatty v0.0.3 h1:ns/ykhmWi7G9O+8a448SecJU3nSMBXJfqQkl0upE1jI=
    dep github.com/mattn/go-runewidth v0.0.3 h1:a+kO+98RDGEfo6asOGMmpodZq4FNtnGP54yps8BzLR4=
    dep github.com/peterh/liner v1.2.1 h1:O4BlKaq/LWu6VRWmol4ByWfzx6MfXc5Op5HETyIy5yg=
    dep github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
    dep github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo=
    dep github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I=
    dep github.com/spf13/cobra v1.1.3 h1:xghbfqPkxzxP3C/f3n5DdpAbdKLj4ZE4BWQI362l53M=
    dep github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
    dep go.starlark.net v0.0.0-20200821142938-949cc6f4b097 h1:YiRMXXgG+Pg26t1fjq+iAjaauKWMC9cmGFrtOEuwDDg=
    dep golang.org/x/arch v0.0.0-20190927153633-4e8777c89be4 h1:QlVATYS7JBoZMVaf+cNjb90WD/beKVHnIxFKT4QaHVI=
    dep golang.org/x/sys v0.0.0-20210514084401-e8d321eab015 h1:hZR0X1kPW+nwyJ9xRxqZk1vx5RUObAPBdKVvXPDUH/E=
    dep gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=

    golint: /Users/stever/go/bin/golint: go1.17
    path golang.org/x/lint/golint
    mod golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug=
    dep golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7 h1:EBZoQjiKKPaLbPrbpssUfuHtwM6KV/vb4U85g/cigFY=

    gopls: /Users/stever/go/bin/gopls: go1.17
    path golang.org/x/tools/gopls
    mod golang.org/x/tools/gopls v0.7.5 h1:8Az52YwcFXTWPvrRomns1C0N+zlgTyyPKWvRazO9GG8=
    dep github.com/BurntSushi/toml v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw=
    dep github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
    dep github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    dep golang.org/x/mod v0.5.1 h1:OJxoQ/rynoF0dcCdI7cLPktw/hR2cueqYfjm43oqK38=
    dep golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
    dep golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
    dep golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
    dep golang.org/x/tools v0.1.9-0.20220114220130-fd7798718afd h1:lTnuArxJC+n54TyvWUPyHhrnGxYvhSi13/aM2Ndr4bs=
    dep golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    dep honnef.co/go/tools v0.2.1 h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=
    dep mvdan.cc/gofumpt v0.1.1 h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
    dep mvdan.cc/xurls/v2 v2.3.0 h1:59Olnbt67UKpxF1EwVBopJvkSUBmgtb468E4GVWIZ1I=

go env
Workspace Folder (schola8m): /Users/stever/src/schola8m
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/stever/Library/Caches/go-build"
GOENV="/Users/stever/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/stever/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/stever/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/stever/src/schola8m/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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/3y/936sl4d50dd_38rvrs041qn80000gn/T/go-build2358998281=/tmp/go-build -gno-record-gcc-switches -fno-common"

Share the Go related settings you have added/edited

Run Preferences: Open Settings (JSON) command to open your settings.json file.
Share all the settings with the go. or ["go"] or gopls prefixes.

  "go.autocompleteUnimportedPackages": true,
  "go.formatTool": "goimports",
  "go.gopath": "/Users/stever/go",
  "go.lintFlags": [],
  "go.lintTool": "golint",
  "go.liveErrors": {
    "enabled": false,
    "delay": 500
  },
  "[go]": {
    "editor.tabSize": 8,
    "editor.insertSpaces": false
  },
  "go.useLanguageServer": true,
  "go.toolsManagement.autoUpdate": true,

Describe the bug

When you select a block of code that has comments in it, and choose the "Extract Method" command, it extracts the statements from that block into a new method, but it leaves the comments in the original location. This is unhelpful because the comments are explaining particular operations in the code that was extracted, and they are now distanced from the statements they explain. What it should do is extract the entire block of code including its comments.

Steps to reproduce the behavior:

  1. Create this file in an empty directory:
package main

type F struct{}

func (f *F) func1() {
	println("a")

	println("b")
	// This line prints the third letter of the alphabet.
	println("c")

	println("d")
}
  1. Select lines 8 through 10 (the two middle printlns).
  2. Click on the lightbulb icon and select "Extract method".

What I see as a result is:

package main

type F struct{}

func (f *F) func1() {
	println("a")

	// This line prints the third letter of the alphabet.
	f.newMethod()

	println("d")
}

func (*F) newMethod() {
	println("b")

	println("c")
}

The comment was left in func1, when it should have stayed above the println("c") in newMethod.
@hyangah
Copy link
Contributor

hyangah commented Jan 27, 2022

Thanks @rothskeller for the report with the simple repro case.

This is a bug in gopls, so I am transferring this to the gopls issue tracker.

@hyangah hyangah changed the title Jan 27, 2022
@hyangah hyangah transferred this issue from golang/vscode-go Jan 27, 2022
@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 Jan 27, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jan 27, 2022
@hyangah hyangah modified the milestones: Unreleased, gopls/on-deck Jan 27, 2022
@hyangah hyangah added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 27, 2022
@ShoshinNikita
Copy link

I think it's not a bug but a limitation of the current implementation of Extract function/method code action.

Other limitations are described here - #37170 (comment). Also see this CL - https://golang.org/cl/313211 (internal/lsp: print comments that would be lost during extract func)

@rothskeller
Copy link
Author

Thanks, @ShoshinNikita, for giving references to suggest that this problem is already known. However, the distinction between bug and limitation is pointless: either way, it is code that does not do what users reasonably expect it to do. At minimum it seems worth having this issue open to track the issue until it can be corrected.

@findleyr findleyr added the Refactoring Issues related to refactoring tools label Apr 11, 2022
@sam-mathews
Copy link

I've just stumbled across this. I agree that from a user perspective it's surprising behaviour. I switched back to manually extracting functions/methods, otherwise I have to comb through and pull each comment across.

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. NeedsFix The path to resolution is known, but the work has not been done. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/backlog
Development

No branches or pull requests
6 participants
@rothskeller @hyangah @gopherbot @ShoshinNikita @findleyr @sam-mathews