fix: speed up LSP (#5650)

# Description

## Problem

The LSP server recently became very slow. The main culprits are these
two PRs:
- #5461
- #5561

Those PRs made it so that when a file is opened, changed or saved, we
re-analyze the entire workspace. It turns out that's too slow (I tried
it on a few big projects and it wasn't slow, but it is slow in one of
our projects). The old behavior was to only check the package where the
file is, not the entire workspace.

Another issue is that inlay hints don't work well inside contracts
because they have generated code that ends up overlapping the real code.

## Summary

This is what this PR does: it only checks the enclosing package when
opening or saving a document (essentially reverting #5461). There's also
a speed-up here because previously the entire workspace's files were
loaded and parsed, even if we only ended up analyzing one package. In
this PR we always load a single package.

The downside is that if you have a workspace with two projects, A and B,
where A depends on B, if you search for references of a type that's
defined in B starting from B, you won't find references to it in project
A (it will work fine if you search for references starting from A,
because when we analyze A we also analyze its dependencies). But that
downgrade in behavior is preferable to a slow experience.

Regarding inlay hints, they are now completely disabled inside
contracts.

## Additional Context

There's also a fix for when hover requests sometimes crashes. Debugging
this I noticed it happens when hovering over modules we can't find, and
that too is related to generated code inside contracts. The fix is to at
least not crash the LSP server and not return any hover info.

## 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.

tooling/lsp/src/lib.rs

@@ -236,38 +236,14 @@ fn byte_span_to_range<'a, F: files::Files<'a> + ?Sized>(
     }
 }
 
-pub(crate) fn resolve_workspace_for_source_path(
-    file_path: &Path,
-    root_path: &Option<PathBuf>,
-) -> Result<Workspace, LspError> {
-    // If there's a LSP root path, starting from file_path go up the directory tree
-    // searching for Nargo.toml files. The last one we find is the one we'll use
-    // (we'll assume Noir workspaces aren't nested)
-    if let Some(root_path) = root_path {
-        let mut current_path = file_path;
-        let mut current_toml_path = None;
-        while current_path.starts_with(root_path) {
-            if let Some(toml_path) = find_file_manifest(current_path) {
-                current_toml_path = Some(toml_path);
-
-                if let Some(next_path) = current_path.parent() {
-                    current_path = next_path;
-                } else {
-                    break;
-                }
-            } else {
-                break;
-            }
-        }
-
-        if let Some(toml_path) = current_toml_path {
-            return resolve_workspace_from_toml(
-                &toml_path,
-                PackageSelection::All,
-                Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
-            )
-            .map_err(|err| LspError::WorkspaceResolutionError(err.to_string()));
-        }
+pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result<Workspace, LspError> {
+    if let Some(toml_path) = find_file_manifest(file_path) {
+        return resolve_workspace_from_toml(
+            &toml_path,
+            PackageSelection::All,
+            Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
+        )
+        .map_err(|err| LspError::WorkspaceResolutionError(err.to_string()));
     }
 
     let Some(parent_folder) = file_path

tooling/lsp/src/notifications/mod.rs

@@ -37,15 +37,9 @@ pub(super) fn on_did_open_text_document(
     state.input_files.insert(params.text_document.uri.to_string(), params.text_document.text);
 
     let document_uri = params.text_document.uri;
-    let only_process_document_uri_package = false;
     let output_diagnostics = true;
 
-    match process_workspace_for_noir_document(
-        state,
-        document_uri,
-        only_process_document_uri_package,
-        output_diagnostics,
-    ) {
+    match process_workspace_for_noir_document(state, document_uri, output_diagnostics) {
         Ok(_) => {
             state.open_documents_count += 1;
             ControlFlow::Continue(())
@@ -62,15 +56,9 @@ pub(super) fn on_did_change_text_document(
     state.input_files.insert(params.text_document.uri.to_string(), text.clone());
 
     let document_uri = params.text_document.uri;
-    let only_process_document_uri_package = true;
-    let output_diagnotics = false;
+    let output_diagnostics = false;
 
-    match process_workspace_for_noir_document(
-        state,
-        document_uri,
-        only_process_document_uri_package,
-        output_diagnotics,
-    ) {
+    match process_workspace_for_noir_document(state, document_uri, output_diagnostics) {
         Ok(_) => ControlFlow::Continue(()),
         Err(err) => ControlFlow::Break(Err(err)),
     }
@@ -90,15 +78,9 @@ pub(super) fn on_did_close_text_document(
     }
 
     let document_uri = params.text_document.uri;
-    let only_process_document_uri_package = true;
-    let output_diagnotics = false;
+    let output_diagnostics = false;
 
-    match process_workspace_for_noir_document(
-        state,
-        document_uri,
-        only_process_document_uri_package,
-        output_diagnotics,
-    ) {
+    match process_workspace_for_noir_document(state, document_uri, output_diagnostics) {
         Ok(_) => ControlFlow::Continue(()),
         Err(err) => ControlFlow::Break(Err(err)),
     }
@@ -109,15 +91,9 @@ pub(super) fn on_did_save_text_document(
     params: DidSaveTextDocumentParams,
 ) -> ControlFlow<Result<(), async_lsp::Error>> {
     let document_uri = params.text_document.uri;
-    let only_process_document_uri_package = false;
-    let output_diagnotics = true;
+    let output_diagnostics = true;
 
-    match process_workspace_for_noir_document(
-        state,
-        document_uri,
-        only_process_document_uri_package,
-        output_diagnotics,
-    ) {
+    match process_workspace_for_noir_document(state, document_uri, output_diagnostics) {
         Ok(_) => ControlFlow::Continue(()),
         Err(err) => ControlFlow::Break(Err(err)),
     }
@@ -129,17 +105,15 @@ pub(super) fn on_did_save_text_document(
 pub(crate) fn process_workspace_for_noir_document(
     state: &mut LspState,
     document_uri: lsp_types::Url,
-    only_process_document_uri_package: bool,
     output_diagnostics: bool,
 ) -> Result<(), async_lsp::Error> {
     let file_path = document_uri.to_file_path().map_err(|_| {
         ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path")
     })?;
 
-    let workspace =
-        resolve_workspace_for_source_path(&file_path, &state.root_path).map_err(|lsp_error| {
-            ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string())
-        })?;
+    let workspace = resolve_workspace_for_source_path(&file_path).map_err(|lsp_error| {
+        ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string())
+    })?;
 
     let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
     insert_all_files_for_workspace_into_file_manager(
@@ -155,10 +129,6 @@ pub(crate) fn process_workspace_for_noir_document(
         .flat_map(|package| -> Vec<Diagnostic> {
             let package_root_dir: String = package.root_dir.as_os_str().to_string_lossy().into();
 
-            if only_process_document_uri_package && !file_path.starts_with(&package.root_dir) {
-                return vec![];
-            }
-
             let (mut context, crate_id) =
                 crate::prepare_package(&workspace_file_manager, &parsed_files, package);
 

tooling/lsp/src/requests/code_lens_request.rs

@@ -63,8 +63,7 @@ fn on_code_lens_request_inner(
         ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not read file from disk")
     })?;
 
-    let workspace =
-        resolve_workspace_for_source_path(file_path.as_path(), &state.root_path).unwrap();
+    let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap();
 
     let package = crate::workspace_package_for_file(&workspace, &file_path).ok_or_else(|| {
         ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file")

tooling/lsp/src/requests/hover.rs

@@ -23,39 +23,45 @@ pub(crate) fn on_hover_request(
     params: HoverParams,
 ) -> impl Future<Output = Result<Option<Hover>, ResponseError>> {
     let result = process_request(state, params.text_document_position_params, |args| {
-        args.interner.reference_at_location(args.location).map(|reference| {
+        args.interner.reference_at_location(args.location).and_then(|reference| {
             let location = args.interner.reference_location(reference);
             let lsp_location = to_lsp_location(args.files, location.file, location.span);
-            Hover {
+            format_reference(reference, &args).map(|formatted| Hover {
                 range: lsp_location.map(|location| location.range),
                 contents: HoverContents::Markup(MarkupContent {
                     kind: MarkupKind::Markdown,
-                    value: format_reference(reference, &args),
+                    value: formatted,
                 }),
-            }
+            })
         })
     });
 
     future::ready(result)
 }
 
-fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -> String {
+fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -> Option<String> {
     match reference {
         ReferenceId::Module(id) => format_module(id, args),
-        ReferenceId::Struct(id) => format_struct(id, args),
-        ReferenceId::StructMember(id, field_index) => format_struct_member(id, field_index, args),
-        ReferenceId::Trait(id) => format_trait(id, args),
-        ReferenceId::Global(id) => format_global(id, args),
-        ReferenceId::Function(id) => format_function(id, args),
-        ReferenceId::Alias(id) => format_alias(id, args),
-        ReferenceId::Local(id) => format_local(id, args),
+        ReferenceId::Struct(id) => Some(format_struct(id, args)),
+        ReferenceId::StructMember(id, field_index) => {
+            Some(format_struct_member(id, field_index, args))
+        }
+        ReferenceId::Trait(id) => Some(format_trait(id, args)),
+        ReferenceId::Global(id) => Some(format_global(id, args)),
+        ReferenceId::Function(id) => Some(format_function(id, args)),
+        ReferenceId::Alias(id) => Some(format_alias(id, args)),
+        ReferenceId::Local(id) => Some(format_local(id, args)),
         ReferenceId::Reference(location, _) => {
             format_reference(args.interner.find_referenced(location).unwrap(), args)
         }
     }
 }
-fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String {
-    let module_attributes = args.interner.module_attributes(&id);
+fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> Option<String> {
+    // Note: it's not clear why `try_module_attributes` might return None here, but it happens.
+    // This is a workaround to avoid panicking in that case (which brings the LSP server down).
+    // Cases where this happens are related to generated code, so once that stops happening
+    // this won't be an issue anymore.
+    let module_attributes = args.interner.try_module_attributes(&id)?;
 
     let mut string = String::new();
     if format_parent_module_from_module_id(
@@ -68,7 +74,7 @@ fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String {
     string.push_str("    ");
     string.push_str("mod ");
     string.push_str(&module_attributes.name);
-    string
+    Some(string)
 }
 
 fn format_struct(id: StructId, args: &ProcessRequestCallbackArgs) -> String {

tooling/lsp/src/requests/inlay_hint.rs

@@ -104,7 +104,11 @@ impl<'a> InlayHintCollector<'a> {
             }
             ItemKind::Global(let_statement) => self.collect_in_let_statement(let_statement),
             ItemKind::Submodules(parsed_submodule) => {
-                self.collect_in_parsed_module(&parsed_submodule.contents);
+                // Inlay hints inside a contract might show up incorrectly because contracts can
+                // have generated code whose location overlaps with real code.
+                if !parsed_submodule.is_contract {
+                    self.collect_in_parsed_module(&parsed_submodule.contents);
+                }
             }
             ItemKind::ModuleDecl(_) => (),
             ItemKind::Import(_) => (),

tooling/lsp/src/requests/mod.rs

@@ -364,8 +364,7 @@ where
             ResponseError::new(ErrorCode::REQUEST_FAILED, "URI is not a valid file path")
         })?;
 
-    let workspace =
-        resolve_workspace_for_source_path(file_path.as_path(), &state.root_path).unwrap();
+    let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap();
     let package = crate::workspace_package_for_file(&workspace, &file_path).ok_or_else(|| {
         ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file")
     })?;

tooling/lsp/src/requests/references.rs

@@ -94,6 +94,10 @@ mod references_tests {
         check_references_succeeds("rename_function", "another_function", 0, false).await;
     }
 
+    // Ignored because making this work slows down everything, so for now things will not work
+    // as ideally, but they'll be fast.
+    // See https://github.com/noir-lang/noir/issues/5460
+    #[ignore]
     #[test]
     async fn test_on_references_request_works_accross_workspace_packages() {
         let (mut state, noir_text_document) = test_utils::init_lsp_server("workspace").await;
@@ -108,13 +112,11 @@ mod references_tests {
         let two_lib = Url::from_file_path(workspace_dir.join("two/src/lib.nr")).unwrap();
 
         // We call this to open the document, so that the entire workspace is analyzed
-        let only_process_document_uri_package = false;
         let output_diagnostics = true;
 
         notifications::process_workspace_for_noir_document(
             &mut state,
             one_lib.clone(),
-            only_process_document_uri_package,
             output_diagnostics,
         )
         .unwrap();