Skip to content

Add check for nil symbol in getJSDocOrTag#2763

Merged
ahejlsberg merged 3 commits intomainfrom
fix-2720
Feb 12, 2026
Merged

Add check for nil symbol in getJSDocOrTag#2763
ahejlsberg merged 3 commits intomainfrom
fix-2720

Conversation

@ahejlsberg
Copy link
Member

Fixes #2720.

Copilot AI review requested due to automatic review settings February 11, 2026 20:18
@ahejlsberg
Copy link
Member Author

ahejlsberg commented Feb 11, 2026

Don't think we need a test here, it was just a missing nil check that wasn't ported from the corresponding Strada code.

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

Fixes a language-service hover panic by guarding against a nil base type symbol when retrieving JSDoc for inherited static members (issue #2720).

Changes:

  • Add a nil check for baseType.Symbol() before calling GetTypeOfSymbol in getJSDocOrTag.

Comment on lines 511 to 513
t := baseType
if isStatic {
if isStatic && baseType.Symbol() != nil {
t = c.GetTypeOfSymbol(baseType.Symbol())
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

Please add a regression test for this panic. getJSDocOrTag is invoked during hover, and this change is specifically preventing a nil-deref crash; having a focused test (e.g., in internal/ls/hover_test.go) that exercises the static-member + base type with nil Symbol() path would help ensure it doesn’t regress again.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +512 to 513
if isStatic && baseType.Symbol() != nil {
t = c.GetTypeOfSymbol(baseType.Symbol())
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

Minor: this calls baseType.Symbol() twice on the same value. Consider assigning it to a local (e.g., baseSym := baseType.Symbol()) to make the nil-check and subsequent use clearer and avoid duplicate calls.

Suggested change
if isStatic && baseType.Symbol() != nil {
t = c.GetTypeOfSymbol(baseType.Symbol())
baseSym := baseType.Symbol()
if isStatic && baseSym != nil {
t = c.GetTypeOfSymbol(baseSym)

Copilot uses AI. Check for mistakes.
@ahejlsberg
Copy link
Member Author

Added regression test by popular demand.

@ahejlsberg ahejlsberg enabled auto-merge February 12, 2026 22:15
@ahejlsberg ahejlsberg added this pull request to the merge queue Feb 12, 2026
Merged via the queue into main with commit e5de8c2 Feb 12, 2026
20 checks passed
@ahejlsberg ahejlsberg deleted the fix-2720 branch February 12, 2026 22:37
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.

panic handling request textDocument/hover

2 participants