diff --git a/Cargo.lock b/Cargo.lock index 9a099308613..7535d132a96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2053,6 +2053,8 @@ dependencies = [ "codespan-reporting", "fm", "lsp-types 0.94.0", + "nargo", + "nargo_toml", "noirc_driver", "noirc_errors", "noirc_frontend", diff --git a/crates/lsp/Cargo.toml b/crates/lsp/Cargo.toml index 3cde18bfba9..00f4f9f9d82 100644 --- a/crates/lsp/Cargo.toml +++ b/crates/lsp/Cargo.toml @@ -13,6 +13,8 @@ codespan-lsp.workspace = true codespan-reporting.workspace = true fm.workspace = true lsp-types.workspace = true +nargo.workspace = true +nargo_toml.workspace = true noirc_driver.workspace = true noirc_errors.workspace = true noirc_frontend.workspace = true diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index df43dac4df7..2f26db87daf 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -1,20 +1,16 @@ -mod lib_hacky; -use std::env; - use std::{ future::{self, Future}, ops::{self, ControlFlow}, - path::PathBuf, pin::Pin, task::{self, Poll}, }; use async_lsp::{ - router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, ErrorCode, - LanguageClient, LspService, ResponseError, + router::Router, AnyEvent, AnyNotification, AnyRequest, ClientSocket, Error, LanguageClient, + LspService, ResponseError, }; use codespan_reporting::files; -use fm::FileManager; +use fm::FILE_EXTENSION; use lsp_types::{ notification, request, CodeLens, CodeLensOptions, CodeLensParams, Command, Diagnostic, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, @@ -22,9 +18,10 @@ use lsp_types::{ InitializeParams, InitializeResult, InitializedParams, Position, PublishDiagnosticsParams, Range, ServerCapabilities, TextDocumentSyncOptions, }; -use noirc_driver::{check_crate, prepare_crate}; +use nargo::prepare_package; +use nargo_toml::{find_package_manifest, resolve_workspace_from_toml}; +use noirc_driver::check_crate; use noirc_errors::{DiagnosticKind, FileDiagnostic}; -use noirc_frontend::{graph::CrateGraph, hir::Context}; use serde_json::Value as JsonValue; use tower::Service; @@ -33,13 +30,12 @@ const TEST_CODELENS_TITLE: &str = "▶\u{fe0e} Run Test"; // State for the LSP gets implemented on this struct and is internal to the implementation pub struct LspState { - root_path: Option, client: ClientSocket, } impl LspState { fn new(client: &ClientSocket) -> Self { - Self { client: client.clone(), root_path: None } + Self { client: client.clone() } } } @@ -49,35 +45,6 @@ pub struct NargoLspService { impl NargoLspService { pub fn new(client: &ClientSocket) -> Self { - // Using conditional running with lib_hacky to prevent non-hacky code from being identified as dead code - // Secondarily, provides a runtime way to stress the non-hacky code. - if env::var("NOIR_LSP_NO_HACK").is_err() { - let state = LspState::new(client); - let mut router = Router::new(state); - router - .request::(lib_hacky::on_initialize) - .request::(lib_hacky::on_shutdown) - .request::(lib_hacky::on_code_lens_request) - .notification::(lib_hacky::on_initialized) - .notification::( - lib_hacky::on_did_change_configuration, - ) - .notification::( - lib_hacky::on_did_open_text_document, - ) - .notification::( - lib_hacky::on_did_change_text_document, - ) - .notification::( - lib_hacky::on_did_close_text_document, - ) - .notification::( - lib_hacky::on_did_save_text_document, - ) - .notification::(lib_hacky::on_exit); - return Self { router }; - } - let state = LspState::new(client); let mut router = Router::new(state); router @@ -134,13 +101,9 @@ impl LspService for NargoLspService { // and params passed in. fn on_initialize( - state: &mut LspState, - params: InitializeParams, + _state: &mut LspState, + _params: InitializeParams, ) -> impl Future> { - if let Some(root_uri) = params.root_uri { - state.root_path = root_uri.to_file_path().ok(); - } - async { let text_document_sync = TextDocumentSyncOptions { save: Some(true.into()), ..Default::default() }; @@ -167,57 +130,52 @@ fn on_shutdown( } fn on_code_lens_request( - state: &mut LspState, + _state: &mut LspState, params: CodeLensParams, ) -> impl Future>, ResponseError>> { - let file_path = ¶ms.text_document.uri.to_file_path().unwrap(); + let file_path = params.text_document.uri.to_file_path().unwrap(); + let program_dir = file_path.parent().unwrap(); - let mut context = match &state.root_path { - Some(root_path) => { - let fm = FileManager::new(root_path); - let graph = CrateGraph::default(); - Context::new(fm, graph) - } - None => { - let err = ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Unable to determine the project root path", - ); - return future::ready(Err(err)); - } - }; + let toml_path = find_package_manifest(program_dir).unwrap(); + let workspace = resolve_workspace_from_toml(&toml_path, None).unwrap(); - let crate_id = prepare_crate(&mut context, file_path); + let mut lenses: Vec = vec![]; - // We ignore the warnings and errors produced by compilation for producing codelenses - // because we can still get the test functions even if compilation fails - let _ = check_crate(&mut context, crate_id, false); + for package in &workspace { + let (mut context, crate_id) = prepare_package(package); + // We ignore the warnings and errors produced by compilation for producing codelenses + // 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_simple_files(); - let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); + let fm = &context.file_manager; + let files = fm.as_simple_files(); + let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); - let mut lenses: Vec = vec![]; - for (func_name, func_id) in tests { - let location = context.function_meta(&func_id).name.location; - let file_id = location.file; - // TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file - if file_id.as_usize() != 0 { - continue; - } + for (func_name, func_id) in tests { + let location = context.function_meta(&func_id).name.location; + let file_id = location.file; - let range = - byte_span_to_range(files, file_id.as_usize(), location.span.into()).unwrap_or_default(); + // 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 command = Command { - title: TEST_CODELENS_TITLE.into(), - command: TEST_COMMAND.into(), - arguments: Some(vec![func_name.into()]), - }; + let range = byte_span_to_range(files, file_id.as_usize(), location.span.into()) + .unwrap_or_default(); + + let command = Command { + title: TEST_CODELENS_TITLE.into(), + command: TEST_COMMAND.into(), + arguments: Some(vec![func_name.into()]), + }; - let lens = CodeLens { range, command: command.into(), data: None }; + let lens = CodeLens { range, command: command.into(), data: None }; - lenses.push(lens); + lenses.push(lens); + } } let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; @@ -264,60 +222,56 @@ fn on_did_save_text_document( state: &mut LspState, params: DidSaveTextDocumentParams, ) -> ControlFlow> { - let file_path = ¶ms.text_document.uri.to_file_path().unwrap(); - let mut context = match &state.root_path { - Some(root_path) => { - let fm = FileManager::new(root_path); - let graph = CrateGraph::default(); - Context::new(fm, graph) - } - None => { - let err = ResponseError::new( - ErrorCode::REQUEST_FAILED, - "Unable to determine the project root path", - ); - return ControlFlow::Break(Err(err.into())); - } - }; + let file_path = params.text_document.uri.to_file_path().unwrap(); + let program_dir = file_path.parent().unwrap(); - let crate_id = prepare_crate(&mut context, file_path); + let toml_path = find_package_manifest(program_dir).unwrap(); + let workspace = resolve_workspace_from_toml(&toml_path, None).unwrap(); let mut diagnostics = Vec::new(); - let file_diagnostics = match check_crate(&mut context, crate_id, false) { - Ok(warnings) => warnings, - Err(errors_and_warnings) => errors_and_warnings, - }; + for package in &workspace { + let (mut context, crate_id) = prepare_package(package); - if !file_diagnostics.is_empty() { - let fm = &context.file_manager; - let files = fm.as_simple_files(); + let file_diagnostics = match check_crate(&mut context, crate_id, false) { + Ok(warnings) => warnings, + Err(errors_and_warnings) => errors_and_warnings, + }; - for FileDiagnostic { file_id, diagnostic } in file_diagnostics { - // TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file - if file_id.as_usize() != 0 { - continue; - } + if !file_diagnostics.is_empty() { + let fm = &context.file_manager; + let files = fm.as_simple_files(); + + for FileDiagnostic { file_id, diagnostic } in file_diagnostics { + // 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 mut range = Range::default(); + let mut range = Range::default(); - // TODO: Should this be the first item in secondaries? Should we bail when we find a range? - for sec in diagnostic.secondaries { - // Not using `unwrap_or_default` here because we don't want to overwrite a valid range with a default range - if let Some(r) = byte_span_to_range(files, file_id.as_usize(), sec.span.into()) { - range = r + // TODO: Should this be the first item in secondaries? Should we bail when we find a range? + for sec in diagnostic.secondaries { + // Not using `unwrap_or_default` here because we don't want to overwrite a valid range with a default range + if let Some(r) = byte_span_to_range(files, file_id.as_usize(), sec.span.into()) + { + range = r + } } + let severity = match diagnostic.kind { + DiagnosticKind::Error => Some(DiagnosticSeverity::ERROR), + DiagnosticKind::Warning => Some(DiagnosticSeverity::WARNING), + }; + diagnostics.push(Diagnostic { + range, + severity, + message: diagnostic.message, + ..Diagnostic::default() + }) } - let severity = match diagnostic.kind { - DiagnosticKind::Error => Some(DiagnosticSeverity::ERROR), - DiagnosticKind::Warning => Some(DiagnosticSeverity::WARNING), - }; - diagnostics.push(Diagnostic { - range, - severity, - message: diagnostic.message, - ..Diagnostic::default() - }) } } diff --git a/crates/lsp/src/lib_hacky.rs b/crates/lsp/src/lib_hacky.rs deleted file mode 100644 index 2705c61238b..00000000000 --- a/crates/lsp/src/lib_hacky.rs +++ /dev/null @@ -1,335 +0,0 @@ -//! NOTE: This is a temporary module until https://github.com/noir-lang/noir/issues/1838 is fixed. -//! This is sectioned off, and currently the default implementation, unless the environment variable NOIR_LSP_NO_HACKS is set. -//! This is mainly so that non-hacky code is not considered dead. -use std::{ - collections::HashMap, - fs, - future::{self, Future}, - ops::{self, ControlFlow}, - path::{Path, PathBuf}, -}; - -use async_lsp::{ErrorCode, LanguageClient, ResponseError}; -use codespan_reporting::files; -use fm::FileManager; -use lsp_types::{ - CodeLens, CodeLensOptions, CodeLensParams, Command, Diagnostic, DiagnosticSeverity, - DidChangeConfigurationParams, DidChangeTextDocumentParams, DidCloseTextDocumentParams, - DidOpenTextDocumentParams, DidSaveTextDocumentParams, InitializeParams, InitializeResult, - InitializedParams, Position, PublishDiagnosticsParams, Range, ServerCapabilities, - TextDocumentSyncOptions, -}; -use noirc_driver::{check_crate, prepare_crate, propagate_dep}; -use noirc_errors::{DiagnosticKind, FileDiagnostic}; -use noirc_frontend::{ - graph::{CrateGraph, CrateId}, - hir::Context, -}; - -// I'm guessing this is here so the `lib.rs` file compiles -use crate::LspState; - -const TEST_COMMAND: &str = "nargo.test"; -const TEST_CODELENS_TITLE: &str = "▶\u{fe0e} Run Test"; - -// Handlers -// The handlers for `request` are not `async` because it compiles down to lifetimes that can't be added to -// the router. To return a future that fits the trait, it is easiest wrap your implementations in an `async {}` -// block but you can also use `std::future::ready`. -// -// Additionally, the handlers for `notification` aren't async at all. -// -// They are not attached to the `NargoLspService` struct so they can be unit tested with only `LspState` -// and params passed in. - -pub fn on_initialize( - state: &mut LspState, - params: InitializeParams, -) -> impl Future> { - if let Some(root_uri) = params.root_uri { - state.root_path = root_uri.to_file_path().ok(); - } - - async { - let text_document_sync = - TextDocumentSyncOptions { save: Some(true.into()), ..Default::default() }; - - let code_lens = CodeLensOptions { resolve_provider: Some(false) }; - - Ok(InitializeResult { - capabilities: ServerCapabilities { - text_document_sync: Some(text_document_sync.into()), - code_lens_provider: Some(code_lens), - // Add capabilities before this spread when adding support for one - ..Default::default() - }, - server_info: None, - }) - } -} - -pub fn on_shutdown( - _state: &mut LspState, - _params: (), -) -> impl Future> { - async { Ok(()) } -} - -pub fn on_code_lens_request( - state: &mut LspState, - params: CodeLensParams, -) -> impl Future>, ResponseError>> { - let actual_path = params.text_document.uri.to_file_path().unwrap(); - let (mut context, crate_id) = match create_context_at_path(&state.root_path, &actual_path) { - Err(err) => return future::ready(Err(err)), - Ok(res) => res, - }; - - // We ignore the warnings and errors produced by compilation for producing codelenses - // 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_simple_files(); - let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); - - let mut lenses: Vec = vec![]; - for (func_name, func_id) in tests { - let location = context.function_meta(&func_id).name.location; - let file_id = location.file; - // TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file - if file_id.as_usize() != 0 { - continue; - } - - let range = - byte_span_to_range(files, file_id.as_usize(), location.span.into()).unwrap_or_default(); - - let command = Command { - title: TEST_CODELENS_TITLE.into(), - command: TEST_COMMAND.into(), - arguments: Some(vec![func_name.into()]), - }; - - let lens = CodeLens { range, command: command.into(), data: None }; - - lenses.push(lens); - } - - let res = if lenses.is_empty() { Ok(None) } else { Ok(Some(lenses)) }; - - future::ready(res) -} - -pub fn on_initialized( - _state: &mut LspState, - _params: InitializedParams, -) -> ControlFlow> { - ControlFlow::Continue(()) -} - -pub fn on_did_change_configuration( - _state: &mut LspState, - _params: DidChangeConfigurationParams, -) -> ControlFlow> { - ControlFlow::Continue(()) -} - -pub fn on_did_open_text_document( - _state: &mut LspState, - _params: DidOpenTextDocumentParams, -) -> ControlFlow> { - ControlFlow::Continue(()) -} - -pub fn on_did_change_text_document( - _state: &mut LspState, - _params: DidChangeTextDocumentParams, -) -> ControlFlow> { - ControlFlow::Continue(()) -} - -pub fn on_did_close_text_document( - _state: &mut LspState, - _params: DidCloseTextDocumentParams, -) -> ControlFlow> { - ControlFlow::Continue(()) -} - -/// Find the nearest parent file with given names. -fn find_nearest_parent_file(path: &Path, filenames: &[&str]) -> Option { - let mut current_path = path; - - while let Some(parent_path) = current_path.parent() { - for filename in filenames { - let mut possible_file_path = parent_path.to_path_buf(); - possible_file_path.push(filename); - if possible_file_path.is_file() { - return Some(possible_file_path); - } - } - current_path = parent_path; - } - - None -} - -fn read_dependencies( - nargo_toml_path: &Path, -) -> Result, Box> { - let content: String = fs::read_to_string(nargo_toml_path)?; - let value: toml::Value = toml::from_str(&content)?; - - let mut dependencies = HashMap::new(); - - if let Some(toml::Value::Table(table)) = value.get("dependencies") { - for (key, value) in table { - if let toml::Value::Table(inner_table) = value { - if let Some(toml::Value::String(path)) = inner_table.get("path") { - dependencies.insert(key.clone(), path.clone()); - } - } - } - } - - Ok(dependencies) -} - -pub fn on_did_save_text_document( - state: &mut LspState, - params: DidSaveTextDocumentParams, -) -> ControlFlow> { - let actual_path = params.text_document.uri.to_file_path().unwrap(); - let actual_file_name = actual_path.file_name(); - let (mut context, crate_id) = match create_context_at_path(&state.root_path, &actual_path) { - Err(err) => return ControlFlow::Break(Err(err.into())), - Ok(res) => res, - }; - - let file_diagnostics = match check_crate(&mut context, crate_id, false) { - Ok(warnings) => warnings, - Err(errors_and_warnings) => errors_and_warnings, - }; - let mut diagnostics = Vec::new(); - - if !file_diagnostics.is_empty() { - let fm = &context.file_manager; - let files = fm.as_simple_files(); - - for FileDiagnostic { file_id, diagnostic } in file_diagnostics { - // TODO(AD): HACK, undo these total hacks once we have a proper approach - if file_id.as_usize() == 0 { - // main.nr case - if actual_file_name.unwrap().to_str() != Some("main.nr") - && actual_file_name.unwrap().to_str() != Some("lib.nr") - { - continue; - } - } else if fm.path(file_id).file_name().unwrap().to_str().unwrap() - != actual_file_name.unwrap().to_str().unwrap().replace(".nr", "") - { - // every other file case - continue; // TODO(AD): HACK, we list all errors, filter by hacky final path component - } - - let mut range = Range::default(); - - // TODO: Should this be the first item in secondaries? Should we bail when we find a range? - for sec in diagnostic.secondaries { - // Not using `unwrap_or_default` here because we don't want to overwrite a valid range with a default range - if let Some(r) = byte_span_to_range(files, file_id.as_usize(), sec.span.into()) { - range = r - } - } - let severity = match diagnostic.kind { - DiagnosticKind::Error => Some(DiagnosticSeverity::ERROR), - DiagnosticKind::Warning => Some(DiagnosticSeverity::WARNING), - }; - diagnostics.push(Diagnostic { - range, - severity, - message: diagnostic.message, - ..Diagnostic::default() - }) - } - } - - let _ = state.client.publish_diagnostics(PublishDiagnosticsParams { - uri: params.text_document.uri, - version: None, - diagnostics, - }); - - ControlFlow::Continue(()) -} - -fn create_context_at_path( - root_path: &Option, - actual_path: &Path, -) -> Result<(Context, CrateId), ResponseError> { - let mut context = match &root_path { - Some(root_path) => { - let fm = FileManager::new(root_path); - let graph = CrateGraph::default(); - Context::new(fm, graph) - } - None => { - let err = ResponseError::new(ErrorCode::REQUEST_FAILED, "Project has not been built"); - return Err(err); - } - }; - - let mut file_path = actual_path.to_path_buf(); - // TODO better naming/unhacking - if let Some(new_path) = find_nearest_parent_file(&file_path, &["lib.nr", "main.nr"]) { - file_path = new_path; // TODO unhack - } - let nargo_toml_path = find_nearest_parent_file(&file_path, &["Nargo.toml"]); - - let current_crate_id = prepare_crate(&mut context, &file_path); - - // TODO(AD): undo hacky dependency resolution - if let Some(nargo_toml_path) = nargo_toml_path { - let dependencies = read_dependencies(&nargo_toml_path); - if let Ok(dependencies) = dependencies { - for (crate_name, dependency_path) in dependencies.iter() { - let path_to_lib = nargo_toml_path - .parent() - .unwrap() // TODO - .join(PathBuf::from(&dependency_path).join("src").join("lib.nr")); - let library_crate = prepare_crate(&mut context, &path_to_lib); - propagate_dep(&mut context, library_crate, &crate_name.parse().unwrap()); - } - } - } - Ok((context, current_crate_id)) -} - -pub fn on_exit(_state: &mut LspState, _params: ()) -> ControlFlow> { - ControlFlow::Continue(()) -} - -fn byte_span_to_range<'a, F: files::Files<'a> + ?Sized>( - files: &'a F, - file_id: F::FileId, - span: ops::Range, -) -> Option { - // TODO(#1683): Codespan ranges are often (always?) off by some amount of characters - if let Ok(codespan_range) = codespan_lsp::byte_span_to_range(files, file_id, span) { - // We have to manually construct a Range because the codespan_lsp restricts lsp-types to the wrong version range - // TODO: codespan is unmaintained and we should probably subsume it. Ref https://github.com/brendanzab/codespan/issues/345 - let range = Range { - start: Position { - line: codespan_range.start.line, - character: codespan_range.start.character, - }, - end: Position { - line: codespan_range.end.line, - character: codespan_range.end.character, - }, - }; - Some(range) - } else { - None - } -}