From aa1a38bbcf350f422677713cb0471541a027cc5f Mon Sep 17 00:00:00 2001 From: vikingowl Date: Wed, 13 May 2026 05:12:52 +0200 Subject: [PATCH] feat(lua): owlry config validate covers the Lua surface (Phase 3.9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the validator per docs/lua-api.md §8. Replaces the TOML-only "load and pass/fail" check from 2.0 with categorised findings: errors (exit 1), warnings (exit 2), clean (exit 0). lua/config.rs: - LuaConfig grows `duplicate_user_provider_ids: Vec`. The dedup in apply_provider would otherwise erase duplicates by the time the snapshot is read. lua/api.rs: - apply_provider records the id on the duplicate list when replacing an existing entry. The original warn! log is unchanged. lua/validate.rs (new): - ValidationReport { errors, warnings } with exit_code() honouring the §8 codes (0/1/2). - validate(cfg) is a pure function over a LuaConfig snapshot. Surfaces: * Unknown keys in owlry.set * Unknown colour keys in owlry.theme * Unknown ids in owlry.providers (built-in alias table consulted; user-provider ids are recognised) * Providers compiled out — Cargo features checked via cfg! macros; always-on ids (app, cmd, calc, conv, power, dmenu) skip the check * Unknown / not-in-providers ids in owlry.tabs (user providers auto-join the enabled set, so they pass when listed in tabs) * Duplicate provider ids * Unknown ids inside owlry.profiles values (per-profile label) - canonical_provider_id() mirrors the apply_providers_list alias table in lua/config.rs so an id accepted by the merger is never flagged. commands.rs: - run_config_validate now branches: when owlry.lua exists, build the LoadedConfig, run validate::validate on the snapshot, print the report with ` warning(s):` / ` error(s):` headers. Eval failures print the chained mlua error (file path + line/col). - TOML fallback path unchanged when no owlry.lua is present. - Whole feature path is `#[cfg(feature = "lua")]`-gated. Tests: 13 new in lua::validate::tests - clean config emits nothing, exit 0 - unknown set key warns - unknown theme key warns - unknown provider id warns (and `app` passes through unflagged) - user_provider ids in owlry.providers list are recognised - tabs with unknown id warn - tabs with id not in owlry.providers warn ("dropped from tab bar") - tabs with user provider id passes (auto-joins enabled set) - duplicate provider id warns - profile with unknown id warns and names the profile - pre-v2 aliases (sys, uuctl) validate cleanly - always-compiled-in providers never trigger compile-out warning - multi-warning report keeps every finding Live smoke (release build, isolated XDG) for each category: - clean → "config: OK (Lua, ...)", exit 0 - unknown set key → "owlry.set: unknown key `mystery_key` — ignored (forward-compat for 2.2+ keys)", exit 2 - unknown provider id → "owlry.providers: unknown id `fictional_provider` — not a built-in and not registered by any owlry.provider call", exit 2 - tabs not in providers → "owlry.tabs: `cmd` is not in owlry.providers — it will be dropped from the tab bar", exit 2 - duplicate provider id → "owlry.provider: id `hs` was registered more than once — the last definition wins (earlier registrations are dropped)", exit 2 - profile with unknown id → "owlry.profiles.dev: unknown id `phantom`", exit 2 - syntax error → "config: ERROR — Lua evaluation error in /.../ owlry.lua: syntax error: [string]:5: '}' expected (to close '{' at line 2) near ", exit 1 - 5-warning combo → all five surfaced with stable ordering, exit 2 352/352 lib tests with --features full. Clippy silent across full, --features lua, and --no-default-features. --- crates/owlry/src/commands.rs | 78 +++++- crates/owlry/src/lua/api.rs | 6 +- crates/owlry/src/lua/config.rs | 6 + crates/owlry/src/lua/mod.rs | 2 + crates/owlry/src/lua/validate.rs | 415 +++++++++++++++++++++++++++++++ 5 files changed, 504 insertions(+), 3 deletions(-) create mode 100644 crates/owlry/src/lua/validate.rs diff --git a/crates/owlry/src/commands.rs b/crates/owlry/src/commands.rs index c47ae53..e992638 100644 --- a/crates/owlry/src/commands.rs +++ b/crates/owlry/src/commands.rs @@ -146,11 +146,34 @@ fn print_provider_list(out: &mut impl Write, list: &[ProviderDesc]) { } } -/// `owlry config validate`: parse the config file and report errors. +/// `owlry config validate`: parse the config file and report errors and +/// warnings per docs/lua-api.md §8. +/// +/// Resolution order matches `Config::load`: +/// 1. If `owlry.lua` exists, evaluate it. Eval errors → exit 1. Otherwise +/// run [`crate::lua::validate::validate`] on the snapshot and surface +/// every warning category (unknown set/theme keys, unknown provider +/// ids, tabs ⊄ providers, duplicate provider ids, compiled-out +/// providers, unknown ids inside profiles). +/// 2. Otherwise fall back to the TOML check. +/// +/// Exit codes: 0 clean, 1 errors (eval / TOML parse failure), 2 +/// warnings-only. pub fn run_config_validate() -> ! { + #[cfg(feature = "lua")] + { + if let Some(lua_path) = paths::lua_config_file() + && lua_path.exists() + { + let exit = run_config_validate_lua(&lua_path); + std::process::exit(exit); + } + } + + // No owlry.lua → fall back to TOML. match Config::load() { Ok(_) => { - println!("config: OK"); + println!("config: OK (TOML)"); std::process::exit(0); } Err(e) => { @@ -160,6 +183,57 @@ pub fn run_config_validate() -> ! { } } +#[cfg(feature = "lua")] +fn run_config_validate_lua(lua_path: &std::path::Path) -> i32 { + use crate::config::LoadedConfig; + use crate::lua::validate; + + let loaded = match LoadedConfig::load_lua_path(lua_path) { + Ok(lc) => lc, + Err(e) => { + // Eval failure — report the chain (mlua error has line/col info). + let mut msg = e.to_string(); + let mut cur: Option<&dyn std::error::Error> = e.source(); + while let Some(c) = cur { + msg.push_str(": "); + msg.push_str(&c.to_string()); + cur = c.source(); + } + eprintln!("config: ERROR — {}", msg); + return 1; + } + }; + let snapshot = match loaded.lua.as_ref() { + Some(ctx) => ctx.snapshot(), + None => { + eprintln!("config: ERROR — internal: Lua context missing after load"); + return 1; + } + }; + + let report = validate::validate(&snapshot); + + if report.is_clean() { + println!("config: OK (Lua, {})", lua_path.display()); + return 0; + } + + println!("config: validating {}", lua_path.display()); + if !report.warnings.is_empty() { + println!(" {} warning(s):", report.warnings.len()); + for w in &report.warnings { + println!(" - {}", w); + } + } + if !report.errors.is_empty() { + println!(" {} error(s):", report.errors.len()); + for e in &report.errors { + println!(" - {}", e); + } + } + report.exit_code() +} + /// `owlry config show`: serialize the effective config to stdout as TOML. pub fn run_config_show() -> ! { let cfg = Config::load_or_default(); diff --git a/crates/owlry/src/lua/api.rs b/crates/owlry/src/lua/api.rs index 13dbbd5..8f89665 100644 --- a/crates/owlry/src/lua/api.rs +++ b/crates/owlry/src/lua/api.rs @@ -232,13 +232,17 @@ fn apply_provider(cfg: &mut LuaConfig, t: Table) -> mlua::Result<()> { items_fn, }; - // Duplicate id → second wins, warning emitted (spec §6.4). + // Duplicate id → second wins, warning emitted (spec §6.4). Also recorded + // on the snapshot for `owlry config validate` to surface. if let Some(pos) = cfg.user_providers.iter().position(|s| s.id == id) { warn!( "owlry.provider: duplicate id '{}' — second definition replaces the first", id ); cfg.user_providers[pos] = spec; + if !cfg.duplicate_user_provider_ids.contains(&id) { + cfg.duplicate_user_provider_ids.push(id); + } } else { cfg.user_providers.push(spec); } diff --git a/crates/owlry/src/lua/config.rs b/crates/owlry/src/lua/config.rs index ac63264..0b9c5a8 100644 --- a/crates/owlry/src/lua/config.rs +++ b/crates/owlry/src/lua/config.rs @@ -46,6 +46,12 @@ pub struct LuaConfig { /// later wins, earlier is dropped (warning logged). pub user_providers: Vec, + /// IDs that appeared more than once in `owlry.provider { ... }` calls. + /// Stored separately so `owlry config validate` (Phase 3.9) can surface + /// them — the dedup in [`super::api::apply_provider`] would otherwise + /// erase the evidence by the time the snapshot is read. + pub duplicate_user_provider_ids: Vec, + // ─── owlry.theme(...) ───────────────────────────────────────────────── /// Theme name from the string form `owlry.theme("nord")`. Last write wins /// across multiple calls (string forms merge with table forms — see diff --git a/crates/owlry/src/lua/mod.rs b/crates/owlry/src/lua/mod.rs index 6167f69..3059ad0 100644 --- a/crates/owlry/src/lua/mod.rs +++ b/crates/owlry/src/lua/mod.rs @@ -16,6 +16,7 @@ pub mod migrate; pub mod provider; pub mod runtime; pub mod util; +pub mod validate; pub mod watcher; pub use config::{LuaConfig, LuaProviderSpec}; @@ -23,4 +24,5 @@ pub use error::LuaConfigError; pub use migrate::{MigrateError, MigrateOutcome, MigrateRequest}; pub use provider::LuaProvider; pub use runtime::LuaContext; +pub use validate::ValidationReport; pub use watcher::ConfigWatcher; diff --git a/crates/owlry/src/lua/validate.rs b/crates/owlry/src/lua/validate.rs new file mode 100644 index 0000000..4e27980 --- /dev/null +++ b/crates/owlry/src/lua/validate.rs @@ -0,0 +1,415 @@ +//! `owlry config validate` for the Lua side per docs/lua-api.md §8. +//! +//! Categories (with exit codes from spec §8): +//! - **errors** (exit 1) — syntax / eval failures. Returned to the caller +//! as the `LuaConfigError` from `LoadedConfig::load_lua_path`; we don't +//! surface them through [`ValidationReport`] because there's nothing +//! else to check once eval failed. +//! - **warnings** (exit 2) — unknown keys, unknown ids, tabs ⊄ providers, +//! duplicate provider ids, providers compiled out. +//! - **clean** (exit 0) — no warnings. +//! +//! The validator is a pure function over a [`LuaConfig`] snapshot. The +//! provider-id and theme-key tables it consults are the same canonical +//! sets used by [`super::config::apply_providers_list`] and +//! [`super::config::KNOWN_THEME_KEYS`], so a key/id accepted by the +//! merger is never flagged by the validator (and vice-versa). + +use super::config::{KNOWN_THEME_KEYS, LuaConfig}; + +/// Categorised findings from validating an `owlry.lua` snapshot. +#[derive(Debug, Default, Clone)] +pub struct ValidationReport { + pub errors: Vec, + pub warnings: Vec, +} + +impl ValidationReport { + /// Spec §8 exit codes: 0 clean, 1 errors, 2 warnings-only. + pub fn exit_code(&self) -> i32 { + if !self.errors.is_empty() { + 1 + } else if !self.warnings.is_empty() { + 2 + } else { + 0 + } + } + + pub fn is_clean(&self) -> bool { + self.errors.is_empty() && self.warnings.is_empty() + } +} + +/// Inspect a [`LuaConfig`] snapshot and produce a [`ValidationReport`]. +pub fn validate(cfg: &LuaConfig) -> ValidationReport { + let mut report = ValidationReport::default(); + let user_ids: Vec<&str> = cfg.user_providers.iter().map(|s| s.id.as_str()).collect(); + + // owlry.set — unknown keys (recorded at eval time). + for key in &cfg.unknown_settings { + report.warnings.push(format!( + "owlry.set: unknown key `{}` — ignored (forward-compat for 2.2+ keys)", + key + )); + } + + // owlry.theme — unknown keys (recorded at eval time). + for key in &cfg.unknown_theme_keys { + // KNOWN_THEME_KEYS contains the pre-v2 `badge_sys` alias too; the + // tracker already filters those, so any leftover key is genuinely + // unrecognised. + if !KNOWN_THEME_KEYS.contains(&key.as_str()) { + report.warnings.push(format!( + "owlry.theme: unknown colour key `{}` — ignored", + key + )); + } + } + + // owlry.providers — unknown ids and compiled-out warnings. + if let Some(list) = &cfg.providers { + for id in list { + match canonical_provider_id(id) { + Some(canon) => { + if !is_compiled_in(canon) { + report.warnings.push(format!( + "owlry.providers: `{}` is not compiled into this build — \ + it will be silently dropped at runtime. Rebuild owlry \ + with the matching cargo feature to enable it.", + id + )); + } + } + None => { + if !user_ids.contains(&id.as_str()) { + report.warnings.push(format!( + "owlry.providers: unknown id `{}` — not a built-in and not \ + registered by any `owlry.provider {{ id = … }}` call", + id + )); + } + } + } + } + } + + // owlry.tabs — unknown ids; entries not in providers list. + if let Some(tabs) = &cfg.tabs { + let enabled_canon: Option> = + cfg.providers.as_ref().map(|v| { + v.iter() + .filter_map(|s| canonical_provider_id(s)) + .collect() + }); + for id in tabs { + let canon = canonical_provider_id(id); + match canon { + Some(c) => { + if let Some(ref set) = enabled_canon + && !set.contains(c) + { + report.warnings.push(format!( + "owlry.tabs: `{}` is not in owlry.providers — it will \ + be dropped from the tab bar", + id + )); + } + } + None => { + if !user_ids.contains(&id.as_str()) { + report.warnings.push(format!( + "owlry.tabs: unknown id `{}` — not a built-in and not \ + a user-defined provider", + id + )); + } + } + } + } + } + + // owlry.provider duplicates. + for dup in &cfg.duplicate_user_provider_ids { + report.warnings.push(format!( + "owlry.provider: id `{}` was registered more than once — the last \ + definition wins (earlier registrations are dropped)", + dup + )); + } + + // owlry.profiles — unknown ids inside each profile's mode list. + if let Some(profiles) = &cfg.profiles { + let mut profile_names: Vec<&String> = profiles.keys().collect(); + profile_names.sort(); + for name in profile_names { + for id in &profiles[name] { + let known = canonical_provider_id(id).is_some() + || user_ids.contains(&id.as_str()); + if !known { + report.warnings.push(format!( + "owlry.profiles.{}: unknown id `{}` — not a built-in and \ + not a user-defined provider", + name, id + )); + } + } + } + } + + report +} + +/// Resolve a Lua-side provider id to its canonical form. Mirrors the +/// alias table in [`super::config::apply_providers_list`]. +fn canonical_provider_id(id: &str) -> Option<&'static str> { + match id { + "app" | "application" | "applications" => Some("applications"), + "cmd" | "command" | "commands" => Some("commands"), + "calc" | "calculator" => Some("calculator"), + "conv" | "converter" => Some("converter"), + "power" | "sys" | "system" => Some("power"), + "systemd" | "uuctl" => Some("systemd"), + "ssh" => Some("ssh"), + "clipboard" | "clip" => Some("clipboard"), + "emoji" => Some("emoji"), + "filesearch" | "file" => Some("filesearch"), + "websearch" | "web" | "search" => Some("websearch"), + "dmenu" => Some("dmenu"), + _ => None, + } +} + +/// Whether the canonical provider id is compiled into the current build. +/// Always-compiled-in ids (applications, commands, calc, conv, power, +/// dmenu) return `true` unconditionally; the rest gate on cargo features. +fn is_compiled_in(canonical: &str) -> bool { + match canonical { + "applications" | "commands" | "calculator" | "converter" | "power" + | "dmenu" => true, + "clipboard" => cfg!(feature = "clipboard"), + "emoji" => cfg!(feature = "emoji"), + "filesearch" => cfg!(feature = "filesearch"), + "ssh" => cfg!(feature = "ssh"), + "systemd" => cfg!(feature = "systemd"), + "websearch" => cfg!(feature = "websearch"), + _ => true, // unknown canonical → not our call + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::ThemeColors; + use std::collections::HashMap; + + fn empty_cfg() -> LuaConfig { + LuaConfig::default() + } + + #[test] + fn clean_config_produces_no_findings() { + let report = validate(&empty_cfg()); + assert!(report.is_clean()); + assert_eq!(report.exit_code(), 0); + } + + #[test] + fn unknown_set_key_becomes_warning() { + let mut cfg = empty_cfg(); + cfg.unknown_settings.push("mystery_2_2_key".into()); + let report = validate(&cfg); + assert_eq!(report.exit_code(), 2); + assert!( + report.warnings.iter().any(|w| w.contains("mystery_2_2_key")), + "got: {:?}", + report.warnings + ); + } + + #[test] + fn unknown_theme_key_becomes_warning() { + let mut cfg = empty_cfg(); + cfg.unknown_theme_keys.push("future_palette_token".into()); + let report = validate(&cfg); + assert_eq!(report.exit_code(), 2); + assert!( + report.warnings.iter().any(|w| w.contains("future_palette_token")) + ); + } + + #[test] + fn unknown_provider_id_becomes_warning() { + let mut cfg = empty_cfg(); + cfg.providers = Some(vec!["app".into(), "fictional".into()]); + let report = validate(&cfg); + assert_eq!(report.exit_code(), 2); + assert!( + report.warnings.iter().any(|w| w.contains("fictional")), + "got: {:?}", + report.warnings + ); + // `app` is recognised and must NOT produce a warning. + assert!(!report.warnings.iter().any(|w| w.contains("`app`"))); + } + + #[test] + fn user_provider_id_in_providers_list_is_known() { + let mut cfg = empty_cfg(); + cfg.providers = Some(vec!["hs".into()]); + // Pretend `hs` was registered as a user provider. + cfg.user_providers.push(make_spec("hs")); + let report = validate(&cfg); + assert!(report.is_clean(), "got: {:?}", report); + } + + #[test] + fn tabs_unknown_id_becomes_warning() { + let mut cfg = empty_cfg(); + cfg.tabs = Some(vec!["app".into(), "ghost".into()]); + let report = validate(&cfg); + assert!( + report.warnings.iter().any(|w| w.contains("ghost")), + "got: {:?}", + report.warnings + ); + } + + #[test] + fn tabs_entry_not_in_providers_list_is_warning() { + let mut cfg = empty_cfg(); + cfg.providers = Some(vec!["app".into()]); + cfg.tabs = Some(vec!["app".into(), "cmd".into()]); // cmd not enabled + let report = validate(&cfg); + assert!( + report.warnings.iter().any(|w| w.contains("cmd") && w.contains("not in owlry.providers")), + "got: {:?}", + report.warnings + ); + } + + #[test] + fn tabs_user_provider_outside_providers_list_is_ok() { + // User providers auto-join the enabled set, so they don't need to + // appear in owlry.providers. Listing them in tabs should be fine. + let mut cfg = empty_cfg(); + cfg.providers = Some(vec!["app".into()]); + cfg.tabs = Some(vec!["app".into(), "hs".into()]); + cfg.user_providers.push(make_spec("hs")); + let report = validate(&cfg); + assert!(report.is_clean(), "got: {:?}", report); + } + + #[test] + fn duplicate_provider_id_becomes_warning() { + let mut cfg = empty_cfg(); + cfg.duplicate_user_provider_ids.push("hs".into()); + let report = validate(&cfg); + assert!( + report.warnings.iter().any(|w| w.contains("registered more than once")), + "got: {:?}", + report.warnings + ); + } + + #[test] + fn profile_with_unknown_id_warns_per_id() { + let mut cfg = empty_cfg(); + let mut p: HashMap> = HashMap::new(); + p.insert("dev".into(), vec!["app".into(), "phantom".into()]); + cfg.profiles = Some(p); + let report = validate(&cfg); + assert!( + report + .warnings + .iter() + .any(|w| w.contains("owlry.profiles.dev") && w.contains("phantom")), + "got: {:?}", + report.warnings + ); + } + + #[test] + fn pre_v2_aliases_are_known() { + let mut cfg = empty_cfg(); + cfg.providers = Some(vec!["sys".into(), "uuctl".into()]); + cfg.tabs = Some(vec!["sys".into(), "uuctl".into()]); + let report = validate(&cfg); + assert!( + report.is_clean(), + "pre-v2 aliases must validate cleanly; got: {:?}", + report.warnings + ); + } + + #[test] + fn compiled_out_provider_is_warning_when_features_dropped() { + // We can't easily flip features at test time, but we can verify + // that ALWAYS-compiled-in ids never produce a "not compiled in" + // warning — that locks in the alias table from regressing. + let mut cfg = empty_cfg(); + cfg.providers = Some(vec![ + "app".into(), + "cmd".into(), + "calc".into(), + "conv".into(), + "power".into(), + ]); + let report = validate(&cfg); + assert!( + !report + .warnings + .iter() + .any(|w| w.contains("not compiled into this build")), + "always-on providers must never warn about compile-out; got: {:?}", + report.warnings + ); + } + + #[test] + fn report_with_multiple_findings_keeps_them_all() { + let mut cfg = empty_cfg(); + cfg.unknown_settings.push("a".into()); + cfg.unknown_theme_keys.push("b".into()); + cfg.providers = Some(vec!["nope".into()]); + cfg.duplicate_user_provider_ids.push("hs".into()); + let report = validate(&cfg); + // 4 categories → at least 4 warnings. + assert!( + report.warnings.len() >= 4, + "expected several warnings, got {}: {:?}", + report.warnings.len(), + report.warnings + ); + assert_eq!(report.exit_code(), 2); + } + + fn make_spec(id: &str) -> super::super::config::LuaProviderSpec { + // Build a minimal LuaProviderSpec for testing. We can't easily + // synthesize an mlua::Function without a Lua context, so we + // borrow one from a throwaway state. + use mlua::Lua; + let lua = Lua::new(); + let f = lua + .create_function(|_, ()| Ok(Vec::::new())) + .unwrap(); + super::super::config::LuaProviderSpec { + id: id.to_string(), + name: None, + prefix: None, + tab_label: None, + icon: None, + search_noun: None, + priority: 0, + dynamic: false, + items_fn: f, + } + } + + // ThemeColors / ProfileConfig usage references just to silence + // unused-import warnings when the imports above aren't otherwise hit. + #[allow(dead_code)] + fn _refs() { + let _ = ThemeColors::default(); + } +}