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

Renaming a .res or .resi to another file extension crashes ReScript language server #392

Closed
cknitt opened this issue Apr 22, 2022 · 6 comments · Fixed by #394 or #422
Closed

Renaming a .res or .resi to another file extension crashes ReScript language server #392

cknitt opened this issue Apr 22, 2022 · 6 comments · Fixed by #394 or #422

Comments

@cknitt
Copy link
Member

cknitt commented Apr 22, 2022

I am running one of the latest master builds of the VS Code extension.

When I rename some A.res or A.resi to another file extension, e.g., A.resx, the ReScript language server crashes. This even occurs when A.res/A.resi is empty.

@cristianoc
Copy link
Collaborator

@cknitt would you give more details on how to repro the crash?

@cknitt
Copy link
Member Author

cknitt commented Apr 22, 2022

@cristianoc In the project I am currently working on, just creating an empty A.res and renaming it to A.resx (by selecting it in the VS Code file tree, pressing enter, modifying the name and pressing enter again) reproduces the crash.

Is it not reproducible for you this way? Then maybe it has something to do with my project setup (yarn workspaces or whatever).

@cknitt
Copy link
Member Author

cknitt commented Apr 22, 2022

It does not seem to be dependent on my project setup. I can even reproduce this as described with a new project created from scratch following https://rescript-lang.org/docs/manual/latest/installation#new-project.

@cristianoc
Copy link
Collaborator

Turns out vscode calls rescript operations such as semanticTokens and documentSymbol on non-rescript file A.resx, and does not even send openedFile on A.resx.

cristianoc added a commit that referenced this issue Apr 22, 2022
cristianoc added a commit that referenced this issue Apr 22, 2022
@cristianoc
Copy link
Collaborator

For what it's worth. Another way to crash the extension: open a non-rescript file, force the editor to consider it ReScript, force an autocomplete.

@cristianoc cristianoc reopened this May 13, 2022
@cristianoc
Copy link
Collaborator

This resurfaced after merging the actions branch.

cristianoc added a commit that referenced this issue May 13, 2022
More robust solution where the file cache is set also for files with extension not `.res` or `.resi`.
In case of rename e.g. from `.res` to `.rex` an opened file notification is sent for the `.rex` file and a crash happens when trying to get the current file content.
After this change, the file content array is set also for the .rex` file, preventing the crash.

Fixes #392
cristianoc added a commit that referenced this issue May 13, 2022
More robust solution where the file cache is set also for files with extension not `.res` or `.resi`.
In case of rename e.g. from `.res` to `.rex` an opened file notification is sent for the `.rex` file and a crash happens when trying to get the current file content.
After this change, the file content array is set also for the .rex` file, preventing the crash.

Fixes #392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants