Skip to content

Conversation

@gabritto
Copy link
Member

@gabritto gabritto commented Dec 12, 2025

Fixes #2290.

We were scanning into an AST node in getTokenAtPosition, basically because in this JSDoc case:

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

identifierProperty's position does not include what would typically be its leading trivia. So the positions between the starting * and identifierProperty effectively don't belong to any node or token in the AST, but getTokenAtPosition operated under the assumption that this was impossible, and therefore that we should scan and find a non-tree token in between * and identifierProperty.

I added code to explicitly track what the next AST node is (nodeAfterLeft) whenever we update the scanner starting position (left), so we can then enforce the invariant of scanning only in between AST nodes.

Note that for now we simply return nil in these cases, like Strada did, but we could actually start detecting it and returning a token. I think this would require extra changes and testing, so I'm leaving that for a future PR.

@jakebailey jakebailey mentioned this pull request Dec 13, 2025
@ahejlsberg
Copy link
Member

@gabritto See my comment here.

@gabritto gabritto marked this pull request as ready for review December 29, 2025 18:06
Copilot AI review requested due to automatic review settings December 29, 2025 18:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes issue #2290 where getTokenAtPosition was incorrectly scanning into AST nodes when the position fell in JSDoc trivia that doesn't belong to any node. The fix introduces tracking of the next AST node (nodeAfterLeft) to enforce the invariant that scanning only occurs between AST nodes, preventing the scanner from advancing into positions that would incorrectly return tokens from within a node's span.

Key changes:

  • Added nodeAfterLeft variable to track the first node after the current left boundary
  • Updated visitNode, visitNodeList functions to maintain nodeAfterLeft tracking
  • Modified scanner loop to respect the nodeAfterLeft boundary when scanning for tokens
  • Added fourslash test to verify completions work correctly in JSDoc trivia positions

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/fourslash/tests/completionsJSDocTrivia_test.go New test case verifying that completions don't crash in JSDoc trivia positions where identifiers don't include their leading trivia
internal/astnav/tokens.go Implements nodeAfterLeft tracking to prevent scanning past the next AST node boundary, fixing the bug where scanner would incorrectly scan into a node's span

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash for completions after JSDoc * on line following string-named property signature

3 participants