Auto merge of #17888 - Tyrubias:remove-invocation-location, r=Veykril

chore(config): remove `invocationLocation` in favor of `invocationStrategy`

These flags were added to help rust-analyzer integrate with repos requiring non-Cargo invocations. The consensus is that having two independent settings are no longer needed. This change removes `invocationLocation` in favor of `invocationStrategy` and changes the internal representation of `InvocationStrategy::Once` to hold the workspace root.

Closes #17848.

Author: bors

crates/project-model/src/build_dependencies.rs

@@ -19,8 +19,8 @@ use serde::Deserialize;
 use toolchain::Tool;
 
 use crate::{
-    utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
-    InvocationStrategy, ManifestPath, Package, Sysroot, TargetKind,
+    utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationStrategy, ManifestPath,
+    Package, Sysroot, TargetKind,
 };
 
 /// Output of the build script and proc-macro building steps for a workspace.
@@ -63,10 +63,7 @@ impl WorkspaceBuildScripts {
         progress: &dyn Fn(String),
         sysroot: &Sysroot,
     ) -> io::Result<WorkspaceBuildScripts> {
-        let current_dir = match &config.invocation_location {
-            InvocationLocation::Root(root) if config.run_build_script_command.is_some() => root,
-            _ => workspace.workspace_root(),
-        };
+        let current_dir = workspace.workspace_root();
 
         let allowed_features = workspace.workspace_features();
         let cmd = Self::build_command(
@@ -89,15 +86,7 @@ impl WorkspaceBuildScripts {
     ) -> io::Result<Vec<WorkspaceBuildScripts>> {
         assert_eq!(config.invocation_strategy, InvocationStrategy::Once);
 
-        let current_dir = match &config.invocation_location {
-            InvocationLocation::Root(root) => root,
-            InvocationLocation::Workspace => {
-                return Err(io::Error::new(
-                    io::ErrorKind::Other,
-                    "Cannot run build scripts from workspace with invocation strategy `once`",
-                ))
-            }
-        };
+        let current_dir = workspace_root;
         let cmd = Self::build_command(
             config,
             &Default::default(),

crates/project-model/src/cargo_workspace.rs

@@ -13,7 +13,7 @@ use serde_json::from_value;
 use span::Edition;
 use toolchain::Tool;
 
-use crate::{utf8_stdout, InvocationLocation, ManifestPath, Sysroot};
+use crate::{utf8_stdout, ManifestPath, Sysroot};
 use crate::{CfgOverrides, InvocationStrategy};
 
 /// [`CargoWorkspace`] represents the logical structure of, well, a Cargo
@@ -100,7 +100,6 @@ pub struct CargoConfig {
     /// Extra env vars to set when invoking the cargo command
     pub extra_env: FxHashMap<String, String>,
     pub invocation_strategy: InvocationStrategy,
-    pub invocation_location: InvocationLocation,
     /// Optional path to use instead of `target` when building
     pub target_dir: Option<Utf8PathBuf>,
 }

crates/project-model/src/lib.rs

@@ -186,20 +186,13 @@ fn utf8_stdout(mut cmd: Command) -> anyhow::Result<String> {
     Ok(stdout.trim().to_owned())
 }
 
-#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
 pub enum InvocationStrategy {
     Once,
     #[default]
     PerWorkspace,
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
-pub enum InvocationLocation {
-    Root(AbsPathBuf),
-    #[default]
-    Workspace,
-}
-
 /// A set of cfg-overrides per crate.
 #[derive(Default, Debug, Clone, Eq, PartialEq)]
 pub struct CfgOverrides {

crates/rust-analyzer/src/config.rs

@@ -80,13 +80,6 @@ config_data! {
         pub(crate) cargo_autoreload: bool           = true,
         /// Run build scripts (`build.rs`) for more precise code analysis.
         cargo_buildScripts_enable: bool  = true,
-        /// Specifies the working directory for running build scripts.
-        /// - "workspace": run build scripts for a workspace in the workspace's root directory.
-        ///   This is incompatible with `#rust-analyzer.cargo.buildScripts.invocationStrategy#` set to `once`.
-        /// - "root": run build scripts in the project's root directory.
-        /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
-        /// is set.
-        cargo_buildScripts_invocationLocation: InvocationLocation = InvocationLocation::Workspace,
         /// Specifies the invocation strategy to use when running the build scripts command.
         /// If `per_workspace` is set, the command will be executed for each workspace.
         /// If `once` is set, the command will be executed once.
@@ -101,8 +94,7 @@ config_data! {
         /// If there are multiple linked projects/workspaces, this command is invoked for
         /// each of them, with the working directory being the workspace root
         /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten
-        /// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and
-        /// `#rust-analyzer.cargo.buildScripts.invocationLocation#`.
+        /// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.
         ///
         /// By default, a cargo invocation will be constructed for the configured
         /// targets and features, with the following base command line:
@@ -182,14 +174,6 @@ config_data! {
         ///
         /// For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,...
         check_ignore: FxHashSet<String> = FxHashSet::default(),
-        /// Specifies the working directory for running checks.
-        /// - "workspace": run checks for workspaces in the corresponding workspaces' root directories.
-        // FIXME: Ideally we would support this in some way
-        ///   This falls back to "root" if `#rust-analyzer.check.invocationStrategy#` is set to `once`.
-        /// - "root": run checks in the project's root directory.
-        /// This config only has an effect when `#rust-analyzer.check.overrideCommand#`
-        /// is set.
-        check_invocationLocation | checkOnSave_invocationLocation: InvocationLocation = InvocationLocation::Workspace,
         /// Specifies the invocation strategy to use when running the check command.
         /// If `per_workspace` is set, the command will be executed for each workspace.
         /// If `once` is set, the command will be executed once.
@@ -212,8 +196,7 @@ config_data! {
         /// If there are multiple linked projects/workspaces, this command is invoked for
         /// each of them, with the working directory being the workspace root
         /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten
-        /// by changing `#rust-analyzer.check.invocationStrategy#` and
-        /// `#rust-analyzer.check.invocationLocation#`.
+        /// by changing `#rust-analyzer.check.invocationStrategy#`.
         ///
         /// If `$saved_file` is part of the command, rust-analyzer will pass
         /// the absolute path of the saved file to the provided command. This is
@@ -1868,12 +1851,6 @@ impl Config {
                 InvocationStrategy::Once => project_model::InvocationStrategy::Once,
                 InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace,
             },
-            invocation_location: match self.cargo_buildScripts_invocationLocation(None) {
-                InvocationLocation::Root => {
-                    project_model::InvocationLocation::Root(self.root_path.clone())
-                }
-                InvocationLocation::Workspace => project_model::InvocationLocation::Workspace,
-            },
             run_build_script_command: self.cargo_buildScripts_overrideCommand(None).clone(),
             extra_args: self.cargo_extraArgs(None).clone(),
             extra_env: self.cargo_extraEnv(None).clone(),
@@ -1930,14 +1907,6 @@ impl Config {
                             crate::flycheck::InvocationStrategy::PerWorkspace
                         }
                     },
-                    invocation_location: match self.check_invocationLocation(None) {
-                        InvocationLocation::Root => {
-                            crate::flycheck::InvocationLocation::Root(self.root_path.clone())
-                        }
-                        InvocationLocation::Workspace => {
-                            crate::flycheck::InvocationLocation::Workspace
-                        }
-                    },
                 }
             }
             Some(_) | None => FlycheckConfig::CargoCommand {
@@ -2348,13 +2317,6 @@ pub(crate) enum InvocationStrategy {
 #[derive(Serialize, Deserialize, Debug, Clone)]
 struct CheckOnSaveTargets(#[serde(with = "single_or_array")] Vec<String>);
 
-#[derive(Serialize, Deserialize, Debug, Clone)]
-#[serde(rename_all = "snake_case")]
-enum InvocationLocation {
-    Root,
-    Workspace,
-}
-
 #[derive(Serialize, Deserialize, Debug, Clone)]
 #[serde(rename_all = "snake_case")]
 enum LifetimeElisionDef {
@@ -3196,14 +3158,6 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
                 "The command will be executed once."
             ],
         },
-        "InvocationLocation" => set! {
-            "type": "string",
-            "enum": ["workspace", "root"],
-            "enumDescriptions": [
-                "The command will be executed in the corresponding workspace root.",
-                "The command will be executed in the project root."
-            ],
-        },
         "Option<CheckOnSaveTargets>" => set! {
             "anyOf": [
                 {

crates/rust-analyzer/src/flycheck.rs

@@ -15,20 +15,13 @@ use toolchain::Tool;
 
 use crate::command::{CommandHandle, ParseFromLine};
 
-#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
 pub(crate) enum InvocationStrategy {
     Once,
     #[default]
     PerWorkspace,
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
-pub(crate) enum InvocationLocation {
-    Root(AbsPathBuf),
-    #[default]
-    Workspace,
-}
-
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) struct CargoOptions {
     pub(crate) target_triples: Vec<String>,
@@ -79,7 +72,6 @@ pub(crate) enum FlycheckConfig {
         args: Vec<String>,
         extra_env: FxHashMap<String, String>,
         invocation_strategy: InvocationStrategy,
-        invocation_location: InvocationLocation,
     },
 }
 
@@ -424,30 +416,17 @@ impl FlycheckActor {
                 cmd.args(&options.extra_args);
                 Some(cmd)
             }
-            FlycheckConfig::CustomCommand {
-                command,
-                args,
-                extra_env,
-                invocation_strategy,
-                invocation_location,
-            } => {
+            FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
                 let mut cmd = Command::new(command);
                 cmd.envs(extra_env);
 
-                match invocation_location {
-                    InvocationLocation::Workspace => {
-                        match invocation_strategy {
-                            InvocationStrategy::Once => {
-                                cmd.current_dir(&self.root);
-                            }
-                            InvocationStrategy::PerWorkspace => {
-                                // FIXME: cmd.current_dir(&affected_workspace);
-                                cmd.current_dir(&self.root);
-                            }
-                        }
+                match invocation_strategy {
+                    InvocationStrategy::Once => {
+                        cmd.current_dir(&self.root);
                     }
-                    InvocationLocation::Root(root) => {
-                        cmd.current_dir(root);
+                    InvocationStrategy::PerWorkspace => {
+                        // FIXME: cmd.current_dir(&affected_workspace);
+                        cmd.current_dir(&self.root);
                     }
                 }
 

crates/rust-analyzer/src/reload.rs

@@ -764,18 +764,22 @@ impl GlobalState {
             FlycheckConfig::CargoCommand { .. } => {
                 crate::flycheck::InvocationStrategy::PerWorkspace
             }
-            FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy,
+            FlycheckConfig::CustomCommand { ref invocation_strategy, .. } => {
+                invocation_strategy.clone()
+            }
         };
 
         self.flycheck = match invocation_strategy {
-            crate::flycheck::InvocationStrategy::Once => vec![FlycheckHandle::spawn(
-                0,
-                sender,
-                config,
-                None,
-                self.config.root_path().clone(),
-                None,
-            )],
+            crate::flycheck::InvocationStrategy::Once => {
+                vec![FlycheckHandle::spawn(
+                    0,
+                    sender,
+                    config,
+                    None,
+                    self.config.root_path().clone(),
+                    None,
+                )]
+            }
             crate::flycheck::InvocationStrategy::PerWorkspace => {
                 self.workspaces
                     .iter()

docs/user/generated_config.adoc

@@ -45,16 +45,6 @@ Automatically refresh project info via `cargo metadata` on
 --
 Run build scripts (`build.rs`) for more precise code analysis.
 --
-[[rust-analyzer.cargo.buildScripts.invocationLocation]]rust-analyzer.cargo.buildScripts.invocationLocation (default: `"workspace"`)::
-+
---
-Specifies the working directory for running build scripts.
-- "workspace": run build scripts for a workspace in the workspace's root directory.
-  This is incompatible with `#rust-analyzer.cargo.buildScripts.invocationStrategy#` set to `once`.
-- "root": run build scripts in the project's root directory.
-This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
-is set.
---
 [[rust-analyzer.cargo.buildScripts.invocationStrategy]]rust-analyzer.cargo.buildScripts.invocationStrategy (default: `"per_workspace"`)::
 +
 --
@@ -75,8 +65,7 @@ option.
 If there are multiple linked projects/workspaces, this command is invoked for
 each of them, with the working directory being the workspace root
 (i.e., the folder containing the `Cargo.toml`). This can be overwritten
-by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and
-`#rust-analyzer.cargo.buildScripts.invocationLocation#`.
+by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.
 
 By default, a cargo invocation will be constructed for the configured
 targets and features, with the following base command line:
@@ -209,16 +198,6 @@ List of `cargo check` (or other command specified in `check.command`) diagnostic
 
 For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,...
 --
-[[rust-analyzer.check.invocationLocation]]rust-analyzer.check.invocationLocation (default: `"workspace"`)::
-+
---
-Specifies the working directory for running checks.
-- "workspace": run checks for workspaces in the corresponding workspaces' root directories.
-  This falls back to "root" if `#rust-analyzer.check.invocationStrategy#` is set to `once`.
-- "root": run checks in the project's root directory.
-This config only has an effect when `#rust-analyzer.check.overrideCommand#`
-is set.
---
 [[rust-analyzer.check.invocationStrategy]]rust-analyzer.check.invocationStrategy (default: `"per_workspace"`)::
 +
 --
@@ -250,8 +229,7 @@ Cargo, you might also want to change
 If there are multiple linked projects/workspaces, this command is invoked for
 each of them, with the working directory being the workspace root
 (i.e., the folder containing the `Cargo.toml`). This can be overwritten
-by changing `#rust-analyzer.check.invocationStrategy#` and
-`#rust-analyzer.check.invocationLocation#`.
+by changing `#rust-analyzer.check.invocationStrategy#`.
 
 If `$saved_file` is part of the command, rust-analyzer will pass
 the absolute path of the saved file to the provided command. This is