Skip to content

fix: Crash for completions after JSDoc * on line following string-named property signature #2342

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.

Sign up for GitHub

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

Open
jgsheppa wants to merge 2 commits into
base: main
Choose a base branch
from
+35 −1
Open

fix: Crash for completions after JSDoc * on line following string-named property signature #2342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
031ae5f
test: Write test for string property crash
jgsheppa Dec 11, 2025
4f3d47e
fix: Expand logic to include type elements in JSDoc
jgsheppa Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/astnav/tokens.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func getTokenAtPosition(
flags := scanner.TokenFlags()
if tokenStart <= position && (position < tokenEnd) {
if token == ast.KindIdentifier || !ast.IsTokenKind(token) {
if ast.IsJSDocKind(current.Kind) {
if ast.IsJSDocKind(current.Kind) || ast.IsTypeElement(current) {
Copy link
Member

@DanielRosenwasser DanielRosenwasser Dec 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel like the right fix- the code path here is meant to be extremely general, and I think you can repro this for any construct that

  1. Contains a list of nodes (e.g. a NodeList)
  2. Intersperses with a *
/**
 * @type {[
 * /*$*/ SomeType,
 * ]} 
 */
error] panic handling request textDocument/completion: did not expect KindTypeReference to have KindIdentifier in its trivia
goroutine 3162 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:26 +0x5e
github.com/microsoft/typescript-go/internal/lsp.(*Server).recover(0xc0001d6008, 0xc00e95fda0)
	github.com/microsoft/typescript-go/internal/lsp/server.go:701 +0x4c
panic({0xbb8320?, 0xc00df28a80?})
	runtime/panic.go:783 +0x132
github.com/microsoft/typescript-go/internal/astnav.getTokenAtPosition(0xc01249e008, 0x13, 0x1, 0x0)
	github.com/microsoft/typescript-go/internal/astnav/tokens.go:196 +0x5f9
github.com/microsoft/typescript-go/internal/astnav.GetTokenAtPosition(...)
	github.com/microsoft/typescript-go/internal/astnav/tokens.go:40
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getCompletionData(0xc00dba6240, {0x109efa8, 0xc00dba61e0}, 0xc003df8c08, 0xc01249e008, 0x13, 0xc00371f400)
	github.com/microsoft/typescript-go/internal/ls/completions.go:454 +0xce
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).getCompletionsAtPosition(0xc00dba6240, {0x109efa8, 0xc00dba61e0}, 0xc01249e008, 0x13, 0x0)
	github.com/microsoft/typescript-go/internal/ls/completions.go:383 +0x2cf
github.com/microsoft/typescript-go/internal/ls.(*LanguageService).ProvideCompletion(0xc00dba6240, {0x109efa8, 0xc00dba61e0}, {0xc003d82390?, 0xc00dba61e0?}, {0x3d82390?, 0xc0?}, 0xc0151180f0)
	github.com/microsoft/typescript-go/internal/ls/completions.go:44 +0xc8
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleCompletion(0xc00110e008?, {0x109efa8?, 0xc00dba61e0?}, 0xc003d82390?, 0x40b92c?)
	github.com/microsoft/typescript-go/internal/lsp/server.go:1011 +0x39
github.com/microsoft/typescript-go/internal/lsp.init.func1.registerLanguageServiceDocumentRequestHandler[...].16({0x109efa8, 0xc00dba61e0}, 0xc00e95fda0)
	github.com/microsoft/typescript-go/internal/lsp/server.go:617 +0x130
github.com/microsoft/typescript-go/internal/lsp.(*Server).handleRequestOrNotification(0xc0001d6008, {0x109efe0?, 0xc00f516410?}, 0xc00e95fda0)
	github.com/microsoft/typescript-go/internal/lsp/server.go:501 +0x14b
github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop.func1()
	github.com/microsoft/typescript-go/internal/lsp/server.go:404 +0x3a
created by github.com/microsoft/typescript-go/internal/lsp.(*Server).dispatchLoop in goroutine 7
	github.com/microsoft/typescript-go/internal/lsp/server.go:424 +0x9ad

Copy link
Member

@DanielRosenwasser DanielRosenwasser Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another example:

/**
 * @type {(
 *  a: string
 * /**/ b
 * ) => string} 
 */

Copy link
Member

@gabritto gabritto Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in general the panic we had before was correct: if we're walking trivia for a non-JSDoc node, we shouldn't find a dangling identifier. Maybe the root problem is that if we're inside a JSDoc, but current is a non-JSDoc node, we run into problems when scanning for the in-between tokens because we're also inside JSDoc?

Copy link
Author

@jgsheppa jgsheppa Dec 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably should have asked some preliminary question in the ticket, but I'll ask them now 😅

The panic would cause the build to fail, as I understand the code, which seems like the desired behavior if there is an additional comment, such as /*$*/ inside of a JSDoc. So what @gabritto sounds right to me. But what is the expected behavior given an example such as the on below?

/**
 * @type {(
 *  a: string
 * /**/ b
 * ) => string} 
 */

Or is my understanding off the mark?

return current
}
panic(fmt.Sprintf("did not expect %s to have %s in its trivia", current.Kind.String(), token.String()))
Expand Down
34 changes: 34 additions & 0 deletions internal/fourslash/tests/completionsJsdocStringProperty_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package fourslash_test

import (
"testing"

"github.com/microsoft/typescript-go/internal/fourslash"
. "github.com/microsoft/typescript-go/internal/fourslash/tests/util"
"github.com/microsoft/typescript-go/internal/testutil"
)

func TestCompletionsJsdocStringProperty(t *testing.T) {
t.Parallel()
defer testutil.RecoverAndFail(t, "Panic on fourslash test")

const content = `/**
* @type {{
* 'string-property': boolean;
*/*$*/ identifierProperty: boolean;
* }}
*/
var someVariable;`

f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
defer done()
f.GoToMarker(t, "$")
f.VerifyCompletions(t, nil, &fourslash.CompletionsExpectedList{
IsIncomplete: false,
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
CommitCharacters: &[]string{".", ",", ";"},
EditRange: Ignored,
},
Items: &fourslash.CompletionsExpectedItems{},
})
}