refactor: Define a TargetSpec wrapper type

Author: Wilfred

crates/rust-analyzer/src/global_state.rs

@@ -18,7 +18,7 @@ use parking_lot::{
     RwLockWriteGuard,
 };
 use proc_macro_api::ProcMacroServer;
-use project_model::{CargoWorkspace, ProjectWorkspace, Target, WorkspaceBuildScripts};
+use project_model::{ProjectWorkspace, WorkspaceBuildScripts};
 use rustc_hash::{FxHashMap, FxHashSet};
 use triomphe::Arc;
 use vfs::{AnchoredPathBuf, ChangedFile, Vfs};
@@ -33,6 +33,7 @@ use crate::{
     mem_docs::MemDocs,
     op_queue::OpQueue,
     reload,
+    target_spec::{CargoTargetSpec, TargetSpec},
     task_pool::{TaskPool, TaskQueue},
 };
 
@@ -502,20 +503,38 @@ impl GlobalStateSnapshot {
         self.vfs_read().file_path(file_id).clone()
     }
 
-    pub(crate) fn cargo_target_for_crate_root(
-        &self,
-        crate_id: CrateId,
-    ) -> Option<(&CargoWorkspace, Target)> {
+    pub(crate) fn target_spec_for_crate(&self, crate_id: CrateId) -> Option<TargetSpec> {
         let file_id = self.analysis.crate_root(crate_id).ok()?;
         let path = self.vfs_read().file_path(file_id).clone();
         let path = path.as_path()?;
-        self.workspaces.iter().find_map(|ws| match ws {
-            ProjectWorkspace::Cargo { cargo, .. } => {
-                cargo.target_by_root(path).map(|it| (cargo, it))
+
+        for workspace in self.workspaces.iter() {
+            match workspace {
+                ProjectWorkspace::Cargo { cargo, .. } => {
+                    let Some(target_idx) = cargo.target_by_root(path) else {
+                        continue;
+                    };
+
+                    let target_data = &cargo[target_idx];
+                    let package_data = &cargo[target_data.package];
+
+                    return Some(TargetSpec::Cargo(CargoTargetSpec {
+                        workspace_root: cargo.workspace_root().to_path_buf(),
+                        cargo_toml: package_data.manifest.clone(),
+                        crate_id,
+                        package: cargo.package_flag(package_data),
+                        target: target_data.name.clone(),
+                        target_kind: target_data.kind,
+                        required_features: target_data.required_features.clone(),
+                        features: package_data.features.keys().cloned().collect(),
+                    }));
+                }
+                ProjectWorkspace::Json { .. } => {}
+                ProjectWorkspace::DetachedFiles { .. } => {}
             }
-            ProjectWorkspace::Json { .. } => None,
-            ProjectWorkspace::DetachedFiles { .. } => None,
-        })
+        }
+
+        None
     }
 
     pub(crate) fn file_exists(&self, file_id: FileId) -> bool {

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

@@ -35,7 +35,6 @@ use triomphe::Arc;
 use vfs::{AbsPath, AbsPathBuf, FileId, VfsPath};
 
 use crate::{
-    cargo_target_spec::CargoTargetSpec,
     config::{Config, RustfmtConfig, WorkspaceSymbolConfig},
     diff::diff,
     global_state::{GlobalState, GlobalStateSnapshot},
@@ -50,6 +49,7 @@ use crate::{
         self, CrateInfoResult, ExternalDocsPair, ExternalDocsResponse, FetchDependencyListParams,
         FetchDependencyListResult, PositionOrRange, ViewCrateGraphParams, WorkspaceSymbolParams,
     },
+    target_spec::TargetSpec,
 };
 
 pub(crate) fn handle_workspace_reload(state: &mut GlobalState, _: ()) -> anyhow::Result<()> {
@@ -771,8 +771,8 @@ pub(crate) fn handle_parent_module(
             Some(&crate_id) => crate_id,
             None => return Ok(None),
         };
-        let cargo_spec = match CargoTargetSpec::for_file(&snap, file_id)? {
-            Some(it) => it,
+        let cargo_spec = match TargetSpec::for_file(&snap, file_id)? {
+            Some(TargetSpec::Cargo(it)) => it,
             None => return Ok(None),
         };
 
@@ -804,7 +804,7 @@ pub(crate) fn handle_runnables(
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
     let offset = params.position.and_then(|it| from_proto::offset(&line_index, it).ok());
-    let cargo_spec = CargoTargetSpec::for_file(&snap, file_id)?;
+    let target_spec = TargetSpec::for_file(&snap, file_id)?;
 
     let expect_test = match offset {
         Some(offset) => {
@@ -821,7 +821,7 @@ pub(crate) fn handle_runnables(
         if should_skip_for_offset(&runnable, offset) {
             continue;
         }
-        if should_skip_target(&runnable, cargo_spec.as_ref()) {
+        if should_skip_target(&runnable, target_spec.as_ref()) {
             continue;
         }
         let mut runnable = to_proto::runnable(&snap, runnable)?;
@@ -834,8 +834,8 @@ pub(crate) fn handle_runnables(
 
     // Add `cargo check` and `cargo test` for all targets of the whole package
     let config = snap.config.runnables();
-    match cargo_spec {
-        Some(spec) => {
+    match target_spec {
+        Some(TargetSpec::Cargo(spec)) => {
             let all_targets = !snap.analysis.is_crate_no_std(spec.crate_id)?;
             for cmd in ["check", "test"] {
                 let mut cargo_args =
@@ -1351,14 +1351,14 @@ pub(crate) fn handle_code_lens(
     }
 
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
-    let cargo_target_spec = CargoTargetSpec::for_file(&snap, file_id)?;
+    let target_spec = TargetSpec::for_file(&snap, file_id)?;
 
     let annotations = snap.analysis.annotations(
         &AnnotationConfig {
-            binary_target: cargo_target_spec
+            binary_target: target_spec
                 .map(|spec| {
                     matches!(
-                        spec.target_kind,
+                        spec.target_kind(),
                         TargetKind::Bin | TargetKind::Example | TargetKind::Test
                     )
                 })
@@ -1765,8 +1765,8 @@ pub(crate) fn handle_open_cargo_toml(
     let _p = tracing::span!(tracing::Level::INFO, "handle_open_cargo_toml").entered();
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
 
-    let cargo_spec = match CargoTargetSpec::for_file(&snap, file_id)? {
-        Some(it) => it,
+    let cargo_spec = match TargetSpec::for_file(&snap, file_id)? {
+        Some(TargetSpec::Cargo(it)) => it,
         None => return Ok(None),
     };
 
@@ -1895,8 +1895,8 @@ fn runnable_action_links(
         return None;
     }
 
-    let cargo_spec = CargoTargetSpec::for_file(snap, runnable.nav.file_id).ok()?;
-    if should_skip_target(&runnable, cargo_spec.as_ref()) {
+    let target_spec = TargetSpec::for_file(snap, runnable.nav.file_id).ok()?;
+    if should_skip_target(&runnable, target_spec.as_ref()) {
         return None;
     }
 
@@ -1961,13 +1961,13 @@ fn prepare_hover_actions(
         .collect()
 }
 
-fn should_skip_target(runnable: &Runnable, cargo_spec: Option<&CargoTargetSpec>) -> bool {
+fn should_skip_target(runnable: &Runnable, cargo_spec: Option<&TargetSpec>) -> bool {
     match runnable.kind {
         RunnableKind::Bin => {
             // Do not suggest binary run on other target than binary
             match &cargo_spec {
                 Some(spec) => !matches!(
-                    spec.target_kind,
+                    spec.target_kind(),
                     TargetKind::Bin | TargetKind::Example | TargetKind::Test
                 ),
                 None => true,
@@ -2044,9 +2044,9 @@ fn run_rustfmt(
         }
         RustfmtConfig::CustomCommand { command, args } => {
             let cmd = Utf8PathBuf::from(&command);
-            let workspace = CargoTargetSpec::for_file(snap, file_id)?;
-            let mut cmd = match workspace {
-                Some(spec) => {
+            let target_spec = TargetSpec::for_file(snap, file_id)?;
+            let mut cmd = match target_spec {
+                Some(TargetSpec::Cargo(spec)) => {
                     // approach: if the command name contains a path separator, join it with the workspace root.
                     // however, if the path is absolute, joining will result in the absolute path being preserved.
                     // as a fallback, rely on $PATH-based discovery.

crates/rust-analyzer/src/lib.rs

@@ -14,7 +14,6 @@
 pub mod cli;
 
 mod caps;
-mod cargo_target_spec;
 mod diagnostics;
 mod diff;
 mod dispatch;
@@ -25,6 +24,7 @@ mod main_loop;
 mod mem_docs;
 mod op_queue;
 mod reload;
+mod target_spec;
 mod task_pool;
 mod version;
 

crates/rust-analyzer/src/lsp/to_proto.rs

@@ -21,7 +21,6 @@ use serde_json::to_value;
 use vfs::AbsPath;
 
 use crate::{
-    cargo_target_spec::CargoTargetSpec,
     config::{CallInfoConfig, Config},
     global_state::GlobalStateSnapshot,
     line_index::{LineEndings, LineIndex, PositionEncoding},
@@ -31,6 +30,7 @@ use crate::{
         LspError,
     },
     lsp_ext::{self, SnippetTextEdit},
+    target_spec::{CargoTargetSpec, TargetSpec},
 };
 
 pub(crate) fn position(line_index: &LineIndex, offset: TextSize) -> lsp_types::Position {
@@ -1347,10 +1347,10 @@ pub(crate) fn runnable(
     runnable: Runnable,
 ) -> Cancellable<lsp_ext::Runnable> {
     let config = snap.config.runnables();
-    let target_spec = CargoTargetSpec::for_file(snap, runnable.nav.file_id)?;
+    let target_spec = TargetSpec::for_file(snap, runnable.nav.file_id)?;
 
     match target_spec {
-        Some(spec) => {
+        Some(TargetSpec::Cargo(spec)) => {
             let workspace_root = spec.workspace_root.clone();
             let target = spec.target.clone();
 
@@ -1549,12 +1549,8 @@ pub(crate) fn test_item(
         id: test_item.id,
         label: test_item.label,
         kind: match test_item.kind {
-            ide::TestItemKind::Crate(id) => 'b: {
-                let Some((cargo_ws, target)) = snap.cargo_target_for_crate_root(id) else {
-                    break 'b lsp_ext::TestItemKind::Package;
-                };
-                let target = &cargo_ws[target];
-                match target.kind {
+            ide::TestItemKind::Crate(id) => match snap.target_spec_for_crate(id) {
+                Some(target_spec) => match target_spec.target_kind() {
                     project_model::TargetKind::Bin
                     | project_model::TargetKind::Lib { .. }
                     | project_model::TargetKind::Example
@@ -1564,6 +1560,7 @@ pub(crate) fn test_item(
                     // benches are not tests needed to be shown in the test explorer
                     project_model::TargetKind::Bench => return None,
                 }
+                None => lsp_ext::TestItemKind::Package,
             }
             ide::TestItemKind::Module => lsp_ext::TestItemKind::Module,
             ide::TestItemKind::Function => lsp_ext::TestItemKind::Test,

crates/rust-analyzer/src/cargo_target_spec.rs renamed to crates/rust-analyzer/src/target_spec.rs

@@ -1,4 +1,4 @@
-//! See `CargoTargetSpec`
+//! See `TargetSpec`
 
 use std::mem;
 
@@ -10,6 +10,35 @@ use vfs::AbsPathBuf;
 
 use crate::global_state::GlobalStateSnapshot;
 
+/// A target represents a thing we can build or test.
+///
+/// We use it to calculate the CLI arguments required to build, run or
+/// test the target.
+#[derive(Clone)]
+pub(crate) enum TargetSpec {
+    Cargo(CargoTargetSpec),
+}
+
+impl TargetSpec {
+    pub(crate) fn for_file(
+        global_state_snapshot: &GlobalStateSnapshot,
+        file_id: FileId,
+    ) -> Cancellable<Option<Self>> {
+        let crate_id = match &*global_state_snapshot.analysis.crates_for(file_id)? {
+            &[crate_id, ..] => crate_id,
+            _ => return Ok(None),
+        };
+
+        Ok(global_state_snapshot.target_spec_for_crate(crate_id))
+    }
+
+    pub(crate) fn target_kind(&self) -> TargetKind {
+        match self {
+            TargetSpec::Cargo(cargo) => cargo.target_kind,
+        }
+    }
+}
+
 /// Abstract representation of Cargo target.
 ///
 /// We use it to cook up the set of cli args we need to pass to Cargo to
@@ -120,35 +149,6 @@ impl CargoTargetSpec {
         (args, extra_args)
     }
 
-    pub(crate) fn for_file(
-        global_state_snapshot: &GlobalStateSnapshot,
-        file_id: FileId,
-    ) -> Cancellable<Option<CargoTargetSpec>> {
-        let crate_id = match &*global_state_snapshot.analysis.crates_for(file_id)? {
-            &[crate_id, ..] => crate_id,
-            _ => return Ok(None),
-        };
-        let (cargo_ws, target) = match global_state_snapshot.cargo_target_for_crate_root(crate_id) {
-            Some(it) => it,
-            None => return Ok(None),
-        };
-
-        let target_data = &cargo_ws[target];
-        let package_data = &cargo_ws[target_data.package];
-        let res = CargoTargetSpec {
-            workspace_root: cargo_ws.workspace_root().to_path_buf(),
-            cargo_toml: package_data.manifest.clone(),
-            package: cargo_ws.package_flag(package_data),
-            target: target_data.name.clone(),
-            target_kind: target_data.kind,
-            required_features: target_data.required_features.clone(),
-            features: package_data.features.keys().cloned().collect(),
-            crate_id,
-        };
-
-        Ok(Some(res))
-    }
-
     pub(crate) fn push_to(self, buf: &mut Vec<String>, kind: &RunnableKind) {
         buf.push("--package".to_owned());
         buf.push(self.package);