Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Format removes imports of vendored packages in use #613

Closed
sindrepm opened this issue Nov 11, 2016 · 38 comments
Closed

Format removes imports of vendored packages in use #613

sindrepm opened this issue Nov 11, 2016 · 38 comments

Comments

@sindrepm
Copy link

sindrepm commented Nov 11, 2016

I just upgraded to the latest version and discovered a bug. When I have imports like the following:

import (
   "github.com/xanzy/go-gitlab"
   "github.com/emicklei/go-restful"
)

Whenever I save the file, those import lines are removed even if they are referenced in the code. This has worked fine up until the last upgrade so not sure what has happened. From what I have found, it only applies to packages which contains a hyphen as shown above (e.g. "go-gitlab" and "go-restful").

I have been able to reference these packages just by the "gitlab" or "restful" names before, but now it seems that the goimports tool doesn't like that? If I alias the imports with "gitlab" or "restful" it works and the lines are not removed.

It is quite an annoying bug so hopefully this can be fixed.

@AlekSi
Copy link
Contributor

AlekSi commented Nov 11, 2016

it only applies to packages which contains a hyphen as shown above

For me it's gopkg.in/Shopify/sarama.v1. I think this is due to package name is different from import path's last element.

It's really annoying.

@AlekSi
Copy link
Contributor

AlekSi commented Nov 11, 2016

It works with "go.formatTool": "gofmt". So my guess it is something between goreturns, GOPATH and vendor/.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Nov 11, 2016

Can you run goreturns and/or goimports from the command line independent of VS Code and see if you still have the issue? This is to determine if the problem is with the formatting tool or with how VS Code uses the formatting tool.

@sindrepm
Copy link
Author

I have tried running goreturns, goimports and gofmt directly on the .go file, and it works fine. The import lines are not removed. However, saving from the editor it immediately removes those imports.

@ramya-rao-a
Copy link
Contributor

Can you share your settings?

@ramya-rao-a
Copy link
Contributor

I tried the below code and I get build errors(syntax error: unexpected -, expecting expression), but no imports get removed on save

package main

import (
    "fmt"
    "github.com/emicklei/go-restful"
)

func main() {

    fmt.Println("Hello")
    go-restful.BodyParameter("hello", "bye")
}

@AlekSi
Copy link
Contributor

AlekSi commented Nov 13, 2016

My settings don't have anything Go-related except "go.gopath" with a single path.

Most, if not all, tools for Go (including gofmt, goimports and goreturns) work by parsing code into AST, modifying AST and printing it back. If Go code has syntax errors, it can't be parsed, and the tool just exits with an error message. That's why import is not removed by tool – tool did nothing.

Go differentiates between the last element of import path and package name. For github.com/xanzy/go-gitlab, github.com/emicklei/go-restful and gopkg.in/Shopify/sarama.v1 package names are gitlab, restful and sarama. I suspect some part of vscode-go assumes that last part of import path equals package name. It's often, but not always, true.

@AlekSi
Copy link
Contributor

AlekSi commented Nov 13, 2016

I created a small example: https://github.com/AlekSi/vscode-bug

@ramya-rao-a
Copy link
Contributor

@AlekSi

Previously, the formatting tool (gofmt, goreturns, goimports) ran on the file on disk, which meant we had to save the file before running the format even if the user didn't want to.

As a means to fix that I had made the change to use stdin as input to the formatting tool instead of passing filename
This broke your scenario of vendor imports, because the formatting tool was unable to find the vendor packages.

Until the next update, as a workaround can you use gofmt as your choice of formatting tool?

@sindrepm
This need not be the same case as yours. Can you share a sample code where you are seeing the imports getting removed?

@ramya-rao-a ramya-rao-a reopened this Nov 14, 2016
@sindrepm
Copy link
Author

@ramya-rao-a

Thanks for the update. I am pretty sure my issue is the same as @AlekSi mentions. It can very easily be reproduce with this simplified code:

package main

import "github.com/xanzy/go-gitlab"

func main() {
    c := gitlab.Client{}
}

Try saving this file and the import line is removed. I do have github.com/xanzy/go-gitlab in my vendor folder.

Your workaround by changing go.formatTool to gofmt worked! With goreturns it fails.

@ramya-rao-a
Copy link
Contributor

@sindrepm Great, the pkg is a vendor pkg for you too. The next update (mostly end of this week), should have the fix

@ramya-rao-a ramya-rao-a changed the title FormatOnSave removes imports in use Format removes imports of vendored packages in use Nov 14, 2016
@ramya-rao-a
Copy link
Contributor

The latest update of the Go extension (0.6.50) has the fix for this.

@AlekSi
Copy link
Contributor

AlekSi commented Nov 24, 2016

Unfortunately, it doesn't work. Please try with https://github.com/AlekSi/vscode-bug.

@AlekSi
Copy link
Contributor

AlekSi commented Nov 24, 2016

