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

goimports fails to consider global variables in package #630

Closed
briantkennedy opened this issue Nov 19, 2016 · 5 comments
Closed

goimports fails to consider global variables in package #630

briantkennedy opened this issue Nov 19, 2016 · 5 comments

Comments

@briantkennedy
Copy link

I just updated to 0.6.49 and format on save is now broken for goimports. After looking at recent changes, I believe that b143a0d triggered the breakage.

The difference is that the text for the file is passed via stdin instead of providing the on-disk filename. The consequence of passing from stdin is that the goimports process is not able to determine what global variables exist within the package. For example, if the global variable "log" is declared foo.go, while saving bar.go which calls "log.Info" or some other function which the "log" package declares, the command will operate on local data and assume you want to import the "log" package rather than using the package global variable which results in the tool incorrectly adding the "log" import.

The fix for this is to do one of the following:

  1. Continue to pass the file contents via stdin and invoke as "goimports -d -srcdir [ filename ]" (see goimports help for the -srcdir flag semantics when a filename is specified)
  2. Do not pass the file contents via stdin and invoke "goimports -d [ filename ]"

Unfortunately, there doesn't appear to be a way to configure the current filename as a parameter in "go.FormatFlags" otherwise that would be an easy workaround.

@ramya-rao-a
Copy link
Contributor

The reason why I changed from passing the filename to passing the filecontents via stdin was that previously formatting an unsaved file, forcefully saved the file.

#613 is an issue that goimports didnt find the imports for vendor packages and so removed them.
The root cause was the change mentioned above.

I put a fix in where I pass the cwd to the process running the formatting tool, so that it could find the vendor packages.

I'll see if that fixes your case. If not will look into your suggestions.

Regardless, I'll push an update on Monday that will contain a fix to your issue

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Nov 21, 2016

I have reverted the said change in the latest update (0.6.50).
This should fix the issue for you

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Dec 7, 2017

@briantkennedy I am working on bringing back the "pass file contents via stdin to formatter" feature.
After digging around, I found that the srcdir option was added by you to goimports. https://go-review.googlesource.com/c/tools/+/23444

But goreturns whose feature set is suppose to be the superset of that of goimports doesn't seem to have this option.

Did you have any plans to add the same option to goreturns?
I have logged sqs/goreturns#40 for reference

@briantkennedy
Copy link
Author

Hi Ramya, thanks for reaching out, but I presently don't have any plans to add it to goreturns. It looks like sqs has plans to implement the feature request, so things should get resolved sooner or later.

@ramya-rao-a
Copy link
Contributor

Thanks @briantkennedy!

halfcrazy added a commit to halfcrazy/vscode-go that referenced this issue Dec 26, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 23, 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

No branches or pull requests

2 participants