From c11ff00f2490acf08799868c323fedf627e24b68 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 3 Oct 2023 17:09:06 +0100 Subject: [PATCH 1/3] chore: break `lsp` down into more modules --- tooling/lsp/src/codelens/mod.rs | 199 +++++++++++++++ tooling/lsp/src/codelens/test_run.rs | 117 +++++++++ tooling/lsp/src/codelens/tests.rs | 66 +++++ tooling/lsp/src/lib.rs | 364 +-------------------------- 4 files changed, 393 insertions(+), 353 deletions(-) create mode 100644 tooling/lsp/src/codelens/mod.rs create mode 100644 tooling/lsp/src/codelens/test_run.rs create mode 100644 tooling/lsp/src/codelens/tests.rs diff --git a/tooling/lsp/src/codelens/mod.rs b/tooling/lsp/src/codelens/mod.rs new file mode 100644 index 00000000000..db2d76074d2 --- /dev/null +++ b/tooling/lsp/src/codelens/mod.rs @@ -0,0 +1,199 @@ +use std::future::{self, Future}; + +use async_lsp::{ErrorCode, LanguageClient, ResponseError}; + +use fm::FILE_EXTENSION; +use nargo::{package::Package, prepare_package, workspace::Workspace}; +use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; +use noirc_driver::check_crate; +use noirc_frontend::hir::FunctionNameMatch; + +use crate::{ + byte_span_to_range, get_non_stdlib_asset, + types::{CodeLens, CodeLensParams, CodeLensResult, Command, LogMessageParams, MessageType}, + LspState, +}; + +mod test_run; +mod tests; + +pub(crate) use {test_run::on_test_run_request, tests::on_tests_request}; + +const ARROW: &str = "▶\u{fe0e}"; +const TEST_COMMAND: &str = "nargo.test"; +const TEST_CODELENS_TITLE: &str = "Run Test"; +const COMPILE_COMMAND: &str = "nargo.compile"; +const COMPILE_CODELENS_TITLE: &str = "Compile"; +const EXECUTE_COMMAND: &str = "nargo.execute"; +const EXECUTE_CODELENS_TITLE: &str = "Execute"; + +fn package_selection_args(workspace: &Workspace, package: &Package) -> Vec { + vec![ + "--program-dir".into(), + workspace.root_dir.display().to_string().into(), + "--package".into(), + package.name.to_string().into(), + ] +} + +pub(super) fn on_code_lens_request( + state: &mut LspState, + params: CodeLensParams, +) -> impl Future> { + let file_path = match params.text_document.uri.to_file_path() { + Ok(file_path) => file_path, + Err(()) => { + return future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + "URI is not a valid file path", + ))) + } + }; + + let root_path = match &state.root_path { + Some(root) => root, + None => { + return future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + "Could not find project root", + ))) + } + }; + + let toml_path = match find_package_manifest(root_path, &file_path) { + Ok(toml_path) => toml_path, + Err(err) => { + // If we cannot find a manifest, we log a warning but return no code lenses + // 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: err.to_string(), + }); + return future::ready(Ok(None)); + } + }; + let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) { + Ok(workspace) => workspace, + Err(err) => { + // If we found a manifest, but the workspace is invalid, we raise an error about it + return future::ready(Err(ResponseError::new(ErrorCode::REQUEST_FAILED, err))); + } + }; + + let mut lenses: Vec = vec![]; + + for package in &workspace { + let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + // We ignore the warnings and errors produced by compilation for producing code lenses + // because we can still get the test functions even if compilation fails + let _ = check_crate(&mut context, crate_id, false); + + let fm = &context.file_manager; + let files = fm.as_file_map(); + let tests = context + .get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Anything); + + for (func_name, test_function) in tests { + let location = context.function_meta(&test_function.get_id()).name.location; + let file_id = location.file; + + // Ignore diagnostics for any file that wasn't the file we saved + // TODO: In the future, we could create "related" diagnostics for these files + // TODO: This currently just appends the `.nr` file extension that we store as a constant, + // but that won't work if we accept other extensions + if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path { + continue; + } + + let range = + byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); + + let test_command = Command { + title: format!("{ARROW} {TEST_CODELENS_TITLE}"), + command: TEST_COMMAND.into(), + arguments: Some( + [ + package_selection_args(&workspace, package), + vec!["--exact".into(), func_name.into()], + ] + .concat(), + ), + }; + + let test_lens = CodeLens { range, command: Some(test_command), data: None }; + + lenses.push(test_lens); + } + + if package.is_binary() { + if let Some(main_func_id) = context.get_main_function(&crate_id) { + let location = context.function_meta(&main_func_id).name.location; + let file_id = location.file; + + // Ignore diagnostics for any file that wasn't the file we saved + // TODO: In the future, we could create "related" diagnostics for these files + // TODO: This currently just appends the `.nr` file extension that we store as a constant, + // but that won't work if we accept other extensions + if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path { + continue; + } + + let range = + byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); + + let compile_command = Command { + title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"), + command: COMPILE_COMMAND.into(), + arguments: Some(package_selection_args(&workspace, package)), + }; + + let compile_lens = CodeLens { range, command: Some(compile_command), data: None }; + + lenses.push(compile_lens); + + let execute_command = Command { + title: EXECUTE_CODELENS_TITLE.to_string(), + command: EXECUTE_COMMAND.into(), + arguments: Some(package_selection_args(&workspace, package)), + }; + + let execute_lens = CodeLens { range, command: Some(execute_command), data: None }; + + lenses.push(execute_lens); + } + } + + if package.is_contract() { + // Currently not looking to deduplicate this since we don't have a clear decision on if the Contract stuff is staying + for contract in context.get_all_contracts(&crate_id) { + let location = contract.location; + let file_id = location.file; + + // Ignore diagnostics for any file that wasn't the file we saved + // TODO: In the future, we could create "related" diagnostics for these files + // TODO: This currently just appends the `.nr` file extension that we store as a constant, + // but that won't work if we accept other extensions + if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path { + continue; + } + + let range = + byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); + + let compile_command = Command { + title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"), + command: COMPILE_COMMAND.into(), + arguments: Some(package_selection_args(&workspace, package)), + }; + + let compile_lens = CodeLens { range, command: Some(compile_command), data: None }; + + lenses.push(compile_lens); + } + } + } + + let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; + + future::ready(res) +} diff --git a/tooling/lsp/src/codelens/test_run.rs b/tooling/lsp/src/codelens/test_run.rs new file mode 100644 index 00000000000..31d9f916985 --- /dev/null +++ b/tooling/lsp/src/codelens/test_run.rs @@ -0,0 +1,117 @@ +use std::future::{self, Future}; + +use async_lsp::{ErrorCode, ResponseError}; +use nargo::{ + ops::{run_test, TestStatus}, + prepare_package, +}; +use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; +use noirc_driver::{check_crate, CompileOptions}; +use noirc_frontend::hir::FunctionNameMatch; + +use crate::{ + get_non_stdlib_asset, + types::{NargoTestRunParams, NargoTestRunResult}, + LspState, +}; + +pub(crate) fn on_test_run_request( + state: &mut LspState, + params: NargoTestRunParams, +) -> impl Future> { + let root_path = match &state.root_path { + Some(root) => root, + None => { + return future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + "Could not find project root", + ))) + } + }; + + let toml_path = match find_package_manifest(root_path, root_path) { + Ok(toml_path) => toml_path, + Err(err) => { + // If we cannot find a manifest, we can't run the test + return future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("{}", err), + ))); + } + }; + + let crate_name = params.id.crate_name(); + let function_name = params.id.function_name(); + + let workspace = match resolve_workspace_from_toml( + &toml_path, + PackageSelection::Selected(crate_name.clone()), + ) { + Ok(workspace) => workspace, + Err(err) => { + // If we found a manifest, but the workspace is invalid, we raise an error about it + return future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("{}", err), + ))); + } + }; + + // Since we filtered on crate name, this should be the only item in the iterator + match workspace.into_iter().next() { + Some(package) => { + let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + if check_crate(&mut context, crate_id, false).is_err() { + let result = NargoTestRunResult { + id: params.id.clone(), + result: "error".to_string(), + message: Some("The project failed to compile".into()), + }; + return future::ready(Ok(result)); + }; + + let test_functions = context.get_all_test_functions_in_crate_matching( + &crate_id, + FunctionNameMatch::Exact(function_name), + ); + + match test_functions.into_iter().next() { + Some((_, test_function)) => { + let test_result = run_test( + &state.solver, + &context, + test_function, + false, + &CompileOptions::default(), + ); + let result = match test_result { + TestStatus::Pass => NargoTestRunResult { + id: params.id.clone(), + result: "pass".to_string(), + message: None, + }, + TestStatus::Fail { message, .. } => NargoTestRunResult { + id: params.id.clone(), + result: "fail".to_string(), + message: Some(message), + }, + TestStatus::CompileError(diag) => NargoTestRunResult { + id: params.id.clone(), + result: "error".to_string(), + message: Some(diag.diagnostic.message), + }, + }; + future::ready(Ok(result)) + } + None => future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("Could not locate test named: {function_name} in {crate_name}"), + ))), + } + } + None => future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + format!("Could not locate package named: {crate_name}"), + ))), + } +} diff --git a/tooling/lsp/src/codelens/tests.rs b/tooling/lsp/src/codelens/tests.rs new file mode 100644 index 00000000000..25572fa9021 --- /dev/null +++ b/tooling/lsp/src/codelens/tests.rs @@ -0,0 +1,66 @@ +use std::future::{self, Future}; + +use async_lsp::{ErrorCode, LanguageClient, ResponseError}; +use lsp_types::{LogMessageParams, MessageType}; +use nargo::prepare_package; +use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; +use noirc_driver::check_crate; + +use crate::{ + get_non_stdlib_asset, get_package_tests_in_crate, + types::{NargoPackageTests, NargoTestsParams, NargoTestsResult}, + LspState, +}; + +pub(crate) fn on_tests_request( + state: &mut LspState, + _params: NargoTestsParams, +) -> impl Future> { + let root_path = match &state.root_path { + Some(root) => root, + None => { + return future::ready(Err(ResponseError::new( + ErrorCode::REQUEST_FAILED, + "Could not find project root", + ))) + } + }; + + let toml_path = match find_package_manifest(root_path, root_path) { + Ok(toml_path) => toml_path, + Err(err) => { + // If we cannot find a manifest, we log a warning but return no tests + // 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: err.to_string(), + }); + return future::ready(Ok(None)); + } + }; + let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) { + Ok(workspace) => workspace, + Err(err) => { + // If we found a manifest, but the workspace is invalid, we raise an error about it + return future::ready(Err(ResponseError::new(ErrorCode::REQUEST_FAILED, err))); + } + }; + + let mut package_tests = Vec::new(); + + for package in &workspace { + let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); + // We ignore the warnings and errors produced by compilation for producing tests + // because we can still get the test functions even if compilation fails + let _ = check_crate(&mut context, crate_id, false); + + // We don't add test headings for a package if it contains no `#[test]` functions + if let Some(tests) = get_package_tests_in_crate(&context, &crate_id, &package.name) { + package_tests.push(NargoPackageTests { package: package.name.to_string(), tests }); + } + } + + let res = if package_tests.is_empty() { Ok(None) } else { Ok(Some(package_tests)) }; + + future::ready(res) +} diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index 840636fe458..48ffefb7f7a 100644 --- a/tooling/lsp/src/lib.rs +++ b/tooling/lsp/src/lib.rs @@ -4,7 +4,7 @@ #![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] use std::{ - future::{self, Future}, + future::Future, ops::{self, ControlFlow}, path::{Path, PathBuf}, pin::Pin, @@ -16,14 +16,12 @@ use async_lsp::{ router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, ErrorCode, LanguageClient, LspService, ResponseError, }; +use codelens::{on_code_lens_request, on_test_run_request, on_tests_request}; use codespan_reporting::files; use fm::FILE_EXTENSION; -use nargo::{ - ops::{run_test, TestStatus}, - prepare_package, -}; +use nargo::prepare_package; use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::{check_crate, CompileOptions}; +use noirc_driver::check_crate; use noirc_errors::{DiagnosticKind, FileDiagnostic}; use noirc_frontend::{ graph::{CrateId, CrateName}, @@ -32,26 +30,18 @@ use noirc_frontend::{ use serde_json::Value as JsonValue; use tower::Service; +mod codelens; mod types; use types::{ - notification, request, CodeLens, CodeLensOptions, CodeLensParams, CodeLensResult, Command, - Diagnostic, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, - DidCloseTextDocumentParams, DidOpenTextDocumentParams, DidSaveTextDocumentParams, - InitializeParams, InitializeResult, InitializedParams, LogMessageParams, MessageType, - NargoCapability, NargoPackageTests, NargoTest, NargoTestId, NargoTestRunParams, - NargoTestRunResult, NargoTestsOptions, NargoTestsParams, NargoTestsResult, Position, - PublishDiagnosticsParams, Range, ServerCapabilities, TextDocumentSyncOptions, Url, + notification, request, CodeLensOptions, Diagnostic, DiagnosticSeverity, + DidChangeConfigurationParams, DidChangeTextDocumentParams, DidCloseTextDocumentParams, + DidOpenTextDocumentParams, DidSaveTextDocumentParams, InitializeParams, InitializeResult, + InitializedParams, LogMessageParams, MessageType, NargoCapability, NargoPackageTests, + NargoTest, NargoTestId, NargoTestsOptions, Position, PublishDiagnosticsParams, Range, + ServerCapabilities, TextDocumentSyncOptions, Url, }; -const ARROW: &str = "▶\u{fe0e}"; -const TEST_COMMAND: &str = "nargo.test"; -const TEST_CODELENS_TITLE: &str = "Run Test"; -const COMPILE_COMMAND: &str = "nargo.compile"; -const COMPILE_CODELENS_TITLE: &str = "Compile"; -const EXECUTE_COMMAND: &str = "nargo.execute"; -const EXECUTE_CODELENS_TITLE: &str = "Execute"; - // This is a struct that wraps a dynamically dispatched `BlackBoxFunctionSolver` // where we proxy the unimplemented stuff to the wrapped backend, but it // allows us to avoid changing function signatures to include the `Box` @@ -192,160 +182,6 @@ fn on_initialize( } } -fn on_test_run_request( - state: &mut LspState, - params: NargoTestRunParams, -) -> impl Future> { - let root_path = match &state.root_path { - Some(root) => root, - None => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Could not find project root", - ))) - } - }; - - let toml_path = match find_package_manifest(root_path, root_path) { - Ok(toml_path) => toml_path, - Err(err) => { - // If we cannot find a manifest, we can't run the test - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - format!("{}", err), - ))); - } - }; - - let crate_name = params.id.crate_name(); - let function_name = params.id.function_name(); - - let workspace = match resolve_workspace_from_toml( - &toml_path, - PackageSelection::Selected(crate_name.clone()), - ) { - Ok(workspace) => workspace, - Err(err) => { - // If we found a manifest, but the workspace is invalid, we raise an error about it - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - format!("{}", err), - ))); - } - }; - - // Since we filtered on crate name, this should be the only item in the iterator - match workspace.into_iter().next() { - Some(package) => { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); - if check_crate(&mut context, crate_id, false).is_err() { - let result = NargoTestRunResult { - id: params.id.clone(), - result: "error".to_string(), - message: Some("The project failed to compile".into()), - }; - return future::ready(Ok(result)); - }; - - let test_functions = context.get_all_test_functions_in_crate_matching( - &crate_id, - FunctionNameMatch::Exact(function_name), - ); - - match test_functions.into_iter().next() { - Some((_, test_function)) => { - let test_result = run_test( - &state.solver, - &context, - test_function, - false, - &CompileOptions::default(), - ); - let result = match test_result { - TestStatus::Pass => NargoTestRunResult { - id: params.id.clone(), - result: "pass".to_string(), - message: None, - }, - TestStatus::Fail { message, .. } => NargoTestRunResult { - id: params.id.clone(), - result: "fail".to_string(), - message: Some(message), - }, - TestStatus::CompileError(diag) => NargoTestRunResult { - id: params.id.clone(), - result: "error".to_string(), - message: Some(diag.diagnostic.message), - }, - }; - future::ready(Ok(result)) - } - None => future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - format!("Could not locate test named: {function_name} in {crate_name}"), - ))), - } - } - None => future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - format!("Could not locate package named: {crate_name}"), - ))), - } -} - -fn on_tests_request( - state: &mut LspState, - _params: NargoTestsParams, -) -> impl Future> { - let root_path = match &state.root_path { - Some(root) => root, - None => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Could not find project root", - ))) - } - }; - - let toml_path = match find_package_manifest(root_path, root_path) { - Ok(toml_path) => toml_path, - Err(err) => { - // If we cannot find a manifest, we log a warning but return no tests - // 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: err.to_string(), - }); - return future::ready(Ok(None)); - } - }; - let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) { - Ok(workspace) => workspace, - Err(err) => { - // If we found a manifest, but the workspace is invalid, we raise an error about it - return future::ready(Err(ResponseError::new(ErrorCode::REQUEST_FAILED, err))); - } - }; - - let mut package_tests = Vec::new(); - - for package in &workspace { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); - // We ignore the warnings and errors produced by compilation for producing tests - // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false); - - // We don't add test headings for a package if it contains no `#[test]` functions - if let Some(tests) = get_package_tests_in_crate(&context, &crate_id, &package.name) { - package_tests.push(NargoPackageTests { package: package.name.to_string(), tests }); - } - } - - let res = if package_tests.is_empty() { Ok(None) } else { Ok(Some(package_tests)) }; - - future::ready(res) -} - fn on_shutdown( _state: &mut LspState, _params: (), @@ -353,184 +189,6 @@ fn on_shutdown( async { Ok(()) } } -fn on_code_lens_request( - state: &mut LspState, - params: CodeLensParams, -) -> impl Future> { - let file_path = match params.text_document.uri.to_file_path() { - Ok(file_path) => file_path, - Err(()) => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "URI is not a valid file path", - ))) - } - }; - - let root_path = match &state.root_path { - Some(root) => root, - None => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Could not find project root", - ))) - } - }; - - let toml_path = match find_package_manifest(root_path, &file_path) { - Ok(toml_path) => toml_path, - Err(err) => { - // If we cannot find a manifest, we log a warning but return no code lenses - // 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: err.to_string(), - }); - return future::ready(Ok(None)); - } - }; - let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) { - Ok(workspace) => workspace, - Err(err) => { - // If we found a manifest, but the workspace is invalid, we raise an error about it - return future::ready(Err(ResponseError::new(ErrorCode::REQUEST_FAILED, err))); - } - }; - - let mut lenses: Vec = vec![]; - - for package in &workspace { - let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset)); - // We ignore the warnings and errors produced by compilation for producing code lenses - // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false); - - let fm = &context.file_manager; - let files = fm.as_file_map(); - let tests = context - .get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Anything); - - for (func_name, test_function) in tests { - let location = context.function_meta(&test_function.get_id()).name.location; - let file_id = location.file; - - // Ignore diagnostics for any file that wasn't the file we saved - // TODO: In the future, we could create "related" diagnostics for these files - // TODO: This currently just appends the `.nr` file extension that we store as a constant, - // but that won't work if we accept other extensions - if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path { - continue; - } - - let range = - byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); - - let test_command = Command { - title: format!("{ARROW} {TEST_CODELENS_TITLE}"), - command: TEST_COMMAND.into(), - arguments: Some(vec![ - "--program-dir".into(), - format!("{}", workspace.root_dir.display()).into(), - "--package".into(), - format!("{}", package.name).into(), - "--exact".into(), - func_name.into(), - ]), - }; - - let test_lens = CodeLens { range, command: Some(test_command), data: None }; - - lenses.push(test_lens); - } - - if package.is_binary() { - if let Some(main_func_id) = context.get_main_function(&crate_id) { - let location = context.function_meta(&main_func_id).name.location; - let file_id = location.file; - - // Ignore diagnostics for any file that wasn't the file we saved - // TODO: In the future, we could create "related" diagnostics for these files - // TODO: This currently just appends the `.nr` file extension that we store as a constant, - // but that won't work if we accept other extensions - if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path { - continue; - } - - let range = - byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); - - let compile_command = Command { - title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"), - command: COMPILE_COMMAND.into(), - arguments: Some(vec![ - "--program-dir".into(), - format!("{}", workspace.root_dir.display()).into(), - "--package".into(), - format!("{}", package.name).into(), - ]), - }; - - let compile_lens = CodeLens { range, command: Some(compile_command), data: None }; - - lenses.push(compile_lens); - - let execute_command = Command { - title: EXECUTE_CODELENS_TITLE.to_string(), - command: EXECUTE_COMMAND.into(), - arguments: Some(vec![ - "--program-dir".into(), - format!("{}", workspace.root_dir.display()).into(), - "--package".into(), - format!("{}", package.name).into(), - ]), - }; - - let execute_lens = CodeLens { range, command: Some(execute_command), data: None }; - - lenses.push(execute_lens); - } - } - - if package.is_contract() { - // Currently not looking to deduplicate this since we don't have a clear decision on if the Contract stuff is staying - for contract in context.get_all_contracts(&crate_id) { - let location = contract.location; - let file_id = location.file; - - // Ignore diagnostics for any file that wasn't the file we saved - // TODO: In the future, we could create "related" diagnostics for these files - // TODO: This currently just appends the `.nr` file extension that we store as a constant, - // but that won't work if we accept other extensions - if fm.path(file_id).with_extension(FILE_EXTENSION) != file_path { - continue; - } - - let range = - byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); - - let compile_command = Command { - title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"), - command: COMPILE_COMMAND.into(), - arguments: Some(vec![ - "--program-dir".into(), - format!("{}", workspace.root_dir.display()).into(), - "--package".into(), - format!("{}", package.name).into(), - ]), - }; - - let compile_lens = CodeLens { range, command: Some(compile_command), data: None }; - - lenses.push(compile_lens); - } - } - } - - let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; - - future::ready(res) -} - fn on_initialized( _state: &mut LspState, _params: InitializedParams, From 67ef03370b826ba85e88241dd14b8cc2f70dd99d Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 3 Oct 2023 19:22:12 +0100 Subject: [PATCH 2/3] chore: add `with_arrow` function --- tooling/lsp/src/codelens/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/codelens/mod.rs b/tooling/lsp/src/codelens/mod.rs index db2d76074d2..b8bfdbb74e4 100644 --- a/tooling/lsp/src/codelens/mod.rs +++ b/tooling/lsp/src/codelens/mod.rs @@ -27,6 +27,10 @@ const COMPILE_CODELENS_TITLE: &str = "Compile"; const EXECUTE_COMMAND: &str = "nargo.execute"; const EXECUTE_CODELENS_TITLE: &str = "Execute"; +fn with_arrow(title: &str) -> String { + format!("{ARROW} {title}") +} + fn package_selection_args(workspace: &Workspace, package: &Package) -> Vec { vec![ "--program-dir".into(), @@ -109,7 +113,7 @@ pub(super) fn on_code_lens_request( byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); let test_command = Command { - title: format!("{ARROW} {TEST_CODELENS_TITLE}"), + title: with_arrow(TEST_CODELENS_TITLE), command: TEST_COMMAND.into(), arguments: Some( [ @@ -142,7 +146,7 @@ pub(super) fn on_code_lens_request( byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); let compile_command = Command { - title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"), + title: with_arrow(COMPILE_CODELENS_TITLE), command: COMPILE_COMMAND.into(), arguments: Some(package_selection_args(&workspace, package)), }; @@ -181,7 +185,7 @@ pub(super) fn on_code_lens_request( byte_span_to_range(files, file_id, location.span.into()).unwrap_or_default(); let compile_command = Command { - title: format!("{ARROW} {COMPILE_CODELENS_TITLE}"), + title: with_arrow(COMPILE_CODELENS_TITLE), command: COMPILE_COMMAND.into(), arguments: Some(package_selection_args(&workspace, package)), }; From 0bba25d5835ad3a7c747d19d468d172327ad9abf Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 3 Oct 2023 19:48:26 +0100 Subject: [PATCH 3/3] chore: keep futures at periphery of LSP code --- tooling/lsp/src/codelens/mod.rs | 51 ++++++------ tooling/lsp/src/codelens/test_run.rs | 114 +++++++++++---------------- tooling/lsp/src/codelens/tests.rs | 44 ++++++----- 3 files changed, 94 insertions(+), 115 deletions(-) diff --git a/tooling/lsp/src/codelens/mod.rs b/tooling/lsp/src/codelens/mod.rs index b8bfdbb74e4..02a91b70074 100644 --- a/tooling/lsp/src/codelens/mod.rs +++ b/tooling/lsp/src/codelens/mod.rs @@ -44,25 +44,20 @@ pub(super) fn on_code_lens_request( state: &mut LspState, params: CodeLensParams, ) -> impl Future> { - let file_path = match params.text_document.uri.to_file_path() { - Ok(file_path) => file_path, - Err(()) => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "URI is not a valid file path", - ))) - } - }; + future::ready(on_code_lens_request_inner(state, params)) +} - let root_path = match &state.root_path { - Some(root) => root, - None => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Could not find project root", - ))) - } - }; +fn on_code_lens_request_inner( + state: &mut LspState, + params: CodeLensParams, +) -> Result { + let file_path = params.text_document.uri.to_file_path().map_err(|_| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path") + })?; + + let root_path = state.root_path.as_deref().ok_or_else(|| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find project root") + })?; let toml_path = match find_package_manifest(root_path, &file_path) { Ok(toml_path) => toml_path, @@ -73,16 +68,14 @@ pub(super) fn on_code_lens_request( typ: MessageType::WARNING, message: err.to_string(), }); - return future::ready(Ok(None)); + return Ok(None); } }; - let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) { - Ok(workspace) => workspace, - Err(err) => { + let workspace = + resolve_workspace_from_toml(&toml_path, PackageSelection::All).map_err(|err| { // If we found a manifest, but the workspace is invalid, we raise an error about it - return future::ready(Err(ResponseError::new(ErrorCode::REQUEST_FAILED, err))); - } - }; + ResponseError::new(ErrorCode::REQUEST_FAILED, err) + })?; let mut lenses: Vec = vec![]; @@ -197,7 +190,9 @@ pub(super) fn on_code_lens_request( } } - let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; - - future::ready(res) + if lenses.is_empty() { + Ok(None) + } else { + Ok(Some(lenses)) + } } diff --git a/tooling/lsp/src/codelens/test_run.rs b/tooling/lsp/src/codelens/test_run.rs index 31d9f916985..66976ab7b38 100644 --- a/tooling/lsp/src/codelens/test_run.rs +++ b/tooling/lsp/src/codelens/test_run.rs @@ -19,43 +19,31 @@ pub(crate) fn on_test_run_request( state: &mut LspState, params: NargoTestRunParams, ) -> impl Future> { - let root_path = match &state.root_path { - Some(root) => root, - None => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Could not find project root", - ))) - } - }; + future::ready(on_test_run_request_inner(state, params)) +} - let toml_path = match find_package_manifest(root_path, root_path) { - Ok(toml_path) => toml_path, - Err(err) => { - // If we cannot find a manifest, we can't run the test - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - format!("{}", err), - ))); - } - }; +fn on_test_run_request_inner( + state: &mut LspState, + params: NargoTestRunParams, +) -> Result { + let root_path = state.root_path.as_deref().ok_or_else(|| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find project root") + })?; + + let toml_path = find_package_manifest(root_path, root_path).map_err(|err| { + // If we cannot find a manifest, we can't run the test + ResponseError::new(ErrorCode::REQUEST_FAILED, err) + })?; let crate_name = params.id.crate_name(); let function_name = params.id.function_name(); - let workspace = match resolve_workspace_from_toml( - &toml_path, - PackageSelection::Selected(crate_name.clone()), - ) { - Ok(workspace) => workspace, - Err(err) => { - // If we found a manifest, but the workspace is invalid, we raise an error about it - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - format!("{}", err), - ))); - } - }; + let workspace = + resolve_workspace_from_toml(&toml_path, PackageSelection::Selected(crate_name.clone())) + .map_err(|err| { + // If we found a manifest, but the workspace is invalid, we raise an error about it + ResponseError::new(ErrorCode::REQUEST_FAILED, err) + })?; // Since we filtered on crate name, this should be the only item in the iterator match workspace.into_iter().next() { @@ -67,7 +55,7 @@ pub(crate) fn on_test_run_request( result: "error".to_string(), message: Some("The project failed to compile".into()), }; - return future::ready(Ok(result)); + return Ok(result); }; let test_functions = context.get_all_test_functions_in_crate_matching( @@ -75,43 +63,37 @@ pub(crate) fn on_test_run_request( FunctionNameMatch::Exact(function_name), ); - match test_functions.into_iter().next() { - Some((_, test_function)) => { - let test_result = run_test( - &state.solver, - &context, - test_function, - false, - &CompileOptions::default(), - ); - let result = match test_result { - TestStatus::Pass => NargoTestRunResult { - id: params.id.clone(), - result: "pass".to_string(), - message: None, - }, - TestStatus::Fail { message, .. } => NargoTestRunResult { - id: params.id.clone(), - result: "fail".to_string(), - message: Some(message), - }, - TestStatus::CompileError(diag) => NargoTestRunResult { - id: params.id.clone(), - result: "error".to_string(), - message: Some(diag.diagnostic.message), - }, - }; - future::ready(Ok(result)) - } - None => future::ready(Err(ResponseError::new( + let (_, test_function) = test_functions.into_iter().next().ok_or_else(|| { + ResponseError::new( ErrorCode::REQUEST_FAILED, format!("Could not locate test named: {function_name} in {crate_name}"), - ))), - } + ) + })?; + + let test_result = + run_test(&state.solver, &context, test_function, false, &CompileOptions::default()); + let result = match test_result { + TestStatus::Pass => NargoTestRunResult { + id: params.id.clone(), + result: "pass".to_string(), + message: None, + }, + TestStatus::Fail { message, .. } => NargoTestRunResult { + id: params.id.clone(), + result: "fail".to_string(), + message: Some(message), + }, + TestStatus::CompileError(diag) => NargoTestRunResult { + id: params.id.clone(), + result: "error".to_string(), + message: Some(diag.diagnostic.message), + }, + }; + Ok(result) } - None => future::ready(Err(ResponseError::new( + None => Err(ResponseError::new( ErrorCode::REQUEST_FAILED, format!("Could not locate package named: {crate_name}"), - ))), + )), } } diff --git a/tooling/lsp/src/codelens/tests.rs b/tooling/lsp/src/codelens/tests.rs index 25572fa9021..91098fceebe 100644 --- a/tooling/lsp/src/codelens/tests.rs +++ b/tooling/lsp/src/codelens/tests.rs @@ -14,37 +14,37 @@ use crate::{ pub(crate) fn on_tests_request( state: &mut LspState, - _params: NargoTestsParams, + params: NargoTestsParams, ) -> impl Future> { - let root_path = match &state.root_path { - Some(root) => root, - None => { - return future::ready(Err(ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Could not find project root", - ))) - } - }; + future::ready(on_tests_request_inner(state, params)) +} + +fn on_tests_request_inner( + state: &mut LspState, + _params: NargoTestsParams, +) -> Result { + let root_path = state.root_path.as_deref().ok_or_else(|| { + ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find project root") + })?; let toml_path = match find_package_manifest(root_path, root_path) { Ok(toml_path) => toml_path, Err(err) => { - // If we cannot find a manifest, we log a warning but return no tests + // If we cannot find a manifest, we log a warning but return no code lenses // 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: err.to_string(), }); - return future::ready(Ok(None)); + return Ok(None); } }; - let workspace = match resolve_workspace_from_toml(&toml_path, PackageSelection::All) { - Ok(workspace) => workspace, - Err(err) => { + + let workspace = + resolve_workspace_from_toml(&toml_path, PackageSelection::All).map_err(|err| { // If we found a manifest, but the workspace is invalid, we raise an error about it - return future::ready(Err(ResponseError::new(ErrorCode::REQUEST_FAILED, err))); - } - }; + ResponseError::new(ErrorCode::REQUEST_FAILED, err) + })?; let mut package_tests = Vec::new(); @@ -60,7 +60,9 @@ pub(crate) fn on_tests_request( } } - let res = if package_tests.is_empty() { Ok(None) } else { Ok(Some(package_tests)) }; - - future::ready(res) + if package_tests.is_empty() { + Ok(None) + } else { + Ok(Some(package_tests)) + } }