Sorry, nevermind, that one is another issue (#623).

@sirianni
Copy link

I am seeing this behavior again. This worked for me a few days ago.

output

@ramya-rao-a
Copy link
Contributor

@sirianni can you try setting "go.formatTool" to "goimports"?

@ehzhang
Copy link

ehzhang commented Dec 22, 2017

I'm also seeing this behavior, just noticed today:

dec-21-2017 19-38-52

This is fixed if i set "go.formatTool": "gofmt". The import is removed when set to "goreturns". I don't have goimports installed.

@sirianni
Copy link

can you try setting "go.formatTool" to "goimports"?

Setting go.formatTool to goimports fixes the issue. Guess this is a bug in goreturns then?

@AlekSi
Copy link
Contributor

AlekSi commented Dec 22, 2017

Same here.

@ramya-rao-a
Copy link
Contributor

Yes, we moved to passing the file contents to the formatting tool instead of having the tool work on the file on disk.

When doing so gofmt and goimports work well, but goreturns has issues with vendor pkgs and global variables that conflict with methods from other pkgs

The same issue existed in goimports before, but is now fixed. I have logged an issue for goreturns to use the same fix. sqs/goreturns#40

Until that issue is fixed, use goimports

@xyi
Copy link

xyi commented Dec 25, 2017

I have the same issue which guide me to here. I think it should be reopened until the issue solved.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Dec 26, 2017

@xyi The fix needs to be done upstream in the goreturns tool. Am sure the author of the tool will welcome a PR if anybody here wants to give it a try. It should be a pretty simple fix.

  • Add new flag -srcdir just like the one taken by goimports
  • Pass the value to goimports

@ramya-rao-a
Copy link
Contributor

Re-opening this issue, so that as soon as we have the upstream issue fixed, the Go extension should be updated to pass the -srcdir flag to goreturns

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jan 5, 2018

This fix is now out in the latest update to the Go extension (0.6.71)
Thanks to @briantkennedy for the upstream fix to goimports and to @halfcrazy for porting the same fix to goreturns

@quetool
Copy link

quetool commented Jan 11, 2018

I have installed 0.6.73 and still happening. I uninstalled it in order to continue coding

@ramya-rao-a
Copy link
Contributor

@quetool Uninstalled what? Can you run Go: Install/Update Tools to ensure all the packages are up to date and try again?

@quetool
Copy link

quetool commented Jan 12, 2018

@ramya-rao-a uninstalled the Go extension. I do not have Go: Install/Update Tools.
captura de pantalla 2018-01-12 a la s 09 02 27

these is my bug (it's my twitter account) https://twitter.com/quetool/status/951539146056749057

@ramya-rao-a
Copy link
Contributor

@quetool Is the package that is getting deleted used elsewhere in the code? Formatting removes unused imports.

@quetool
Copy link

quetool commented Jan 12, 2018

@ramya-rao-a Yes, is used in some functions to send messages to slack. This is what you are asking?

captura de pantalla 2018-01-12 a la s 11 43 55

@ramya-rao-a
Copy link
Contributor

Can you update all the dependent Go tools and try again? You can run the command Go: Install/Update Tools from the command pallet. Its not available in the menu

@yaoweizhen
Copy link

yaoweizhen commented Jan 13, 2018

Formatting removes unused imports.

That makes no sense to remove unused imports quietly, instead, if it could be a warning.

@yaoweizhen
Copy link

Go: Install/Update Tools

Missing ">" , the command should be ">Go: Install/Update Tools"

@Newt0n
Copy link

Newt0n commented Feb 6, 2018

This issue still happening on 1.19.3(macOS), it's really really annoying for daily work, "goimports"/"goreturns" both don't work for me, any new fix?

@ramya-rao-a
Copy link
Contributor

@Newt0n Did you try updating the tools as suggested in #613 (comment)?

@viet-wego
Copy link

viet-wego commented Feb 7, 2018

VSCode Version 1.19.3
Go Extension: 0.6.73
Tried >Go: Install/Update Tools => the same
Updated go.formatTool to gofmt => solved
The problem comes from goreturns not this extension.
Thank you every one.

@Newt0n
Copy link

Newt0n commented Feb 7, 2018

@ramya-rao-a Yes, I've tried Go: Install/Update Tools but has no magic, only update go.formatTool to gofmt works, just like @VietTD .

Seems like goreturns not exists in Update List:

Installing 13 tools at /Users/Newton/go/bin
  gocode
  gopkgs
  go-outline
  go-symbols
  guru
  gorename
  gomodifytags
  goplay
  impl
  godef
  godoc
  golint
  gotests

Installing github.com/nsf/gocode SUCCEEDED
Installing github.com/uudashr/gopkgs/cmd/gopkgs SUCCEEDED
Installing github.com/ramya-rao-a/go-outline SUCCEEDED
Installing github.com/acroca/go-symbols SUCCEEDED
Installing golang.org/x/tools/cmd/guru SUCCEEDED
Installing golang.org/x/tools/cmd/gorename SUCCEEDED
Installing github.com/fatih/gomodifytags SUCCEEDED
Installing github.com/haya14busa/goplay/cmd/goplay SUCCEEDED
Installing github.com/josharian/impl SUCCEEDED
Installing github.com/rogpeppe/godef SUCCEEDED
Installing golang.org/x/tools/cmd/godoc SUCCEEDED
Installing github.com/golang/lint/golint SUCCEEDED
Installing github.com/cweill/gotests/... SUCCEEDED

All tools successfully installed. You're ready to Go :).

@halfcrazy
Copy link

@ramya-rao-a
Copy link
Contributor

@Newt0n, @VietTD

  • Add the setting "go.formatTools": "goreturns"
  • Delete goreturns binary from your GOPATH. @Newt0n for you it would be /Users/Newton/go/bin
  • Try manually formatting your Go file, you will be prompted that goreturns is missing and an option to install it. Choose "Install"

@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.