-
Notifications
You must be signed in to change notification settings - Fork 219
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
LSP: Support completions #1577
Labels
Comments
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 9, 2024
# Description ## Problem Part of #1577 ## Summary Implements autocompletion for use statements, so typing anything after `use`. https://github.com/user-attachments/assets/a9ecd738-30c2-41fc-8ea0-3ea93d5d99ab Note: the gray hints are probably coming from copilot, I should have turned it off before recording 😅 ## Additional Context To begin implementing autocompletion I thought starting with `use` would be the easiest thing, but also not having to look up the std (or your project) module hierarchy could be useful. To implement this I had to allow parsing `use foo::`, that is, allowing trailing colons, because otherwise that wouldn't parse to a use statement and we wouldn't have any information to do the autocompletion. There are still some cases that don't work. For example if you write `use ` and explicitly ask for autocompletion, you don't get anything. The reason is that `use ` gives a parse error so no AST is produced. We could tackle those in later PRs (I didn't want to continue making this PR bigger). This PR also introduces a simple way to test autocompletions, so next autocompletion PRs should be much smaller. Another thought: at first I was worried that caching `def_maps` in the LSP state would increase the memory usage by a lot, but in retrospective I think that's not a heavy data structure, or at least `NodeInterner`, which was already cached, is probably much larger in memory footprint. I tried this PR in a few projects and the memory usage was fine. ## 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-merge-queue bot
pushed a commit
that referenced
this issue
Aug 12, 2024
# Description ## Problem Part of #1577 ## Summary LSP will now suggest completions anywhere there's a Path. Here are some scenarios: ### Autocomplete functions in full paths, with parameters ![lsp-complete-function](https://github.com/user-attachments/assets/48f1bc08-50e6-4c10-b22a-8c99dd67736d) ### Autocomplete functions when they exist in a use statement ![lsp-complete-function-after-use](https://github.com/user-attachments/assets/74f27049-ec4d-4722-972b-586e1f834969) ### Built-in functions are also suggested ![lsp-builtin-functions](https://github.com/user-attachments/assets/17ecd98b-7001-4981-a1ba-f54ec4aa8ca3) ### Autocomplete local variables ![lsp-local-var](https://github.com/user-attachments/assets/671efe94-4a4d-4cd8-a66b-982d67276383) ### Autocomplete types in several type positions ![lsp-suggest-type](https://github.com/user-attachments/assets/e52a965a-59b1-47a9-aff2-c5c7cbce8e37) ### Built-in types are also suggested ![lsp-builtin-type](https://github.com/user-attachments/assets/9b3c0bb6-b429-447a-9b13-7d1d939127dc) ### Autocomplete type parameters ![lsp-type-parameter](https://github.com/user-attachments/assets/e8bd3e8d-ac20-48f9-b416-426d9ddfcd4e) ## Additional Context Sorry for the large diff, the bulk of it is traversing the AST. I thought about using something like the Visitor pattern, but I'm not exactly sure how it would look in Rust and whether it would greatly reduce the amount of code. It could be a separate refactor, though! (and we have tons of tests now, so it should be safe to do too). Another big chunk of code is tests... So the actual logic isn't that large. Some more things could be done on top of this, like suggesting `Self` if it's available, but I wanted to stop making this PR bigger 😊 ## 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.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 13, 2024
…#5714) # Description ## Problem Part of #1577 ## Summary Suggest struct fields and methods after you type "." or "::". ![lsp-methods](https://github.com/user-attachments/assets/ff242c4f-6a90-4bc1-bc90-9343141f169d) Here it's working in a project inside Aztec-Packages: ![lsp-methods-2](https://github.com/user-attachments/assets/0b3c9496-b2ea-4d6c-9e50-2fc4295420e1) ## Additional Context Some things don't work well yet. For example if you chain some calls, like `[1, 2, 3].as_slice().` nothing is offered there. It has to do with how things are stored in `id_to_location`. Or if you type `if something.` then nothing gets offered because that doesn't parse right. We could improve those things on later PRs, though, but at least autocompletion now is much better than before. ## 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-merge-queue bot
pushed a commit
that referenced
this issue
Aug 13, 2024
…#5714) # Description ## Problem Part of #1577 ## Summary Suggest struct fields and methods after you type "." or "::". ![lsp-methods](https://github.com/user-attachments/assets/ff242c4f-6a90-4bc1-bc90-9343141f169d) Here it's working in a project inside Aztec-Packages: ![lsp-methods-2](https://github.com/user-attachments/assets/0b3c9496-b2ea-4d6c-9e50-2fc4295420e1) ## Additional Context Some things don't work well yet. For example if you chain some calls, like `[1, 2, 3].as_slice().` nothing is offered there. It has to do with how things are stored in `id_to_location`. Or if you type `if something.` then nothing gets offered because that doesn't parse right. We could improve those things on later PRs, though, but at least autocompletion now is much better than before. ## 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.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 15, 2024
# Description ## Problem Part of #1577 (there are many more completions we could support and it's hard to list all of them upfront, so I'll just link to that issue from now on) ## Summary Suggest tuple fields (0, 1) in autocompletion, which is useful if you don't know the type you are handling with (maybe it's a result of another expression that's not in a variable). Also I noticed tuple methods showed up duplicate, so that's fixed too. Before: ![lsp-complete-tuple-before](https://github.com/user-attachments/assets/9dc36a32-9017-4b3d-a16c-091ea6ab5a42) After: ![lsp-complete-tuple-after](https://github.com/user-attachments/assets/70b6d9fd-4f4f-4d57-ab45-55a270fa64cb) ## Additional Context The two first commits are just refactors, so I'd suggest going commit by commit. ## 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.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 15, 2024
# Description ## Problem Part of #1577 ## Summary ![lsp-complete-constructor-fields](https://github.com/user-attachments/assets/7b822fd8-1852-492f-9d7f-48ae45e171a9) ^ The above is just an example of it working, I'm not encouraging using constructors of std types 😄 ## Additional Context This is the last autocompletion I had in mind while implementing autocompletion, so I'll likely won't send any other LSP PRs for a while... That said, if you think of other autocompletions (that don't involve auto-importing, because that's a big thing) let me know (or capture an issue) and we can work on it. ## 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.
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 15, 2024
# Description ## Problem Part of #1577 ## Summary ![lsp-complete-trait-methods](https://github.com/user-attachments/assets/2d608450-1d6d-475d-8433-9b9f6e60c357) ## Additional Context Also includes a couple of refactors to further simplify things. ## 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-merge-queue bot
pushed a commit
that referenced
this issue
Aug 15, 2024
# Description ## Problem Part of #1577 ## Summary ![lsp-complete-trait-methods](https://github.com/user-attachments/assets/2d608450-1d6d-475d-8433-9b9f6e60c357) ## Additional Context Also includes a couple of refactors to further simplify things. ## 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-merge-queue bot
pushed a commit
that referenced
this issue
Aug 16, 2024
# Description ## Problem Part of #1577 ## Summary I said I wasn't going to send any more LSP features, but I had this one in the back of my head since I started working on autocompletion. I had the idea of how to do it in my head, but I wanted to first work on the easier stuff (regular autocompletion). It turns out auto-import wasn't that hard to implement after all! ![lsp-autoimport](https://github.com/user-attachments/assets/9d2268fb-5caf-4b42-9bb3-b01f6ca40a9b) ![lsp-autoimport-nested](https://github.com/user-attachments/assets/ac4cd18a-d87d-4311-82d0-61f6d5a8dd19) ## Additional Context In this PR every new imported item is added as a full path. Ideally, like in Rust Analyzer, we'd group common use prefixes, add to existing ones, etc. We can do that! But maybe it could be part of a follow-up PR, or maybe we could do it much later. Or maybe `nargo fmt` could handle this, so the auto-import doesn't do it, but once you save the file it's done. That's why I didn't spend time on that in this PR. There's another thing I noticed while working on this PR: Rust Analyzer will offer matches that match **any** part of the name, not just the beginning. So, in Noir, if you type `merkle` it should probably offer `compute_merkle_root` as an auto-import completion, which is nice because maybe you are looking for stuff related to "merkle" but don't know the full name... but I didn't want to introduce that change in this PR (it also works for every other autocompletion). But, as always, this could be done later on. ## 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.
This was referenced Aug 19, 2024
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 19, 2024
…5752) # Description ## Problem Part of #1577 ## Summary As I mentioned in #5741, I noticed Rust Analyzer will suggests methods and names where the prefix you are writing matches any part of the name. Well, kind of, the logic is not exactly that, but that's what's implemented in this PR. It should make finding things much easier (if you are looking for stuff related to "merkle" or "hasher" you just need to type that). ![lsp-complete-any-part](https://github.com/user-attachments/assets/a65adc20-fc96-4682-b1c3-8961c434a133) It works in any context, for example struct fields: ![lsp-complete-any-part-struct-field](https://github.com/user-attachments/assets/f52193ef-adf7-493b-afa5-dbae9009857e) ## Additional Context None. ## 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-merge-queue bot
pushed a commit
that referenced
this issue
Aug 19, 2024
# Description ## Problem Part of #1577 ## Summary Autocompletion wasn't triggering in function parameter types, or in impl names. The reason is that when you start writing them the body will still be missing, and that discarded the entire fn or impl. In this PR we make that parse well, but produce an error (and create a fake empty body). Now autocompletion works fine in these scenarios: ![lsp-recover-fn](https://github.com/user-attachments/assets/e2358d49-c2e3-473b-bd4c-bfc6c0ff43e9) ![lsp-recover-impl](https://github.com/user-attachments/assets/a8fb1d44-86ce-4514-b167-f1ff6eb4b9b7) ## Additional Context I'm not sure about the many new "parser error" introduced, maybe there should be a generic "expected these tokens"? I think chumsky has one of those errors but it's kind of tied to the parsing logic. On the other hand adding new errors is simple, so maybe it's fine. ## 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-merge-queue bot
pushed a commit
that referenced
this issue
Aug 20, 2024
# Description ## Problem Part of #1577 ## Summary We recently added `super::` to `use` so I thought it would be nice if the auto-importer used that if possible. ![lsp-use-super](https://github.com/user-attachments/assets/4f8cbf37-f844-4293-832e-bc9685295837) ## Additional Context This PR also includes a small refactor in the hover code because the code to format a module path exists there too (but the two are different enough that I didn't reuse them). ## 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. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 13, 2024
# Description ## Problem Part of #1577 ## Summary Now that we have the "Implement missing members" code action, it's a small step to have autocompletion suggest a trait method as you are typing its name (like in Rust Analyzer). The only tricky part was that `fn foo` produced a parse error so I made that parse to a function without arguments and without body (but still reporting a parser error). A nice side-effect of this is that if you type that and you think "Hm, what should the parameters be" the rest of the LSP features don't stop working (like, inlay hints don't disappear, etc.) Even though "Implement missing members" might seem to be better than offering completions one by one, this completion has two advantages: 1. It offers default methods 2. It teaches the user that LSP can suggest trait methods, so they could think "hm, I wonder if there's a code action that suggests all of them" (at least this was my experience for discovering the code action) ![lsp-suggest-trait-impl-method](https://github.com/user-attachments/assets/00d1a6f8-597b-4686-ab20-78cc145f22f4) ## 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
The LSP should support making completion suggestions following https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion
Happy Case
A user should receive completion suggestions when editing code while using an LSP client, such as the vscode plugin.
Alternatives Considered
No response
Additional Context
No response
Would you like to submit a PR for this Issue?
No
Support Needs
No response
The text was updated successfully, but these errors were encountered: