mirror of
https://github.com/j178/prek.git
synced 2026-04-25 02:11:36 +02:00
Validate shell support by language
This commit is contained in:
+44
-1
@@ -266,6 +266,7 @@ impl HookBuilder {
|
||||
let HookOptions {
|
||||
language_version,
|
||||
additional_dependencies,
|
||||
shell,
|
||||
..
|
||||
} = &self.hook_spec.options;
|
||||
|
||||
@@ -310,6 +311,37 @@ impl HookBuilder {
|
||||
}
|
||||
}
|
||||
|
||||
if shell.is_some() {
|
||||
match self.repo.as_ref() {
|
||||
Repo::Meta { .. } => {
|
||||
return Err(Error::Hook {
|
||||
hook: self.hook_spec.id.clone(),
|
||||
error: anyhow::anyhow!(
|
||||
"Hook specified `shell` but meta hooks do not support shell execution",
|
||||
),
|
||||
});
|
||||
}
|
||||
Repo::Builtin { .. } => {
|
||||
return Err(Error::Hook {
|
||||
hook: self.hook_spec.id.clone(),
|
||||
error: anyhow::anyhow!(
|
||||
"Hook specified `shell` but builtin hooks do not support shell execution",
|
||||
),
|
||||
});
|
||||
}
|
||||
Repo::Remote { .. } | Repo::Local { .. } => {}
|
||||
}
|
||||
|
||||
if !language.supports_shell() {
|
||||
return Err(Error::Hook {
|
||||
hook: self.hook_spec.id.clone(),
|
||||
error: anyhow::anyhow!(
|
||||
"Hook specified `shell` but the language `{language}` does not support shell execution",
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -502,6 +534,13 @@ impl HookEntry {
|
||||
|
||||
/// Split the entry into a list of commands.
|
||||
pub(crate) fn split(&self) -> Result<Vec<String>, Error> {
|
||||
if let Some(shell) = self.shell {
|
||||
panic!(
|
||||
"internal error: attempted to split shell entry `{}` using {shell:?}",
|
||||
self.hook
|
||||
);
|
||||
}
|
||||
|
||||
let splits = shlex::split(&self.entry).ok_or_else(|| Error::Hook {
|
||||
hook: self.hook.clone(),
|
||||
error: anyhow::anyhow!("Failed to parse entry `{}` as commands", &self.entry),
|
||||
@@ -519,6 +558,10 @@ impl HookEntry {
|
||||
pub(crate) fn raw(&self) -> &str {
|
||||
&self.entry
|
||||
}
|
||||
|
||||
pub(crate) fn shell(&self) -> Option<Shell> {
|
||||
self.shell
|
||||
}
|
||||
}
|
||||
|
||||
impl Shell {
|
||||
@@ -997,7 +1040,7 @@ mod tests {
|
||||
use crate::languages::version::LanguageRequest;
|
||||
use crate::workspace::Project;
|
||||
|
||||
use super::{Hook, HookBuilder, Repo, bash_argv, cmd_argv, powershell_argv};
|
||||
use super::{Hook, HookBuilder, Repo};
|
||||
|
||||
#[tokio::test]
|
||||
async fn hook_builder_build_fills_and_merges_attributes() -> Result<()> {
|
||||
|
||||
@@ -129,6 +129,24 @@ impl LanguageImpl for Unimplemented {
|
||||
// system: only system version, no env, no additional deps
|
||||
|
||||
impl Language {
|
||||
pub(crate) fn supports_shell(self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::Bun
|
||||
| Self::Deno
|
||||
| Self::Dotnet
|
||||
| Self::Golang
|
||||
| Self::Haskell
|
||||
| Self::Lua
|
||||
| Self::Node
|
||||
| Self::Python
|
||||
| Self::Ruby
|
||||
| Self::Script
|
||||
| Self::Swift
|
||||
| Self::System
|
||||
)
|
||||
}
|
||||
|
||||
pub fn supported(lang: Language) -> bool {
|
||||
matches!(
|
||||
lang,
|
||||
|
||||
@@ -256,6 +256,13 @@ impl ScriptTag {
|
||||
/// Effectively, we are implementing a new `python-script` language which works like `script`.
|
||||
/// But we don't want to introduce a new language just for this for now.
|
||||
pub(crate) async fn extract_pep723_metadata(hook: &mut Hook) -> Result<()> {
|
||||
if hook.entry.shell().is_some() {
|
||||
trace!(
|
||||
"Skipping reading PEP 723 metadata for hook `{hook}` because `shell` treats `entry` as shell source",
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
if !hook.additional_dependencies.is_empty() {
|
||||
trace!(
|
||||
"Skipping reading PEP 723 metadata for hook `{hook}` because it already has `additional_dependencies`",
|
||||
|
||||
@@ -19,6 +19,7 @@ mod python;
|
||||
mod ruby;
|
||||
mod rust;
|
||||
mod script;
|
||||
mod shell;
|
||||
mod swift;
|
||||
mod system;
|
||||
mod unimplemented;
|
||||
|
||||
@@ -0,0 +1,190 @@
|
||||
use assert_fs::fixture::{FileWriteStr, PathChild};
|
||||
|
||||
use crate::common::{TestContext, cmd_snapshot};
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn bash_shell_adapter_runs_entry() -> anyhow::Result<()> {
|
||||
let context = TestContext::new();
|
||||
context.init_project();
|
||||
context.write_pre_commit_config(indoc::indoc! {r#"
|
||||
repos:
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: bash-shell
|
||||
name: bash-shell
|
||||
language: system
|
||||
files: ^input\.txt$
|
||||
shell: bash
|
||||
entry: |
|
||||
items=("$@")
|
||||
printf 'bash:%s:%s\n' "${items[0]}" "${items[1]}"
|
||||
args: [configured]
|
||||
verbose: true
|
||||
"#});
|
||||
context.work_dir().child("input.txt").write_str("input")?;
|
||||
context.git_add(".");
|
||||
|
||||
cmd_snapshot!(context.filters(), context.run(), @r"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
bash-shell...............................................................Passed
|
||||
- hook id: bash-shell
|
||||
- duration: [TIME]
|
||||
|
||||
bash:configured:input.txt
|
||||
|
||||
----- stderr -----
|
||||
");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pwsh_shell_adapter_runs_entry() -> anyhow::Result<()> {
|
||||
if which::which("pwsh").is_err() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let context = TestContext::new();
|
||||
context.init_project();
|
||||
context.write_pre_commit_config(indoc::indoc! {r#"
|
||||
repos:
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: pwsh-shell
|
||||
name: pwsh-shell
|
||||
language: system
|
||||
files: ^input\.txt$
|
||||
shell: pwsh
|
||||
entry: |
|
||||
Write-Output "pwsh:$($args[0]):$($args[1])"
|
||||
args: [configured]
|
||||
verbose: true
|
||||
"#});
|
||||
context.work_dir().child("input.txt").write_str("input")?;
|
||||
context.git_add(".");
|
||||
|
||||
cmd_snapshot!(context.filters(), context.run(), @r"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
pwsh-shell...............................................................Passed
|
||||
- hook id: pwsh-shell
|
||||
- duration: [TIME]
|
||||
|
||||
pwsh:configured:input.txt
|
||||
|
||||
----- stderr -----
|
||||
");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn powershell_shell_adapter_runs_entry() -> anyhow::Result<()> {
|
||||
let context = TestContext::new();
|
||||
context.init_project();
|
||||
context.write_pre_commit_config(indoc::indoc! {r#"
|
||||
repos:
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: powershell-shell
|
||||
name: powershell-shell
|
||||
language: system
|
||||
files: ^input\.txt$
|
||||
shell: powershell
|
||||
entry: |
|
||||
Write-Output "powershell:$($args[0]):$($args[1])"
|
||||
args: [configured]
|
||||
verbose: true
|
||||
"#});
|
||||
context.work_dir().child("input.txt").write_str("input")?;
|
||||
context.git_add(".");
|
||||
|
||||
cmd_snapshot!(context.filters(), context.run(), @r"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
powershell-shell.........................................................Passed
|
||||
- hook id: powershell-shell
|
||||
- duration: [TIME]
|
||||
|
||||
powershell:configured:input.txt
|
||||
|
||||
----- stderr -----
|
||||
");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn cmd_shell_adapter_runs_entry() -> anyhow::Result<()> {
|
||||
let context = TestContext::new();
|
||||
context.init_project();
|
||||
context.write_pre_commit_config(indoc::indoc! {r"
|
||||
repos:
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: cmd-shell
|
||||
name: cmd-shell
|
||||
language: system
|
||||
files: ^input\.txt$
|
||||
shell: cmd
|
||||
entry: |
|
||||
@echo off
|
||||
echo cmd:%1:%2
|
||||
args: [configured]
|
||||
verbose: true
|
||||
"});
|
||||
context.work_dir().child("input.txt").write_str("input")?;
|
||||
context.git_add(".");
|
||||
|
||||
cmd_snapshot!(context.filters(), context.run(), @r"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
cmd-shell................................................................Passed
|
||||
- hook id: cmd-shell
|
||||
- duration: [TIME]
|
||||
|
||||
cmd:configured:input.txt
|
||||
|
||||
----- stderr -----
|
||||
");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_rejected_for_pygrep() {
|
||||
let context = TestContext::new();
|
||||
context.init_project();
|
||||
context.write_pre_commit_config(indoc::indoc! {r"
|
||||
repos:
|
||||
- repo: local
|
||||
hooks:
|
||||
- id: check-todo
|
||||
name: check-todo
|
||||
language: pygrep
|
||||
entry: TODO
|
||||
shell: sh
|
||||
always_run: true
|
||||
pass_filenames: false
|
||||
"});
|
||||
context.git_add(".");
|
||||
|
||||
cmd_snapshot!(context.filters(), context.run(), @r"
|
||||
success: false
|
||||
exit_code: 2
|
||||
----- stdout -----
|
||||
|
||||
----- stderr -----
|
||||
error: Failed to init hooks
|
||||
caused by: Invalid hook `check-todo`
|
||||
caused by: Hook specified `shell` but the language `pygrep` does not support shell execution
|
||||
");
|
||||
}
|
||||
@@ -58,8 +58,9 @@ manifest semantics. For the upstream reference, see:
|
||||
|
||||
When `shell` is set, `entry` is treated as shell source. Hook `args` and
|
||||
filenames are passed as script arguments, so POSIX shell entries should read
|
||||
them with `"$@"`. See [`shell`](configuration.md#shell) for the exact shell
|
||||
adapter commands.
|
||||
them with `"$@"`. `shell` is supported only for language backends that use
|
||||
the shell-aware entry resolver; see [`shell`](configuration.md#shell) for
|
||||
the supported languages and exact shell adapter commands.
|
||||
|
||||
!!! note "Manifest fields only"
|
||||
|
||||
|
||||
@@ -871,11 +871,14 @@ Run `entry` through a predefined shell adapter.
|
||||
- Type: one of `sh`, `bash`, `pwsh`, `powershell`, `cmd`
|
||||
- Default: `null` (run `entry` directly without a shell)
|
||||
- `prek`-only
|
||||
- Supported for `bun`, `deno`, `dotnet`, `golang`, `haskell`, `lua`, `node`, `python`, `ruby`, `script`, `swift`, and `system` hooks.
|
||||
|
||||
When `shell` is omitted, `prek` preserves the default no-shell behavior: it parses `entry` into argv, invokes the command directly, and appends `args` and matching filenames as process arguments.
|
||||
|
||||
When `shell` is set, `entry` is treated as source for that shell. `prek` writes the source to a temporary script file, runs it with the selected shell adapter, and passes hook `args` followed by matching filenames as script arguments.
|
||||
|
||||
`shell` is rejected for language backends that do not run `entry` through the shell-aware entry resolver, including `docker`, `docker_image`, `fail`, `julia`, `pygrep`, and `rust`, and for `repo: meta` and `repo: builtin` hooks.
|
||||
|
||||
| `shell` | Adapter command | Script arguments |
|
||||
| -- | -- | -- |
|
||||
| `bash` | `bash --noprofile --norc -eo pipefail <script>` | `"$@"` |
|
||||
|
||||
@@ -138,7 +138,7 @@ available through `%*`.
|
||||
|
||||
## Language Interaction
|
||||
|
||||
Most language backends already share the same shape:
|
||||
Supported language backends already share the same shape:
|
||||
|
||||
1. Resolve the hook entry.
|
||||
2. Create a process.
|
||||
@@ -149,6 +149,12 @@ The new hook entry abstraction should provide a single way to build the concrete
|
||||
argv for a batch, so managed and unmanaged language backends can opt into shell
|
||||
execution consistently.
|
||||
|
||||
Backends that still treat `entry` as language-specific data or parse it outside
|
||||
the shell-aware resolver should reject `shell` during validation instead of
|
||||
silently ignoring it. This includes `docker`, `docker_image`, `fail`, `julia`,
|
||||
`pygrep`, `rust`, and predefined `repo: meta` and `repo: builtin` hooks in the
|
||||
initial implementation.
|
||||
|
||||
`language: script` needs one special rule: when `shell` is unset, the first
|
||||
`entry` token remains a repository-relative script path, matching existing
|
||||
behavior. When `shell` is set, `entry` is shell source and no repository-relative
|
||||
|
||||
Reference in New Issue
Block a user