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: crash in goimports with line directives #51916

Closed
griesemer opened this issue Mar 24, 2022 · 3 comments
Closed

x/tools/gopls: crash in goimports with line directives #51916

griesemer opened this issue Mar 24, 2022 · 3 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.8.4

Comments

@griesemer
Copy link
Contributor

gopls version: v0.8.1 (devel go1.19-e33711f8c1 Tue Mar 15 19:03:38 2022 -0700)
gopls flags:
update flags: proxy
extension version: 0.32.0
go version: devel 1.19-2f6c07744f
environment: Visual Studio Code darwin
initialization error: undefined
issue timestamp: Thu, 24 Mar 2022 04:46:28 GMT
restart history:
Thu, 24 Mar 2022 04:45:11 GMT: activation (enabled: true)

ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.

  1. cd $GOROOT/src/bytes
  2. open the CWD with VSCode: code . (code is a small script that launches VSCode with . in this case)
  3. open (choose) bytes.go file
  4. add the line comment //line math.go:100 on the first line (before the copyright notice)
  5. delete the import keyword on line 10 (to provoke an error) and save the file
  6. using the VSCode terminal (in the CWD), run: go build -gcflags=-L - this will produce an error like:
$ go build -gcflags=-L
# bytes
math.go:108[/Users/gri/goroot/src/bytes/bytes.go:10:1]: syntax error: non-declaration statement outside function body
  1. Option+click on the filename in square brackets to locate the cursor at the error position.
  2. Command+Z in the editor to undo the deletion of the import keyword (don't yet save the file).
  3. After 1s or so the crash appears.

I've been able to reproduce this fairly reliably with these 9 steps.

panic: invalid line number 111 (should be < 19)

goroutine 23670 [running]:
go/token.(*File).MergeLine(0xc024266c60, 0x183%3F)
	  position.go:158  0x1ee
golang.org/x/tools/internal/imports.sortImports({0x0, 0x0}, 0x1c04cc0%3F, 0xc042678200)
	  sortimports.go:56  0x5e9
golang.org/x/tools/internal/imports.formatFile(0xc032ebde80%3F, 0xc042678200%3F, {0xc000712000, 0x7d44, 0x7d45}, 0x0, 0xc035ebabd0)
	  imports.go:108  0x8b
golang.org/x/tools/internal/imports.ApplyFixes({0x0, 0x0, 0x0}, {0x0, 0x0}, {0xc000712000, 0x7d44, 0x7d45}, 0xc035ebabd0, 0x2)
	  imports.go:103  0x165
golang.org/x/tools/internal/lsp/source.computeFixEdits({0x1c0d2d0, 0xc0302fcb40}, 0xc002ce0180, 0x7d44%3F, {0x0, 0x0, 0x0})
	  format.go:195  0x177
golang.org/x/tools/internal/lsp/source.computeImportEdits({0x1c0d2d0, 0xc0302fcb40}, 0xc002ce0180, 0x100f227%3F)
	  format.go:140  0xce
golang.org/x/tools/internal/lsp/source.AllImportsFixes.func1(0x18701a0%3F)
	  format.go:121  0x4a
golang.org/x/tools/internal/lsp/cache.(*importsState).runProcessEnvFunc(0xc0003e29c0, {0x1c05200, 0xc035eba270}, 0xc0302fcb40, 0xc03196c140)
	  imports.go:114  0x5a3
golang.org/x/tools/internal/lsp/cache.(*snapshot).RunProcessEnvFunc(0x1c05158%3F, {0x1c05200%3F, 0xc035eba270%3F}, 0x1c05468%3F)
	  view.go:374  0x34
golang.org/x/tools/internal/lsp/source.AllImportsFixes({0x1c05158, 0xc031108980}, {0x1c0d2d0%3F, 0xc0302fcb40}, {0x1c05468, 0xc02c8f4420})
	  format.go:120  0x308
golang.org/x/tools/internal/lsp.(*Server).codeAction(0x185b440%3F, {0x1c05158, 0xc031108980}, 0xc0436c2000)
	  code_action.go:98  0x97b
golang.org/x/tools/internal/lsp.(*Server).CodeAction(0xc034326000%3F, {0x1c05158%3F, 0xc031108980%3F}, 0x185b440%3F)
	  server_gen.go:16  0x25
golang.org/x/tools/internal/lsp/protocol.serverDispatch({0x1c05158, 0xc031108980}, {0x1c10280, 0xc000274360}, 0xc035eba150, {0x1c053c0, 0xc031108840})
	  tsserver.go:478  0x17a8
golang.org/x/tools/internal/lsp/protocol.ServerHandler.func1({0x1c05158, 0xc031108980}, 0xc035eba150, {0x1c053c0, 0xc031108840})
	  protocol.go:154  0x90
golang.org/x/tools/internal/lsp/lsprpc.handshaker.func1({0x1c05158, 0xc031108980}, 0xc035eba150, {0x1c053c0%3F, 0xc031108840%3F})
	  lsprpc.go:506  0xa43
golang.org/x/tools/internal/jsonrpc2.MustReplyHandler.func1({0x1c05158, 0xc031108980}, 0xc042604d20, {0x1c053c0%3F, 0xc031108840%3F})
	  handler.go:35  0xf6
golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1.2()
	  handler.go:103  0xa3
created by golang.org/x/tools/internal/jsonrpc2.AsyncHandler.func1
	  handler.go:100  0x20a
[Error - 9:46:21 PM]

OPTIONAL: If you would like to share more information, you can attach your complete gopls logs.

NOTE: THESE MAY CONTAIN SENSITIVE INFORMATION ABOUT YOUR CODEBASE.
DO NOT SHARE LOGS IF YOU ARE WORKING IN A PRIVATE REPOSITORY.

<OPTIONAL: ATTACH LOGS HERE>
@findleyr
Copy link
Contributor

Thank you for the reproducer!

Transferring to the Go issue tracker.

@findleyr findleyr changed the title gopls: automated issue report (crash) x/tools/gopls: crash in goimports with line directives Mar 24, 2022
@findleyr findleyr transferred this issue from golang/vscode-go Mar 24, 2022
@findleyr findleyr added this to the gopls/v0.8.2 milestone Mar 24, 2022
@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 Mar 24, 2022
@findleyr findleyr added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed upstream-tools labels Mar 24, 2022
@findleyr findleyr modified the milestones: gopls/v0.8.2, gopls/v0.8.3 Mar 31, 2022
@pjweinb
Copy link

pjweinb commented Apr 5, 2022

For gopls developers, an easier example: put //line math.go:400 at the top of internal/imports/sortimports.go and save the file:
(may be caused by using fset.Position() instead of fset.PositionFor(, false))

panic: invalid line number 414 (should be < 21)

goroutine 135 [running]:
go/token.(*File).MergeLine(0xc006a30000, 0x126?)
/usr/lib/google-golang/src/go/token/position.go:158 +0x1ee
golang.org/x/tools/internal/imports.sortImports({0x0, 0x0}, 0x1171200?, 0xc006eca080)
/usr/local/google/home/pjw/tools/internal/imports/sortimports.go:56 +0x5e9
golang.org/x/tools/internal/imports.formatFile(0xc006a197c0?, 0xc006eca080?, {0xc0006a6000, 0x1f2f, 0x1f30}, 0x0, 0xc006962390)
/usr/local/google/home/pjw/tools/internal/imports/imports.go:108 +0x8b
golang.org/x/tools/internal/imports.ApplyFixes({0x0, 0x0, 0x0}, {0x0, 0x0}, {0xc0006a6000, 0x1f2f, 0x1f30}, 0xc006962390, 0x2)
/usr/local/google/home/pjw/tools/internal/imports/imports.go:103 +0x165
golang.org/x/tools/internal/lsp/source.computeFixEdits({0x1179590, 0xc003a78000}, 0xc0051021e0, 0x1f2f?, {0x0, 0x0, 0x0})
/usr/local/google/home/pjw/tools/internal/lsp/source/format.go:195 +0x177
golang.org/x/tools/internal/lsp/source.computeImportEdits({0x1179590, 0xc003a78000}, 0xc0051021e0, 0x57f127?)
/usr/local/google/home/pjw/tools/internal/lsp/source/format.go:140 +0xce
golang.org/x/tools/internal/lsp/source.AllImportsFixes.func1(0xdd8f20?)
/usr/local/google/home/pjw/tools/internal/lsp/source/format.go:121 +0x4a
golang.org/x/tools/internal/lsp/cache.(*importsState).runProcessEnvFunc(0xc00024cea0, {0x11716f8, 0xc006962240}, 0xc003a78000, 0xc006539280)
/usr/local/google/home/pjw/tools/internal/lsp/cache/imports.go:114 +0x5a3
golang.org/x/tools/internal/lsp/cache.(*snapshot).RunProcessEnvFunc(0x1171650?, {0x11716f8?, 0xc006962240?}, 0x1171960?)
/usr/local/google/home/pjw/tools/internal/lsp/cache/view.go:374 +0x34
golang.org/x/tools/internal/lsp/source.AllImportsFixes({0x1171650, 0xc00041ab00}, {0x1179590?, 0xc003a78000}, {0x1171960, 0xc0034097a0})
/usr/local/google/home/pjw/tools/internal/lsp/source/format.go:120 +0x308

@pjweinb pjweinb self-assigned this Apr 5, 2022
@gopherbot
Copy link

Change https://go.dev/cl/398395 mentions this issue: internal/imports: ignore some line directives

@findleyr findleyr modified the milestones: gopls/v0.8.3, gopls/v0.8.4 Apr 5, 2022
@rsc rsc unassigned pjweinb Jun 22, 2022
rinchsan pushed a commit to rinchsan/gosimports that referenced this issue Aug 14, 2022
@pjweinbgo @rinchsan
internal/imports: ignore some line directives
c27bdbb 
This is a minimal fix for the reported panic. The existing code
used FileSet.Position() where it should have used FileSet.PositionFor(, false).

gopls does this in lots of places; each use of Position() needs to
be thought about.

x/tools/go/ast/astutil/import.go:273,274 is another example.

Fixes: golang/go#51916

Change-Id: I42de6affca2c97c09efb7974a9e44322556b3ffd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/398395
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Trust: Peter Weinberger <pjw@google.com>
@golang golang locked and limited conversation to collaborators Jun 22, 2023
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. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.8.4
Development

No branches or pull requests
4 participants
@pjweinb @griesemer @gopherbot @findleyr