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: organizeImports disabled when gofumpt is used #44136

Closed
bhcleek opened this issue Feb 6, 2021 · 12 comments
Closed

x/tools/gopls: organizeImports disabled when gofumpt is used #44136

bhcleek opened this issue Feb 6, 2021 · 12 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.

Comments

@bhcleek
Copy link
Contributor

bhcleek commented Feb 6, 2021

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

$ go version
go version go1.15.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/bhcleek/go/bin"
GOCACHE="/Users/bhcleek/Library/Caches/go-build"
GOENV="/Users/bhcleek/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/bhcleek/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/bhcleek/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"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/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 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/f9/d6g779t53gq_20w0zm2f_1zm0000gn/T/go-build652228814=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I enabled gofumpt and a local imports value for gopls and tried to organize imports with gopls.

What did you expect to see?

The imports organized and the local modules put in their own section.

What did you see instead?

The imports were not organized.

Here are the relevant logs when gofumpt is not enabled (the last log is response to the codeAction/organizeImports request):

gopls logs
===== sent =====
Content-Length: 681

{"method":"initialize","jsonrpc":"2.0","id":1,"params":{"rootUri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","capabilities":{"workspace":{"workspaceFolders":true,"configuration":true,"didChangeConfiguration":{"dynamicRegistration":true}},"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["source.organizeImports","refactor.rewrite"]}}},"completion":{"completionItem":{"snippetSupport":true}},"hover":{"contentFormat":["plaintext"]}}},"processId":31705,"workspaceFolders":[{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","name":"/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options"}]}}

===== received =====

Content-Length: 260


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:41:53 Handshake session update\n\tupdate_session=5\n\tdebug_address="127.0.0.1:52121"\n\tlogfile=""\n\tserver="1"\n\tgopls_path="/Users/bhcleek/go/bin/gopls"\n"}}

===== stderr =====

serve.go:399: debug server listening at http://localhost:52121

===== received =====

Content-Length: 2733


{"jsonrpc":"2.0","result":{"capabilities":{"textDocumentSync":{"change":2,"openClose":true,"save":{}},"completionProvider":{"triggerCharacters":["."]},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor.extract","refactor.rewrite","source.fixAll","source.organizeImports"]},"codeLensProvider":{},"documentLinkProvider":{},"workspaceSymbolProvider":true,"documentFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"renameProvider":true,"foldingRangeProvider":true,"executeCommandProvider":{"commands":["gopls.generate","gopls.fill_struct","gopls.regenerate_cgo","gopls.test","gopls.tidy","gopls.undeclared_name","gopls.add_dependency","gopls.upgrade_dependency","gopls.remove_dependency","gopls.vendor","gopls.extract_variable","gopls.extract_function","gopls.gc_details","gopls.generate_gopls_mod"]},"callHierarchyProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":"workspace/didChangeWorkspaceFolders"}}},"serverInfo":{"name":"gopls","version":"{"path":"golang.org/x/tools/gopls","version":"v0.5.4","sum":"h1:PkQWDfzw8CxoseHDQniF2GhzmndN8KnQL6gcG2gnF3Y=","deps":[{"path":"github.com/BurntSushi/toml","version":"v0.3.1","sum":"h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ="},{"path":"github.com/google/go-cmp","version":"v0.5.1","sum":"h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k="},{"path":"github.com/sergi/go-diff","version":"v1.1.0","sum":"h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0="},{"path":"golang.org/x/mod","version":"v0.3.0","sum":"h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4="},{"path":"golang.org/x/sync","version":"v0.0.0-20201020160332-67f06af15bc9","sum":"h1:SQFwaSi55rU7vdNs9Yr0Z324VNlrF+0wMqRXT4St8ck="},{"path":"golang.org/x/tools","version":"v0.0.0-20201123203206-735e963e909b","sum":"h1:U8XGResmZKfdo8rCjWPsuiB4cu7N0PJSpQwc9lZr9Rg="},{"path":"golang.org/x/xerrors","version":"v0.0.0-20200804184101-5ec99f83aff1","sum":"h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE="},{"path":"honnef.co/go/tools","version":"v0.0.1-2020.1.6","sum":"h1:W18jzjh8mfPez+AwGLxmOImucz/IFjpNlrKVnaj2YVc="},{"path":"mvdan.cc/gofumpt","version":"v0.0.0-20200927160801-5bfeb2e70dd6","sum":"h1:z+/YqapuV7VZPvBb3GYmuEJbA88M3PFUxaHilHYVCpQ="},{"path":"mvdan.cc/xurls/v2","version":"v2.2.0","sum":"h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A="}]}"}},"id":1}

