From 32fca82ddc1338df419bafa709cd03d7381a811d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 2 Nov 2022 13:05:16 +0100 Subject: [PATCH] Add $manifest_path substitution for cargo override commands --- crates/flycheck/src/lib.rs | 70 +++++++++++-------- crates/paths/src/lib.rs | 3 + crates/project-model/src/build_scripts.rs | 28 ++++++-- crates/project-model/src/project_json.rs | 1 + crates/rust-analyzer/src/config.rs | 12 ++-- .../src/handlers/notification.rs | 22 +++--- crates/rust-analyzer/src/main_loop.rs | 3 +- crates/rust-analyzer/src/reload.rs | 15 +++- docs/user/generated_config.adoc | 12 ++-- editors/code/package.json | 4 +- 10 files changed, 109 insertions(+), 61 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 2de719af92ce..169e14934edb 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -89,8 +89,9 @@ impl FlycheckHandle { sender: Box, config: FlycheckConfig, workspace_root: AbsPathBuf, + project_file: AbsPathBuf, ) -> FlycheckHandle { - let actor = FlycheckActor::new(id, sender, config, workspace_root); + let actor = FlycheckActor::new(id, sender, config, workspace_root, project_file); let (sender, receiver) = unbounded::(); let thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker) .name("Flycheck".to_owned()) @@ -100,8 +101,8 @@ impl FlycheckHandle { } /// Schedule a re-start of the cargo check worker. - pub fn restart(&self) { - self.sender.send(StateChange::Restart).unwrap(); + pub fn restart(&self, saved_file: Option) { + self.sender.send(StateChange::Restart { saved_file }).unwrap(); } /// Stop this cargo check worker. @@ -152,7 +153,7 @@ pub enum Progress { } enum StateChange { - Restart, + Restart { saved_file: Option }, Cancel, } @@ -165,6 +166,8 @@ struct FlycheckActor { /// Either the workspace root of the workspace we are flychecking, /// or the project root of the project. root: AbsPathBuf, + /// The Cargo.toml or rust-project.json file of the project. + project_file: AbsPathBuf, /// CargoHandle exists to wrap around the communication needed to be able to /// run `cargo check` without blocking. Currently the Rust standard library /// doesn't provide a way to read sub-process output without blocking, so we @@ -184,9 +187,17 @@ impl FlycheckActor { sender: Box, config: FlycheckConfig, workspace_root: AbsPathBuf, + project_file: AbsPathBuf, ) -> FlycheckActor { tracing::info!(%id, ?workspace_root, "Spawning flycheck"); - FlycheckActor { id, sender, config, root: workspace_root, command_handle: None } + FlycheckActor { + id, + sender, + config, + root: workspace_root, + project_file, + command_handle: None, + } } fn report_progress(&self, progress: Progress) { @@ -212,7 +223,7 @@ impl FlycheckActor { tracing::debug!(flycheck_id = self.id, "flycheck cancelled"); self.cancel_check_process(); } - Event::RequestStateChange(StateChange::Restart) => { + Event::RequestStateChange(StateChange::Restart { saved_file }) => { // Cancel the previously spawned process self.cancel_check_process(); while let Ok(restart) = inbox.recv_timeout(Duration::from_millis(50)) { @@ -222,7 +233,7 @@ impl FlycheckActor { } } - let command = self.check_command(); + let command = self.check_command(saved_file); let formatted_command = format!("{:?}", command); tracing::debug!(?command, "will restart flycheck"); @@ -296,8 +307,10 @@ impl FlycheckActor { } } - fn check_command(&self) -> Command { - let (mut cmd, args) = match &self.config { + fn check_command(&self, _saved_file: Option) -> Command { + // FIXME: Figure out the story for exposing the saved file to the custom flycheck command + // as it can be absent + match &self.config { FlycheckConfig::CargoCommand { command, target_triples, @@ -312,7 +325,7 @@ impl FlycheckActor { let mut cmd = Command::new(toolchain::cargo()); cmd.arg(command); cmd.current_dir(&self.root); - cmd.arg("--workspace"); + cmd.args(&["--workspace", "--manifest-path"]).arg(self.project_file.as_os_str()); cmd.arg(if *ansi_color_output { "--message-format=json-diagnostic-rendered-ansi" @@ -341,7 +354,8 @@ impl FlycheckActor { } } cmd.envs(extra_env); - (cmd, extra_args) + cmd.args(extra_args); + cmd } FlycheckConfig::CustomCommand { command, @@ -352,30 +366,24 @@ impl FlycheckActor { } => { let mut cmd = Command::new(command); cmd.envs(extra_env); + args.iter().for_each(|arg| { + match invocation_strategy { + InvocationStrategy::Once => cmd.arg(arg), + InvocationStrategy::PerWorkspace => cmd.arg(arg.replace( + "$manifest_path", + &self.project_file.as_os_str().to_string_lossy(), + )), + }; + }); 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); - } - } - } - InvocationLocation::Root(root) => { - cmd.current_dir(root); - } - } + InvocationLocation::Workspace => cmd.current_dir(&self.root), + InvocationLocation::Root(root) => cmd.current_dir(root), + }; - (cmd, args) + cmd } - }; - - cmd.args(args); - cmd + } } fn send(&self, check_task: Message) { diff --git a/crates/paths/src/lib.rs b/crates/paths/src/lib.rs index 88b8d0aee3a4..6d5df7e4d2d4 100644 --- a/crates/paths/src/lib.rs +++ b/crates/paths/src/lib.rs @@ -231,6 +231,9 @@ impl AbsPath { pub fn exists(&self) -> bool { self.0.exists() } + pub fn with_extension>(&self, extension: S) -> AbsPathBuf { + AbsPathBuf::try_from(self.0.with_extension(extension)).unwrap() + } // endregion:delegate-methods } diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs index fb0f3ab7d174..c9f9bd9520b2 100644 --- a/crates/project-model/src/build_scripts.rs +++ b/crates/project-model/src/build_scripts.rs @@ -60,11 +60,21 @@ impl WorkspaceBuildScripts { fn build_command( config: &CargoConfig, allowed_features: &FxHashSet, + manifest_path: Option, ) -> io::Result { let mut cmd = match config.run_build_script_command.as_deref() { Some([program, args @ ..]) => { let mut cmd = Command::new(program); - cmd.args(args); + for arg in args { + if let Some(manifest_path) = &manifest_path { + cmd.arg(arg.replace( + "$manifest_path", + &manifest_path.as_os_str().to_string_lossy(), + )); + } else { + cmd.arg(arg); + } + } cmd } _ => { @@ -139,7 +149,11 @@ impl WorkspaceBuildScripts { let allowed_features = workspace.workspace_features(); match Self::run_per_ws( - Self::build_command(config, &allowed_features)?, + Self::build_command( + config, + &allowed_features, + Some(workspace.workspace_root().join("Cargo.toml")), + )?, workspace, current_dir, progress, @@ -149,8 +163,12 @@ impl WorkspaceBuildScripts { { // building build scripts failed, attempt to build with --keep-going so // that we potentially get more build data - let mut cmd = Self::build_command(config, &allowed_features)?; - cmd.args(["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1"); + let mut cmd = Self::build_command( + config, + &allowed_features, + Some(workspace.workspace_root().join("Cargo.toml")), + )?; + cmd.args(&["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1"); let mut res = Self::run_per_ws(cmd, workspace, current_dir, progress)?; res.error = Some(error); Ok(res) @@ -177,7 +195,7 @@ impl WorkspaceBuildScripts { )) } }; - let cmd = Self::build_command(config, &Default::default())?; + let cmd = Self::build_command(config, &Default::default(), None)?; // NB: Cargo.toml could have been modified between `cargo metadata` and // `cargo check`. We shouldn't assume that package ids we see here are // exactly those from `config`. diff --git a/crates/project-model/src/project_json.rs b/crates/project-model/src/project_json.rs index 80897f7478cf..e9e5139ea4de 100644 --- a/crates/project-model/src/project_json.rs +++ b/crates/project-model/src/project_json.rs @@ -65,6 +65,7 @@ pub struct ProjectJson { pub(crate) sysroot: Option, /// e.g. `path/to/sysroot/lib/rustlib/src/rust` pub(crate) sysroot_src: Option, + /// the folder containing `rust-project.json`) project_root: AbsPathBuf, crates: Vec, } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index af2a8436efb9..409601dde694 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -81,8 +81,10 @@ config_data! { /// is set. cargo_buildScripts_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. + /// - `per_workspace`: Executes the command for each workspace separately. + /// Allows the use of the `$manifest_path` substitution variable in the override command + /// which will be replaced by the path of the workspace's Cargo.toml. + /// - `once`: Executes the command once. /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#` /// is set. cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"", @@ -164,8 +166,10 @@ config_data! { /// is set. check_invocationLocation | checkOnSave_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. + /// - `per_workspace`: Executes the command for each workspace separately. + /// Allows the use of the `$manifest_path` substitution variable in the override command + /// which will be replaced by the path of the workspace's Cargo.toml or rust-project.json. + /// - `once`: Executes the command once. /// This config only has an effect when `#rust-analyzer.cargo.check.overrideCommand#` /// is set. check_invocationStrategy | checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"", diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index f9070d273539..1bc3798849d4 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -10,7 +10,7 @@ use lsp_types::{ DidOpenTextDocumentParams, DidSaveTextDocumentParams, WorkDoneProgressCancelParams, }; use triomphe::Arc; -use vfs::{AbsPathBuf, ChangeKind, VfsPath}; +use vfs::{AbsPath, AbsPathBuf, ChangeKind, VfsPath}; use crate::{ config::Config, @@ -132,7 +132,8 @@ pub(crate) fn handle_did_save_text_document( ) -> anyhow::Result<()> { if let Ok(vfs_path) = from_proto::vfs_path(¶ms.text_document.uri) { // Re-fetch workspaces if a workspace related file has changed - if let Some(abs_path) = vfs_path.as_path() { + let abs_path = vfs_path.as_path(); + if let Some(abs_path) = abs_path { if reload::should_refresh_for_change(abs_path, ChangeKind::Modify) { state .fetch_workspaces_queue @@ -140,13 +141,13 @@ pub(crate) fn handle_did_save_text_document( } } - if !state.config.check_on_save() || run_flycheck(state, vfs_path) { + if !state.config.check_on_save() || run_flycheck(state, &vfs_path, abs_path) { return Ok(()); } } else if state.config.check_on_save() { // No specific flycheck was triggered, so let's trigger all of them. for flycheck in state.flycheck.iter() { - flycheck.restart(); + flycheck.restart(None); } } Ok(()) @@ -232,10 +233,11 @@ pub(crate) fn handle_did_change_watched_files( Ok(()) } -fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool { +fn run_flycheck(state: &mut GlobalState, vfs_path: &VfsPath, abs_path: Option<&AbsPath>) -> bool { let _p = profile::span("run_flycheck"); - let file_id = state.vfs.read().0.file_id(&vfs_path); + let file_id = state.vfs.read().0.file_id(vfs_path); + let abs_path = abs_path.map(AbsPath::to_owned); if let Some(file_id) = file_id { let world = state.snapshot(); let mut updated = false; @@ -287,7 +289,7 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool { for (id, _) in workspace_ids.clone() { if id == flycheck.id() { updated = true; - flycheck.restart(); + flycheck.restart(abs_path.clone()); continue; } } @@ -295,7 +297,7 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool { // No specific flycheck was triggered, so let's trigger all of them. if !updated { for flycheck in world.flycheck.iter() { - flycheck.restart(); + flycheck.restart(abs_path.clone()); } } Ok(()) @@ -330,14 +332,14 @@ pub(crate) fn handle_run_flycheck( let _p = profile::span("handle_run_flycheck"); 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) { + if run_flycheck(state, &vfs_path, vfs_path.as_path()) { return Ok(()); } } } // No specific flycheck was triggered, so let's trigger all of them. for flycheck in state.flycheck.iter() { - flycheck.restart(); + flycheck.restart(None); } Ok(()) } diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index cdf41c955d26..5f1382e31f92 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -7,7 +7,6 @@ use std::{ use always_assert::always; use crossbeam_channel::{select, Receiver}; -use flycheck::FlycheckHandle; use ide_db::base_db::{SourceDatabaseExt, VfsPath}; use lsp_server::{Connection, Notification, Request}; use lsp_types::notification::Notification as _; @@ -299,7 +298,7 @@ impl GlobalState { if became_quiescent { if self.config.check_on_save() { // Project has loaded properly, kick off initial flycheck - self.flycheck.iter().for_each(FlycheckHandle::restart); + self.flycheck.iter().for_each(|flycheck| flycheck.restart(None)); } if self.config.prefill_caches() { self.prime_caches_queue.request_op("became quiescent".to_string(), ()); diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 3fae08b82e27..c34c2e0ae574 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -590,30 +590,39 @@ impl GlobalState { Box::new(move |msg| sender.send(msg).unwrap()), config, self.config.root_path().clone(), + self.config.root_path().clone(), )], flycheck::InvocationStrategy::PerWorkspace => { self.workspaces .iter() .enumerate() .filter_map(|(id, w)| match w { - ProjectWorkspace::Cargo { cargo, .. } => Some((id, cargo.workspace_root())), + ProjectWorkspace::Cargo { cargo, .. } => { + Some((id, cargo.workspace_root(), true)) + } ProjectWorkspace::Json { project, .. } => { // Enable flychecks for json projects if a custom flycheck command was supplied // in the workspace configuration. match config { - FlycheckConfig::CustomCommand { .. } => Some((id, project.path())), + FlycheckConfig::CustomCommand { .. } => { + Some((id, project.path(), false)) + } _ => None, } } ProjectWorkspace::DetachedFiles { .. } => None, }) - .map(|(id, root)| { + .map(|(id, root, cargo_project)| { let sender = sender.clone(); FlycheckHandle::spawn( id, Box::new(move |msg| sender.send(msg).unwrap()), config.clone(), root.to_path_buf(), + match cargo_project { + true => root.join("Cargo.toml"), + false => root.join("rust-project.json"), + }, ) }) .collect() diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 71feed0f72ca..e669b4edf423 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -44,8 +44,10 @@ is set. + -- 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. +- `per_workspace`: Executes the command for each workspace separately. + Allows the use of the `$manifest_path` substitution variable in the override command + which will be replaced by the path of the workspace's Cargo.toml. +- `once`: Executes the command once. This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#` is set. -- @@ -182,8 +184,10 @@ is set. + -- 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. +- `per_workspace`: Executes the command for each workspace separately. + Allows the use of the `$manifest_path` substitution variable in the override command + which will be replaced by the path of the workspace's Cargo.toml or rust-project.json. +- `once`: Executes the command once. This config only has an effect when `#rust-analyzer.cargo.check.overrideCommand#` is set. -- diff --git a/editors/code/package.json b/editors/code/package.json index 233e7bf44b16..746234776e47 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -544,7 +544,7 @@ ] }, "rust-analyzer.cargo.buildScripts.invocationStrategy": { - "markdownDescription": "Specifies the invocation strategy to use when running the build scripts command.\nIf `per_workspace` is set, the command will be executed for each workspace.\nIf `once` is set, the command will be executed once.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.", + "markdownDescription": "Specifies the invocation strategy to use when running the build scripts command.\n- `per_workspace`: Executes the command for each workspace separately.\n Allows the use of the `$manifest_path` substitution variable in the override command\n which will be replaced by the path of the workspace's Cargo.toml.\n- `once`: Executes the command once.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.", "default": "per_workspace", "type": "string", "enum": [ @@ -725,7 +725,7 @@ ] }, "rust-analyzer.check.invocationStrategy": { - "markdownDescription": "Specifies the invocation strategy to use when running the check command.\nIf `per_workspace` is set, the command will be executed for each workspace.\nIf `once` is set, the command will be executed once.\nThis config only has an effect when `#rust-analyzer.cargo.check.overrideCommand#`\nis set.", + "markdownDescription": "Specifies the invocation strategy to use when running the check command.\n- `per_workspace`: Executes the command for each workspace separately.\n Allows the use of the `$manifest_path` substitution variable in the override command\n which will be replaced by the path of the workspace's Cargo.toml or rust-project.json.\n- `once`: Executes the command once.\nThis config only has an effect when `#rust-analyzer.cargo.check.overrideCommand#`\nis set.", "default": "per_workspace", "type": "string", "enum": [