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: unimported completions produces invalid edits with windows line endings when CRLF is used #46791

Closed
DmitriyVTitov opened this issue May 9, 2021 · 32 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

@DmitriyVTitov
Copy link

What version of Go, VS Code & VS Code Go extension are you using?

  • Run go version to get version of Go from the VS Code integrated terminal.
    • go version go1.16.3 windows/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • golang.org/x/tools/gopls v0.6.11
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.56.0 x64
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.24.2
  • Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) > Go: Locate Configured Go Tools command.

gopath: C:\Users\dtsp\YandexDisk\go
GOROOT: c:\go
PATH: C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0;C:\WINDOWS\System32\OpenSSH;C:\Program Files\Git\cmd;C:\Program Files\nodejs;C:\Program Files\Graphviz 2.44.1\bin;C:\Go\bin;C:\Users\dtsp\AppData\Local\Microsoft\WindowsApps;C:\Users\dtsp\AppData\Roaming\npm;C:\Users\dtsp\go\bin;C:\Users\dtsp\YandexDisk\go\bin;C:\Users\dtsp\AppData\Local\GitHubDesktop\bin;c:\flutter\bin;C:\msys64\mingw64\bin;C:\Users\dtsp\AppData\Local\Microsoft\WindowsApps;C:\Users\dtsp\AppData\Local\Programs\Microsoft VS Code\bin;C:\Users\dtsp\go\bin

gopkgs: C:\Users\dtsp\YandexDisk\go\bin\gopkgs.exe installed
go-outline: C:\Users\dtsp\YandexDisk\go\bin\go-outline.exe installed
gotests: C:\Users\dtsp\YandexDisk\go\bin\gotests.exe installed
gomodifytags: C:\Users\dtsp\YandexDisk\go\bin\gomodifytags.exe installed
impl: C:\Users\dtsp\YandexDisk\go\bin\impl.exe installed
goplay: C:\Users\dtsp\YandexDisk\go\bin\goplay.exe installed
dlv: C:\Users\dtsp\YandexDisk\go\bin\dlv.exe installed
dlv-dap: dlv-dap not installed
staticcheck: C:\Users\dtsp\YandexDisk\go\bin\staticcheck.exe installed
gopls: C:\Users\dtsp\YandexDisk\go\bin\gopls.exe installed

Describe the bug

When second imported package is called in source code it is autoimported in the same line as first one.

Steps to reproduce the behavior:

package ex

import "fmt"

func ex() {
	fmt.Println("ABC")
}

Then add for ex. math.Sqrt(2)

package ex

import (
	"fmt"	"math"
)


func ex() {
	fmt.Println("ABC")
	math.Sqrt(2)
}

Screenshots or recordings

image

@findleyr
Copy link
Contributor

Thank you for the report.

This may be an formatting issue related to the use of go-diff v1.2.0: https://golang.org/issue/45732.

Could you run go version -m "C:\Users\dtsp\YandexDisk\go\bin\gopls.exe", and report the version of github.com/sergi/go-diff you are using?

@DmitriyVTitov
Copy link
Author

Could you run go version -m "C:\Users\dtsp\YandexDisk\go\bin\gopls.exe", and report the version of github.com/sergi/go-diff you are using?

Sure!

PS > go version -m "C:\Users\dtsp\YandexDisk\go\bin\gopls.exe"
...\go\bin\gopls.exe: go1.16.3
...
dep github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
...

@findleyr
Copy link
Contributor

Thanks! Well, that rules out this known issue.

Are you able to reliably reproduce? If so, would you be able to collect rpc trace logs during a repro session, and share them here? Instructions for how to collect logs are here:
https://github.com/golang/vscode-go/blob/master/docs/troubleshooting.md#collect-gopls-information

@DmitriyVTitov
Copy link
Author

Thanks! Well, that rules out this known issue.

Are you able to reliably reproduce? If so, would you be able to collect rpc trace logs during a repro session, and share them here? Instructions for how to collect logs are here:
https://github.com/golang/vscode-go/blob/master/docs/troubleshooting.md#collect-gopls-information

