diff --git a/CHANGELOG.md b/CHANGELOG.md index f72308fd0..311c6ff32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog - **Fixed** Running `vp run` in a package that sits inside a workspace folder but isn't listed in its `pnpm-workspace.yaml` `packages` or `package.json` `workspaces` now runs that package's own tasks, instead of treating the enclosing workspace as its root. When both configurations exist, `pnpm-workspace.yaml` takes precedence ([#859](https://github.com/voidzero-dev/vite-task/pull/859)). -- **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). +- **Fixed** A mistyped or misplaced setting in a task or in the root `cache` setting now gets an error that names it, instead of `data did not match any variant of untagged enum ...`. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, and an `input` entry of `{ pattern: "src/**" }` fails with ``missing field `base` `` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815), [#864](https://github.com/voidzero-dev/vite-task/pull/864)). - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p ` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)). - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)). - **Fixed** On Linux, concurrent system calls in tracked programs, such as `pnpm dlx`, no longer cause a stack overflow ([#854](https://github.com/voidzero-dev/vite-task/pull/854)). diff --git a/crates/vt_graph/src/config/mod.rs b/crates/vt_graph/src/config/mod.rs index 967ef82a1..b2fcc42d7 100644 --- a/crates/vt_graph/src/config/mod.rs +++ b/crates/vt_graph/src/config/mod.rs @@ -5,9 +5,9 @@ use std::{collections::BTreeSet, sync::Arc}; use rustc_hash::FxHashSet; use serde::Serialize; pub use user::{ - AutoTracking, Command, EnabledCacheConfig, GlobWithBase, InputBase, ResolvedGlobalCacheConfig, - UserCacheConfig, UserDependencyType, UserDependsOnEntry, UserDependsOnFrom, - UserGlobalCacheConfig, UserInputEntry, UserInputsConfig, UserOutputEntry, + AutoTracking, Command, DetailedGlobalCacheConfig, EnabledCacheConfig, GlobWithBase, InputBase, + ResolvedGlobalCacheConfig, UserCacheConfig, UserDependencyType, UserDependsOnEntry, + UserDependsOnFrom, UserGlobalCacheConfig, UserInputEntry, UserInputsConfig, UserOutputEntry, UserPackageDependency, UserRunConfig, UserTaskConfig, UserTaskDefinition, }; use vt_path::AbsolutePath; diff --git a/crates/vt_graph/src/config/user.rs b/crates/vt_graph/src/config/user.rs index 92f00a2ef..7fa7cb392 100644 --- a/crates/vt_graph/src/config/user.rs +++ b/crates/vt_graph/src/config/user.rs @@ -1,9 +1,16 @@ //! Configuration structures for user-defined tasks in `vite.config.*` +//! +//! Enums that accept more than one JSON shape implement `Deserialize` with +//! [`UntaggedEnumVisitor`] instead of `#[serde(untagged)]`, which replaces each variant's +//! error with a generic one. use std::sync::Arc; use rustc_hash::FxHashMap; -use serde::{Deserialize, Deserializer}; +use serde::{ + Deserialize, Deserializer, + de::{self, IntoDeserializer as _}, +}; use serde_untagged::UntaggedEnumVisitor; #[cfg(all(test, not(clippy)))] use ts_rs::TS; @@ -28,9 +35,8 @@ pub enum InputBase { } /// Glob pattern with explicit base directory for resolution. -#[derive(Debug, Deserialize, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] #[cfg_attr(all(test, not(clippy)), derive(TS))] -#[serde(deny_unknown_fields)] pub struct GlobWithBase { /// The glob pattern (positive or negative starting with `!`) pub pattern: Str, @@ -39,24 +45,67 @@ pub struct GlobWithBase { } /// Automatic file-tracking directive for input fingerprinting or output archiving. -#[derive(Debug, Deserialize, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] #[cfg_attr(all(test, not(clippy)), derive(TS))] -#[serde(deny_unknown_fields)] pub struct AutoTracking { /// Enable automatic file tracking for this input or output list. pub auto: bool, } +/// Deserializes an input or output entry: a glob string, `{ pattern, base }`, or `{ auto }`. +fn deserialize_glob_entry<'de, D: Deserializer<'de>, T>( + deserializer: D, + glob: fn(Str) -> T, + glob_with_base: fn(GlobWithBase) -> T, + auto: fn(AutoTracking) -> T, +) -> Result { + /// Both object forms in one struct, so a mistyped key is reported with every accepted key. + /// A field is `None` only when it's absent, so `null` is rejected as it is in either form. + #[derive(Deserialize)] + #[serde(deny_unknown_fields)] + struct EntryObject { + #[serde(default, deserialize_with = "present")] + pattern: Option, + #[serde(default, deserialize_with = "present")] + base: Option, + #[serde(default, deserialize_with = "present")] + auto: Option, + } + + fn present<'de, D: Deserializer<'de>, V: Deserialize<'de>>( + deserializer: D, + ) -> Result, D::Error> { + V::deserialize(deserializer).map(Some) + } + + UntaggedEnumVisitor::new() + .expecting("a glob pattern, `{ pattern, base }`, or `{ auto: true }`") + .string(|pattern| Ok(glob(pattern.into()))) + .map(|map| match map.deserialize::()? { + EntryObject { pattern: Some(pattern), base: Some(base), auto: None } => { + Ok(glob_with_base(GlobWithBase { pattern, base })) + } + EntryObject { pattern: None, base: None, auto: Some(enabled) } => { + Ok(auto(AutoTracking { auto: enabled })) + } + EntryObject { auto: Some(_), .. } => { + Err(de::Error::custom("`auto` can't be combined with `pattern` or `base`")) + } + EntryObject { pattern: None, .. } => Err(de::Error::missing_field("pattern")), + EntryObject { base: None, .. } => Err(de::Error::missing_field("base")), + }) + .deserialize(deserializer) +} + /// A single input entry in the `input` array. /// /// Inputs can be: /// - Glob patterns as strings (resolved relative to the package directory) /// - Object form with explicit base: `{ "pattern": "...", "base": "workspace" | "package" }` /// - Automatic tracking directives: `{ "auto": true }` -#[derive(Debug, Deserialize, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting -#[cfg_attr(all(test, not(clippy)), derive(TS))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(untagged))] pub enum UserInputEntry { /// Glob pattern (positive or negative starting with `!`), resolved relative to package dir Glob(Str), @@ -66,6 +115,12 @@ pub enum UserInputEntry { Auto(AutoTracking), } +impl<'de> Deserialize<'de> for UserInputEntry { + fn deserialize>(deserializer: D) -> Result { + deserialize_glob_entry(deserializer, Self::Glob, Self::GlobWithBase, Self::Auto) + } +} + /// The inputs configuration for cache fingerprinting. /// /// Default (when field omitted): `[{auto: true}]` - infer from file accesses. @@ -86,10 +141,9 @@ pub enum UserDependencyType { } /// The `from` selector for object-form `dependsOn` entries. -#[derive(Debug, Deserialize, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting -#[cfg_attr(all(test, not(clippy)), derive(TS), ts(rename = "DependsOnFrom"))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(rename = "DependsOnFrom", untagged))] pub enum UserDependsOnFrom { /// Select one package.json dependency field. Single(UserDependencyType), @@ -100,6 +154,25 @@ pub enum UserDependsOnFrom { ), } +impl<'de> Deserialize<'de> for UserDependsOnFrom { + fn deserialize>(deserializer: D) -> Result { + UntaggedEnumVisitor::new() + .expecting( + "a package.json dependency field name, such as `dependencies` or \ + `devDependencies`, or a non-empty array of them", + ) + .string(|field| { + UserDependencyType::deserialize(field.into_deserializer()).map(Self::Single) + }) + .seq(|seq| { + Vec1::try_from_vec(seq.deserialize()?).map(Self::Multiple).map_err(|_| { + de::Error::invalid_length(0, &"at least one package.json dependency field name") + }) + }) + .deserialize(deserializer) + } +} + impl UserDependsOnFrom { #[must_use] pub fn as_slice(&self) -> &[UserDependencyType] { @@ -124,10 +197,9 @@ pub struct UserPackageDependency { } /// A single `dependsOn` entry. -#[derive(Debug, Deserialize, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting -#[cfg_attr(all(test, not(clippy)), derive(TS), ts(rename = "DependsOnEntry"))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(rename = "DependsOnEntry", untagged))] pub enum UserDependsOnEntry { /// Same-package task or `package#task` specifier. Task(Str), @@ -135,16 +207,25 @@ pub enum UserDependsOnEntry { Package(UserPackageDependency), } +impl<'de> Deserialize<'de> for UserDependsOnEntry { + fn deserialize>(deserializer: D) -> Result { + UntaggedEnumVisitor::new() + .expecting("a task name or an object with `task` and `from`") + .string(|task| Ok(Self::Task(task.into()))) + .map(|map| map.deserialize().map(Self::Package)) + .deserialize(deserializer) + } +} + /// A single output entry in the `output` array. /// /// Outputs can be: /// - Glob patterns as strings (resolved relative to the package directory) /// - Object form with explicit base: `{ "pattern": "...", "base": "workspace" | "package" }` /// - Automatic tracking directive: `{ "auto": true }` -#[derive(Debug, Deserialize, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting -#[cfg_attr(all(test, not(clippy)), derive(TS))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(untagged))] pub enum UserOutputEntry { /// Glob pattern (positive or negative starting with `!`), resolved relative to package dir Glob(Str), @@ -154,6 +235,12 @@ pub enum UserOutputEntry { Auto(AutoTracking), } +impl<'de> Deserialize<'de> for UserOutputEntry { + fn deserialize>(deserializer: D) -> Result { + deserialize_glob_entry(deserializer, Self::Glob, Self::GlobWithBase, Self::Auto) + } +} + /// The value of a task's `cache` field. #[derive(Debug, PartialEq, Eq)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting @@ -165,7 +252,6 @@ pub enum UserCacheConfig { Config(EnabledCacheConfig), } -// Not `#[serde(untagged)]`, which replaces each variant's error with a generic one. impl<'de> Deserialize<'de> for UserCacheConfig { fn deserialize>(deserializer: D) -> Result { UntaggedEnumVisitor::new() @@ -299,10 +385,9 @@ impl Default for UserTaskOptions { } } /// The command to run for a task: a single string or a sequence of strings. -#[derive(Debug, Deserialize, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting -#[cfg_attr(all(test, not(clippy)), derive(TS))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(untagged))] pub enum Command { /// A single command string. Single(Str), @@ -310,6 +395,16 @@ pub enum Command { Array(Arc<[Str]>), } +impl<'de> Deserialize<'de> for Command { + fn deserialize>(deserializer: D) -> Result { + UntaggedEnumVisitor::new() + .expecting("a command string or an array of command strings") + .string(|command| Ok(Self::Single(command.into()))) + .seq(|seq| seq.deserialize().map(Self::Array)) + .deserialize(deserializer) + } +} + /// Full user-defined task configuration in `vite.config.*`, including the command and options. #[derive(Debug, Deserialize, PartialEq)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting @@ -342,7 +437,6 @@ pub enum UserTaskDefinition { CommandShorthand(Command), } -// Not `#[serde(untagged)]`, which replaces each variant's error with a generic one. impl<'de> Deserialize<'de> for UserTaskDefinition { fn deserialize>(deserializer: D) -> Result { UntaggedEnumVisitor::new() @@ -367,37 +461,53 @@ impl<'de> Deserialize<'de> for UserTaskDefinition { /// /// This option can only be set in the workspace root's config file. /// Setting it in a package's config will result in an error. -#[derive(Debug, Clone, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] // TS derive macro generates code using std types that clippy disallows; skip derive during linting -#[cfg_attr(all(test, not(clippy)), derive(TS), ts(optional_fields))] -#[serde(untagged, deny_unknown_fields)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(untagged))] pub enum UserGlobalCacheConfig { Bool(bool), /// Detailed cache configuration with separate control for scripts and tasks. - Detailed { - /// Enable caching for package.json scripts not defined in the `tasks` map. - /// - /// When `false`, package.json scripts will not be cached. - /// When `true`, package.json scripts will be cached with default settings. - /// - /// Default: `false` - scripts: Option, - - /// Global cache kill switch for task entries. - /// - /// When `false`, overrides all tasks to disable caching, even tasks with `cache: true` - /// or a `cache` object. - /// When `true`, respects each task's individual `cache` setting - /// (each task's `cache` defaults to `true` if omitted). - /// - /// Default: `true` - tasks: Option, - - /// Remote cache shared by tasks in the workspace. - // Parsed but left out of the public types while remote caching is unreleased. - #[cfg_attr(all(test, not(clippy)), ts(skip))] - remote: Option, - }, + Detailed(#[cfg_attr(all(test, not(clippy)), ts(inline))] DetailedGlobalCacheConfig), +} + +impl<'de> Deserialize<'de> for UserGlobalCacheConfig { + fn deserialize>(deserializer: D) -> Result { + UntaggedEnumVisitor::new() + .expecting("a boolean or an object of cache settings") + .bool(|enabled| Ok(Self::Bool(enabled))) + .map(|map| map.deserialize().map(Self::Detailed)) + .deserialize(deserializer) + } +} + +/// Detailed root-level cache configuration with separate control for scripts and tasks. +#[derive(Debug, Clone, Deserialize, PartialEq, Eq)] +// TS derive macro generates code using std types that clippy disallows; skip derive during linting +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(optional_fields))] +#[serde(deny_unknown_fields)] +pub struct DetailedGlobalCacheConfig { + /// Enable caching for package.json scripts not defined in the `tasks` map. + /// + /// When `false`, package.json scripts will not be cached. + /// When `true`, package.json scripts will be cached with default settings. + /// + /// Default: `false` + pub scripts: Option, + + /// Global cache kill switch for task entries. + /// + /// When `false`, overrides all tasks to disable caching, even tasks with `cache: true` + /// or a `cache` object. + /// When `true`, respects each task's individual `cache` setting + /// (each task's `cache` defaults to `true` if omitted). + /// + /// Default: `true` + pub tasks: Option, + + /// Remote cache shared by tasks in the workspace. + // Parsed but left out of the public types while remote caching is unreleased. + #[cfg_attr(all(test, not(clippy)), ts(skip))] + pub remote: Option, } /// Resolved global cache configuration with concrete values. @@ -424,7 +534,11 @@ impl ResolvedGlobalCacheConfig { Some(UserGlobalCacheConfig::Bool(false)) => { Self { scripts: false, tasks: false, remote_url: None } } - Some(UserGlobalCacheConfig::Detailed { scripts, tasks, remote }) => Self { + Some(UserGlobalCacheConfig::Detailed(DetailedGlobalCacheConfig { + scripts, + tasks, + remote, + })) => Self { scripts: scripts.unwrap_or(false), tasks: tasks.unwrap_or(true), remote_url: remote.as_ref().map(|remote| Arc::clone(&remote.url)), @@ -969,7 +1083,7 @@ mod tests { #[test] fn test_input_mixed_auto_and_glob_with_base_error() { - // An object with both "auto" and "pattern"/"base" should fail due to deny_unknown_fields + // An object with both "auto" and "pattern"/"base" should fail let user_config_json = json!({ "input": [{ "auto": true, "pattern": "src/**", "base": "workspace" }] }); @@ -977,6 +1091,19 @@ mod tests { assert!(result.is_err(), "mixing auto and pattern/base fields should produce an error"); } + #[test] + fn test_input_null_field_error() { + // An explicit `null` is rejected, not treated as an absent field + for entry in [ + json!({ "pattern": "src/**", "base": "workspace", "auto": null }), + json!({ "auto": false, "pattern": null }), + ] { + let user_config_json = json!({ "input": [entry] }); + let result = serde_json::from_value::(user_config_json); + assert!(result.is_err(), "a null field should produce an error"); + } + } + #[test] fn test_input_mixed_with_glob_base() { let user_config_json = json!({ @@ -1113,6 +1240,50 @@ mod tests { "invalid type: integer `123`, expected a command string, an array of command \ strings, or a task object", ), + ( + json!({ "command": 123 }), + "invalid type: integer `123`, expected a command string or an array of command \ + strings", + ), + ( + json!({ + "command": "echo", + "dependsOn": [{ "task": "build", "form": "dependencies" }] + }), + "unknown field `form`, expected `task` or `from`", + ), + ( + json!({ "command": "echo", "dependsOn": [{ "task": "build", "from": "devDeps" }] }), + "unknown variant `devDeps`, expected one of `dependencies`, `devDependencies`, \ + `peerDependencies`", + ), + ( + json!({ "command": "echo", "dependsOn": [{ "task": "build", "from": [] }] }), + "invalid length 0, expected at least one package.json dependency field name", + ), + ( + json!({ + "command": "echo", + "cache": { "input": [{ "pattern": "src/**", "bas": "workspace" }] } + }), + "unknown field `bas`, expected one of `pattern`, `base`, `auto`", + ), + ( + json!({ "command": "echo", "cache": { "input": [{ "pattern": "src/**" }] } }), + "missing field `base`", + ), + ( + json!({ + "command": "echo", + "cache": { "input": [{ "auto": true, "pattern": "src/**" }] } + }), + "`auto` can't be combined with `pattern` or `base`", + ), + ( + json!({ "command": "echo", "cache": { "output": [123] } }), + "invalid type: integer `123`, expected a glob pattern, `{ pattern, base }`, or \ + `{ auto: true }`", + ), ]; for (task, expected) in cases { let user_config_json = json!({ "tasks": { "build": task } }); @@ -1120,4 +1291,14 @@ mod tests { assert_eq!(error.to_string(), expected); } } + + #[test] + fn test_invalid_global_cache_error_names_the_problem() { + let error = serde_json::from_value::(json!({ "cache": { "script": true } })) + .unwrap_err(); + assert_eq!( + error.to_string(), + "unknown field `script`, expected one of `scripts`, `tasks`, `remote`" + ); + } }