Skip to content

Commit

Permalink
feat: LSP path completion (#5712)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
asterite authored Aug 12, 2024
1 parent a31f82e commit 3c6b998
Show file tree
Hide file tree
Showing 7 changed files with 1,700 additions and 84 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions compiler/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ tracing.workspace = true
petgraph = "0.6"
rangemap = "1.4.0"
lalrpop-util = { version = "0.20.2", features = ["lexer"] }
strum = "0.24"
strum_macros = "0.24"


[dev-dependencies]
base64.workspace = true
strum = "0.24"
strum_macros = "0.24"

[build-dependencies]
lalrpop = "0.20.2"
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/def_map/module_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl ModuleData {
}
}

pub(crate) fn scope(&self) -> &ItemScope {
pub fn scope(&self) -> &ItemScope {
&self.scope
}

Expand Down
3 changes: 1 addition & 2 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -883,8 +883,7 @@ impl AsRef<str> for SecondaryAttribute {

/// Note that `self` is not present - it is a contextual keyword rather than a true one as it is
/// only special within `impl`s. Otherwise `self` functions as a normal identifier.
#[derive(PartialEq, Eq, Hash, Debug, Copy, Clone, PartialOrd, Ord)]
#[cfg_attr(test, derive(strum_macros::EnumIter))]
#[derive(PartialEq, Eq, Hash, Debug, Copy, Clone, PartialOrd, Ord, strum_macros::EnumIter)]
pub enum Keyword {
As,
Assert,
Expand Down
1 change: 1 addition & 0 deletions tooling/lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ noirc_frontend.workspace = true
noirc_artifacts.workspace = true
serde.workspace = true
serde_json.workspace = true
strum = "0.24"
tower.workspace = true
async-lsp = { workspace = true, features = ["omni-trait"] }
serde_with = "3.2.0"
Expand Down
Loading

0 comments on commit 3c6b998

Please sign in to comment.