From b21d4e804c176a89e1da557a941af0d7306d87d6 Mon Sep 17 00:00:00 2001 From: Terry <186405119+tlq5l@users.noreply.github.com> Date: Wed, 7 Oct 2026 03:54:01 +0700 Subject: [PATCH 1/2] fix(config): name the unknown field when a task config doesn't parse A mistyped task setting such as `cache: { outputs: [...] }` failed with "data did not match any variant of untagged enum UserTaskDefinition". Deserialize `UserTaskDefinition` and `UserCacheConfig` by value type so the error of the form the user wrote is shown instead. Refs voidzero-dev/vite-plus#2635 Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + crates/vt_graph/src/config/user.rs | 105 +++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34d209565..14d4f1f04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **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)). - **Changed** When a task isn't cached because it wrote a file it also read, `vp run --last-details` now says the task read and wrote the file, and shows the `cache: { input, output }` exclusions that let the task be cached ([#784](https://github.com/voidzero-dev/vite-task/pull/784)). - **Changed** The run summary now says a task that wrote a file it also read was `not cached because it modified its inputs`, and the statistics in `vp run --verbose` and `vp run --last-details` use the singular for a count of one, e.g. `1 task • 1 cache miss` ([#783](https://github.com/voidzero-dev/vite-task/pull/783)). - **Fixed** An invalid glob in `--filter` no longer shows its error message twice ([#763](https://github.com/voidzero-dev/vite-task/pull/763)). diff --git a/crates/vt_graph/src/config/user.rs b/crates/vt_graph/src/config/user.rs index a0ef5888a..acb89329e 100644 --- a/crates/vt_graph/src/config/user.rs +++ b/crates/vt_graph/src/config/user.rs @@ -3,7 +3,13 @@ use std::sync::Arc; use rustc_hash::FxHashMap; -use serde::Deserialize; +use serde::{ + Deserialize, Deserializer, + de::{ + MapAccess, SeqAccess, Visitor, + value::{MapAccessDeserializer, SeqAccessDeserializer}, + }, +}; #[cfg(all(test, not(clippy)))] use ts_rs::TS; use vec1::Vec1; @@ -154,10 +160,9 @@ pub enum UserOutputEntry { } /// The value of a task's `cache` field. -#[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), ts(rename = "TaskCache"))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(rename = "TaskCache", untagged))] pub enum UserCacheConfig { /// `true` enables caching with default settings; `false` disables caching. Bool(bool), @@ -165,6 +170,33 @@ pub enum UserCacheConfig { Config(EnabledCacheConfig), } +// Not `#[serde(untagged)]`: that replaces the error of every variant with a generic one, so a +// mistyped setting would not be named. +impl<'de> Deserialize<'de> for UserCacheConfig { + fn deserialize>(deserializer: D) -> Result { + struct CacheConfigVisitor; + + impl<'de> Visitor<'de> for CacheConfigVisitor { + type Value = UserCacheConfig; + + fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("a boolean or an object of cache settings") + } + + fn visit_bool(self, enabled: bool) -> Result { + Ok(UserCacheConfig::Bool(enabled)) + } + + fn visit_map>(self, map: A) -> Result { + Deserialize::deserialize(MapAccessDeserializer::new(map)) + .map(UserCacheConfig::Config) + } + } + + deserializer.deserialize_any(CacheConfigVisitor) + } +} + impl Default for UserCacheConfig { /// Caching enabled with default settings, used when `cache` is omitted. fn default() -> Self { @@ -319,10 +351,9 @@ pub struct UserTaskConfig { } /// User-defined task configuration or command-only shorthand in `vite.config.*`. -#[derive(Debug, Deserialize, PartialEq)] +#[derive(Debug, PartialEq)] // 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 = "TaskDefinition"))] -#[serde(untagged)] +#[cfg_attr(all(test, not(clippy)), derive(TS), ts(rename = "TaskDefinition", untagged))] pub enum UserTaskDefinition { /// Full task object form. Object(UserTaskConfig), @@ -330,6 +361,38 @@ pub enum UserTaskDefinition { CommandShorthand(Command), } +// Not `#[serde(untagged)]`: that replaces the error of every variant with a generic one, so a +// mistyped field would not be named. +impl<'de> Deserialize<'de> for UserTaskDefinition { + fn deserialize>(deserializer: D) -> Result { + struct TaskDefinitionVisitor; + + impl<'de> Visitor<'de> for TaskDefinitionVisitor { + type Value = UserTaskDefinition; + + fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("a command string, an array of command strings, or a task object") + } + + fn visit_str(self, command: &str) -> Result { + Ok(UserTaskDefinition::CommandShorthand(Command::Single(command.into()))) + } + + fn visit_seq>(self, seq: A) -> Result { + Deserialize::deserialize(SeqAccessDeserializer::new(seq)) + .map(|commands| UserTaskDefinition::CommandShorthand(Command::Array(commands))) + } + + fn visit_map>(self, map: A) -> Result { + Deserialize::deserialize(MapAccessDeserializer::new(map)) + .map(UserTaskDefinition::Object) + } + } + + deserializer.deserialize_any(TaskDefinitionVisitor) + } +} + /// Root-level cache configuration. /// /// Controls caching behavior for the entire workspace. @@ -1066,4 +1129,32 @@ mod tests { .is_err() ); } + + #[test] + fn test_invalid_task_error_names_the_problem() { + let cases = [ + (json!({ "command": "echo", "outputs": [] }), "unknown field `outputs`"), + ( + json!({ "command": "echo", "cache": { "outputs": [] } }), + "unknown field `outputs`, expected one of `env`, `untrackedEnv`, `input`, \ + `output`, `remote`", + ), + ( + json!({ "command": "echo", "cache": "yes" }), + "invalid type: string \"yes\", expected a boolean or an object of cache settings", + ), + (json!({ "cache": false }), "missing field `command`"), + (json!(["echo one", 2]), "invalid type: integer `2`, expected a string"), + ( + json!(123), + "invalid type: integer `123`, expected a command string, an array of command \ + strings, or a task object", + ), + ]; + for (task, expected) in cases { + let user_config_json = json!({ "tasks": { "build": task } }); + let error = serde_json::from_value::(user_config_json).unwrap_err(); + assert_eq!(error.to_string(), expected); + } + } } From 0bd5afd69350809db710d3785fa72e04e560c9a9 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Mon, 19 Oct 2026 10:12:26 +0800 Subject: [PATCH 2/2] refactor(config): use serde-untagged for task and cache definitions Replace the hand-written `Visitor` impls for `UserTaskDefinition` and `UserCacheConfig` with `serde_untagged::UntaggedEnumVisitor`. Error messages are unchanged. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 30 ++++++++++++ Cargo.toml | 1 + crates/vt_graph/Cargo.toml | 1 + crates/vt_graph/src/config/user.rs | 73 +++++++----------------------- 4 files changed, 49 insertions(+), 56 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 99c4b31fa..40dad5d5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1182,6 +1182,17 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" +[[package]] +name = "erased-serde" +version = "0.4.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2add8a07dd6a8d93ff627029c51de145e12686fbc36ecb298ac22e74cf02dec" +dependencies = [ + "serde", + "serde_core", + "typeid", +] + [[package]] name = "errno" version = "0.3.14" @@ -3861,6 +3872,18 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-untagged" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f9faf48a4a2d2693be24c6289dbe26552776eb7737074e6722891fadbe6c5058" +dependencies = [ + "erased-serde", + "serde", + "serde_core", + "typeid", +] + [[package]] name = "serde_bytes" version = "0.11.19" @@ -4780,6 +4803,12 @@ dependencies = [ "rand 0.9.2", ] +[[package]] +name = "typeid" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c" + [[package]] name = "typenum" version = "1.20.0" @@ -5071,6 +5100,7 @@ dependencies = [ "pretty_assertions", "rustc-hash", "serde", + "serde-untagged", "serde_json", "thiserror 2.0.18", "tracing", diff --git a/Cargo.toml b/Cargo.toml index 4fe61fa87..74ef3590f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -130,6 +130,7 @@ rustls = { version = "0.23.44", default-features = false } # SeccompAction::UserNotif (SECCOMP_RET_USER_NOTIF) was added after the latest published release (v0.5.0) seccompiler = { git = "https://github.com/rust-vmm/seccompiler", rev = "08587106340b8e3cb361c7561411510039436857" } serde = "1.0.219" +serde-untagged = "0.1.9" serde_bytes = "0.11.19" serde_json = "1.0.140" serde_norway = "0.9.42" diff --git a/crates/vt_graph/Cargo.toml b/crates/vt_graph/Cargo.toml index 603b9faeb..ef0cb970d 100644 --- a/crates/vt_graph/Cargo.toml +++ b/crates/vt_graph/Cargo.toml @@ -14,6 +14,7 @@ wincode = { workspace = true, features = ["derive"] } petgraph = { workspace = true } rustc-hash = { workspace = true } serde = { workspace = true, features = ["derive", "rc"] } +serde-untagged = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } diff --git a/crates/vt_graph/src/config/user.rs b/crates/vt_graph/src/config/user.rs index 22fef730c..92f00a2ef 100644 --- a/crates/vt_graph/src/config/user.rs +++ b/crates/vt_graph/src/config/user.rs @@ -3,13 +3,8 @@ use std::sync::Arc; use rustc_hash::FxHashMap; -use serde::{ - Deserialize, Deserializer, - de::{ - MapAccess, SeqAccess, Visitor, - value::{MapAccessDeserializer, SeqAccessDeserializer}, - }, -}; +use serde::{Deserialize, Deserializer}; +use serde_untagged::UntaggedEnumVisitor; #[cfg(all(test, not(clippy)))] use ts_rs::TS; use vec1::Vec1; @@ -170,30 +165,14 @@ pub enum UserCacheConfig { Config(EnabledCacheConfig), } -// Not `#[serde(untagged)]`: that replaces the error of every variant with a generic one, so a -// mistyped setting would not be named. +// Not `#[serde(untagged)]`, which replaces each variant's error with a generic one. impl<'de> Deserialize<'de> for UserCacheConfig { fn deserialize>(deserializer: D) -> Result { - struct CacheConfigVisitor; - - impl<'de> Visitor<'de> for CacheConfigVisitor { - type Value = UserCacheConfig; - - fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("a boolean or an object of cache settings") - } - - fn visit_bool(self, enabled: bool) -> Result { - Ok(UserCacheConfig::Bool(enabled)) - } - - fn visit_map>(self, map: A) -> Result { - Deserialize::deserialize(MapAccessDeserializer::new(map)) - .map(UserCacheConfig::Config) - } - } - - deserializer.deserialize_any(CacheConfigVisitor) + UntaggedEnumVisitor::new() + .expecting("a boolean or an object of cache settings") + .bool(|enabled| Ok(Self::Bool(enabled))) + .map(|map| map.deserialize().map(Self::Config)) + .deserialize(deserializer) } } @@ -363,35 +342,17 @@ pub enum UserTaskDefinition { CommandShorthand(Command), } -// Not `#[serde(untagged)]`: that replaces the error of every variant with a generic one, so a -// mistyped field would not be named. +// Not `#[serde(untagged)]`, which replaces each variant's error with a generic one. impl<'de> Deserialize<'de> for UserTaskDefinition { fn deserialize>(deserializer: D) -> Result { - struct TaskDefinitionVisitor; - - impl<'de> Visitor<'de> for TaskDefinitionVisitor { - type Value = UserTaskDefinition; - - fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("a command string, an array of command strings, or a task object") - } - - fn visit_str(self, command: &str) -> Result { - Ok(UserTaskDefinition::CommandShorthand(Command::Single(command.into()))) - } - - fn visit_seq>(self, seq: A) -> Result { - Deserialize::deserialize(SeqAccessDeserializer::new(seq)) - .map(|commands| UserTaskDefinition::CommandShorthand(Command::Array(commands))) - } - - fn visit_map>(self, map: A) -> Result { - Deserialize::deserialize(MapAccessDeserializer::new(map)) - .map(UserTaskDefinition::Object) - } - } - - deserializer.deserialize_any(TaskDefinitionVisitor) + UntaggedEnumVisitor::new() + .expecting("a command string, an array of command strings, or a task object") + .string(|command| Ok(Self::CommandShorthand(Command::Single(command.into())))) + .seq(|seq| { + seq.deserialize().map(|commands| Self::CommandShorthand(Command::Array(commands))) + }) + .map(|map| map.deserialize().map(Self::Object)) + .deserialize(deserializer) } }