Skip to content
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

feat(lsp): re-add code lens feature with improved performance #3829

Merged
merged 22 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7714eaa
feat: consider single package in workspace for code lens
kobyhallx Dec 14, 2023
167bf06
feat: use cached code lenses
kobyhallx Dec 14, 2023
318b5d2
feat(lsp): improve LSP code lens performance
kobyhallx Dec 15, 2023
a2943b1
feat(lsp): accept option to disable code lens
kobyhallx Dec 15, 2023
2c06e91
chore: document `LspInitializationOptions`
kobyhallx Dec 15, 2023
a0018eb
Merge remote-tracking branch 'origin/master' into kh-cached-code-lens
kobyhallx Dec 15, 2023
a3fd9a4
fix: failing test
kobyhallx Dec 15, 2023
cc1c484
chore: fmt
kobyhallx Dec 15, 2023
daf36bc
chore: clippy
kobyhallx Dec 18, 2023
44bc510
Merge branch 'master' into kh-cached-code-lens
kobyhallx Dec 18, 2023
308dcef
chore: adjust to newer api
kobyhallx Dec 18, 2023
59e8f7a
Merge branch 'master' into kh-cached-code-lens
kobyhallx Dec 19, 2023
f8f9616
chore: review nits
kobyhallx Dec 19, 2023
c0683ad
chore: review nits - better default
kobyhallx Dec 19, 2023
6681d15
Merge branch 'master' into kh-cached-code-lens
kobyhallx Dec 19, 2023
07e2276
Merge branch 'master' into kh-cached-code-lens
kobyhallx Dec 19, 2023
ca19e4b
chore: move prepare_source fn to lsp from nargo
kobyhallx Dec 19, 2023
d34bc71
Merge remote-tracking branch 'origin/master' into kh-cached-code-lens
kobyhallx Dec 19, 2023
f0f68c9
chore: merge fixups
kobyhallx Dec 19, 2023
9c6a48a
Merge branch 'master' into kh-cached-code-lens
kobyhallx Dec 19, 2023
795eb21
chore: move test along prepare_source
kobyhallx Dec 19, 2023
438c09a
Merge branch 'master' into kh-cached-code-lens
kobyhallx Dec 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 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
46 changes: 43 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 @@ -18,6 +18,9 @@ use async_lsp::{
ResponseError,
};
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 @@ -27,10 +30,11 @@ use notifications::{
on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized,
};
use requests::{
on_formatting, on_goto_definition_request, on_initialize, on_profile_run_request, on_shutdown,
on_test_run_request, on_tests_request,
on_code_lens_request, on_formatting, on_goto_definition_request, on_initialize,
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 @@ -41,12 +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}.
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
#[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>,
cached_lenses: HashMap<String, Vec<CodeLens>>,
}

impl LspState {
Expand All @@ -56,6 +68,7 @@ impl LspState {
root_path: None,
solver: WrapperSolver(Box::new(solver)),
input_files: HashMap::new(),
cached_lenses: HashMap::new(),
}
}
}
Expand All @@ -72,6 +85,7 @@ impl NargoLspService {
.request::<request::Initialize, _>(on_initialize)
.request::<request::Formatting, _>(on_formatting)
.request::<request::Shutdown, _>(on_shutdown)
.request::<request::CodeLens, _>(on_code_lens_request)
.request::<request::NargoTests, _>(on_tests_request)
.request::<request::NargoTestRun, _>(on_test_run_request)
.request::<request::NargoProfileRun, _>(on_profile_run_request)
Expand Down Expand Up @@ -175,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> {
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
let package_root = find_file_manifest(file_path);

let toml_path = match package_root {
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
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(
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
&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: 53 additions & 31 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,7 +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.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(
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
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),
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
)
.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 @@ -51,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 @@ -69,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),
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
)
.into()));
.into()))
}
};

Expand All @@ -118,6 +131,15 @@ pub(super) fn on_did_save_text_document(
});
}

let collected_lenses = crate::requests::collect_lenses_for_package(
&context,
crate_id,
&workspace,
package,
Some(&file_path),
);
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
Loading