OK, I've created simple src file before bug occurs. Then I've cleaned output and then I've added line which causes bug. Logs are attached.
If you need full log from src file creation let me know.

gopls.log

@DmitriyVTitov
Copy link
Author

I started to record log when I had this snippet:

package main

import "fmt"

func main() {
	fmt.Println("A")
}

Then I've added math.Sqrt()

And gopls autoimported math like this:

package main

import (
	"fmt"	"math"
)


func main() {
	fmt.Println("A")
	math.Sqrt()
}

@findleyr
Copy link
Contributor

Thanks for your responsiveness!

We're a bit baffled by the edits in your log. It looks like gopls sends the following edits:

[Trace - 18:38:33.818 PM] Received response 'textDocument/codeAction - (186)' in 119ms.
Result: [{"title":"Organize Imports","kind":"source.organizeImports","edit":{"documentChanges":[{"textDocument":{"version":52,"uri":"file:///C:/Users/dtsp/YandexDisk/go/src/sf/snippets/issue/issue.go"},"edits":[{"range":{"start":{"line":0,"character":12},"end":{"line":0,"character":13}},"newText":""},{"range":{"start":{"line":1,"character":0},"end":{"line":1,"character":1}},"newText":""}]}]}}]

But then the editor sends edits to insert "math":

[Trace - 18:38:34.634 PM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file:///c%3A/Users/dtsp/YandexDisk/go/src/sf/snippets/issue/issue.go","version":53},"contentChanges":[{"range":{"start":{"line":2,"character":12},"end":{"line":2,"character":12}},"rangeLength":0,"text":"\t\"math\"\r\n)\r\n"},{"range":{"start":{"line":2,"character":7},"end":{"line":2,"character":7}},"rangeLength":0,"text":"(\r\n\t"}]}

Do you by any chance have other extensions installed that might be triggering these edits? It looks like something is fighting with gopls.

@DmitriyVTitov
Copy link
Author

Don't know but this bug occures only when i switched to gopls. I can reproduce this on two my macnines. They are pretty much identical, but extensions are the same as before gopls.
So can't tell you.

@DmitriyVTitov
Copy link
Author

My format tool is goimports.

@findleyr
Copy link
Contributor

but extensions are the same as before gopls

I seem to recall that extensions which could previously coexist with vscode-go might night coexist well with gopls, because of the way they interact with buffer state (gopls maintains an active copy of all open buffers). So just because there was no collision before gopls was enabled does not mean that there won't be collision with gopls. So, with that being said, do you have any other extensions installed that format Go code?

My format tool is goimports.

If you have gopls enabled, that tool should not run.

Just to narrow things down, can you run gopls imports path/to/issue.go to see if gopls in isolation reproduces the issue?

@DmitriyVTitov
Copy link
Author

DmitriyVTitov commented May 10, 2021

I cannot reproduce this with CLI. But have another interesting result:

PS > gopls imports ...\issue\issue.go
package main

import (
        "math"
)


func main() {
        fmt.Print("a")
        math.Sqrt(2)
}

There is no "fmt" package :) Or this is by design?

Original isuue does not reproduce after I've imported packages properly. Only on the first import. Maybe there is a conflict, I don't know. But if I recreate src file bug occures.

@hyangah
Copy link
Contributor

hyangah commented May 11, 2021

I think the first thing to check is to disable all other extensions and see if it's still reproducible.

And, can you please share your vscode settings so we can try to locally produce it? (Command Palette -> "Preferences: Open Settings (JSON)", "Preferences: Open Workspace Settings (JSON)")

@DmitriyVTitov
Copy link
Author

It reproduces with all extensions disabled. Here there are only 3 extensions - Russian Language, Go and VSCode Icons.
image

Workspace settings empty. Here are my global settings.

