LSP: Support code actions #1579
Labels
Comments
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 2, 2024
# Description ## Problem Part of #1579 ## Summary There are many code actions we could support, but the most useful ones are importing unknown identifiers or fully-qualifying them. This is what this PR is about.  Also thanks to VS Code that it notices that if you request code action on an error, it actually requests code actions for that error span... so it seems like the two are connected, but they are not! And it works just fine :-) Also fixes a bug I found in "go to definition" when there was no definition to resolve to (precisely the case this code action works on). ## Additional Context Note that in Rust Analyzer it shows as "Qualify [name]" or "Import [name]" and if there are multiple possibilities, a popup shows up letting you choose which one. I wanted to do the same, but looking at the server trace for Rust Analyzer I see each code action has a "group" property, and that's how they are grouped, but... I couldn't find that property in the lsp-types crate, or even in the LSP specification 😮 . So for now each possibility will show up (no intermediate popup). But I think that's better than not having this functionality at all, and we could always improve it later if we find out how. ## 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 <[email protected]>
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 3, 2024
# Description ## Problem Part of #1579 ## Summary My second mostly-used code action in Rust is "Fill struct fields" (and "Fill match arms", but we don't have match in Noir yet). This PR implements that.  ## Additional Context We don't have `todo!()` in Noir, so I used `()` instead. I think the most helpful thing about this code action is filling out the field names, so using `()` or `todo!()` is almost the same as you'll have to replace either with something else. ## 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
Sep 12, 2024
# Description ## Problem Part of #1579 ## Summary Adds a code action to add missing trait impl methods and types. Default methods are not includeded. Here it's working for `Add`:  Here for `BigField`:  Here for a complex type in Aztec-Packages:  ## Additional Context I found this code action in Rust very useful! It saves a lot of time, plus there's no need to copy-paste :-) ## 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
Sep 23, 2024
# Description ## Problem Part of #1579 ## Summary This implements another commonly used feature: "remove unused import/s". This is when the entire `use` item is removed:  This is when part of the `use` item is removed:  This is when "Remove all the unused imports" is chosen with a selection range:  ## Additional Context It works exactly the same as Rust Analyzer: 1. You get a "Remove unused import", "Remove unused imports" or "Remove the entire `use` item" on each `use` item that has unused import(s) 2. You also get a "Remove all the unused imports" code action. This will remove all unused imports in the current selection. I need to clarify 2 because it's something I learned while coding this: a code action is suggested in the selection you have (the selection is the cursor in case there's no selection, though VSCode would sometimes use the current line's error span as the selection). That's why if you have the cursor on an import and choose "Remove all the unused imports" it just removes the imports in the current `use` item. You can get VSCode to actually remove **all** the unused imports if you select all the `use` items, then ask for code actions, then choose "Remove all the unused imports". To be honest this is kind of annoying, but it kind of also makes sense. We _could_ make "Remove all the unused imports" remove all of them regardless of what's the selection (or, well, if at least one selection has unused items) but we could do that as a follow-up. --- Other things to know: 1. This works by creating a new `UseTree` from the original `UseTree` but with imports removed, then converting that to a string (formatting it with `nargo fmt`). What happens if there were comments in the middle of the import? They are lost! But... this is also what Rust Analyzer does. I guess worse case scenario is you revert that code action and manually remove things (comments in these positions are very unlikely to happen). 2. I didn't test the "Remove all the unused imports" action because right now our tests don't support a selection range. But given that the logic for that is relatively simple (just get all the "remove unused import" edits and present that as a new code action) I thought it was okay not to test it (I did try it out, though). We could eventually add tests for this, though. ## 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 various code actions following https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction
Code actions can perform many behaviors, but a common one would be to import missing identifiers.
Happy Case
A user should be presented with useful, contextual code actions 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: