Skip to content

Conversation

@gabritto
Copy link
Member

@gabritto gabritto commented Dec 8, 2025

Fixes #2241.

We needed target: esnext to repro the crash in tests.

I think the only kinds of expressions and subexpressions that can appear as a computed property name in type nodes we produce are identifiers and property/element accesses, so I've added support for those.

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 a crash in the inlay hints language server feature when processing property access expressions within computed property names. The issue specifically occurred with target: esnext when using expressions like [Symbol.dispose]() in object literals.

Key Changes:

  • Added handling for ast.KindPropertyAccessExpression in the inlay hints display parts visitor
  • Added a test case to verify the fix and prevent regression

Reviewed changes

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

File Description
internal/ls/inlay_hints.go Added a new case to handle PropertyAccessExpression nodes in the getInlayHintLabelParts switch statement, properly visiting the expression, adding a dot separator, and visiting the name
internal/fourslash/tests/inlayHintsUsing_test.go Added a test case that reproduces the crash scenario using Symbol.dispose in a computed property name with target: esnext

@gabritto
Copy link
Member Author

gabritto commented Dec 8, 2025

Forgot to push the baselines 🙁

@DanielRosenwasser
Copy link
Member

I think the only kinds of expressions and subexpressions that can appear as a computed property name in type nodes we produce are identifiers and property accesses (e.g. element accesses produce a grammar error), so I've only added a new case for property access expressions, but I could be wrong.

But this still causes an inlay hint crash.

using _defer = {
	[Symbol["dispose"]]() {},
};

So I think the right thing to do is probably guard early on not providing inlay hints if the name is not a supported node type.

@gabritto
Copy link
Member Author

gabritto commented Dec 8, 2025

using _defer = {
	[Symbol["dispose"]]() {},
};

How does that crash? I can't repro it with my PR.

@gabritto
Copy link
Member Author

gabritto commented Dec 8, 2025

using _defer = {
	[Symbol["dispose"]]() {},
};

How does that crash? I can't repro it with my PR.

Ok, I guess we can produce element accesses too in the node builder, but pretty sure nothing else. Going to update with a test for that too.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 8, 2025

What I mean more is that we shouldn't try to produce inlay hints in the first place for those - we should modify visitVariableLikeDeclaration, or maybe isHintableDeclaration should return false depending on the kind of decl.Name().

@gabritto
Copy link
Member Author

gabritto commented Dec 8, 2025

What I mean more is that we shouldn't try to produce inlay hints in the first place for those - we should modify visitVariableLikeDeclaration, or maybe isHintableDeclaration should return false depending on the kind of decl.Name().

The function I updated only ever visits type nodes that we ourselves produce from a checker type, so modulo bugs it should only have a limited number of (well-formed) ASTs to deal with.

@gabritto gabritto requested a review from jakebailey December 8, 2025 19:31
@gabritto gabritto enabled auto-merge December 8, 2025 19:42
@gabritto gabritto added this pull request to the merge queue Dec 8, 2025
Merged via the queue into main with commit a767276 Dec 8, 2025
22 checks passed
@gabritto gabritto deleted the gabritto/issue2241 branch December 8, 2025 19:59
@DanielRosenwasser
Copy link
Member

Oh, duh, I misunderstood, sorry.

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 when code contains disposer logic. Request textDocument/inlayHint failed.

4 participants