===== sent =====

Content-Length: 52


{"method":"initialized","jsonrpc":"2.0","params":{}}

===== sent =====

Content-Length: 252


{"method":"workspace/didChangeWorkspaceFolders","jsonrpc":"2.0","params":{"event":{"added":[{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","name":"/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options"}],"removed":[]}}}

===== sent =====

Content-Length: 554


{"method":"textDocument/didOpen","jsonrpc":"2.0","params":{"textDocument":{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go","version":2,"languageId":"go","text":"package main\n\nimport (\n\t"context"\n\t"fmt"\n\t"mymod/pkg1"\n\t"mymod/pkg2"\n\n\t"github.com/pkg/errors"\n)\n\nfunc main() {\n\tfmt.Println("vim-go")\n}\n\nfunc dummyFunc(ctx context.Context) {\n\tfmt.Println(ctx)\n\tpkg1.Foo()\n\tpkg2.Bar()\n\terr := errors.Wrap(errors.New("some error"), "some wrapping info")\n\tfmt.Println(err)\n}\n"}}}

===== received =====

Content-Length: 99


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":4,"message":"Loading packages..."}}

===== received =====

Content-Length: 179


{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[{"scopeUri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","section":"gopls"}]},"id":1}

===== sent =====

Content-Length: 154


{"id":1,"jsonrpc":"2.0","result":[{"usePlaceholders":true,"buildFlags":[],"hoverKind":"Structured","staticcheck":true,"matcher":"fuzzy","local":"mymod"}]}

===== received =====

Content-Length: 268


{"jsonrpc":"2.0","method":"client/registerCapability","params":{"registrations":[{"id":"workspace/didChangeConfiguration","method":"workspace/didChangeConfiguration"},{"id":"workspace/didChangeWorkspaceFolders","method":"workspace/didChangeWorkspaceFolders"}]},"id":2}

===== sent =====

Content-Length: 38


{"id":2,"jsonrpc":"2.0","result":null}

===== received =====

Content-Length: 839


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:41:53 go env for /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n(root /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options)\n(go version go version go1.15.5 darwin/amd64)\n(valid build configuration = true)\n(build flags: [])\nGONOPROXY=.internal.digitalocean.com,github.com/digitalocean\nGOPRIVATE=.internal.digitalocean.com,github.com/digitalocean\nGOROOT=/usr/local/go\nGOSUMDB=sum.golang.org\nGOCACHE=/Users/bhcleek/Library/Caches/go-build\nGOINSECURE=\nGONOSUMDB=*.internal.digitalocean.com,github.com/digitalocean\nGOMOD=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/go.mod\nGOPROXY=https://proxy.golang.org,direct\nGO111MODULE=\nGOFLAGS=\nGOMODCACHE=/Users/bhcleek/go/pkg/mod\nGOPATH=/Users/bhcleek/go\n\n"}}

===== received =====

Content-Length: 99


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":4,"message":"Loading packages..."}}

===== received =====

Content-Length: 179


{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[{"scopeUri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","section":"gopls"}]},"id":3}

===== sent =====

Content-Length: 154


{"id":3,"jsonrpc":"2.0","result":[{"usePlaceholders":true,"buildFlags":[],"hoverKind":"Structured","staticcheck":true,"matcher":"fuzzy","local":"mymod"}]}

===== received =====

Content-Length: 839


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:41:53 go env for /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n(root /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options)\n(go version go version go1.15.5 darwin/amd64)\n(valid build configuration = true)\n(build flags: [])\nGOROOT=/usr/local/go\nGO111MODULE=\nGOCACHE=/Users/bhcleek/Library/Caches/go-build\nGOMOD=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/go.mod\nGOSUMDB=sum.golang.org\nGOMODCACHE=/Users/bhcleek/go/pkg/mod\nGONOSUMDB=.internal.digitalocean.com,github.com/digitalocean\nGOPRIVATE=.internal.digitalocean.com,github.com/digitalocean\nGONOPROXY=*.internal.digitalocean.com,github.com/digitalocean\nGOPATH=/Users/bhcleek/go\nGOPROXY=https://proxy.golang.org,direct\nGOFLAGS=\nGOINSECURE=\n\n"}}

===== received =====

Content-Length: 246


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:41:53 go/packages.Load\n\tsnapshot=0\n\tdirectory=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n\tquery=[builtin mymod/...]\n\tpackages=4\n"}}

===== received =====

Content-Length: 246


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:41:54 go/packages.Load\n\tsnapshot=0\n\tdirectory=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n\tquery=[builtin mymod/...]\n\tpackages=4\n"}}

===== received =====

Content-Length: 106


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":3,"message":"Finished loading packages."}}

===== received =====

Content-Length: 214


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":1,"message":"2021/02/06 09:41:54 creating workspace dir: mkdir /var/folders/f9/d6g779t53gq_20w0zm2f_1zm0000gn/T/gopls-31705.workspace: file exists\n"}}

===== received =====

Content-Length: 106


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":3,"message":"Finished loading packages."}}

===== sent =====

Content-Length: 559


{"method":"textDocument/didChange","jsonrpc":"2.0","params":{"contentChanges":[{"text":"package main\n\nimport (\n\t"context"\n\t"fmt"\n\t"mymod/pkg1"\n\t"mymod/pkg2"\n\n\t"github.com/pkg/errors"\n)\n\nfunc main() {\n\tfmt.Println("vim-go")\n}\n\nfunc dummyFunc(ctx context.Context) {\n\tfmt.Println(ctx)\n\tpkg1.Foo()\n\tpkg2.Bar()\n\terr := errors.Wrap(errors.New("some error"), "some wrapping info")\n\tfmt.Println(err)\n}\n"}],"textDocument":{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go","version":3}}}

===== sent =====

Content-Length: 290


{"method":"textDocument/codeAction","jsonrpc":"2.0","id":2,"params":{"context":{"only":["source.organizeImports"]},"range":{"end":{"character":0,"line":22},"start":{"character":0,"line":0}},"textDocument":{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go"}}}

===== received =====

Content-Length: 364


{"jsonrpc":"2.0","result":[{"title":"Organize Imports","kind":"source.organizeImports","disabled":{"reason":""},"edit":{"documentChanges":[{"textDocument":{"version":3,"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go"},"edits":[{"range":{"start":{"line":5,"character":0},"end":{"line":5,"character":0}},"newText":"\n"}]}]}}],"id":2}

And here they are when gofumpt is enabled (the last log is response to the codeAction/organizeImports request):

gopls logs
===== sent =====
Content-Length: 681

{"method":"initialize","jsonrpc":"2.0","id":1,"params":{"rootUri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","capabilities":{"workspace":{"workspaceFolders":true,"configuration":true,"didChangeConfiguration":{"dynamicRegistration":true}},"textDocument":{"codeAction":{"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["source.organizeImports","refactor.rewrite"]}}},"completion":{"completionItem":{"snippetSupport":true}},"hover":{"contentFormat":["plaintext"]}}},"processId":31562,"workspaceFolders":[{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","name":"/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options"}]}}

===== received =====

Content-Length: 260


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:39:43 Handshake session update\n\tupdate_session=4\n\tdebug_address="127.0.0.1:52091"\n\tlogfile=""\n\tserver="1"\n\tgopls_path="/Users/bhcleek/go/bin/gopls"\n"}}

===== stderr =====

serve.go:399: debug server listening at http://localhost:52091

===== received =====

Content-Length: 2733


{"jsonrpc":"2.0","result":{"capabilities":{"textDocumentSync":{"change":2,"openClose":true,"save":{}},"completionProvider":{"triggerCharacters":["."]},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor.extract","refactor.rewrite","source.fixAll","source.organizeImports"]},"codeLensProvider":{},"documentLinkProvider":{},"workspaceSymbolProvider":true,"documentFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"renameProvider":true,"foldingRangeProvider":true,"executeCommandProvider":{"commands":["gopls.generate","gopls.fill_struct","gopls.regenerate_cgo","gopls.test","gopls.tidy","gopls.undeclared_name","gopls.add_dependency","gopls.upgrade_dependency","gopls.remove_dependency","gopls.vendor","gopls.extract_variable","gopls.extract_function","gopls.gc_details","gopls.generate_gopls_mod"]},"callHierarchyProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":"workspace/didChangeWorkspaceFolders"}}},"serverInfo":{"name":"gopls","version":"{"path":"golang.org/x/tools/gopls","version":"v0.5.4","sum":"h1:PkQWDfzw8CxoseHDQniF2GhzmndN8KnQL6gcG2gnF3Y=","deps":[{"path":"github.com/BurntSushi/toml","version":"v0.3.1","sum":"h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ="},{"path":"github.com/google/go-cmp","version":"v0.5.1","sum":"h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k="},{"path":"github.com/sergi/go-diff","version":"v1.1.0","sum":"h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0="},{"path":"golang.org/x/mod","version":"v0.3.0","sum":"h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4="},{"path":"golang.org/x/sync","version":"v0.0.0-20201020160332-67f06af15bc9","sum":"h1:SQFwaSi55rU7vdNs9Yr0Z324VNlrF+0wMqRXT4St8ck="},{"path":"golang.org/x/tools","version":"v0.0.0-20201123203206-735e963e909b","sum":"h1:U8XGResmZKfdo8rCjWPsuiB4cu7N0PJSpQwc9lZr9Rg="},{"path":"golang.org/x/xerrors","version":"v0.0.0-20200804184101-5ec99f83aff1","sum":"h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE="},{"path":"honnef.co/go/tools","version":"v0.0.1-2020.1.6","sum":"h1:W18jzjh8mfPez+AwGLxmOImucz/IFjpNlrKVnaj2YVc="},{"path":"mvdan.cc/gofumpt","version":"v0.0.0-20200927160801-5bfeb2e70dd6","sum":"h1:z+/YqapuV7VZPvBb3GYmuEJbA88M3PFUxaHilHYVCpQ="},{"path":"mvdan.cc/xurls/v2","version":"v2.2.0","sum":"h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A="}]}"}},"id":1}

===== sent =====

Content-Length: 52


{"method":"initialized","jsonrpc":"2.0","params":{}}

===== sent =====

Content-Length: 252


{"method":"workspace/didChangeWorkspaceFolders","jsonrpc":"2.0","params":{"event":{"added":[{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","name":"/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options"}],"removed":[]}}}

===== sent =====

Content-Length: 554


{"method":"textDocument/didOpen","jsonrpc":"2.0","params":{"textDocument":{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go","version":2,"languageId":"go","text":"package main\n\nimport (\n\t"context"\n\t"fmt"\n\t"mymod/pkg1"\n\t"mymod/pkg2"\n\n\t"github.com/pkg/errors"\n)\n\nfunc main() {\n\tfmt.Println("vim-go")\n}\n\nfunc dummyFunc(ctx context.Context) {\n\tfmt.Println(ctx)\n\tpkg1.Foo()\n\tpkg2.Bar()\n\terr := errors.Wrap(errors.New("some error"), "some wrapping info")\n\tfmt.Println(err)\n}\n"}}}

===== received =====

Content-Length: 99


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":4,"message":"Loading packages..."}}

===== received =====

Content-Length: 179


{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[{"scopeUri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","section":"gopls"}]},"id":1}

===== sent =====

Content-Length: 169


{"id":1,"jsonrpc":"2.0","result":[{"usePlaceholders":true,"buildFlags":[],"hoverKind":"Structured","staticcheck":true,"matcher":"fuzzy","gofumpt":true,"local":"mymod"}]}

===== received =====

Content-Length: 268


{"jsonrpc":"2.0","method":"client/registerCapability","params":{"registrations":[{"id":"workspace/didChangeConfiguration","method":"workspace/didChangeConfiguration"},{"id":"workspace/didChangeWorkspaceFolders","method":"workspace/didChangeWorkspaceFolders"}]},"id":2}

===== sent =====

Content-Length: 38


{"id":2,"jsonrpc":"2.0","result":null}

===== received =====

Content-Length: 839


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:39:43 go env for /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n(root /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options)\n(go version go version go1.15.5 darwin/amd64)\n(valid build configuration = true)\n(build flags: [])\nGOINSECURE=\nGOFLAGS=\nGOPRIVATE=.internal.digitalocean.com,github.com/digitalocean\nGOSUMDB=sum.golang.org\nGONOPROXY=.internal.digitalocean.com,github.com/digitalocean\nGONOSUMDB=*.internal.digitalocean.com,github.com/digitalocean\nGOPROXY=https://proxy.golang.org,direct\nGO111MODULE=\nGOMOD=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/go.mod\nGOPATH=/Users/bhcleek/go\nGOMODCACHE=/Users/bhcleek/go/pkg/mod\nGOCACHE=/Users/bhcleek/Library/Caches/go-build\nGOROOT=/usr/local/go\n\n"}}

===== received =====

Content-Length: 99


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":4,"message":"Loading packages..."}}

===== received =====

Content-Length: 179


{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[{"scopeUri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options","section":"gopls"}]},"id":3}

===== sent =====

Content-Length: 169


{"id":3,"jsonrpc":"2.0","result":[{"usePlaceholders":true,"buildFlags":[],"hoverKind":"Structured","staticcheck":true,"matcher":"fuzzy","gofumpt":true,"local":"mymod"}]}

===== received =====

Content-Length: 839


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:39:43 go env for /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n(root /Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options)\n(go version go version go1.15.5 darwin/amd64)\n(valid build configuration = true)\n(build flags: [])\nGOMOD=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/go.mod\nGOSUMDB=sum.golang.org\nGO111MODULE=\nGOMODCACHE=/Users/bhcleek/go/pkg/mod\nGONOPROXY=.internal.digitalocean.com,github.com/digitalocean\nGONOSUMDB=.internal.digitalocean.com,github.com/digitalocean\nGOROOT=/usr/local/go\nGOCACHE=/Users/bhcleek/Library/Caches/go-build\nGOPRIVATE=*.internal.digitalocean.com,github.com/digitalocean\nGOFLAGS=\nGOINSECURE=\nGOPATH=/Users/bhcleek/go\nGOPROXY=https://proxy.golang.org,direct\n\n"}}

===== received =====

Content-Length: 246


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:39:43 go/packages.Load\n\tsnapshot=0\n\tdirectory=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n\tquery=[builtin mymod/...]\n\tpackages=4\n"}}

===== received =====

Content-Length: 246


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":3,"message":"2021/02/06 09:39:44 go/packages.Load\n\tsnapshot=0\n\tdirectory=/Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options\n\tquery=[builtin mymod/...]\n\tpackages=4\n"}}

===== received =====

Content-Length: 106


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":3,"message":"Finished loading packages."}}

===== received =====

Content-Length: 214


{"jsonrpc":"2.0","method":"window/logMessage","params":{"type":1,"message":"2021/02/06 09:39:44 creating workspace dir: mkdir /var/folders/f9/d6g779t53gq_20w0zm2f_1zm0000gn/T/gopls-31562.workspace: file exists\n"}}

===== received =====

Content-Length: 106


{"jsonrpc":"2.0","method":"window/showMessage","params":{"type":3,"message":"Finished loading packages."}}

===== sent =====

Content-Length: 559


{"method":"textDocument/didChange","jsonrpc":"2.0","params":{"contentChanges":[{"text":"package main\n\nimport (\n\t"context"\n\t"fmt"\n\t"mymod/pkg1"\n\t"mymod/pkg2"\n\n\t"github.com/pkg/errors"\n)\n\nfunc main() {\n\tfmt.Println("vim-go")\n}\n\nfunc dummyFunc(ctx context.Context) {\n\tfmt.Println(ctx)\n\tpkg1.Foo()\n\tpkg2.Bar()\n\terr := errors.Wrap(errors.New("some error"), "some wrapping info")\n\tfmt.Println(err)\n}\n"}],"textDocument":{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go","version":3}}}

===== sent =====

Content-Length: 290


{"method":"textDocument/codeAction","jsonrpc":"2.0","id":2,"params":{"context":{"only":["source.organizeImports"]},"range":{"end":{"character":0,"line":22},"start":{"character":0,"line":0}},"textDocument":{"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go"}}}

===== received =====

Content-Length: 364


{"jsonrpc":"2.0","result":[{"title":"Organize Imports","kind":"source.organizeImports","disabled":{"reason":""},"edit":{"documentChanges":[{"textDocument":{"version":3,"uri":"file:///Users/bhcleek/src/vim-go/issues/3151/vim-go-gopls-options/main.go"},"edits":[{"range":{"start":{"line":5,"character":0},"end":{"line":5,"character":0}},"newText":"\n"}]}]}}],"id":2}

This was originally reported on fatih/vim-go#3151
@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 Feb 6, 2021
@gopherbot gopherbot added this to the Unreleased milestone Feb 6, 2021
@bhcleek bhcleek mentioned this issue Feb 6, 2021
Closed
@bhcleek
Copy link
Contributor Author

bhcleek commented Feb 9, 2021

I took another look at this, and I think the problem isn't so much that organizeImports is disabled. Looking more closely at the logs, it's clear that organizeImports is disabled in both scenarios. It seems, though, that when gofumpt is used, it reformats the imports to remove empty lines between groups of imports. Is this intentional? Are gofumpt and organizeImports inherently incompatible?

I don't understand why organizeImports is disabled in the scenarios above in the original description; perhaps that deserves its own issue?

@heschi
Copy link
Contributor

heschi commented Feb 9, 2021

The presence of the disabled attribute is a glitch in our JSON serialization, which I believe has been fixed at tip. VS Code, at least, ignores it if the reason is empty.

I believe this behavior is gofumpt working as designed. The import path mymodule/pkg1 is not obviously third-party, since it has no dot, and it looks to me like like the code (https://github.com/mvdan/gofumpt/blob/c296abe75203bf63a6c9a65760361cb66461fe98/format/format.go#L583) will treat it as stdlib in the absence of other evidence. So it gets merged into the top block.

cc @mvdan in case I'm missing something.

@heschi heschi added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 9, 2021
@bhcleek
Copy link
Contributor Author

bhcleek commented Feb 10, 2021

Vim-go also ignores the disabled attribute for now, though I was thinking about changing that.

The organizeImports code actions seem to be working correctly, but gofumpt is messing with the import groups.

When configuring, notice the "local":"mymod" that's sent:

{"id":1,"jsonrpc":"2.0","result":[{"usePlaceholders":true,"buildFlags":[],"hoverKind":"Structured","staticcheck":true,"matcher":"fuzzy","local":"mymod"}]}

Adjusting imports honors that, but then gofumpt messes with the import groups. I take your point, about mymod looking like a stdlib package, but it's not. Perhaps the gofumpt algorithm used by gopls should honor the local setting?

I can modify vim-go to adjust imports after formatting as a workaround, though.

@bhcleek
Copy link
Contributor Author

bhcleek commented Feb 10, 2021

FWIW, I've adjusted vim-go to organize imports after formatting on save so that the local imports will meet user's expectations.

@mvdan
Copy link
Member

mvdan commented Feb 10, 2021

What @heschik said is right. #32819 is the issue to canonically document that module names without dots in the first path element are reserved. Arguably, tools like goimports or gopls allowing flags like -local=mymod should be considered a bug given the documentation issue above.

You might also be interested in #37641.

gofumpt itself is meant as a replacement for gofmt and not goimports, which is why it doesn't have a -local flag right now. It might in the future, it's hard to say. One could argue that it's a mistake for gofumpt to follow #32819, given that it's not yet a well documented principle. Maybe I'll temporarily relax the heuristics for a year or so if many people keep running into this issue. But it's also true that anyone calling their module mymod is going directly against the soon-to-be-documented rules, so they should probably rename their module sooner or later to prevent tools and editors from misbehaving :)

@bhcleek
Copy link
Contributor Author

bhcleek commented Feb 10, 2021

@mvdan #32819 seems to be about documenting what's required to retrieve a module over the network. In my mind, that's a distinct concern from whether or not import paths should be considered stdlib or not.

@heschi
Copy link
Contributor

heschi commented Feb 10, 2021

As far as I can tell there is not much to be done from the gopls side here. In my opinion, if a user chooses to enable gofumpt, gopls should always preserve gofumpt formatting. In that sense, the fact that organize imports is doing its usual grouping before being overriden by formatting might be a bug. (Though not one I would bother fixing.)

I'll leave this bug open for further discussion if you like, but to me this seems like something better suited for the gofumpt tracker.

@bhcleek
Copy link
Contributor Author

bhcleek commented Feb 10, 2021

Fair. I have a workaround implemented in vim-go already (organize imports after formatting).

@bhcleek bhcleek closed this as completed Feb 10, 2021
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Feb 10, 2021
@mvdan
Copy link
Member

mvdan commented Feb 11, 2021

@mvdan #32819 seems to be about documenting what's required to retrieve a module over the network. In my mind, that's a distinct concern from whether or not import paths should be considered stdlib or not.

I'm surprised you arrived at that conclusion. It is one of the reasons, but not the only one. For example, the issue says:

Another advantage of strongly encouraging the use of .tld is that it's much easier to tell if a package is from the standard library or not. Without the separation, one would have to keep a list of all standard library package paths, which easily gets out of date. Or run go list std, which adds work.

The issue is also about reserving the paths entirely, so any distinction about the reasons seems unnecessary. It's entirely reasonable for a tool to assume that such a path is not from an external module, given that it's reserved.

@mvdan
Copy link
Member

mvdan commented Feb 11, 2021

organize imports after formatting

That's of course your decision for vim-go, but it seems a bit confusing from the user's point of view; if they enabled gofumpt, they should get its formatting. I get your workaround as a temporary workaround, but not as a proper long-term solution :)

@bhcleek
Copy link
Contributor Author

bhcleek commented Feb 11, 2021

Yeah, the issue does say that, but module names without dots are entirely valid and always have been. Yes, it's possible to collide with stdlib, but the idea that module names without dots are reserved seems like a new rule that would catch a lot of people off guard and potentially create a lot of work for Go users unnecessarily.

It's entirely reasonable for a tool to assume that such a path is not from an external module, given that it's reserved.

Is it reserved now, though? I work with modules regularly that don't have dots without any ill effects. Everyone one I've shown that issue to shakes their head when they see it. I'm happy to take this convo to that issue if it's useful for you.

That's of course your decision for vim-go, but it seems a bit confusing from the user's point of view; if they enabled gofumpt, they should get its formatting. I get your workaround as a temporary workaround, but not as a proper long-term solution :)

gofumpt has several benefits aside from the organization of imports. Vim-go has the ability already to format with one tool and organize imports with another. Certainly there could be conflicts between those two things, but when a user has configured the local setting for gopls and has chosen to use gofumpt, it seems reasonable that they really do want the imports to be organized according to the local setting. If they don't want that, then they can elide the local setting and use only gofumpt, but without this change there is no way to get the local imports organized and use gofumpt if the ordering of formatting and organizing imports is reversed. Putting this in the control of users seems like the most flexible and affords the greatest flexibility to satisfy users' needs.

@mvdan
Copy link
Member

mvdan commented Feb 12, 2021

I'm happy to take this convo to that issue if it's useful for you.

Yes, your points seem to refer mostly to that issue and not this one.

@golang golang locked and limited conversation to collaborators Feb 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
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
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bhcleek @mvdan @stamblerre @gopherbot @heschi