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 completion corrupts import decl (client=BBEdit) #70646

Closed
johnataravir opened this issue Dec 2, 2024 · 7 comments
Closed
Labels
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

@johnataravir
Copy link

gopls version

Build info

golang.org/x/tools/gopls (devel)
golang.org/x/tools/gopls@(devel)
github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/mod@v0.20.0 h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0=
golang.org/x/sync@v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
golang.org/x/telemetry@v0.0.0-20240829154258-f29ab539cc98 h1:Wm3cG5X6sZ0RSVRc/H1/sciC4AT6HAKgLCSH2lbpR/c=
golang.org/x/text@v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653 h1:6bJEg2w2kUHWlfdJaESYsmNfI1LKAZQi6zCa7LUn7eI=
golang.org/x/vuln@v1.0.4 h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
honnef.co/go/tools@v0.4.7 h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs=
mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.23.1

go env

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/john/Library/Caches/go-build'
GOENV='/Users/john/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/john/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/john/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.3'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/john/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/john/Documents/Cellscale/code/pulumi/cell-factory/go.mod'
GOWORK='/Users/john/Documents/Cellscale/code/go.work'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9f/mkz_6zm91z7fm60rylr5tz5r0000gn/T/go-build2455567505=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I tried to auto-complete a function from a module that hadn't yet been added to my code. The problem can be reproduced for me by auto-completing any function from any yet-to-be-included module, on two similarly-configured MacOS systems. (I've tried on standard lib modules and imported modules that I've already run go get for.)

What did you see happen?

The import statement became corrupted, e.g.:

package main

import (
	"cry/randSeqll-factos"
	"context"
	"lg"os/exec"
	"ogs
	"ons/xtec

	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/compute"
	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/organizations"
	"github.com/pulumi/pulumi-gcp/sdk/v8/go/go/pacts"
o"github.com/pulumi/pulumi-gcp/sdk/v8/gooptup//gcp/storage"
	"github.com/pulumi/pulumi/sdk/v3/gmpute"
"gthb.umii/sdk/v3/go/auto"
	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
	"cell-factory/randSeq"
)

func main() {

		_, err = storage.NewBucket(<# 1: #>)

What did you expect to see?

I expected a library to be added cleanly to the import statement, like this storage library when auto-completing the storage.NewBucket() function:

package main

import (
	"os"
	"fmt"
	"os/exec"
	"log"
	"context"
	"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/organizations"
	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/projects"
	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/compute"
	"github.com/pulumi/pulumi/sdk/v3/go/auto"
	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
	"cell-factory/randSeq"
	"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/storage"
)

func main() {
		_, err = storage.NewBucket(<# 1: #>)

}

Editor and settings

I'm using BBEdit 15.1.2 on MacOS. BBEdit only uses configuration files for language servers if you want to do something unusual, so I can't paste mine here, but I follow the defaults pretty closely. The only notable config is the reference URL template, for which I used the default suggested in BBEdit's docs: https://www.google.com/search?&q=__LANGUAGENAME__%20__SYMBOLNAME__.

Logs

2024-11-08 15:42:02.535: stderr output from server: [Trace - 15:42:02.534 PM] Sending notification 'textDocument/didChange'.
Params: {"contentChanges":[{"range":{"start":{"line":102,"character":19},"end":{"line":102,"character":22}},"text":"NewBucket(<# 1: #>)"},{"range":{"start":{"line":3,"character":2},"end":{"line":3,"character":2}},"text":"cell-fact"},{"range":{"start":{"line":3,"character":3},"end":{"line":3,"character":4}},"text":"ry/randSeq"},{"range":{"start":{"line":4,"character":2},"end":{"line":4,"character":2}},"text":"context"\n\t""},{"range":{"start":{"line":5,"character":2},"end":{"line":5,"character":2}},"text":"l"},{"range":{"start":{"line":5,"character":3},"end":{"line":6,"character":1}},"text":"g"},{"range":{"start":{"line":6,"character":2},"end":{"line":6,"character":3}},"text":""},{"range":{"start":{"line":6,"character":4},"end":{"line":6,"character":5}},"text":"s"},{"range":{"start":{"line":7,"character":2},"end":{"line":7,"character":3}},"text":""},{"range":{"start":{"line":7,"character":4},"end":{"line":7,"character":6}},"text":"s/"},{"range":{"start":{"line":7,"character":8},"end":{"line":7,"character":9}},"text":"ec"},{"range":{"start":{"line":8,"character":0},"end":{"line":11,"character":40}},"text":"\n\t"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/compute"\n\t"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/organizations"\n\t"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/projects"\n\t"github.com/pulumi/pulumi-gcp/sdk/v8/go/gcp/storage"\n\t"github.com/pulumi/pulumi/sdk/v3"},{"range":{"start":{"line":11,"character":42},"end":{"line":11,"character":44}},"text":"o"},{"range":{"start":{"line":11,"character":45},"end":{"line":11,"character":49}},"text":"a"},{"range":{"start":{"line":12,"character":0},"end":{"line":12,"character":1}},"text":"o"},{"range":{"start":{"line":12,"character":40},"end":{"line":12,"character":40}},"text":"o/"},{"range":{"start":{"line":12,"character":41},"end":{"line":12,"character":41}},"text":"ptup"},{"range":{"start":{"line":13,"character":35},"end":{"line":13,"character":40}},"text":""},{"range":{"start":{"line":14,"character":0},"end":{"line":14,"character":1}},"text":""},{"range":{"start":{"line":14,"character":2},"end":{"line":14,"character":3}},"text":""},{"range":{"start":{"line":14,"character":4},"end":{"line":14,"character":5}},"text":""},{"range":{"start":{"line":14,"character":6},"end":{"line":14,"character":22}},"text":"umi"}],"textDocument":{"uri":"file:///Users/john/Documents/Cellscale/code/pulumi/cell-factory/main.go","version":51}}

2024-11-08 15:42:02.733: stderr output from server: [Trace - 15:42:02.732 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2024/11/08 15:42:02 go/packages.Load #24\n\tview_id="1"\n\tsnapshot=131\n\tdirectory=/Users/john/Documents/Cellscale/code/cellscale/\n\tquery=[file=/Users/john/Documents/Cellscale/code/pulumi/cell-factory/main.go]\n\tpackages=1\n\tduration=120.820083ms\n"}

2024-11-08 15:42:02.733: Server message: 2024/11/08 15:42:02 go/packages.Load #24
view_id="1"
snapshot=131
directory=/Users/john/Documents/Cellscale/code/cellscale/
query=[file=/Users/john/Documents/Cellscale/code/pulumi/cell-factory/main.go]
packages=1
duration=120.820083ms

2024-11-08 15:42:02.794: stderr output from server: [Trace - 15:42:02.793 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/john/Documents/Cellscale/code/pulumi/cell-factory/main.go","version":51,"diagnostics":[{"range":{"start":{"line":5,"character":5},"end":{"line":5,"character":5}},"severity":1,"source":"syntax","message":"expected ';', found os"}]}

2024-11-08 15:42:03.739: stderr output from server: [Trace - 15:42:03.738 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2024/11/08 15:42:03 go/packages.Load #25\n\tview_id="4"\n\tsnapshot=131\n\tdirectory=/Users/john/Documents/Cellscale/code/pulumi/cell-factory/\n\tquery=[cell-factory]\n\tpackages=1\n\tduration=144.397584ms\n"}

2024-11-08 15:42:03.739: Server message: 2024/11/08 15:42:03 go/packages.Load #25
view_id="4"
snapshot=131
directory=/Users/john/Documents/Cellscale/code/pulumi/cell-factory/
query=[cell-factory]
packages=1
duration=144.397584ms

2024-11-08 15:42:03.967: stderr output from server: [Trace - 15:42:03.966 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2024/11/08 15:42:03 go/packages.Load #26\n\tview_id="2"\n\tsnapshot=131\n\tdirectory=/Users/john/Documents/Cellscale/code/cellscale/randSeq/\n\tquery=[cell-factory]\n\tpackages=1\n\tduration=120.878583ms\n"}

2024-11-08 15:42:03.967: Server message: 2024/11/08 15:42:03 go/packages.Load #26
view_id="2"
snapshot=131
directory=/Users/john/Documents/Cellscale/code/cellscale/randSeq/
query=[cell-factory]
packages=1
duration=120.878583ms

@johnataravir johnataravir added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Dec 2, 2024
@gopherbot gopherbot added this to the Unreleased milestone Dec 2, 2024
@adonovan
Copy link
Member

adonovan commented Dec 2, 2024

File corruption is a serious bug, but unfortunately it is impossible to tell from the portion of the session log which part--client or server--is at fault. My gut feeling is that this must be a bug in BBEdit's LSP client, since I wasn't even aware it had LSP support till now, and we haven't heard of similar reports with other clients.

If you are able, could you repeat the experiment on an open-source file at a known revision, and include the entire session log, including the textDocument/completion request and response? Thanks.

@adonovan adonovan changed the title x/tools/gopls: Lib import corruption when autocompleting a function x/tools/gopls: unimported completion corrupts import decl (client=BBEdit) Dec 2, 2024
@johnataravir
Copy link
Author

Thanks, Alan. I've attached the log: LanguageServerProtocol-Go.txt and a .zip of the original and modified files: gopls_bbedit.zip. Here are the steps I took to reproduce this today:

  • git clone https://github.com/pulumi/automation-api-examples
  • Edit /go/inline_program/main.go.
  • Remove line 12 ("github.com/pulumi/pulumi/sdk/v3/go/pulumi").
  • After line 26, start a new line, copy the following line until the pulumi reference, then auto-complete:
  • deployFunc := func(ctx *pulumi

The import statement before the completion looks like this:

import (
	"context"
	"fmt"
	"os"

	"github.com/pulumi/pulumi-aws/sdk/v4/go/aws/s3"
	"github.com/pulumi/pulumi/sdk/v3/go/auto"
	"github.com/pulumi/pulumi/sdk/v3/go/auto/optdestroy"
	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
)

And after, like this:

import (
	"context"
	"fmt"
	"os"

	"github.com/pulumi/pulumi-aws/sdk/v4/go/aws/s3"
	"github.com/pulumi/pulumi/sdk/v3/go/auto"
	"github.com/pulumi/pulumi/sdk/v3/go/auto/optdestroy"
	"github.com/pulumi/pulumi/sdk/v3/go/auto/optumulumii/sdk/v3/go/p"
	"github.com/pulumi/pulup"
)

I agree there's a good chance it's something in BBEdit. I did talk to them first, but they couldn't find an issue. If it's not a problem in gopls, hopefully I can go back to them with something that makes the issue more clear. Thanks again for the help!

@adonovan
Copy link
Member

adonovan commented Dec 5, 2024

Thanks for the helpful information. Just to confirm we're on the same page: does "After line 26" mean after having deleted the import, insert the new code after the existing comment, before the existing deployFunc := ... statement? In VS Code, the offered completion was for the pulumi package itself, and tab caused it to add the import, correctly. So I couldn't reproduce the problem in VS Code.

The gopls completion response is correct, if weird: instead of simply inserting a new import at the end, it inserts three chunks, the first of which contains an interior newline, into the middle last existing import:

      "additionalTextEdits": [
        {
          "range": {
            "start": {
              "line": 10,
              "character": 45
            },
            "end": {
              "line": 10,
              "character": 45
            }
          },
          "newText": "up\"\n\t\"github.com/pulumi/pul"
        },
        {
          "range": {
            "start": {
              "line": 10,
              "character": 46
            },
            "end": {
              "line": 10,
              "character": 46
            }
          },
          "newText": "mi/sdk/v3/go/"
        },
        {
          "range": {
            "start": {
              "line": 10,
              "character": 47
            },
            "end": {
              "line": 10,
              "character": 47
            }
          },
          "newText": "ulumi"
        }
      ]

Initial state:

	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
)

First insertion:

	"github.com/pulumi/pulumi/sdk/v3/go/auto/opt«up"
	"github.com/pulumi/pul»up"
)

Second insertion:

	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
	"github.com/pulumi/pulu«mi/sdk/v3/go/»p"
)

Third insertion:

	"github.com/pulumi/pulumi/sdk/v3/go/auto/optup"
	"github.com/pulumi/pulumi/sdk/v3/go/p«ulumi»"
)

But this is a valid edit. I wonder whether BBEdit's edit logic is confused by the sequence of insertions seeming to overlap. Conceptually, they should be applied in reverse order so that the newly inserted text has no effect on the interpretation of later line/column data.

@johnataravir
Copy link
Author

My pleasure! Thanks for the analysis, and yes, it sounds like you reproduced my process exactly. Sounds like it's time for me to get back in touch with BBEdit. I'll send them this issue and see what they say.

@findleyr findleyr added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 12, 2024
@gopherbot
Copy link
Contributor

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.)

@gopherbot gopherbot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 12, 2025
@johnataravir
Copy link
Author

BBEdit has found the issue and fixed it in pre-release. It should be fixed for everyone soon. Thanks for all the help in identifying the problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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