{
  "terminal.integrated.shell.windows": "C:\\WINDOWS\\System32\\WindowsPowerShell\\v1.0\\powershell.exe",
  "vetur.experimental.templateInterpolationService": false,
  "go.testFlags": ["-v"],
  "files.associations": {
    "*.sql": "pgsql",
    "*.go": "go"
  },
  "[javascript]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  },
  "dart.checkForSdkUpdates": false,
  "[html]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode"
  },
  "workbench.startupEditor": "newUntitledFile",
  "workbench.iconTheme": "vscode-icons",
  "[vue]": {
    "editor.defaultFormatter": "octref.vetur"
  },
  "prettier.disableLanguages": [],
  "[go]": {},
  "go.formatTool": "goimports",
  "vsicons.dontShowNewVersionMessage": true,
  "window.zoomLevel": 1,
  "dart.openDevTools": "flutter",
  "timeline.excludeSources": ["git-history"],
  "extensions.ignoreRecommendations": false,
  "editor.suggestSelection": "first",
  "vsintellicode.modify.editor.suggestSelection": "automaticallyOverrodeDefaultValue",
  "editor.codeActionsOnSave": { "source.fixAll.eslint": true },
  "editor.formatOnPaste": true,
  "editor.formatOnSave": true,
  "editor.formatOnType": true,
  "auto-close-tag.activationOnLanguage": [
    "xml",
    "php",
    "blade",
    "ejs",
    "jinja",
    "javascript",
    "javascriptreact",
    "typescript",
    "typescriptreact",
    "plaintext",
    "markdown",
    "vue",
    "liquid",
    "erb",
    "lang-cfml",
    "cfml",
    "HTML (EEx)",
    "HTML (Eex)",
    "plist"
  ],
  "editor.fontLigatures": null,
  "workbench.editor.pinnedTabSizing": "compact",
  "prettier.semi": false,
  "editor.renameOnType": true,
  "go.lintTool": "staticcheck",
  "go.toolsManagement.autoUpdate": true
}

@wtask
Copy link

wtask commented May 11, 2021

Does it occurred for multi-module repo? Does it occurred after save file action? Does your workspace contain more than one repository?
As my experience talks, any problems arise only for multi-modular repos. In other cases, everything works more or less smoothly.

@DmitriyVTitov
Copy link
Author

Screenshot shows workspace with only one folder with single file. Autoimports occures when I type exported method (or press tab for autocompetion). I don't even press "Save". Folder is part of the module. There are no nested modules.

@stamblerre
Copy link
Contributor

Can you please remove "go.formatTool": "goimports" from your settings? I think it's conflicting with gopls.

/cc @hyangah

@wtask

This comment has been minimized.

@stamblerre

This comment has been minimized.

@wtask

This comment has been minimized.

@DmitriyVTitov
Copy link
Author

Can you please remove "go.formatTool": "goimports" from your settings? I think it's conflicting with gopls.

/cc @hyangah

I've tried, even restarted VS Code, but result is the same.

@stamblerre
Copy link
Contributor

Ok, thanks for trying. One more idea--can you please remove the

  "editor.formatOnPaste": true,
  "editor.formatOnSave": true,
  "editor.formatOnType": true,

settings as well?

@DmitriyVTitov
Copy link
Author

Ok, thanks for trying. One more idea--can you please remove the

  "editor.formatOnPaste": true,
  "editor.formatOnSave": true,
  "editor.formatOnType": true,

settings as well?

No luck :)

Here is short video with this issue.
https://disk.yandex.ru/i/LUeLUZOGUAiEDg

@gopherbot
Copy link

Change https://golang.org/cl/319089 mentions this issue: gopls/internal/regtest: add a failing regtest for vscode-go#1489

@findleyr
Copy link
Contributor

Thanks for the video, which made it clear that the problem was with unimported completions. With that context we can spot in your logs an incorrect completion item:

"additionalTextEdits":[
{"range":{"start":{"line":0,"character":12},"end":{"line":0,"character":13}},"newText":""},
{"range":{"start":{"line":1,"character":0},"end":{"line":1,"character":1}},"newText":""},
{"range":{"start":{"line":2,"character":7},"end":{"line":2,"character":7}},"newText":"(\n\t"},
{"range":{"start":{"line":2,"character":13},"end":{"line":2,"character":13}},"newText":"\t\"math\"\n)\n"}
]

