-
Notifications
You must be signed in to change notification settings - Fork 773
Make (*WatchedFiles[T]).Clone propagate nil
#2252
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
Conversation
There was a problem hiding this 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 panic that occurred when WatchEnabled is false but TypingsLocation is set. In this scenario, the typingsWatch field (and other watch fields) remain nil, but ATA can still attempt to clone them, causing a nil pointer dereference when acquiring the mutex.
- Added nil check to
WatchedFiles[T].Clone()to propagate nil instead of panicking - Added unit test to verify nil propagation behavior
- Added integration test to ensure ATA works correctly with WatchEnabled=false
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/project/watch.go | Added nil check in Clone method to prevent panic when called on nil receiver |
| internal/project/watch_test.go | Added unit test verifying Clone returns nil when called on nil WatchedFiles |
| internal/project/ata/ata_test.go | Added integration test ensuring ATA doesn't panic with WatchEnabled=false but TypingsLocation set |
|
Where do we end up trying to call clone on nil? I can imagine this fix works, but I'm not sure why we even get that far |
In |
|
So this would be hard to emulate in VS Code (or it just doesn't happen) because it supports the capability typescript-go/internal/lsp/server.go Lines 1005 to 1008 in 7c0aa9e
typescript-go/internal/lsp/server.go Lines 1032 to 1049 in 7c0aa9e
And I believe what happens is that when creating the first project, this conditionally sets typescript-go/internal/project/project.go Lines 151 to 174 in d876d05
So it's not always there. |
Fixes #2248.