Skip to content

Commit

Permalink
feat(lsp): improve LSP code lens performance
Browse files Browse the repository at this point in the history
  • Loading branch information
kobyhallx committed Dec 15, 2023
1 parent 167bf06 commit 318b5d2
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 102 deletions.
18 changes: 9 additions & 9 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions tooling/lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ serde_json.workspace = true
tower.workspace = true
async-lsp = { workspace = true, features = ["omni-trait"] }
serde_with = "3.2.0"
thiserror.workspace = true
fm.workspace = true

[target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies]
Expand Down
42 changes: 39 additions & 3 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
collections::HashMap,
future::Future,
ops::{self, ControlFlow},
path::PathBuf,
path::{Path, PathBuf},
pin::Pin,
task::{self, Poll},
};
Expand All @@ -19,6 +19,8 @@ use async_lsp::{
};
use fm::codespan_files as files;
use lsp_types::CodeLens;
use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;
use noirc_frontend::{
graph::{CrateId, CrateName},
hir::{Context, FunctionNameMatch},
Expand All @@ -32,6 +34,7 @@ use requests::{
on_profile_run_request, on_shutdown, on_test_run_request, on_tests_request,
};
use serde_json::Value as JsonValue;
use thiserror::Error;
use tower::Service;

mod notifications;
Expand All @@ -42,13 +45,20 @@ mod types;
use solver::WrapperSolver;
use types::{notification, request, NargoTest, NargoTestId, Position, Range, Url};

#[derive(Debug, Error)]
pub enum LspError {
/// Error while Resolving Workspace - {0}.
#[error("Failed to Resolve Workspace - {0}")]
WorkspaceResolutionError(String),
}

// State for the LSP gets implemented on this struct and is internal to the implementation
pub struct LspState {
root_path: Option<PathBuf>,
client: ClientSocket,
solver: WrapperSolver,
input_files: HashMap<String, String>,
collected_lenses: Option<Vec<CodeLens>>,
cached_lenses: HashMap<String, Vec<CodeLens>>,
}

impl LspState {
Expand All @@ -58,7 +68,7 @@ impl LspState {
root_path: None,
solver: WrapperSolver(Box::new(solver)),
input_files: HashMap::new(),
collected_lenses: None,
cached_lenses: HashMap::new(),
}
}
}
Expand Down Expand Up @@ -179,3 +189,29 @@ fn byte_span_to_range<'a, F: files::Files<'a> + ?Sized>(
None
}
}

pub(crate) fn resolve_workspace_for_source_path(
file_path: &Path,
) -> Result<nargo::workspace::Workspace, LspError> {
let package_root = find_file_manifest(file_path);

let toml_path = match package_root {
Some(toml_path) => toml_path,
None => {
return Err(LspError::WorkspaceResolutionError(format!(
"Nargo.toml not found for file: {:?}",
file_path
)))
}
};

let workspace = match resolve_workspace_from_toml(
&toml_path,
PackageSelection::All,
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
) {
Ok(workspace) => workspace,
Err(err) => return Err(LspError::WorkspaceResolutionError(format!("{err}"))),
};
Ok(workspace)
}
84 changes: 50 additions & 34 deletions tooling/lsp/src/notifications/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ use std::ops::ControlFlow;

use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use nargo::prepare_package;
use nargo_toml::{find_file_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING};
use noirc_driver::check_crate;
use noirc_errors::{DiagnosticKind, FileDiagnostic};

use crate::requests::collect_lenses_for_package;
use crate::types::{
notification, Diagnostic, DiagnosticSeverity, DidChangeConfigurationParams,
DidChangeTextDocumentParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams,
DidSaveTextDocumentParams, InitializedParams, LogMessageParams, MessageType, NargoPackageTests,
PublishDiagnosticsParams,
DidSaveTextDocumentParams, InitializedParams, NargoPackageTests, PublishDiagnosticsParams,
};

use crate::{byte_span_to_range, get_package_tests_in_crate, LspState};
use crate::{
byte_span_to_range, get_package_tests_in_crate, resolve_workspace_for_source_path, LspState,
};

pub(super) fn on_initialized(
_state: &mut LspState,
Expand Down Expand Up @@ -42,8 +43,38 @@ pub(super) fn on_did_change_text_document(
params: DidChangeTextDocumentParams,
) -> ControlFlow<Result<(), async_lsp::Error>> {
let text = params.content_changes.into_iter().next().unwrap().text;
state.input_files.insert(params.text_document.uri.to_string(), text);
state.collected_lenses = Some(Vec::new());
state.input_files.insert(params.text_document.uri.to_string(), text.clone());

let (mut context, crate_id) = nargo::prepare_source(text);
let _ = check_crate(&mut context, crate_id, false, false);

let workspace = match resolve_workspace_for_source_path(
params.text_document.uri.to_file_path().unwrap().as_path(),
) {
Ok(workspace) => workspace,
Err(lsp_error) => {
return ControlFlow::Break(Err(ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("{}", lsp_error),
)
.into()))
}
};
let package = match workspace.members.first() {
Some(package) => package,
None => {
return ControlFlow::Break(Err(ResponseError::new(
ErrorCode::REQUEST_FAILED,
"Selected workspace has no members",
)
.into()))
}
};

let lenses = collect_lenses_for_package(&context, crate_id, &workspace, package, None);

state.cached_lenses.insert(params.text_document.uri.to_string(), lenses);

ControlFlow::Continue(())
}

Expand All @@ -52,6 +83,7 @@ pub(super) fn on_did_close_text_document(
params: DidCloseTextDocumentParams,
) -> ControlFlow<Result<(), async_lsp::Error>> {
state.input_files.remove(&params.text_document.uri.to_string());
state.cached_lenses.remove(&params.text_document.uri.to_string());
ControlFlow::Continue(())
}

Expand All @@ -70,34 +102,14 @@ pub(super) fn on_did_save_text_document(
}
};

let package_root = find_file_manifest(file_path.as_path());

let toml_path = match package_root {
Some(toml_path) => toml_path,
None => {
// If we cannot find a manifest, we log a warning but return no diagnostics
// We can reconsider this when we can build a file without the need for a Nargo.toml file to resolve deps
let _ = state.client.log_message(LogMessageParams {
typ: MessageType::WARNING,
message: format!("Nargo.toml not found for file: {:}", file_path.display()),
});
return ControlFlow::Continue(());
}
};

let workspace = match resolve_workspace_from_toml(
&toml_path,
PackageSelection::All,
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
) {
Ok(workspace) => workspace,
Err(err) => {
// If we found a manifest, but the workspace is invalid, we raise an error about it
let workspace = match resolve_workspace_for_source_path(&file_path) {
Ok(value) => value,
Err(lsp_error) => {
return ControlFlow::Break(Err(ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("{err}"),
format!("{}", lsp_error),
)
.into()));
.into()))
}
};

Expand All @@ -120,9 +132,13 @@ pub(super) fn on_did_save_text_document(
}

let collected_lenses = crate::requests::collect_lenses_for_package(
&context, crate_id, &file_path, &workspace, package,
&context,
crate_id,
&workspace,
package,
Some(&file_path),
);
state.collected_lenses = Some(collected_lenses);
state.cached_lenses.insert(params.text_document.uri.to_string(), collected_lenses);

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

0 comments on commit 318b5d2

Please sign in to comment.