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

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

Closed
rothskeller opened this issue Jan 26, 2022 · 5 comments
Closed

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

rothskeller opened this issue Jan 26, 2022 · 5 comments
Assignees
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. 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

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.

@findleyr findleyr self-assigned this Nov 14, 2024
@findleyr findleyr modified the milestones: gopls/backlog, gopls/v0.17.0 Nov 19, 2024
@findleyr findleyr added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Nov 19, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/629376 mentions this issue: gopls/internal/golang/extract: preserve comments in extracted block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. 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
Development

No branches or pull requests

6 participants