This edit shows that gopls is trying to insert an edit after the newline (line 2 character 13), rather than on the next line. Per the LSP spec: "Positions are line end character agnostic. So you can not specify a position that denotes \r|\n or \n| where | represents the character offset." Presumably VS Code is interpreting this position as the end of the line, rather than the start of the next line, resulting in the broken formatting you experience.

This is a gopls bug, and I'm not sure how we haven't encountered it before. In any case I've reproduced in a regression test, and we should be able to fix.

I should say I'm only 98% sure this is the underlying problem, but we're going to fix this first.

Let me reiterate how much we appreciate your willingness to follow up on this report.

@findleyr findleyr changed the title Issue with autoimport packages gopls: unimported completions produces invalid edits with windows line endings May 11, 2021
@gopherbot
Copy link

Change https://golang.org/cl/319129 mentions this issue: internal/lsp/source: compute imports text edits from scratch

@findleyr findleyr self-assigned this May 12, 2021
gopherbot referenced this issue in golang/tools May 13, 2021
Unimported completion computes invalid text edits with windows line
endings.

To enable this test, add support for windows line endings in the regtest
framework. Doing this required decoupling the txtar encoding from the
sandbox, which was a good change anyway.

For golang/vscode-go#1489

Change-Id: I6c1075fd38d24090271a7a7f33b11ddd8f9decf5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/319089
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@lengyuxuan
Copy link

@AWZ20U@BOY3OWPR@{1NYLS

you can set end of line sequence to LF, my problem has been solved.

@hyangah
Copy link
Contributor

hyangah commented Jun 16, 2021

The root cause was confirmed and it needs fix in gopls.
I am transferring this issue for better tracking.

@hyangah hyangah transferred this issue from golang/vscode-go Jun 16, 2021
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jun 16, 2021
@hyangah hyangah changed the title gopls: unimported completions produces invalid edits with windows line endings x/tools/gopls: unimported completions produces invalid edits with windows line endings Jun 16, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 16, 2021
@gopherbot gopherbot added this to the Unreleased milestone Jun 16, 2021
@hyangah hyangah modified the milestones: Unreleased, gopls/v0.7.1 Jun 16, 2021
@stamblerre stamblerre self-assigned this Jun 21, 2021
@hyangah hyangah changed the title x/tools/gopls: unimported completions produces invalid edits with windows line endings x/tools/gopls: unimported completions produces invalid edits with windows line endings when CRLF is used Jul 16, 2021
@stamblerre stamblerre modified the milestones: gopls/v0.7.1, gopls/v0.7.2 Jul 26, 2021
@hyangah
Copy link
Contributor

hyangah commented Jul 26, 2021

@Piotr1215 verified https://go-review.googlesource.com/c/tools/+/319129/ fixed the issue, but reported a minor problem - gopls adds an extra newline at the end of the import block - See golang/vscode-go#1646 (comment)

@stamblerre
Copy link
Contributor

stamblerre commented Jul 30, 2021

@Piotr1215: Do you mind sharing a complete Go file that reproduces the extra newline issue? I can't seem to reproduce that issue.

@Piotr1215
Copy link

hi @stamblerre , no problem. Here is also another recording, showing the extra line and some jitter:

https://www.screencast.com/t/Fsvwlu3j0o

Code file

package main

import (
	"fmt"
)

func main() {

	fmt.Println("hello world")
}

@stamblerre
Copy link
Contributor

@Piotr1215: Can you please share your gopls logs when you see this happen? I am not able to reproduce this myself, and in fact, gopls should not even be running any import organization on this file because there are no missing imports. My only guess is that this could be the formatter--or there are some other issues at play here. Information on how to capture logs can be found here.

@stamblerre stamblerre added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 6, 2021
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@Piotr1215
Copy link

Hi, sorry for late reply. I’ve swapped to Linux a while ago and the problem disappeared. I gave the same plugins and settings synced.

@golang golang locked and limited conversation to collaborators Sep 11, 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

8 participants