Skip to content
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

Fixes #7246, wrap getTypeContextAtPosition in try/catch #7247

Merged
merged 3 commits into from Jan 1, 2022

Conversation

tm1000
Copy link
Contributor

@tm1000 tm1000 commented Dec 30, 2021

See #7246 and psalm/psalm-vscode-plugin#107

Specifically if something crashes deep in Psalm we should try to catch most of these crashes in the language server and continue (if possible). This is one such instance where continuing is fine as the error comes from the result of live typing so the code completion (that is erroring) is wrong as the time of typing the letters.

This is really hard to actually give a code reproduction because it relies on Psalm having a live cache and then that cache changing while live typing. You can see this in the reference issue against psalm-vscode-plugin

@tm1000
Copy link
Contributor Author

tm1000 commented Dec 30, 2021

FYI: The Psalm error should be fixed but it's out of the scope of this PR because it was already broken before my code additions

@orklah
Copy link
Collaborator

orklah commented Dec 30, 2021

Yeah, it's a false positive, and a pretty convoluted one. You should be able to extract the conditional that checks both variable are not null from the try/catch and it should work fine.

@tm1000
Copy link
Contributor Author

tm1000 commented Dec 30, 2021

@orklah are you talking about this PR or the bugs outside the context of this PR (sorry just a little confused :-) )

@orklah
Copy link
Collaborator

orklah commented Dec 31, 2021

Sorry, I was talking about that failure in CI: https://github.com/vimeo/psalm/runs/4669003160?check_suite_focus=true#step:4:30

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Jan 1, 2022
@orklah orklah merged commit 8dc1a31 into vimeo:master Jan 1, 2022
@orklah
Copy link
Collaborator

orklah commented Jan 1, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants