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: AllImportsFixes misbehavior edge case #47328

Closed
suntong opened this issue Jul 21, 2021 · 10 comments
Closed

x/tools/gopls: AllImportsFixes misbehavior edge case #47328

suntong opened this issue Jul 21, 2021 · 10 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@suntong
Copy link

suntong commented Jul 21, 2021

What version of Go are you using (go version)?

$ go version
go version go1.15.9 linux/amd64

$ gopls version
gopls, built in GOPATH mode master
    gopls, built in GOPATH mode@master

Does this issue reproduce with the latest release?

Yes, I'm using the latest gopls go code repo directly.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/me/.cache/go-build"
GOENV="/home/me/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/path/to/Go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/path/to/Go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.15"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.15/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build987591591=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Edit a special .go file under Emacs, using the LSP + go mode, using the following hook when saving the file:

  (defun lsp-go-install-save-hooks ()
    "LSP Go save hooks."
    (add-hook 'before-save-hook #'lsp-format-buffer t t)
    (add-hook 'before-save-hook #'lsp-organize-imports t t)
    )

What did you expect to see?

lsp-organize-imports and lsp-format-buffer do their jobs as expected.

What did you see instead?

My go code structure was totally messed up after the save:

image

and the changes doesn't make sense at all.

Steps to duplicate it

  • Take a look at this change:
    go-openwechat/owc-insight@63988c7
  • download the simple go project,
  • remove that extra space on line 94, and
  • do a save while in go LSP mode with the above save hook

Further explanation:

  • I wasn't able to make such change/correction in my desktop in the first place, and have to correct it with another editor to save the proper version first.
  • I then try to replicate the situation (first step is go-openwechat/owc-insight@63988c7 ) on my laptop, to repeat such change in my laptop and got the same result. I.e.,

Both edits will cause the .go file to be dramatically changed as above picture.
The change on both of my two machines are exactly the same.

  • My desktop is using Ubuntu 20.4 LTS
  • My laptop is using Debian 10 (buster) within Windows WSL

This is really a simple file/repo, and I have no idea how to make the case even simpler.
So please download the simple go project and make the above simple change and see what you got.

Thanks a lot for your help.

Further discussion:

  • Making other changes in other place of the file and save, is OK. The structure changed only when removing that extra space on that peculiar empty line.
  • Furthermore, removing second tab char while leave the first tab char still there and save, is OK too!!!
  • It is only when both the tab chars on that peculiar empty line are removed then save, when the structure get changed.

In my Emacs lsp-log buffer, I saw the error message is

imports fixes: AllImportsFixes: /home/me/l/gg/suntong/owc-insight/main.go:15:16: expected 'STRING', found string (and 20 more errors)
	file="/home/me/l/gg/suntong/owc-insight/main.go"

which is pointing to:

	"github.com/eatMoreApple/openwechat"
	 -------------^

Don't know if the unusual uppercase package name has anything to do with the problem.

@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 Jul 21, 2021
@gopherbot gopherbot added this to the Unreleased milestone Jul 21, 2021
@findleyr
Copy link
Contributor

Thanks for filing this issue!

I tried to reproduce, but am not able. Before trying to replicate your exact set-up, I think we should try a couple things:

  1. Most importantly, capture LSP logs from a repro. Instructions here. If you could attach the complete logs for a session where you reproduce the bug, that would be very helpful for us.
  2. Try with a release version of gopls. Your gopls is at master. Instruction here.

@suntong
Copy link
Author

suntong commented Jul 22, 2021

Thanks for checking will do...

@stamblerre stamblerre added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 22, 2021
@suntong
Copy link
Author

suntong commented Jul 22, 2021

I managed to have my emacs lsp-mode to start gopls with the -rpc.trace flag. However, I don't need to attach the complete logs for a session any more, because.

Using a release version of gopls instead of master fixes the problem!

So the edge case only exhibits in master.

I'm happy myself for now. If anyone interested to dig deeper in this edge case, please let me know. thx!

@findleyr
Copy link
Contributor

