Skip to content

Commit

Permalink
Remove simplistic interpolation for manifest-path
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Oct 19, 2022
1 parent 7db5029 commit 4673236
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 87 deletions.
26 changes: 5 additions & 21 deletions crates/flycheck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub use cargo_metadata::diagnostic::{

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationStrategy {
OnceInRoot,
Once,
#[default]
PerWorkspace,
}
Expand Down Expand Up @@ -317,26 +317,10 @@ impl FlycheckActor {
(cmd, args, invocation_strategy)
}
};
if let InvocationStrategy::PerWorkspace = invocation_strategy {
let mut with_manifest_path = false;
for arg in args {
if let Some(_) = arg.find("$manifest_path") {
with_manifest_path = true;
cmd.arg(arg.replace(
"$manifest_path",
&self.root.join("Cargo.toml").display().to_string(),
));
} else {
cmd.arg(arg);
}
}

if !with_manifest_path {
cmd.current_dir(&self.root);
}
} else {
cmd.args(args);
}
match invocation_strategy {
InvocationStrategy::PerWorkspace => cmd.current_dir(&self.root),
InvocationStrategy::Once => cmd.args(args),
};
cmd
}

Expand Down
27 changes: 2 additions & 25 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,30 +62,7 @@ impl WorkspaceBuildScripts {
let mut cmd = match config.run_build_script_command.as_deref() {
Some([program, args @ ..]) => {
let mut cmd = Command::new(program);

// FIXME: strategy and workspace root are coupled, express that in code
if let (InvocationStrategy::PerWorkspace, Some(workspace_root)) =
(config.invocation_strategy, workspace_root)
{
let mut with_manifest_path = false;
for arg in args {
if let Some(_) = arg.find("$manifest_path") {
with_manifest_path = true;
cmd.arg(arg.replace(
"$manifest_path",
&workspace_root.join("Cargo.toml").display().to_string(),
));
} else {
cmd.arg(arg);
}
}

if !with_manifest_path {
cmd.current_dir(workspace_root);
}
} else {
cmd.args(args);
}
cmd.args(args);
cmd
}
_ => {
Expand Down Expand Up @@ -176,7 +153,7 @@ impl WorkspaceBuildScripts {
workspaces: &[&CargoWorkspace],
progress: &dyn Fn(String),
) -> io::Result<Vec<WorkspaceBuildScripts>> {
assert_eq!(config.invocation_strategy, InvocationStrategy::OnceInRoot);
assert_eq!(config.invocation_strategy, InvocationStrategy::Once);
let cmd = Self::build_command(config, 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
Expand Down
2 changes: 1 addition & 1 deletion crates/project-model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ fn utf8_stdout(mut cmd: Command) -> Result<String> {

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationStrategy {
OnceInRoot,
Once,
#[default]
PerWorkspace,
}
30 changes: 11 additions & 19 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,9 @@ config_data! {
/// Run build scripts (`build.rs`) for more precise code analysis.
cargo_buildScripts_enable: bool = "true",
/// 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 and all
/// occurrences of `$manifest_path` in the command will be replaced by the corresponding
/// manifest path of the workspace that the command is being invoked for. If interpolation
/// for the manifest path happens at least once, the commands will be executed from the
/// project root, otherwise the commands will be executed from the corresponding workspace
/// root.
/// If `once_in_root` is set, the command will be executed once in the project root.
/// If `per_workspace` is set, the command will be executed for each workspace from the
/// corresponding workspace root.
/// If `once` is set, the command will be executed once in the project root.
/// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
/// is set.
cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
Expand Down Expand Up @@ -134,13 +130,9 @@ config_data! {
/// Set to `"all"` to pass `--all-features` to Cargo.
checkOnSave_features: Option<CargoFeaturesDef> = "null",
/// Specifies the invocation strategy to use when running the checkOnSave command.
/// If `per_workspace` is set, the command will be executed for each workspace and all
/// occurrences of `$manifest_path` in the command will be replaced by the corresponding
/// manifest path of the workspace that the command is being invoked for. If interpolation
/// for the manifest path happens at least once, the commands will be executed from the
/// project root, otherwise the commands will be executed from the corresponding workspace
/// root.
/// If `once_in_root` is set, the command will be executed once in the project root.
/// If `per_workspace` is set, the command will be executed for each workspace from the
/// corresponding workspace root.
/// If `once` is set, the command will be executed once in the project root.
/// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
/// is set.
checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
Expand Down Expand Up @@ -1079,7 +1071,7 @@ impl Config {
unset_test_crates: UnsetTestCrates::Only(self.data.cargo_unsetTest.clone()),
wrap_rustc_in_build_scripts: self.data.cargo_buildScripts_useRustcWrapper,
invocation_strategy: match self.data.cargo_buildScripts_invocationStrategy {
InvocationStrategy::OnceInRoot => project_model::InvocationStrategy::OnceInRoot,
InvocationStrategy::Once => project_model::InvocationStrategy::Once,
InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace,
},
run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(),
Expand All @@ -1106,7 +1098,7 @@ impl Config {
return None;
}
let invocation_strategy = match self.data.checkOnSave_invocationStrategy {
InvocationStrategy::OnceInRoot => flycheck::InvocationStrategy::OnceInRoot,
InvocationStrategy::Once => flycheck::InvocationStrategy::Once,
InvocationStrategy::PerWorkspace => flycheck::InvocationStrategy::PerWorkspace,
};
let flycheck_config = match &self.data.checkOnSave_overrideCommand {
Expand Down Expand Up @@ -1622,7 +1614,7 @@ enum CargoFeaturesDef {
#[derive(Deserialize, Debug, Clone)]
#[serde(rename_all = "snake_case")]
enum InvocationStrategy {
OnceInRoot,
Once,
PerWorkspace,
}

Expand Down Expand Up @@ -2042,9 +2034,9 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
},
"InvocationStrategy" => set! {
"type": "string",
"enum": ["per_workspace", "once_in_root"],
"enum": ["per_workspace", "once"],
"enumDescriptions": [
"The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.",
"The command will be executed for each workspace from the corresponding workspace root.",
"The command will be executed once in the project root."
],
},
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ impl GlobalState {
| FlycheckConfig::CustomCommand { invocation_strategy, .. }) = config;

self.flycheck = match invocation_strategy {
flycheck::InvocationStrategy::OnceInRoot => vec![FlycheckHandle::spawn(
flycheck::InvocationStrategy::Once => vec![FlycheckHandle::spawn(
0,
Box::new(move |msg| sender.send(msg).unwrap()),
config.clone(),
Expand Down
20 changes: 6 additions & 14 deletions docs/user/generated_config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,9 @@ Run build scripts (`build.rs`) for more precise code analysis.
+
--
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 and all
occurrences of `$manifest_path` in the command will be replaced by the corresponding
manifest path of the workspace that the command is being invoked for. If interpolation
for the manifest path happens at least once, the commands will be executed from the
project root, otherwise the commands will be executed from the corresponding workspace
root.
If `once_in_root` is set, the command will be executed once in the project root.
If `per_workspace` is set, the command will be executed for each workspace from the
corresponding workspace root.
If `once` is set, the command will be executed once in the project root.
This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
is set.
--
Expand Down Expand Up @@ -136,13 +132,9 @@ Set to `"all"` to pass `--all-features` to Cargo.
+
--
Specifies the invocation strategy to use when running the checkOnSave command.
If `per_workspace` is set, the command will be executed for each workspace and all
occurrences of `$manifest_path` in the command will be replaced by the corresponding
manifest path of the workspace that the command is being invoked for. If interpolation
for the manifest path happens at least once, the commands will be executed from the
project root, otherwise the commands will be executed from the corresponding workspace
root.
If `once_in_root` is set, the command will be executed once in the project root.
If `per_workspace` is set, the command will be executed for each workspace from the
corresponding workspace root.
If `once` is set, the command will be executed once in the project root.
This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
is set.
--
Expand Down
12 changes: 6 additions & 6 deletions editors/code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -422,15 +422,15 @@
"type": "boolean"
},
"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 and all\noccurrences of `$manifest_path` in the command will be replaced by the corresponding\nmanifest path of the workspace that the command is being invoked for. If interpolation\nfor the manifest path happens at least once, the commands will be executed from the\nproject root, otherwise the commands will be executed from the corresponding workspace\nroot.\nIf `once_in_root` is set, the command will be executed once in the project root.\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.\nIf `per_workspace` is set, the command will be executed for each workspace from the\ncorresponding workspace root.\nIf `once` is set, the command will be executed once in the project root.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.",
"default": "per_workspace",
"type": "string",
"enum": [
"per_workspace",
"once_in_root"
"once"
],
"enumDescriptions": [
"The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.",
"The command will be executed for each workspace from the corresponding workspace root.",
"The command will be executed once in the project root."
]
},
Expand Down Expand Up @@ -560,15 +560,15 @@
]
},
"rust-analyzer.checkOnSave.invocationStrategy": {
"markdownDescription": "Specifies the invocation strategy to use when running the checkOnSave command.\nIf `per_workspace` is set, the command will be executed for each workspace and all\noccurrences of `$manifest_path` in the command will be replaced by the corresponding\nmanifest path of the workspace that the command is being invoked for. If interpolation\nfor the manifest path happens at least once, the commands will be executed from the\nproject root, otherwise the commands will be executed from the corresponding workspace\nroot.\nIf `once_in_root` is set, the command will be executed once in the project root.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.",
"markdownDescription": "Specifies the invocation strategy to use when running the checkOnSave command.\nIf `per_workspace` is set, the command will be executed for each workspace from the\ncorresponding workspace root.\nIf `once` is set, the command will be executed once in the project root.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.",
"default": "per_workspace",
"type": "string",
"enum": [
"per_workspace",
"once_in_root"
"once"
],
"enumDescriptions": [
"The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.",
"The command will be executed for each workspace from the corresponding workspace root.",
"The command will be executed once in the project root."
]
},
Expand Down

0 comments on commit 4673236

Please sign in to comment.