Configure flycheck using workspace.discoverConfig

crates/base-db/src/input.rs

@@ -432,10 +432,15 @@ impl CrateGraph {
 
     /// Returns all transitive reverse dependencies of the given crate,
     /// including the crate itself.
-    pub fn transitive_rev_deps(&self, of: CrateId) -> impl Iterator<Item = CrateId> {
+    ///
+    /// The result is topologically ordered, like [Self::crates_in_topological_order] -- dependencies
+    /// of a crate come before the crate itself. The crate you pass in is first.
+    pub fn transitive_rev_deps(&self, of: CrateId) -> Vec<CrateId> {
+        let mut rev_deps = Vec::new();
+        let mut visited = FxHashSet::default();
+        visited.insert(of);
+        rev_deps.push(of);
         let mut worklist = vec![of];
-        let mut rev_deps = FxHashSet::default();
-        rev_deps.insert(of);
 
         let mut inverted_graph = FxHashMap::<_, Vec<_>>::default();
         self.arena.iter().for_each(|(krate, data)| {
@@ -446,15 +451,16 @@ impl CrateGraph {
 
         while let Some(krate) = worklist.pop() {
             if let Some(krate_rev_deps) = inverted_graph.get(&krate) {
-                krate_rev_deps
-                    .iter()
-                    .copied()
-                    .filter(|&rev_dep| rev_deps.insert(rev_dep))
-                    .for_each(|rev_dep| worklist.push(rev_dep));
+                krate_rev_deps.iter().copied().filter(|&rev_dep| visited.insert(rev_dep)).for_each(
+                    |rev_dep| {
+                        rev_deps.push(rev_dep);
+                        worklist.push(rev_dep);
+                    },
+                );
             }
         }
 
-        rev_deps.into_iter()
+        rev_deps
     }
 
     /// Returns all crates in the graph, sorted in topological order (ie. dependencies of a crate

crates/hir/src/lib.rs

@@ -215,7 +215,7 @@ impl Crate {
         self,
         db: &dyn HirDatabase,
     ) -> impl Iterator<Item = Crate> {
-        db.crate_graph().transitive_rev_deps(self.id).map(|id| Crate { id })
+        db.crate_graph().transitive_rev_deps(self.id).into_iter().map(|id| Crate { id })
     }
 
     pub fn root_module(self) -> Module {

crates/ide/src/lib.rs

@@ -577,7 +577,7 @@ impl Analysis {
 
     /// Returns crates this file belongs too.
     pub fn transitive_rev_deps(&self, crate_id: CrateId) -> Cancellable<Vec<CrateId>> {
-        self.with_db(|db| db.crate_graph().transitive_rev_deps(crate_id).collect())
+        self.with_db(|db| db.crate_graph().transitive_rev_deps(crate_id))
     }
 
     /// Returns crates this file *might* belong too.

crates/project-model/src/project_json.rs

@@ -183,12 +183,11 @@ impl ProjectJson {
         &self.project_root
     }
 
-    pub fn crate_by_root(&self, root: &AbsPath) -> Option<Crate> {
+    pub fn crate_by_root(&self, root: &AbsPath) -> Option<&Crate> {
         self.crates
             .iter()
             .filter(|krate| krate.is_workspace_member)
             .find(|krate| krate.root_module == root)
-            .cloned()
     }
 
     /// Returns the path to the project's manifest, if it exists.
@@ -219,13 +218,17 @@ impl ProjectJson {
     pub fn runnables(&self) -> &[Runnable] {
         &self.runnables
     }
+
+    pub fn runnable_template(&self, kind: RunnableKind) -> Option<&Runnable> {
+        self.runnables().iter().find(|r| r.kind == kind)
+    }
 }
 
 /// A crate points to the root module of a crate and lists the dependencies of the crate. This is
 /// useful in creating the crate graph.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Crate {
-    pub(crate) display_name: Option<CrateDisplayName>,
+    pub display_name: Option<CrateDisplayName>,
     pub root_module: AbsPathBuf,
     pub(crate) edition: Edition,
     pub(crate) version: Option<String>,
@@ -319,6 +322,10 @@ pub enum RunnableKind {
 
     /// Run a single test.
     TestOne,
+
+    /// Template for checking a target, emitting rustc JSON diagnostics.
+    /// May include {label} which will get the label from the `build` section of a crate.
+    Flycheck,
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
@@ -420,6 +427,7 @@ pub struct RunnableData {
 #[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub enum RunnableKindData {
+    Flycheck,
     Check,
     Run,
     TestOne,
@@ -490,6 +498,7 @@ impl From<RunnableKindData> for RunnableKind {
             RunnableKindData::Check => RunnableKind::Check,
             RunnableKindData::Run => RunnableKind::Run,
             RunnableKindData::TestOne => RunnableKind::TestOne,
+            RunnableKindData::Flycheck => RunnableKind::Flycheck,
         }
     }
 }

crates/rust-analyzer/src/global_state.rs

@@ -93,6 +93,7 @@ pub(crate) struct GlobalState {
     pub(crate) flycheck_sender: Sender<FlycheckMessage>,
     pub(crate) flycheck_receiver: Receiver<FlycheckMessage>,
     pub(crate) last_flycheck_error: Option<String>,
+    pub(crate) flycheck_formatted_commands: Vec<String>,
 
     // Test explorer
     pub(crate) test_run_session: Option<Vec<CargoTestHandle>>,
@@ -240,6 +241,7 @@ impl GlobalState {
             flycheck_sender,
             flycheck_receiver,
             last_flycheck_error: None,
+            flycheck_formatted_commands: vec![],
 
             test_run_session: None,
             test_run_sender,
@@ -695,7 +697,7 @@ impl GlobalStateSnapshot {
                     let Some(krate) = project.crate_by_root(path) else {
                         continue;
                     };
-                    let Some(build) = krate.build else {
+                    let Some(build) = krate.build.clone() else {
                         continue;
                     };
 

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

@@ -3,20 +3,21 @@
 
 use std::ops::{Deref, Not as _};
 
+use ide::CrateId;
 use itertools::Itertools;
 use lsp_types::{
     CancelParams, DidChangeConfigurationParams, DidChangeTextDocumentParams,
     DidChangeWatchedFilesParams, DidChangeWorkspaceFoldersParams, DidCloseTextDocumentParams,
     DidOpenTextDocumentParams, DidSaveTextDocumentParams, WorkDoneProgressCancelParams,
 };
 use paths::Utf8PathBuf;
-use stdx::TupleExt;
+use project_model::project_json;
 use triomphe::Arc;
 use vfs::{AbsPathBuf, ChangeKind, VfsPath};
 
 use crate::{
     config::{Config, ConfigChange},
-    flycheck::Target,
+    flycheck::{self, BinTarget},
     global_state::{FetchWorkspaceRequest, GlobalState},
     lsp::{from_proto, utils::apply_document_changes},
     lsp_ext::{self, RunFlycheckParams},
@@ -296,41 +297,68 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
         let source_root_id = world.analysis.source_root_id(file_id).ok();
         let mut updated = false;
         let task = move || -> std::result::Result<(), ide::Cancelled> {
-            // Is the target binary? If so we let flycheck run only for the workspace that contains the crate.
-            let target = TargetSpec::for_file(&world, file_id)?.and_then(|it| {
-                let tgt_kind = it.target_kind();
-                let (tgt_name, crate_id) = match it {
-                    TargetSpec::Cargo(c) => (c.target, c.crate_id),
-                    TargetSpec::ProjectJson(p) => (p.label, p.crate_id),
-                };
+            #[derive(Debug)]
+            enum FlycheckScope {
+                /// Cargo workspace but user edited a binary target. There should be no
+                /// downstream crates. We let flycheck run only for the workspace that
+                /// contains the crate.
+                Binary { package_name: Option<String>, bin_target: BinTarget, crate_id: CrateId },
+                /// Limit flycheck to crates actually containing the file_id, because the user does not want to
+                /// flycheck with --workspace.
+                NoDownstream,
+                /// Run on any affected workspace
+                Workspace,
+            }
 
-                let tgt = match tgt_kind {
-                    project_model::TargetKind::Bin => Target::Bin(tgt_name),
-                    project_model::TargetKind::Example => Target::Example(tgt_name),
-                    project_model::TargetKind::Test => Target::Test(tgt_name),
-                    project_model::TargetKind::Bench => Target::Benchmark(tgt_name),
-                    _ => return None,
-                };
+            let scope = TargetSpec::for_file(&world, file_id)?
+                .map(|it| {
+                    let tgt_kind = it.target_kind();
+                    let (package_name, tgt_name, crate_id) = match it {
+                        TargetSpec::Cargo(c) => (Some(c.package), c.target, c.crate_id),
+                        TargetSpec::ProjectJson(p) => (None, p.label, p.crate_id),
+                    };
 
-                Some((tgt, crate_id))
-            });
+                    if let Some(bin_target) = BinTarget::from_target_kind(tgt_kind, tgt_name) {
+                        return FlycheckScope::Binary { package_name, bin_target, crate_id };
+                    }
+                    if !world.config.flycheck_workspace(source_root_id) {
+                        FlycheckScope::NoDownstream
+                    } else {
+                        FlycheckScope::Workspace
+                    }
+                })
+                // XXX: is this right?
+                .unwrap_or(FlycheckScope::Workspace);
 
-            let crate_ids = match target {
-                // Trigger flychecks for the only crate which the target belongs to
-                Some((_, krate)) => vec![krate],
-                None => {
+            tracing::debug!("flycheck scope: {:?}", scope);
+
+            let crate_ids = match scope {
+                FlycheckScope::Workspace => {
                     // Trigger flychecks for all workspaces that depend on the saved file
-                    // Crates containing or depending on the saved file
+                    // i.e. have crates containing or depending on the saved file
                     world
                         .analysis
                         .crates_for(file_id)?
                         .into_iter()
+                        // These are topologically sorted. So `id` is first.
                         .flat_map(|id| world.analysis.transitive_rev_deps(id))
+                        // FIXME: If there are multiple crates_for(file_id), once you flatten
+                        // multiple transitive_rev_deps, it's no longer guaranteed to be toposort.
                         .flatten()
                         .unique()
                         .collect::<Vec<_>>()
                 }
+                FlycheckScope::NoDownstream => {
+                    // Trigger flychecks in all workspaces, but only for the exact crate that has
+                    // this file, and not for any workspaces that don't have that file.
+                    world.analysis.crates_for(file_id)?
+                }
+                // Trigger flychecks for the only crate which the target belongs to
+                FlycheckScope::Binary { crate_id, .. } => {
+                    vec![crate_id]
+                }
             };
+
             let crate_root_paths: Vec<_> = crate_ids
                 .iter()
                 .filter_map(|&crate_id| {
@@ -352,20 +380,51 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
                     | 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())
-                    }),
+                    } => {
+                        // Iterate crate_root_paths first because it is in topological
+                        // order^[1], and we can therefore find the actual crate your saved
+                        // file was in rather than some random downstream dependency.
+                        // Thus with `[check] workspace = false` we can flycheck the
+                        // smallest number of crates (just A) instead of checking B and C
+                        // in response to every file save in A.
+                        //
+                        // A <- B <- C
+                        //
+                        // [1]: But see FIXME above where we flatten.
+                        crate_root_paths.iter().find_map(|root| {
+                            let target = cargo.target_by_root(root)?;
+                            let pkg = cargo[target].package;
+                            let pkg_name = cargo[pkg].name.clone();
+                            Some(flycheck::PackageSpecifier::Cargo {
+                                // This is very hacky. But we are iterating through a lot of
+                                // crates, many of which are reverse deps, and it doesn't make
+                                // sense to attach --bin XXX to some random downstream dep in a
+                                // different workspace.
+                                bin_target: match &scope {
+                                    FlycheckScope::Binary {
+                                        package_name: bin_pkg_name,
+                                        bin_target,
+                                        ..
+                                    } if bin_pkg_name.as_ref() == Some(&pkg_name)
+                                        && bin_target.name() == cargo[target].name =>
+                                    {
+                                        Some(bin_target.clone())
+                                    }
+                                    _ => None,
+                                },
+                                cargo_canonical_name: pkg_name,
+                            })
+                        })
+                    }
                     project_model::ProjectWorkspaceKind::Json(project) => {
-                        if !project.crates().any(|(_, krate)| {
-                            crate_root_paths.contains(&krate.root_module.as_path())
-                        }) {
-                            return None;
-                        }
-                        None
+                        let krate_flycheck = crate_root_paths.iter().find_map(|root| {
+                            let krate = project.crate_by_root(root)?;
+                            project_json_flycheck(project, krate)
+                        });
+
+                        // If there is no matching crate, returns None and doesn't hit this
+                        // workspace in the loop below.
+                        Some(krate_flycheck?)
                     }
                     project_model::ProjectWorkspaceKind::DetachedFile { .. } => return None,
                 };
@@ -379,14 +438,20 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
                 for (id, package) in workspace_ids.clone() {
                     if id == flycheck.id() {
                         updated = true;
-                        match package.filter(|_| {
-                            !world.config.flycheck_workspace(source_root_id) || target.is_some()
+                        match package.filter(|spec| {
+                            // Any of these cases, and we can't flycheck the whole workspace.
+                            !world.config.flycheck_workspace(source_root_id)
+                                || flycheck.cannot_run_workspace()
+                                    // No point flychecking the whole workspace when you edited a
+                                    // main.rs. It cannot have dependencies.
+                                || matches!(
+                                    spec,
+                                    flycheck::PackageSpecifier::Cargo { bin_target: Some(_), .. }
+                                )
                         }) {
-                            Some(package) => flycheck
-                                .restart_for_package(package, target.clone().map(TupleExt::head)),
+                            Some(spec) => flycheck.restart_for_package(spec),
                             None => flycheck.restart_workspace(saved_file.clone()),
                         }
-                        continue;
                     }
                 }
             }
@@ -446,3 +511,22 @@ pub(crate) fn handle_abort_run_test(state: &mut GlobalState, _: ()) -> anyhow::R
     }
     Ok(())
 }
+
+fn project_json_flycheck(
+    _project_json: &project_json::ProjectJson,
+    krate: &project_json::Crate,
+) -> Option<flycheck::PackageSpecifier> {
+    if let Some(build_info) = krate.build.as_ref() {
+        let label = build_info.label.clone();
+        Some(flycheck::PackageSpecifier::BuildInfo { label })
+    } else {
+        // No build_info field, so assume this is built by cargo.
+        let cargo_canonical_name =
+            krate.display_name.as_ref().map(|x| x.canonical_name().to_owned())?.to_string();
+        Some(flycheck::PackageSpecifier::Cargo {
+            cargo_canonical_name,
+            // In JSON world, can we even describe crates that are checkable with `cargo check --bin XXX`?
+            bin_target: None,
+        })
+    }
+}

crates/rust-analyzer/src/main_loop.rs

@@ -951,8 +951,24 @@ impl GlobalState {
             FlycheckMessage::ClearDiagnostics { id } => self.diagnostics.clear_check(id),
 
             FlycheckMessage::Progress { id, progress } => {
+                let format_with_id = |user_facing_command: String| {
+                    if self.flycheck.len() == 1 {
+                        user_facing_command
+                    } else {
+                        format!("{user_facing_command} (#{})", id + 1)
+                    }
+                };
+
+                self.flycheck_formatted_commands
+                    .resize_with(self.flycheck.len().max(id + 1), || {
+                        format_with_id(self.config.flycheck(None).to_string())
+                    });
+
                 let (state, message) = match progress {
-                    flycheck::Progress::DidStart => (Progress::Begin, None),
+                    flycheck::Progress::DidStart { user_facing_command } => {
+                        self.flycheck_formatted_commands[id] = format_with_id(user_facing_command);
+                        (Progress::Begin, None)
+                    }
                     flycheck::Progress::DidCheckCrate(target) => (Progress::Report, Some(target)),
                     flycheck::Progress::DidCancel => {
                         self.last_flycheck_error = None;
@@ -970,13 +986,8 @@ impl GlobalState {
                     }
                 };
 
-                // When we're running multiple flychecks, we have to include a disambiguator in
-                // the title, or the editor complains. Note that this is a user-facing string.
-                let title = if self.flycheck.len() == 1 {
-                    format!("{}", self.config.flycheck(None))
-                } else {
-                    format!("{} (#{})", self.config.flycheck(None), id + 1)
-                };
+                // Clone because we &mut self for report_progress
+                let title = self.flycheck_formatted_commands[id].clone();
                 self.report_progress(
                     &title,
                     state,

crates/rust-analyzer/src/reload.rs

@@ -24,7 +24,9 @@ use itertools::Itertools;
 use load_cargo::{load_proc_macro, ProjectFolders};
 use lsp_types::FileSystemWatcher;
 use proc_macro_api::ProcMacroServer;
-use project_model::{ManifestPath, ProjectWorkspace, ProjectWorkspaceKind, WorkspaceBuildScripts};
+use project_model::{
+    project_json, ManifestPath, ProjectWorkspace, ProjectWorkspaceKind, WorkspaceBuildScripts,
+};
 use stdx::{format_to, thread::ThreadIntent};
 use triomphe::Arc;
 use vfs::{AbsPath, AbsPathBuf, ChangeKind};
@@ -808,6 +810,7 @@ impl GlobalState {
                     0,
                     sender,
                     config,
+                    crate::flycheck::FlycheckConfigJson::default(),
                     None,
                     self.config.root_path().clone(),
                     None,
@@ -825,13 +828,25 @@ impl GlobalState {
                                 | ProjectWorkspaceKind::DetachedFile {
                                     cargo: Some((cargo, _, _)),
                                     ..
-                                } => (cargo.workspace_root(), Some(cargo.manifest_path())),
+                                } => (
+                                    crate::flycheck::FlycheckConfigJson::default(),
+                                    cargo.workspace_root(),
+                                    Some(cargo.manifest_path()),
+                                ),
                                 ProjectWorkspaceKind::Json(project) => {
+                                    let config_json = crate::flycheck::FlycheckConfigJson {
+                                        single_template: project
+                                            .runnable_template(project_json::RunnableKind::Flycheck)
+                                            .cloned(),
+                                    };
                                     // Enable flychecks for json projects if a custom flycheck command was supplied
                                     // in the workspace configuration.
                                     match config {
+                                        _ if config_json.any_configured() => {
+                                            (config_json, project.path(), None)
+                                        }
                                         FlycheckConfig::CustomCommand { .. } => {
-                                            (project.path(), None)
+                                            (config_json, project.path(), None)
                                         }
                                         _ => return None,
                                     }
@@ -841,11 +856,12 @@ impl GlobalState {
                             ws.sysroot.root().map(ToOwned::to_owned),
                         ))
                     })
-                    .map(|(id, (root, manifest_path), sysroot_root)| {
+                    .map(|(id, (config_json, root, manifest_path), sysroot_root)| {
                         FlycheckHandle::spawn(
                             id,
                             sender.clone(),
                             config.clone(),
+                            config_json,
                             sysroot_root,
                             root.to_path_buf(),
                             manifest_path.map(|it| it.to_path_buf()),
@@ -855,6 +871,7 @@ impl GlobalState {
             }
         }
         .into();
+        self.flycheck_formatted_commands = vec![];
     }
 }