Great to hear, but we'd of course be very interested in fixing a bug at master :)

Could you try with gopls built at the current master? It's it possible your gopls was very old?

@suntong
Copy link
Author

suntong commented Jul 22, 2021

OK.

The following is after a fresh go get -v -u golang.org/x/tools/gopls:

From my Emacs lsp-log buffer:

Command "gopls -logfile /tmp/gopls.log -rpc.trace" is present on the path.
Command "gopls -logfile /tmp/gopls.log -rpc.trace" is present on the path.
Found the following clients for /home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go: (server-id gopls, priority 0)
The following clients were selected based on priority: (server-id gopls, priority 0)
2021/07/22 15:57:18 go env for /home/tong/l/wk/Go/src/github.com/suntong/owc-insight
(root /home/tong/l/wk/Go/src/github.com/suntong/owc-insight)
(go version go version go1.15.9 linux/amd64)
(valid build configuration = true)
(build flags: [])
GOFLAGS=
GOINSECURE=
GONOPROXY=
GONOSUMDB=
GOMOD=
GOPRIVATE=
GOPROXY=https://proxy.golang.org,direct
GOPATH=/home/tong/l/g:/home/tong/l/wk/Go
GO111MODULE=off
GOMODCACHE=/home/tong/l/g/pkg/mod
GOCACHE=/home/tong/.cache/go-build
GOROOT=/usr/lib/go-1.15
GOSUMDB=sum.golang.org


2021/07/22 15:57:18 go/packages.Load
	snapshot=0
	directory=/home/tong/l/wk/Go/src/github.com/suntong/owc-insight
	query=[./... builtin]
	packages=2

Creating watchers for following 1 folders:
  /lfs/workv/wv/wk/Go/src/github.com/suntong/owc-insight
Failed to create a watch for No file notification package available: message
2021/07/22 15:57:18 falling back to safe trimming due to type errors: [/usr/lib/go-1.15/src/runtime/vdso_linux.go:54:38: invalid operation: division by zero /usr/lib/go-1.15/src/runtime/vdso_linux.go:55:38: invalid operation: division by zero] or still-missing identifiers: map[memRecordCycle:true pageBits:true]
	package="runtime"

2021/07/22 15:57:18 discovered missing identifiers: map[options:true]
	package="vendor/golang.org/x/text/unicode/bidi"

2021/07/22 15:57:18 discovered missing identifiers: map[encoder:true]
	package="image/png"

2021/07/22 15:57:48 background imports cache refresh starting

2021/07/22 15:57:48 background refresh finished after 16.524427ms

2021/07/22 15:58:06 imports fixes: AllImportsFixes: /home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go:15:16: expected 'STRING', found string (and 20 more errors)
	file="/home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go"

2021/07/22 15:58:06 go/packages.Load
	snapshot=50
	directory=/home/tong/l/wk/Go/src/github.com/suntong/owc-insight
	query=[file=/home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go]
	packages=1

2021/07/22 15:58:06 go/packages.Load
	snapshot=50
	package="command-line-arguments"
	files=[/home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go]

2021/07/22 15:58:06 go/packages.Load
	snapshot=50
	directory=/home/tong/l/wk/Go/src/github.com/suntong/owc-insight
	query=[file=/home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go]
	packages=1

2021/07/22 15:58:06 go/packages.Load
	snapshot=50
	package="command-line-arguments"
	files=[/home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go]

2021/07/22 15:58:06 workspace package _/home/tong/l/wk/Go/src/github.com/suntong/owc-insight not associated with file:///home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go
	level=1

2021/07/22 15:58:06 go/packages.Load
	snapshot=51
	directory=/home/tong/l/wk/Go/src/github.com/suntong/owc-insight
	query=[_/home/tong/l/wk/Go/src/github.com/suntong/owc-insight]
	packages=1

2021/07/22 15:58:06 go/packages.Load
	snapshot=51
	package="_/home/tong/l/wk/Go/src/github.com/suntong/owc-insight"
	files=[]

