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

Bug: LSP features don't work well with unsaved documents #5562

Closed
asterite opened this issue Jul 19, 2024 · 0 comments · Fixed by #5561
Closed

Bug: LSP features don't work well with unsaved documents #5562

asterite opened this issue Jul 19, 2024 · 0 comments · Fixed by #5561
Labels
bug Something isn't working

Comments

@asterite
Copy link
Collaborator

Aim

LSP features should work well if a file has unsaved contents. For example, document symbols, inlay hints, references, etc., should all work well if, say, a struct is moved to a difference place in a file but that file isn't yet saved: references to that struct should take you to the new location, even though the file is unsaved.

Expected Behavior

This is expected behavior:

lsp-good.mov

Note how when we edit the file but don't save it, inlay hints are correctly updated. We can even define a struct, add a reference to it and jump to it, all while the file is unsaved.

Bug

This is how it currently works:

lsp-wrong.mov

Note how inlay hints still are always based on the saved version of the file, and how you can't jump to "Foo".

To Reproduce

No response

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

No response

Installation Method

None

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

Yes

Support Needs

No response

@asterite asterite added the bug Something isn't working label Jul 19, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 23, 2024
# Description

## Problem

Resolves #5562

## Summary

I noticed that our LSP code puts unsaved files into an `input_files`
dictionary, but that dictionary was not used when adding files to a
FileManager, meaning that file sources always corresponded to disk
contents, never unsaved content.

Additionally, when we got a notification that a file changed, only that
file (without everything it depended on) was processed, just to get code
lenses. That means that references to types and variables weren't
updated here, so things like hover, go to reference and inlay hints
wouldn't work well.

To fix this, a few things have been done in this PR:
1. When creating a FileManager, always add these unsaved files first.
2. When we get the notification that a file was changed, type-check only
the current package but without producing errors (at least Rust Analyzer
doesn't update the Problems list until you save a file). This is also
how it used to work before this PR.
3. When a file is closed, we also type-check the current package to
update references (to avoid keeping references to that unsaved code
locations).

Type-checking a package on every edit sounds a bit too much... but I
checked a few big crates and type-checking just a single package (in a
workspace of many packages) takes around 300ms on my machine (with a
debug build), so I think it's acceptable. If it's slower then it might
still be good enough (it's worse if things break and you get incorrect
inlay hints or error notifications). Maybe ideally we'd process all
top-level definitions in a package, but only fully type-check (looking
inside function definitions) in the file that just changed, but I
couldn't find an easy way to do that, so that would be an optimization
for later (if really needed).

## Additional Context

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant