Run flycheck for workspace if target is binary

Since querying for a crate's target is a salsa db and therefore blocking, flycheck task is now deferred out of main thread by using `GlobalState`s `deferred_task_queue`

Author: alibektas

crates/rust-analyzer/src/handlers/notification.rs

@@ -1,7 +1,7 @@
 //! This module is responsible for implementing handlers for Language Server
 //! Protocol. This module specifically handles notifications.
 
-use std::ops::{Deref, Not as _};
+use std::ops::Not as _;
 
 use itertools::Itertools;
 use lsp_types::{
@@ -11,13 +11,14 @@ use lsp_types::{
 };
 use paths::Utf8PathBuf;
 use triomphe::Arc;
-use vfs::{AbsPathBuf, ChangeKind, VfsPath};
+use vfs::{AbsPathBuf, ChangeKind};
 
 use crate::{
     config::{Config, ConfigChange},
     global_state::{FetchWorkspaceRequest, GlobalState},
     lsp::{from_proto, utils::apply_document_changes},
     lsp_ext::{self, RunFlycheckParams},
+    main_loop::QueuedTask,
     mem_docs::DocumentData,
     reload,
 };
@@ -282,100 +283,6 @@ pub(crate) fn handle_did_change_watched_files(
     Ok(())
 }
 
-fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
-    let _p = tracing::info_span!("run_flycheck").entered();
-
-    let file_id = state.vfs.read().0.file_id(&vfs_path);
-    if let Some(file_id) = file_id {
-        let world = state.snapshot();
-        let mut updated = false;
-        let task = move || -> std::result::Result<(), ide::Cancelled> {
-            // Trigger flychecks for all workspaces that depend on the saved file
-            // Crates containing or depending on the saved file
-            let crate_ids: Vec<_> = world
-                .analysis
-                .crates_for(file_id)?
-                .into_iter()
-                .flat_map(|id| world.analysis.transitive_rev_deps(id))
-                .flatten()
-                .unique()
-                .collect();
-
-            let crate_root_paths: Vec<_> = crate_ids
-                .iter()
-                .filter_map(|&crate_id| {
-                    world
-                        .analysis
-                        .crate_root(crate_id)
-                        .map(|file_id| {
-                            world.file_id_to_file_path(file_id).as_path().map(ToOwned::to_owned)
-                        })
-                        .transpose()
-                })
-                .collect::<ide::Cancellable<_>>()?;
-            let crate_root_paths: Vec<_> = crate_root_paths.iter().map(Deref::deref).collect();
-
-            // Find all workspaces that have at least one target containing the saved file
-            let workspace_ids = world.workspaces.iter().enumerate().filter_map(|(idx, ws)| {
-                let package = match &ws.kind {
-                    project_model::ProjectWorkspaceKind::Cargo { cargo, .. }
-                    | project_model::ProjectWorkspaceKind::DetachedFile {
-                        cargo: Some((cargo, _)),
-                        ..
-                    } => cargo.packages().find_map(|pkg| {
-                        let has_target_with_root = cargo[pkg]
-                            .targets
-                            .iter()
-                            .any(|&it| crate_root_paths.contains(&cargo[it].root.as_path()));
-                        has_target_with_root.then(|| cargo[pkg].name.clone())
-                    }),
-                    project_model::ProjectWorkspaceKind::Json(project) => {
-                        if !project.crates().any(|(_, krate)| {
-                            crate_root_paths.contains(&krate.root_module.as_path())
-                        }) {
-                            return None;
-                        }
-                        None
-                    }
-                    project_model::ProjectWorkspaceKind::DetachedFile { .. } => return None,
-                };
-                Some((idx, package))
-            });
-
-            let saved_file = vfs_path.as_path().map(|p| p.to_owned());
-
-            // Find and trigger corresponding flychecks
-            for flycheck in world.flycheck.iter() {
-                for (id, package) in workspace_ids.clone() {
-                    if id == flycheck.id() {
-                        updated = true;
-                        match package.filter(|_| !world.config.flycheck_workspace()) {
-                            Some(package) => flycheck.restart_for_package(package),
-                            None => flycheck.restart_workspace(saved_file.clone()),
-                        }
-                        continue;
-                    }
-                }
-            }
-            // No specific flycheck was triggered, so let's trigger all of them.
-            if !updated {
-                for flycheck in world.flycheck.iter() {
-                    flycheck.restart_workspace(saved_file.clone());
-                }
-            }
-            Ok(())
-        };
-        state.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, move |_| {
-            if let Err(e) = std::panic::catch_unwind(task) {
-                tracing::error!("flycheck task panicked: {e:?}")
-            }
-        });
-        true
-    } else {
-        false
-    }
-}
-
 pub(crate) fn handle_cancel_flycheck(state: &mut GlobalState, _: ()) -> anyhow::Result<()> {
     let _p = tracing::info_span!("handle_cancel_flycheck").entered();
     state.flycheck.iter().for_each(|flycheck| flycheck.cancel());
@@ -395,9 +302,8 @@ pub(crate) fn handle_run_flycheck(
     let _p = tracing::info_span!("handle_run_flycheck").entered();
     if let Some(text_document) = params.text_document {
         if let Ok(vfs_path) = from_proto::vfs_path(&text_document.uri) {
-            if run_flycheck(state, vfs_path) {
-                return Ok(());
-            }
+            let _ = state.deferred_task_queue.sender.send(QueuedTask::Flycheck(vfs_path));
+            return Ok(());
         }
     }
     // No specific flycheck was triggered, so let's trigger all of them.

crates/rust-analyzer/src/main_loop.rs

@@ -856,10 +856,9 @@ impl GlobalState {
                     let world = self.snapshot();
                     let mut updated = false;
                     let task = move || -> std::result::Result<(), ide::Cancelled> {
-                        // Is the target binary? If so we let flycheck trigger only for the workspace that contains it.
+                        // Is the target binary? If so we let flycheck run only for the workspace that contains the crate.
                         let target_is_bin = TargetSpec::for_file(&world, file_id)?
                             .is_some_and(|x| x.target_kind() == project_model::TargetKind::Bin);
-
                         let crate_ids = if target_is_bin {
                             // Trigger flychecks for the only workspace which the binary crate belongs to
                             world

crates/rust-analyzer/tests/slow-tests/main.rs

@@ -17,16 +17,15 @@ mod testdir;
 use std::{collections::HashMap, path::PathBuf, time::Instant};
 
 use lsp_types::{
-    notification::{DidOpenTextDocument, DidSaveTextDocument},
+    notification::DidOpenTextDocument,
     request::{
         CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest,
         InlayHintRequest, InlayHintResolveRequest, WillRenameFiles, WorkspaceSymbolRequest,
     },
     CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams,
-    DidSaveTextDocumentParams, DocumentFormattingParams, FileRename, FormattingOptions,
-    GotoDefinitionParams, HoverParams, InlayHint, InlayHintLabel, InlayHintParams,
-    PartialResultParams, Position, Range, RenameFilesParams, TextDocumentIdentifier,
-    TextDocumentItem, TextDocumentPositionParams, Url, WorkDoneProgressParams,
+    DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams,
+    InlayHint, InlayHintLabel, InlayHintParams, PartialResultParams, Position, Range,
+    RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams,
 };
 use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams};
 use serde_json::json;
@@ -1395,116 +1394,3 @@ version = "0.0.0"
 
     server.request::<WorkspaceSymbolRequest>(Default::default(), json!([]));
 }
-
-#[test]
-/// When flycheck is run on a binary, only the binary crate
-/// should be taken in consideration.
-fn flycheck_only_for_single_crate() {
-    if skip_slow_tests() {
-        return;
-    }
-
-    let tmp_dir = TestDir::new();
-    let path_str = tmp_dir.path().to_owned();
-
-    let server = Project::with_fixture(
-        r#"
-//- /foo/Cargo.toml
-[workspace]
-members = ["crates/*"]
-resolver = "2"
-
-
-[workspace.dependencies]
-luib = { version = "0.1.0" , path = "./crates/luib" }
-luib2 = { version = "0.1.0" , path = "./crates/luib2" }
-
-
-
-//- /foo/crates/luib/Cargo.toml
-[package]
-name = "luib"
-version = "0.1.0"
-edition = "2021"
-
-[dependencies]
-luib2.workspace = true
-
-//- /foo/crates/luib/src/lib.rs
-pub fn add(left: u64, right: u64) -> u64 {
-    left + right
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn it_works() {
-        let result = add(2, 2);
-        assert_eq!(result, 4);
-    }
-}
-
-//- /foo/crates/buin/Cargo.toml
-[package]
-name = "buin"
-version = "0.1.0"
-edition = "2021"
-
-[dependencies]
-luib.workspace = true
-luib2.workspace = true
-
-//- /foo/crates/buin/src/main.rs
-use luib;
-fn main() {
-    luib::add(3, 4);
-    println!("Hello, world!");
-}
-
-//- /foo/crates/luib2/Cargo.toml
-[package]
-name = "luib2"
-version = "0.1.0"
-edition = "2021"
-
-[dependencies]
-
-//- /foo/crates/luib2/src/lib.rs
-pub fn add(left: u64, right: u64) -> u64 {
-    left + right
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn it_works() {
-        let result = add(2, 2);
-        assert_eq!(result, 4);
-    }
-}
-"#,
-    )
-    .root("foo")
-    .server()
-    .wait_until_workspace_is_loaded();
-
-    let mut path_to_file = path_str;
-    path_to_file.push("foo");
-    path_to_file.push("crates");
-    path_to_file.push("buin");
-    path_to_file.push("src");
-    path_to_file.push("main.rs");
-
-    dbg!(&path_to_file);
-
-    server.notification::<DidSaveTextDocument>(DidSaveTextDocumentParams {
-        text_document: TextDocumentIdentifier {
-            uri: Url::parse(&format!("file://{}", path_to_file)).unwrap(),
-        },
-        text: None,
-    });
-}