2021/07/22 15:58:06 reloadOrphanedFiles reloading
	query=[file:///home/tong/l/wk/Go/src/github.com/suntong/owc-insight/logging.go]

2021/07/22 15:58:06 reloadOrphanedFiles reloading
	query=[file:///home/tong/l/wk/Go/src/github.com/suntong/owc-insight/logging.go]

2021/07/22 15:58:06 reloadOrphanedFiles reloading
	query=[file:///home/tong/l/wk/Go/src/github.com/suntong/owc-insight/logging.go]

2021/07/22 15:58:06 imports fixes: AllImportsFixes: /home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go:15:16: expected 'STRING', found string (and 20 more errors)
	file="/home/tong/l/wk/Go/src/github.com/suntong/owc-insight/main.go"

2021/07/22 15:58:31 background imports cache refresh starting

2021/07/22 15:58:31 background refresh finished after 16.217481ms

The gopls.log is over 63k, so I'll attach instead.
gopls.log

@findleyr
Copy link
Contributor

@suntong can you please share the output of go version -m $(which gopls) (assuming which gopls returns the faulty gopls you just built).

@suntong
Copy link
Author

suntong commented Jul 22, 2021

NP.

$ go version -m $(which gopls)
/home/tong/l/wk/Go/bin/gopls: go1.15.9

$ go version
go version go1.15.9 linux/amd64

This is the only go & gopls that I have.
and it is from my desktop.

$ lsb_release -a 
No LSB modules are available.
Distributor ID:	Debian
Description:	Debian GNU/Linux bullseye/sid
Release:	testing
Codename:	bullseye

$ uname -rm 
5.10.0-6-amd64 x86_64

@hyangah
Copy link
Contributor

hyangah commented Jul 23, 2021

It looks like that gopls (/home/tong/l/wk/Go/bin/gopls) was indeed built in GOPATH mode.

Please follow the installation instruction as described in https://github.com/golang/tools/tree/master/gopls#installation https://github.com/golang/tools/blob/master/gopls/doc/advanced.md#unstable-versions i.e., run the following command from a directory where there is no go.mod. I.e. home directory or a temporary directory.

GO111MODULE=on go get golang.org/x/tools/gopls@master golang.org/x/tools@master

gopls -v version should output the build info like this:

$ gopls -v version
Build info
----------
golang.org/x/tools/gopls master
    golang.org/x/tools/gopls@v0.0.0-20210722172326-c740bfd9b268 h1:2iG2PzHcZHm8yHmSrAAe70cVJPzLDMIpb4SOTEM+NpA=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/google/go-cmp@v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/mod@v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
    golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ=
    golang.org/x/sys@v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
    golang.org/x/tools@v0.1.3-0.20210608163600-9ed039809d4c h1:Pv9gNyJFYVdpUAVZYJ1BDSU4eGgXQ+0f3DIGAdolO5s=
    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
    honnef.co/go/tools@v0.2.0 h1:ws8AfbgTX3oIczLPNPCu5166oBg9ST2vNs0rcht+mDE=
    mvdan.cc/gofumpt@v0.1.1 h1:bi/1aS/5W00E2ny5q65w9SnKpWEF/UIOqDYBILpo9rA=
    mvdan.cc/xurls/v2@v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=

Then, if the problem keeps occurring, please share the gopls trace again. Thanks!

FYI it's no longer recommended to use go get -u for tools installation with newer versions of Go.

EDIT: update the installation instruction to get the unstable version built at the master.

@findleyr
Copy link
Contributor

FWIW, I suspected that the problem is sergi/go-diff@v1.2. It would be nice to verify that gopls works at master when not installed with -u.

@suntong
Copy link
Author

suntong commented Jul 24, 2021

I suspected that the problem is sergi/go-diff@v1.2. It would be nice to verify that gopls works at master when not installed with -u.

Yep, I used the installation instruction from @hyangah to get the unstable version built at the master, getting exact output of gopls -v version as him, and now the problem is gone.

Thanks a lot everyone!

@suntong suntong closed this as completed Jul 24, 2021
@golang golang locked and limited conversation to collaborators Jul 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants