Skip to content

Commit

Permalink
feat: LSP autocompletion for use statement (#5704)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
asterite authored Aug 9, 2024
1 parent 2166c94 commit 226aeb1
Show file tree
Hide file tree
Showing 11 changed files with 868 additions and 87 deletions.
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/hir/def_map/module_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ impl ModuleData {
&self.scope
}

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

fn declare(
&mut self,
name: Ident,
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ pub enum ParserErrorReason {
ExpectedFieldName(Token),
#[error("expected a pattern but found a type - {0}")]
ExpectedPatternButFoundType(Token),
#[error("expected an identifier after ::")]
ExpectedIdentifierAfterColons,
#[error("Expected a ; separating these two statements")]
MissingSeparatingSemi,
#[error("constrain keyword is deprecated")]
Expand Down
30 changes: 28 additions & 2 deletions compiler/noirc_frontend/src/parser/parser/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,19 @@ pub fn path_no_turbofish() -> impl NoirParser<Path> {
}

fn path_inner<'a>(segment: impl NoirParser<PathSegment> + 'a) -> impl NoirParser<Path> + 'a {
let segments = segment.separated_by(just(Token::DoubleColon)).at_least(1);
let segments = segment
.separated_by(just(Token::DoubleColon))
.at_least(1)
.then(just(Token::DoubleColon).then_ignore(none_of(Token::LeftBrace).rewind()).or_not())
.validate(|(path_segments, trailing_colons), span, emit_error| {
if trailing_colons.is_some() {
emit_error(ParserError::with_reason(
ParserErrorReason::ExpectedIdentifierAfterColons,
span,
));
}
path_segments
});
let make_path = |kind| move |segments, span| Path { segments, kind, span };

let prefix = |key| keyword(key).ignore_then(just(Token::DoubleColon));
Expand Down Expand Up @@ -69,7 +81,7 @@ mod test {
use super::*;
use crate::parser::{
parse_type,
parser::test_helpers::{parse_all_failing, parse_with},
parser::test_helpers::{parse_all_failing, parse_recover, parse_with},
};

#[test]
Expand Down Expand Up @@ -111,4 +123,18 @@ mod test {
vec!["crate", "crate::std::crate", "foo::bar::crate", "foo::dep"],
);
}

#[test]
fn parse_path_with_trailing_colons() {
let src = "foo::bar::";

let (path, errors) = parse_recover(path_no_turbofish(), src);
let path = path.unwrap();
assert_eq!(path.segments.len(), 2);
assert_eq!(path.segments[0].ident.0.contents, "foo");
assert_eq!(path.segments[1].ident.0.contents, "bar");

assert_eq!(errors.len(), 1);
assert_eq!(errors[0].message, "expected an identifier after ::");
}
}
23 changes: 15 additions & 8 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use fm::{codespan_files as files, FileManager};
use fxhash::FxHashSet;
use lsp_types::{
request::{
DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest, References,
Rename,
Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest,
References, Rename,
},
CodeLens,
};
Expand All @@ -36,7 +36,10 @@ use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelecti
use noirc_driver::{file_manager_with_stdlib, prepare_crate, NOIR_ARTIFACT_VERSION_STRING};
use noirc_frontend::{
graph::{CrateId, CrateName},
hir::{def_map::parse_file, Context, FunctionNameMatch, ParsedFiles},
hir::{
def_map::{parse_file, CrateDefMap},
Context, FunctionNameMatch, ParsedFiles,
},
node_interner::NodeInterner,
parser::ParserError,
ParsedModule,
Expand All @@ -48,11 +51,11 @@ use notifications::{
on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized,
};
use requests::{
on_code_lens_request, on_document_symbol_request, on_formatting, on_goto_declaration_request,
on_goto_definition_request, on_goto_type_definition_request, on_hover_request, on_initialize,
on_inlay_hint_request, on_prepare_rename_request, on_profile_run_request,
on_references_request, on_rename_request, on_shutdown, on_test_run_request, on_tests_request,
LspInitializationOptions,
on_code_lens_request, on_completion_request, on_document_symbol_request, on_formatting,
on_goto_declaration_request, on_goto_definition_request, on_goto_type_definition_request,
on_hover_request, on_initialize, on_inlay_hint_request, on_prepare_rename_request,
on_profile_run_request, on_references_request, on_rename_request, on_shutdown,
on_test_run_request, on_tests_request, LspInitializationOptions,
};
use serde_json::Value as JsonValue;
use thiserror::Error;
Expand All @@ -62,6 +65,7 @@ mod notifications;
mod requests;
mod solver;
mod types;
mod utils;

#[cfg(test)]
mod test_utils;
Expand All @@ -86,6 +90,7 @@ pub struct LspState {
cached_lenses: HashMap<String, Vec<CodeLens>>,
cached_definitions: HashMap<String, NodeInterner>,
cached_parsed_files: HashMap<PathBuf, (usize, (ParsedModule, Vec<ParserError>))>,
cached_def_maps: HashMap<String, BTreeMap<CrateId, CrateDefMap>>,
options: LspInitializationOptions,
}

Expand All @@ -103,6 +108,7 @@ impl LspState {
cached_definitions: HashMap::new(),
open_documents_count: 0,
cached_parsed_files: HashMap::new(),
cached_def_maps: HashMap::new(),
options: Default::default(),
}
}
Expand Down Expand Up @@ -136,6 +142,7 @@ impl NargoLspService {
.request::<Rename, _>(on_rename_request)
.request::<HoverRequest, _>(on_hover_request)
.request::<InlayHintRequest, _>(on_inlay_hint_request)
.request::<Completion, _>(on_completion_request)
.notification::<notification::Initialized>(on_initialized)
.notification::<notification::DidChangeConfiguration>(on_did_change_configuration)
.notification::<notification::DidOpenTextDocument>(on_did_open_text_document)
Expand Down
6 changes: 3 additions & 3 deletions tooling/lsp/src/notifications/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(super) fn on_did_change_configuration(
ControlFlow::Continue(())
}

pub(super) fn on_did_open_text_document(
pub(crate) fn on_did_open_text_document(
state: &mut LspState,
params: DidOpenTextDocumentParams,
) -> ControlFlow<Result<(), async_lsp::Error>> {
Expand Down Expand Up @@ -153,8 +153,8 @@ pub(crate) fn process_workspace_for_noir_document(
Some(&file_path),
);
state.cached_lenses.insert(document_uri.to_string(), collected_lenses);

state.cached_definitions.insert(package_root_dir, context.def_interner);
state.cached_definitions.insert(package_root_dir.clone(), context.def_interner);
state.cached_def_maps.insert(package_root_dir.clone(), context.def_maps);

let fm = &context.file_manager;
let files = fm.as_file_map();
Expand Down
Loading

0 comments on commit 226aeb1

Please sign in to comment.