From 9fc65c701f3a38c071acfa0cb7ae6100b3891a4a Mon Sep 17 00:00:00 2001 From: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com> Date: Mon, 3 Aug 2026 14:52:45 -0700 Subject: [PATCH] feat!: enforce #[non_exhaustive] policy via clippy lints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Set clippy::exhaustive_enums and clippy::exhaustive_structs to `deny` as workspace lints so the SemVer growth question is an explicit, reviewed decision for every public type, and settle the existing surface against a documented policy (specs/non-exhaustive-policy.md). The level is `deny`, not `warn`: an undecided public type is a hard error under a plain `cargo clippy`, so it fails locally while the type is being written rather than relying on CI's `-D warnings`. openjd-cli (a binary) and openjd-for-js (publish = false) are exempt at the crate level. The rule: types that mirror a numbered OpenJD spec section grow by extension RFC and are gated by the decode-time extension allowlist, so they are marked #[non_exhaustive] (76 types) — e.g. template::JobTemplate, HostRequirements, the parameter-definition/UserInterface families, plus PathFormat, HostContext, and the template growth-axis enums. Types that represent a decidable concept, are caller-constructed configuration, or are the instantiated job:: model the sessions runtime constructs and exhaustively matches (with no allowlist in front of them) stay closed with an #[expect(..., reason = "...")] recording why (80 sites). Key distinction, documented in the policy: template::* (deserialize-time input a consumer reads) is non_exhaustive; job::* (instantiated model the runtime executes) stays closed, so a new field is a compile error on the runner rather than a silently ignored `_` arm — the same rule already applied to the runtime state machines (ActionState, SessionState, ...). Adds PathMappingRule::new and CallerLimits builder methods as the supported construction path for the two non_exhaustive types that callers legitimately build by hand, and migrates call sites. openjd-cli (a binary) and openjd-for-js (publish = false) are exempted from the lint at the crate level. BREAKING CHANGE: many public enums and structs in openjd-expr, openjd-model, openjd-sessions, and openjd-snapshots are now #[non_exhaustive]. Downstream Rust consumers must add wildcard match arms and use `..` patterns / the provided constructors instead of struct literals. No functional behavior change. Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com> --- Cargo.toml | 24 ++ crates/openjd-cli/Cargo.toml | 7 + crates/openjd-expr/Cargo.toml | 3 + crates/openjd-expr/src/eval/evaluator.rs | 4 + crates/openjd-expr/src/format_string.rs | 4 + crates/openjd-expr/src/function_library.rs | 4 + crates/openjd-expr/src/path_mapping.rs | 28 +++ crates/openjd-expr/src/profile.rs | 1 + crates/openjd-expr/src/range_expr.rs | 4 + crates/openjd-expr/src/symbol_table.rs | 8 + crates/openjd-expr/src/uri_path.rs | 4 + crates/openjd-expr/src/value.rs | 4 + .../integration/test_function_context.rs | 24 +- .../tests/integration/test_path_mapping.rs | 210 +++--------------- .../integration/test_path_mapping_platform.rs | 18 +- crates/openjd-for-js/Cargo.toml | 7 + crates/openjd-for-js/src/expr.rs | 13 +- crates/openjd-for-js/src/model.rs | 26 ++- crates/openjd-model/Cargo.toml | 3 + crates/openjd-model/src/error.rs | 16 ++ .../src/job/create_job/parameters.rs | 12 +- crates/openjd-model/src/job/mod.rs | 19 ++ .../src/job/step_dependency_graph.rs | 8 + crates/openjd-model/src/template/actions.rs | 8 + .../src/template/constrained_strings.rs | 17 ++ .../openjd-model/src/template/environment.rs | 3 + .../src/template/environment_template.rs | 1 + .../src/template/expr_parameters.rs | 19 ++ .../src/template/host_requirements.rs | 3 + .../openjd-model/src/template/job_template.rs | 1 + .../openjd-model/src/template/parameters.rs | 22 ++ crates/openjd-model/src/template/parse.rs | 8 + crates/openjd-model/src/template/step.rs | 4 + .../src/template/task_parameters.rs | 20 ++ crates/openjd-model/src/types.rs | 85 +++++++ .../tests/integration/test_caller_limits.rs | 88 ++------ .../integration/test_template_public_api.rs | 12 + crates/openjd-sessions/Cargo.toml | 3 + crates/openjd-sessions/src/action.rs | 16 ++ crates/openjd-sessions/src/action_status.rs | 4 + crates/openjd-sessions/src/embedded_files.rs | 1 + crates/openjd-sessions/src/runner/mod.rs | 12 + crates/openjd-sessions/src/session.rs | 24 +- crates/openjd-sessions/src/session_user.rs | 5 + crates/openjd-sessions/src/subprocess.rs | 4 + crates/openjd-sessions/src/tempdir.rs | 4 + crates/openjd-sessions/src/win32.rs | 5 + .../integration/test_cross_user_windows.rs | 6 + .../tests/integration/test_path_mapping.rs | 12 +- .../test_path_mapping_materialize.rs | 28 +-- .../tests/integration/test_session.rs | 21 +- .../integration/test_session_scenarios.rs | 10 +- .../integration/test_windows_permissions.rs | 3 + crates/openjd-snapshots/Cargo.toml | 3 + crates/openjd-snapshots/src/codec.rs | 7 + crates/openjd-snapshots/src/data_cache.rs | 12 + crates/openjd-snapshots/src/hash.rs | 4 + crates/openjd-snapshots/src/manifest.rs | 31 +++ crates/openjd-snapshots/src/ops/cache_sync.rs | 6 + crates/openjd-snapshots/src/ops/collect.rs | 4 + crates/openjd-snapshots/src/ops/diff.rs | 4 + crates/openjd-snapshots/src/ops/download.rs | 7 + crates/openjd-snapshots/src/ops/hash_op.rs | 6 + .../openjd-snapshots/src/ops/hash_upload.rs | 6 + crates/openjd-snapshots/src/ops/partition.rs | 4 + .../tests/integration/test_compose.rs | 5 +- .../tests/integration/test_download.rs | 22 +- .../tests/integration/test_join.rs | 5 +- .../tests/integration/test_manifest.rs | 13 +- .../tests/integration/test_round_trip.rs | 7 +- specs/model/public-api.md | 18 ++ specs/non-exhaustive-policy.md | 107 +++++++++ 72 files changed, 782 insertions(+), 389 deletions(-) create mode 100644 specs/non-exhaustive-policy.md diff --git a/Cargo.toml b/Cargo.toml index 91b7f7aa..15e372d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,30 @@ authors = ["Amazon Web Services"] keywords = ["openjd", "openjobdescription", "render-farm", "job-template", "rendering"] categories = ["command-line-utilities", "parser-implementations"] +# Workspace lints. Inherited by each crate via `[lints] workspace = true`. +# +# `exhaustive_enums` / `exhaustive_structs` are `restriction`-group lints that +# fire only on *exported* (publicly reachable) types — private and `pub(crate)` +# items never trigger them. Enabling them here makes the SemVer growth question +# an explicit, reviewed decision for every new public type instead of something +# a contributor has to remember. Every existing site is either marked +# `#[non_exhaustive]` or carries an `#[expect(..., reason = "...")]` recording +# why it is intentionally closed. +# +# The rule for which way to go is documented in +# `specs/non-exhaustive-policy.md`. In short: types that mirror a numbered +# OpenJD spec section grow by extension RFC and are marked `#[non_exhaustive]`; +# types representing a decidable logical concept, and runtime state machines a +# consumer must react to, stay closed. +# +# Level is `deny`, not `warn`: a new public type that has not made this +# decision is a hard error at every invocation, not just under CI's +# `-D warnings`. Local `cargo clippy` / `cargo build` fails too, so the +# decision surfaces while the type is being written rather than in CI. +[workspace.lints.clippy] +exhaustive_enums = "deny" +exhaustive_structs = "deny" + [workspace.dependencies] shlex = "2" serde = { version = "1", features = ["derive"] } diff --git a/crates/openjd-cli/Cargo.toml b/crates/openjd-cli/Cargo.toml index 2e7e04e8..3c26ed19 100644 --- a/crates/openjd-cli/Cargo.toml +++ b/crates/openjd-cli/Cargo.toml @@ -37,3 +37,10 @@ windows = { version = "0.62", features = [ "Win32_Foundation", "Win32_System_Console", ] } + +# Exempt: a binary crate: its `pub` items are clap argument structs and internal +# helpers, not a library API any downstream crate can depend on. +# See specs/non-exhaustive-policy.md. +[lints.clippy] +exhaustive_enums = "allow" +exhaustive_structs = "allow" diff --git a/crates/openjd-expr/Cargo.toml b/crates/openjd-expr/Cargo.toml index c3dcde24..da9faa11 100644 --- a/crates/openjd-expr/Cargo.toml +++ b/crates/openjd-expr/Cargo.toml @@ -30,3 +30,6 @@ serde_json = { workspace = true } ruff_python_parser = { package = "rustpython-ruff_python_parser", version = "0.15.8" } ruff_python_ast = { package = "rustpython-ruff_python_ast", version = "0.15.8" } ruff_text_size = { package = "rustpython-ruff_text_size", version = "0.15.8" } + +[lints] +workspace = true diff --git a/crates/openjd-expr/src/eval/evaluator.rs b/crates/openjd-expr/src/eval/evaluator.rs index d0a20f59..162b2839 100644 --- a/crates/openjd-expr/src/eval/evaluator.rs +++ b/crates/openjd-expr/src/eval/evaluator.rs @@ -47,6 +47,10 @@ pub const DEFAULT_OPERATION_LIMIT: usize = 10_000_000; /// Result of expression evaluation. #[derive(Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct EvalResult { pub value: ExprValue, pub peak_memory: usize, diff --git a/crates/openjd-expr/src/format_string.rs b/crates/openjd-expr/src/format_string.rs index b966ebdc..71e858c7 100644 --- a/crates/openjd-expr/src/format_string.rs +++ b/crates/openjd-expr/src/format_string.rs @@ -543,6 +543,10 @@ fn parse_segments(input: &str, profile: &ExprProfile) -> Result, Ex /// Carries the position of the failing interpolation within the format string /// so callers can produce caret-style diagnostics or structured error responses. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct FormatStringValidationError { /// Description of what went wrong (e.g. "Undefined variable 'Param.X'"). pub message: String, diff --git a/crates/openjd-expr/src/function_library.rs b/crates/openjd-expr/src/function_library.rs index 9fdc5971..b05d22d1 100644 --- a/crates/openjd-expr/src/function_library.rs +++ b/crates/openjd-expr/src/function_library.rs @@ -45,6 +45,10 @@ pub type FunctionImpl = Arc< /// A registered function overload. #[derive(Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct FunctionEntry { pub signature: ExprType, // TypeCode::Signature pub implementation: FunctionImpl, diff --git a/crates/openjd-expr/src/path_mapping.rs b/crates/openjd-expr/src/path_mapping.rs index 98f46c8e..77fb46bd 100644 --- a/crates/openjd-expr/src/path_mapping.rs +++ b/crates/openjd-expr/src/path_mapping.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; /// Path format (POSIX, Windows, or URI). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "UPPERCASE")] +#[non_exhaustive] pub enum PathFormat { #[serde(alias = "posix", alias = "Posix")] Posix, @@ -36,6 +37,7 @@ impl PathFormat { /// #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(deny_unknown_fields)] +#[non_exhaustive] pub struct PathMappingRule { pub source_path_format: PathFormat, pub source_path: String, @@ -43,6 +45,32 @@ pub struct PathMappingRule { } impl PathMappingRule { + /// Construct a path mapping rule. + /// + /// Prefer this over a struct literal: `PathMappingRule` is + /// `#[non_exhaustive]`, so literal construction is not available to + /// other crates. + /// + /// # Examples + /// + /// ``` + /// use openjd_expr::{PathFormat, PathMappingRule}; + /// + /// let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "Z:\\shared"); + /// assert_eq!(rule.source_path, "/mnt/shared"); + /// ``` + pub fn new( + source_path_format: PathFormat, + source_path: impl Into, + destination_path: impl Into, + ) -> Self { + Self { + source_path_format, + source_path: source_path.into(), + destination_path: destination_path.into(), + } + } + /// Apply this rule using host-native output separators. /// Equivalent to Python's behavior (uses `os.name` to pick separator). pub fn apply(&self, path: &str) -> Option { diff --git a/crates/openjd-expr/src/profile.rs b/crates/openjd-expr/src/profile.rs index 65aed728..fd2b87e7 100644 --- a/crates/openjd-expr/src/profile.rs +++ b/crates/openjd-expr/src/profile.rs @@ -95,6 +95,7 @@ impl ExprExtension { /// the previous split between `FunctionLibrary::with_host_context` and /// `FunctionLibrary::with_unresolved_host_context`. #[derive(Debug, Clone, Default)] +#[non_exhaustive] pub enum HostContext { /// No host-context functions are registered. Default. #[default] diff --git a/crates/openjd-expr/src/range_expr.rs b/crates/openjd-expr/src/range_expr.rs index 5fc23269..39f0c4c6 100644 --- a/crates/openjd-expr/src/range_expr.rs +++ b/crates/openjd-expr/src/range_expr.rs @@ -33,6 +33,10 @@ pub const MAX_RANGE_EXPR_CHUNKS: usize = 10_000; /// Error raised when parsing a range expression fails. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct RangeExprError { pub expr: String, pub message: String, diff --git a/crates/openjd-expr/src/symbol_table.rs b/crates/openjd-expr/src/symbol_table.rs index ea635afd..5fe63539 100644 --- a/crates/openjd-expr/src/symbol_table.rs +++ b/crates/openjd-expr/src/symbol_table.rs @@ -59,6 +59,10 @@ pub const MAX_SYMBOL_TABLE_ENTRIES: usize = 100_000; /// /// For example, setting `"A.B.C"` when `"A.B"` is already a scalar value. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct SymbolTableError { pub key: String, pub conflict: String, @@ -90,6 +94,10 @@ impl From for crate::error::ExpressionError { /// Entry in a symbol table: either a nested table or a value. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum SymbolTableEntry { Table(SymbolTable), Value(ExprValue), diff --git a/crates/openjd-expr/src/uri_path.rs b/crates/openjd-expr/src/uri_path.rs index 7032b723..bd5f5ee2 100644 --- a/crates/openjd-expr/src/uri_path.rs +++ b/crates/openjd-expr/src/uri_path.rs @@ -10,6 +10,10 @@ /// Parsed URI: authority (`scheme://host`) and path segments. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "internal mechanics, publicly reachable but not a spec surface" +)] pub struct UriParts { pub authority: String, pub path_parts: Vec, diff --git a/crates/openjd-expr/src/value.rs b/crates/openjd-expr/src/value.rs index 8fce1c91..bf778ea1 100644 --- a/crates/openjd-expr/src/value.rs +++ b/crates/openjd-expr/src/value.rs @@ -1399,6 +1399,10 @@ impl From for ExprValue { } /// Zero-allocation iterator over list elements. +#[expect( + clippy::exhaustive_enums, + reason = "structural mirror of another type; not independently extensible" +)] pub enum ListIter<'a> { Bool(std::slice::Iter<'a, bool>), Int(std::slice::Iter<'a, i64>), diff --git a/crates/openjd-expr/tests/integration/test_function_context.rs b/crates/openjd-expr/tests/integration/test_function_context.rs index 491b3a5d..f6ca083e 100644 --- a/crates/openjd-expr/tests/integration/test_function_context.rs +++ b/crates/openjd-expr/tests/integration/test_function_context.rs @@ -177,11 +177,7 @@ fn method_syntax_with_host_context() { // === Path mapping rules === #[test] fn with_path_mapping_rules() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/old".into(), - destination_path: "/new".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/old", "/new"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/old/file.txt".into())) .unwrap(); @@ -199,11 +195,7 @@ fn with_path_mapping_rules() { } #[test] fn unmatched_path_unchanged() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/old".into(), - destination_path: "/new".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/old", "/new"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/other/file.txt".into())) .unwrap(); @@ -334,11 +326,7 @@ fn path_with_suffix_without_host_context() { // === Function-syntax apply_path_mapping with rules === #[test] fn function_syntax_with_path_mapping_rules() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/old/path".into(), - destination_path: "/new/path".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/old/path", "/new/path"); let lib = FunctionLibrary::for_profile( &ExprProfile::current().with_host_context(HostContext::with_rules(vec![rule])), ); @@ -354,11 +342,7 @@ fn function_syntax_with_path_mapping_rules() { } #[test] fn function_syntax_unmatched_path_unchanged() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/specific/path".into(), - destination_path: "/mapped/path".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/specific/path", "/mapped/path"); let lib = FunctionLibrary::for_profile( &ExprProfile::current().with_host_context(HostContext::with_rules(vec![rule])), ); diff --git a/crates/openjd-expr/tests/integration/test_path_mapping.rs b/crates/openjd-expr/tests/integration/test_path_mapping.rs index bf5916df..dac73771 100644 --- a/crates/openjd-expr/tests/integration/test_path_mapping.rs +++ b/crates/openjd-expr/tests/integration/test_path_mapping.rs @@ -36,11 +36,7 @@ fn eval_with_rules_fmt( // === TestPathMappingRuleFromPosix === #[test] fn posix_to_windows_basic() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".to_string(), - destination_path: "Z:\\shared".to_string(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "Z:\\shared"); let mut st = SymbolTable::new(); st.set( "P", @@ -54,11 +50,7 @@ fn posix_to_windows_basic() { // === TestPathMappingRuleValidation === #[test] fn path_mapping_preserves_type() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/old".to_string(), - destination_path: "/new".to_string(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/old", "/new"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/old/file.txt".to_string())) .unwrap(); @@ -69,11 +61,7 @@ fn path_mapping_preserves_type() { // === TestPathMappingRuleFromPosix === #[test] fn posix_exact_match() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/mnt/shared".into())) .unwrap(); @@ -82,11 +70,7 @@ fn posix_exact_match() { } #[test] fn posix_trailing_slash_preserved() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/mnt/shared/".into())) .unwrap(); @@ -95,11 +79,7 @@ fn posix_trailing_slash_preserved() { } #[test] fn posix_no_match_different_path() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/other/path".into())) .unwrap(); @@ -110,11 +90,7 @@ fn posix_no_match_different_path() { #[test] fn unmapped_posix_path_normalized_to_windows_format() { // When no rule matches and format is Windows, separators should be normalized - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/other/path/file.txt".into())) .unwrap(); @@ -129,11 +105,7 @@ fn unmapped_posix_path_normalized_to_windows_format() { #[test] fn posix_no_match_same_prefix() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/mnt/sharedextra/file".into())) .unwrap(); @@ -142,11 +114,7 @@ fn posix_no_match_same_prefix() { } #[test] fn posix_with_subpath() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/shared"); let mut st = SymbolTable::new(); st.set( "P", @@ -160,11 +128,7 @@ fn posix_with_subpath() { // === TestPathMappingRuleFromWindows === #[test] fn windows_with_subpath() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "Z:\\shared".into(), - destination_path: "/mnt/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "Z:\\shared", "/mnt/shared"); let mut st = SymbolTable::new(); st.set( "P", @@ -181,11 +145,7 @@ fn windows_with_subpath() { } #[test] fn windows_exact_match() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "Z:\\shared".into(), - destination_path: "/mnt/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "Z:\\shared", "/mnt/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("Z:\\shared".into())).unwrap(); let r = eval_with_rules_fmt( @@ -198,11 +158,7 @@ fn windows_exact_match() { } #[test] fn windows_no_match() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "Z:\\shared".into(), - destination_path: "/mnt/shared".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "Z:\\shared", "/mnt/shared"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("C:\\other".into())).unwrap(); let r = eval_with_rules_fmt( @@ -217,11 +173,7 @@ fn windows_no_match() { // === TestPathMappingRuleFromUri === #[test] fn uri_with_subpath() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("s3://bucket/prefix/file.txt".into())) .unwrap(); @@ -230,11 +182,7 @@ fn uri_with_subpath() { } #[test] fn uri_nested_subpath() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set( "P", @@ -246,11 +194,7 @@ fn uri_nested_subpath() { } #[test] fn uri_exact_match() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("s3://bucket/prefix".into())) .unwrap(); @@ -259,11 +203,7 @@ fn uri_exact_match() { } #[test] fn uri_no_match_different_bucket() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("s3://other/prefix/file.txt".into())) .unwrap(); @@ -272,11 +212,7 @@ fn uri_no_match_different_bucket() { } #[test] fn uri_no_match_prefix_overlap() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set( "P", @@ -288,11 +224,7 @@ fn uri_no_match_prefix_overlap() { } #[test] fn uri_no_match_different_scheme() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("gs://bucket/prefix/file.txt".into())) .unwrap(); @@ -301,11 +233,7 @@ fn uri_no_match_different_scheme() { } #[test] fn uri_no_match_filesystem() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("/local/file.txt".into())) .unwrap(); @@ -314,11 +242,7 @@ fn uri_no_match_filesystem() { } #[test] fn uri_https_scheme() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "https://host/path".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "https://host/path", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("https://host/path/file.txt".into())) .unwrap(); @@ -327,11 +251,7 @@ fn uri_https_scheme() { } #[test] fn uri_custom_scheme() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "fsx://vol/path".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "fsx://vol/path", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("fsx://vol/path/file.txt".into())) .unwrap(); @@ -340,11 +260,7 @@ fn uri_custom_scheme() { } #[test] fn uri_trailing_slash() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local/data".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local/data"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("s3://bucket/prefix/".into())) .unwrap(); @@ -356,11 +272,7 @@ fn uri_trailing_slash() { #[test] fn posix_trailing_slash_exact_output() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/src".into(), - destination_path: "/dst".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/src", "/dst"); // No trailing slash → no trailing slash assert_eq!( rule.apply_with_format("/src", PathFormat::Posix), @@ -383,11 +295,7 @@ fn posix_trailing_slash_exact_output() { #[test] fn uri_trailing_slash_exact_output() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/prefix".into(), - destination_path: "/local".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/prefix", "/local"); // No trailing slash → no trailing slash assert_eq!( rule.apply_with_format("s3://bucket/prefix", PathFormat::Posix), @@ -421,25 +329,13 @@ mod apply_unit { use openjd_expr::path_mapping::PathFormat; use openjd_expr::PathMappingRule; fn posix_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: src.into(), - destination_path: dst.into(), - } + PathMappingRule::new(PathFormat::Posix, src, dst) } fn windows_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: src.into(), - destination_path: dst.into(), - } + PathMappingRule::new(PathFormat::Windows, src, dst) } fn uri_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: src.into(), - destination_path: dst.into(), - } + PathMappingRule::new(PathFormat::Uri, src, dst) } // ── POSIX ── @@ -805,11 +701,7 @@ mod serde_tests { #[test] fn posix_to_dict() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/new/prefix".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/mnt/shared", "/new/prefix"); let val = serde_json::to_value(&rule).unwrap(); assert_eq!(val["source_path_format"], "POSIX"); assert_eq!(val["source_path"], "/mnt/shared"); @@ -829,11 +721,7 @@ mod serde_tests { #[test] fn windows_to_dict() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "C:\\projects".into(), - destination_path: "/mnt/projects".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "C:\\projects", "/mnt/projects"); let val = serde_json::to_value(&rule).unwrap(); assert_eq!(val["source_path_format"], "WINDOWS"); assert_eq!(val["source_path"], "C:\\projects"); @@ -853,11 +741,7 @@ mod serde_tests { #[test] fn uri_to_dict() { - let rule = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/assets".into(), - destination_path: "/local".into(), - }; + let rule = PathMappingRule::new(PathFormat::Uri, "s3://bucket/assets", "/local"); let val = serde_json::to_value(&rule).unwrap(); assert_eq!(val["source_path_format"], "URI"); assert_eq!(val["source_path"], "s3://bucket/assets"); @@ -866,11 +750,7 @@ mod serde_tests { #[test] fn uri_roundtrip_dict() { - let original = PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: "s3://bucket/assets".into(), - destination_path: "/local".into(), - }; + let original = PathMappingRule::new(PathFormat::Uri, "s3://bucket/assets", "/local"); let json = serde_json::to_string(&original).unwrap(); let restored: PathMappingRule = serde_json::from_str(&json).unwrap(); assert_eq!(restored.source_path_format, original.source_path_format); @@ -930,11 +810,7 @@ mod serde_tests { #[test] fn windows_no_match_same_prefix_eval() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "C:\\projects".into(), - destination_path: "/mnt/projects".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "C:\\projects", "/mnt/projects"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("C:\\projects2\\file.txt".into())) .unwrap(); @@ -949,11 +825,7 @@ fn windows_no_match_same_prefix_eval() { #[test] fn windows_trailing_backslash_preserved_eval() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "C:\\projects".into(), - destination_path: "/mnt/projects".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "C:\\projects", "/mnt/projects"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("C:\\projects\\subdir\\".into())) .unwrap(); @@ -968,11 +840,7 @@ fn windows_trailing_backslash_preserved_eval() { #[test] fn windows_trailing_forward_slash_preserved_eval() { - let rule = PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "C:\\projects".into(), - destination_path: "/mnt/projects".into(), - }; + let rule = PathMappingRule::new(PathFormat::Windows, "C:\\projects", "/mnt/projects"); let mut st = SymbolTable::new(); st.set("P", ExprValue::String("C:\\projects\\subdir/".into())) .unwrap(); @@ -988,11 +856,7 @@ fn windows_trailing_forward_slash_preserved_eval() { // === apply_path_mapping only accepts string, not path (spec §2.2.6) === #[test] fn apply_path_mapping_rejects_path_input() { - let rule = PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/src".into(), - destination_path: "/dst".into(), - }; + let rule = PathMappingRule::new(PathFormat::Posix, "/src", "/dst"); let mut st = SymbolTable::new(); st.set("P", ExprValue::new_path("/src/file.txt", PathFormat::Posix)) .unwrap(); @@ -1016,11 +880,7 @@ mod uri_case_sensitivity { use openjd_expr::{PathFormat, PathMappingRule}; fn uri_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: src.into(), - destination_path: dst.into(), - } + PathMappingRule::new(PathFormat::Uri, src, dst) } #[test] diff --git a/crates/openjd-expr/tests/integration/test_path_mapping_platform.rs b/crates/openjd-expr/tests/integration/test_path_mapping_platform.rs index f3116c08..4ba3f65b 100644 --- a/crates/openjd-expr/tests/integration/test_path_mapping_platform.rs +++ b/crates/openjd-expr/tests/integration/test_path_mapping_platform.rs @@ -13,27 +13,15 @@ use openjd_expr::path_mapping::{ }; fn posix_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: src.to_string(), - destination_path: dst.to_string(), - } + PathMappingRule::new(PathFormat::Posix, src, dst) } fn windows_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: src.to_string(), - destination_path: dst.to_string(), - } + PathMappingRule::new(PathFormat::Windows, src, dst) } fn uri_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Uri, - source_path: src.to_string(), - destination_path: dst.to_string(), - } + PathMappingRule::new(PathFormat::Uri, src, dst) } // ========================================================================= diff --git a/crates/openjd-for-js/Cargo.toml b/crates/openjd-for-js/Cargo.toml index 4fc8a16c..927a8351 100644 --- a/crates/openjd-for-js/Cargo.toml +++ b/crates/openjd-for-js/Cargo.toml @@ -26,3 +26,10 @@ js-sys = "0.3" [dependencies.web-sys] version = "0.3" features = ["console"] + +# Exempt: `publish = false` WASM bindings whose Rust surface is an implementation +# detail of the npm package; JS callers use plain-object literals. +# See specs/non-exhaustive-policy.md. +[lints.clippy] +exhaustive_enums = "allow" +exhaustive_structs = "allow" diff --git a/crates/openjd-for-js/src/expr.rs b/crates/openjd-for-js/src/expr.rs index 3ecb1ecd..4d8afaaa 100644 --- a/crates/openjd-for-js/src/expr.rs +++ b/crates/openjd-for-js/src/expr.rs @@ -157,6 +157,9 @@ impl JsPathFormat { openjd_expr::PathFormat::Posix => JsPathFormat::Posix, openjd_expr::PathFormat::Windows => JsPathFormat::Windows, openjd_expr::PathFormat::Uri => JsPathFormat::Uri, + // `PathFormat` is `#[non_exhaustive]`; POSIX is the safe default + // for any future path flavor the JS binding does not yet model. + _ => JsPathFormat::Posix, } } } @@ -178,11 +181,11 @@ impl JsPathMappingRule { dest_path: &str, ) -> JsPathMappingRule { JsPathMappingRule { - inner: openjd_expr::PathMappingRule { - source_path_format: source_format.into_inner(), - source_path: source_path.to_string(), - destination_path: dest_path.to_string(), - }, + inner: openjd_expr::PathMappingRule::new( + source_format.into_inner(), + source_path, + dest_path, + ), } } } diff --git a/crates/openjd-for-js/src/model.rs b/crates/openjd-for-js/src/model.rs index 38c8822f..6f2dad9c 100644 --- a/crates/openjd-for-js/src/model.rs +++ b/crates/openjd-for-js/src/model.rs @@ -222,14 +222,26 @@ impl JsCallerLimits { /// Convert to the Rust-side struct for a call into /// `openjd_model`. Cheap — copies six `Option` scalars. pub fn as_rust(&self) -> openjd_model::CallerLimits { - openjd_model::CallerLimits { - max_step_count: self.max_step_count, - max_env_count: self.max_env_count, - max_task_count: self.max_task_count, - max_step_script_size: self.max_step_script_size, - max_environment_size: self.max_environment_size, - max_template_size: self.max_template_size, + let mut limits = openjd_model::CallerLimits::new(); + if let Some(v) = self.max_step_count { + limits = limits.with_max_step_count(v); } + if let Some(v) = self.max_env_count { + limits = limits.with_max_env_count(v); + } + if let Some(v) = self.max_task_count { + limits = limits.with_max_task_count(v); + } + if let Some(v) = self.max_step_script_size { + limits = limits.with_max_step_script_size(v); + } + if let Some(v) = self.max_environment_size { + limits = limits.with_max_environment_size(v); + } + if let Some(v) = self.max_template_size { + limits = limits.with_max_template_size(v); + } + limits } /// Deserialize a JS value into `JsCallerLimits`. Returns `None` diff --git a/crates/openjd-model/Cargo.toml b/crates/openjd-model/Cargo.toml index f41746e6..b8a32d40 100644 --- a/crates/openjd-model/Cargo.toml +++ b/crates/openjd-model/Cargo.toml @@ -23,3 +23,6 @@ regex = { workspace = true } [dev-dependencies] tempfile = "3" + +[lints] +workspace = true diff --git a/crates/openjd-model/src/error.rs b/crates/openjd-model/src/error.rs index 384b84cf..4fc2537d 100644 --- a/crates/openjd-model/src/error.rs +++ b/crates/openjd-model/src/error.rs @@ -78,6 +78,10 @@ impl From for ModelError { /// An element in a validation error path. #[derive(Debug, Clone, PartialEq, Eq)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum PathElement { Field(String), Index(usize), @@ -85,6 +89,10 @@ pub enum PathElement { /// A single validation error with its location in the template. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct ValidationError { /// Location of the error in the template structure (e.g., which field /// in the JSON/YAML tree). Used by consumers to navigate to or annotate @@ -101,6 +109,10 @@ pub struct ValidationError { /// Structured diagnostic data for a validation error. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct ErrorDetail { /// Human-readable error summary without source pointers. pub summary: String, @@ -111,6 +123,10 @@ pub struct ErrorDetail { /// A diagnostic span identifying a specific character range in source text. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct DiagnosticSpan { /// Human-readable description of the diagnostic at this source location. pub summary: String, diff --git a/crates/openjd-model/src/job/create_job/parameters.rs b/crates/openjd-model/src/job/create_job/parameters.rs index c2655afc..c32ac4ff 100644 --- a/crates/openjd-model/src/job/create_job/parameters.rs +++ b/crates/openjd-model/src/job/create_job/parameters.rs @@ -783,6 +783,10 @@ fn value_matches_type(value: &openjd_expr::ExprValue, param_type: JobParameterTy } /// Options controlling how PATH parameters are resolved in [`preprocess_job_parameters`]. +#[expect( + clippy::exhaustive_structs, + reason = "internal mechanics, publicly reachable but not a spec surface" +)] pub struct PathParameterOptions<'a> { /// Directory containing the job template. Relative PATH defaults are joined to this. pub job_template_dir: &'a str, @@ -1079,7 +1083,9 @@ fn normalize_path_str(path: &str, format: openjd_expr::path_mapping::PathFormat) use openjd_expr::path_mapping::PathFormat; let sep = match format { PathFormat::Windows => '\\', - PathFormat::Posix | PathFormat::Uri => '/', + // `PathFormat` is `#[non_exhaustive]`; POSIX-style `/` is the correct + // default for any future non-Windows path flavor. + _ => '/', }; // Detect and preserve the root prefix @@ -1128,7 +1134,9 @@ fn path_is_within(path: &str, base: &str, format: openjd_expr::path_mapping::Pat use openjd_expr::path_mapping::PathFormat; let sep = match format { PathFormat::Windows => '\\', - PathFormat::Posix | PathFormat::Uri => '/', + // `PathFormat` is `#[non_exhaustive]`; POSIX-style `/` is the correct + // default for any future non-Windows path flavor. + _ => '/', }; // Exact match — the path resolves to the base directory itself. if path == base { diff --git a/crates/openjd-model/src/job/mod.rs b/crates/openjd-model/src/job/mod.rs index fe27628e..bc4f15ab 100644 --- a/crates/openjd-model/src/job/mod.rs +++ b/crates/openjd-model/src/job/mod.rs @@ -21,6 +21,25 @@ //! `to_bits()` after normalizing `-0.0` to `0.0`, consistent with //! `-0.0 == 0.0`. Types with `f64` fields implement `PartialEq` but not //! `Eq`. +//! +//! # Exhaustiveness +//! +//! Every type in this module is deliberately left exhaustive (no +//! `#[non_exhaustive]`), unlike the `crate::template` types. These are the +//! resolved, post-decode model: the `openjd-sessions` runtime constructs and +//! exhaustively matches them across the crate boundary, and the decode-time +//! extension allowlist no longer stands between a new field and the code +//! (extensions are already resolved by the time a value here exists). A new +//! field is therefore runtime behavior the runner must handle — as +//! `WRAP_ACTIONS` added `on_wrap_env_enter` to `EnvironmentActions` — so a +//! compile error is the desired signal rather than a silently ignored `_` +//! arm. Same rationale as the runtime state machines. See +//! `specs/non-exhaustive-policy.md`. +#![expect( + clippy::exhaustive_structs, + clippy::exhaustive_enums, + reason = "see the module-level Exhaustiveness note above" +)] pub mod create_job; pub mod step_dependency_graph; diff --git a/crates/openjd-model/src/job/step_dependency_graph.rs b/crates/openjd-model/src/job/step_dependency_graph.rs index 21bf3cec..f8cb01e8 100644 --- a/crates/openjd-model/src/job/step_dependency_graph.rs +++ b/crates/openjd-model/src/job/step_dependency_graph.rs @@ -13,6 +13,10 @@ type NodeIndex = usize; /// A step-to-step dependency edge. #[derive(Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct StepDependencyEdge { /// Index of the step that is depended upon. pub origin: NodeIndex, @@ -22,6 +26,10 @@ pub struct StepDependencyEdge { /// A node in the dependency graph. #[derive(Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct StepDependencyNode { /// The step this node represents (index into job.steps). pub step_index: usize, diff --git a/crates/openjd-model/src/template/actions.rs b/crates/openjd-model/src/template/actions.rs index c80023f6..992fb98d 100644 --- a/crates/openjd-model/src/template/actions.rs +++ b/crates/openjd-model/src/template/actions.rs @@ -10,6 +10,7 @@ use serde::Deserialize; /// §5 Action #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct Action { pub command: FormatString, pub args: Option>, @@ -28,6 +29,7 @@ pub struct Action { /// rationale, and openjd-specifications Template Schemas §5.3 / RFC 0008 /// "Cancelation behavior" for the normative rules. #[derive(Debug, Clone)] +#[non_exhaustive] pub enum CancelationMode { /// §5.3.1 — immediate termination, no extra fields allowed. Terminate, @@ -128,6 +130,7 @@ impl<'de> Deserialize<'de> for CancelationMode { /// §3.5.1 StepActions #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StepActions { pub on_run: Action, } @@ -135,6 +138,7 @@ pub struct StepActions { /// §4.1 EnvironmentActions #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct EnvironmentActions { pub on_enter: Option, /// RFC 0008 — wraps inner environments' `onEnter` actions. Requires the @@ -152,6 +156,10 @@ pub struct EnvironmentActions { /// RFC 0008: the per-hook companion template variable a wrap hook exposes /// in addition to `WrappedAction.*`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum WrapHookScope { /// `WrappedEnv.Name` — available in `onWrapEnvEnter` and `onWrapEnvExit`. EnvName, diff --git a/crates/openjd-model/src/template/constrained_strings.rs b/crates/openjd-model/src/template/constrained_strings.rs index 9a19893c..836f55d5 100644 --- a/crates/openjd-model/src/template/constrained_strings.rs +++ b/crates/openjd-model/src/template/constrained_strings.rs @@ -11,6 +11,11 @@ use std::sync::LazyLock; /// §7.1 Identifier: `[A-Za-z_][A-Za-z0-9_]*`, length 1..=512 (64 base, 512 with FEATURE_BUNDLE_1) #[derive(Debug, Clone, PartialEq, Eq, Hash)] +// `#[allow]`, not `#[expect]`: the lint fires only in test-enabled builds of +// this crate, so an `#[expect]` would be unfulfilled in the plain lib build. +// Same rationale as the other newtypes here — a single-field wrapper cannot +// gain a second field. See specs/non-exhaustive-policy.md. +#[allow(clippy::exhaustive_structs)] pub struct Identifier(pub String); static IDENTIFIER_RE: LazyLock = @@ -58,6 +63,12 @@ impl std::fmt::Display for Identifier { /// §7.2 Description: any unicode except Cc category, length 0..=2048 #[derive(Debug, Clone, PartialEq, Eq)] +#[expect( + clippy::exhaustive_structs, + reason = "single-field wrapper over one validated value; it cannot gain a second \ + field without ceasing to be a newtype, and callers construct and \ + destructure it directly" +)] pub struct Description(pub String); impl Description { @@ -93,6 +104,12 @@ impl serde::Serialize for Description { /// §1.1.2 ExtensionName: `[A-Z_0-9]{3,128}` #[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[expect( + clippy::exhaustive_structs, + reason = "single-field wrapper over one validated value; it cannot gain a second \ + field without ceasing to be a newtype, and callers construct and \ + destructure it directly" +)] pub struct ExtensionName(pub String); static EXTENSION_NAME_RE: LazyLock = diff --git a/crates/openjd-model/src/template/environment.rs b/crates/openjd-model/src/template/environment.rs index 20abd675..206ec7ce 100644 --- a/crates/openjd-model/src/template/environment.rs +++ b/crates/openjd-model/src/template/environment.rs @@ -14,6 +14,7 @@ use std::collections::HashMap; /// §4 Environment #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct Environment { pub name: String, pub description: Option, @@ -24,6 +25,7 @@ pub struct Environment { /// §4.1 EnvironmentScript #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct EnvironmentScript { #[serde(rename = "let")] pub let_bindings: Option>, @@ -34,6 +36,7 @@ pub struct EnvironmentScript { /// §6 EmbeddedFile #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct EmbeddedFile { pub name: String, #[serde(rename = "type")] diff --git a/crates/openjd-model/src/template/environment_template.rs b/crates/openjd-model/src/template/environment_template.rs index 9bb2a3d4..285a1d63 100644 --- a/crates/openjd-model/src/template/environment_template.rs +++ b/crates/openjd-model/src/template/environment_template.rs @@ -12,6 +12,7 @@ use serde::Deserialize; /// §1.2 EnvironmentTemplate #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct EnvironmentTemplate { pub specification_version: String, pub extensions: Option>, diff --git a/crates/openjd-model/src/template/expr_parameters.rs b/crates/openjd-model/src/template/expr_parameters.rs index 26179209..03301299 100644 --- a/crates/openjd-model/src/template/expr_parameters.rs +++ b/crates/openjd-model/src/template/expr_parameters.rs @@ -15,6 +15,7 @@ use serde::Deserialize; /// User interface definition for BOOL parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct BoolUserInterface { pub control: Option, pub label: Option, @@ -24,6 +25,7 @@ pub struct BoolUserInterface { /// User interface definition for RANGE_EXPR parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct RangeExprUserInterface { pub control: Option, pub label: Option, @@ -33,6 +35,7 @@ pub struct RangeExprUserInterface { /// User interface definition for `LIST[STRING]` and `LIST[BOOL]` parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListSimpleUserInterface { pub control: Option, pub label: Option, @@ -42,6 +45,7 @@ pub struct ListSimpleUserInterface { /// User interface definition for `LIST[PATH]` parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListPathUserInterface { pub control: Option, pub label: Option, @@ -53,6 +57,7 @@ pub struct ListPathUserInterface { /// User interface definition for `LIST[INT]` parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListIntUserInterface { pub control: Option, pub label: Option, @@ -63,6 +68,7 @@ pub struct ListIntUserInterface { /// User interface definition for `LIST[FLOAT]` parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListFloatUserInterface { pub control: Option, pub label: Option, @@ -74,6 +80,7 @@ pub struct ListFloatUserInterface { /// User interface definition for `LIST[LIST[INT]]` parameters (HIDDEN only). #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct HiddenOnlyUserInterface { pub control: Option, pub label: Option, @@ -83,6 +90,7 @@ pub struct HiddenOnlyUserInterface { /// §2.9 JobBoolParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobBoolParameterDefinition { pub name: Identifier, pub description: Option, @@ -173,6 +181,7 @@ impl JobBoolParameterDefinition { /// §2.10 JobRangeExprParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobRangeExprParameterDefinition { pub name: Identifier, pub description: Option, @@ -255,6 +264,7 @@ impl JobRangeExprParameterDefinition { /// §2.11 JobListStringParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobListStringParameterDefinition { pub name: Identifier, pub description: Option, @@ -267,6 +277,7 @@ pub struct JobListStringParameterDefinition { #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListStringItemConstraints { pub allowed_values: Option>, pub min_length: Option, @@ -325,6 +336,7 @@ impl JobListStringParameterDefinition { /// §2.12 JobListPathParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobListPathParameterDefinition { pub name: Identifier, pub description: Option, @@ -394,6 +406,7 @@ impl JobListPathParameterDefinition { /// §2.13 JobListIntParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobListIntParameterDefinition { pub name: Identifier, pub description: Option, @@ -406,6 +419,7 @@ pub struct JobListIntParameterDefinition { #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListIntItemConstraints { pub allowed_values: Option>, pub min_value: Option, @@ -464,6 +478,7 @@ impl JobListIntParameterDefinition { /// §2.14 JobListFloatParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobListFloatParameterDefinition { pub name: Identifier, pub description: Option, @@ -476,6 +491,7 @@ pub struct JobListFloatParameterDefinition { #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListFloatItemConstraints { pub allowed_values: Option>, pub min_value: Option, @@ -576,6 +592,7 @@ impl JobListFloatParameterDefinition { /// §2.15 JobListBoolParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobListBoolParameterDefinition { pub name: Identifier, pub description: Option, @@ -631,6 +648,7 @@ impl JobListBoolParameterDefinition { /// §2.16 JobListListIntParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobListListIntParameterDefinition { pub name: Identifier, pub description: Option, @@ -643,6 +661,7 @@ pub struct JobListListIntParameterDefinition { #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ListListIntItemConstraints { pub min_length: Option, pub max_length: Option, diff --git a/crates/openjd-model/src/template/host_requirements.rs b/crates/openjd-model/src/template/host_requirements.rs index 5d6729b8..5a78c4a4 100644 --- a/crates/openjd-model/src/template/host_requirements.rs +++ b/crates/openjd-model/src/template/host_requirements.rs @@ -10,6 +10,7 @@ use serde::Deserialize; /// §3.3 HostRequirements #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct HostRequirements { pub amounts: Option>, pub attributes: Option>, @@ -18,6 +19,7 @@ pub struct HostRequirements { /// §3.3.1 AmountRequirement #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct AmountRequirement { pub name: String, pub min: Option, @@ -27,6 +29,7 @@ pub struct AmountRequirement { /// §3.3.2 AttributeRequirement #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct AttributeRequirement { pub name: String, pub any_of: Option>, diff --git a/crates/openjd-model/src/template/job_template.rs b/crates/openjd-model/src/template/job_template.rs index 7ebbe56c..176c3b72 100644 --- a/crates/openjd-model/src/template/job_template.rs +++ b/crates/openjd-model/src/template/job_template.rs @@ -14,6 +14,7 @@ use serde::Deserialize; /// §1.1 JobTemplate #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobTemplate { pub specification_version: String, #[serde(rename = "$schema")] diff --git a/crates/openjd-model/src/template/parameters.rs b/crates/openjd-model/src/template/parameters.rs index 42c36548..450853c0 100644 --- a/crates/openjd-model/src/template/parameters.rs +++ b/crates/openjd-model/src/template/parameters.rs @@ -55,6 +55,7 @@ impl<'de, T: serde::de::DeserializeOwned> Deserialize<'de> for NullableVec { /// types are available (BOOL, RANGE_EXPR, `LIST[*]`). #[derive(Debug, Clone)] #[allow(non_camel_case_types)] +#[non_exhaustive] pub enum JobParameterDefinition { STRING(JobStringParameterDefinition), INT(JobIntParameterDefinition), @@ -542,6 +543,7 @@ impl JobParameterDefinition { /// User interface definition for STRING parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StringUserInterface { pub control: Option, pub label: Option, @@ -551,6 +553,7 @@ pub struct StringUserInterface { /// User interface definition for INT parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct IntUserInterface { pub control: Option, pub label: Option, @@ -561,6 +564,7 @@ pub struct IntUserInterface { /// User interface definition for FLOAT parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct FloatUserInterface { pub control: Option, pub label: Option, @@ -572,6 +576,7 @@ pub struct FloatUserInterface { /// User interface definition for PATH parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct PathUserInterface { pub control: Option, pub label: Option, @@ -583,6 +588,7 @@ pub struct PathUserInterface { /// §2.7 JobPathParameterFileFilter #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct FileFilter { pub label: String, pub patterns: Vec, @@ -617,6 +623,7 @@ pub(crate) fn validate_ui_label( /// §2.1 JobStringParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobStringParameterDefinition { pub name: Identifier, pub description: Option, @@ -822,6 +829,12 @@ impl JobStringParameterDefinition { /// An `i64` that deserializes from YAML integers, integer-valued floats (e.g. `42.0`), or /// numeric strings (e.g. `"42"`). Rejects booleans, nulls, and non-integer floats. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "single-field wrapper over one validated value; it cannot gain a second \ + field without ceasing to be a newtype, and callers construct and \ + destructure it directly" +)] pub struct FlexInt(pub i64); impl<'de> Deserialize<'de> for FlexInt { @@ -867,6 +880,12 @@ impl std::fmt::Display for FlexInt { /// Preserves the original string representation when parsed from a string, which is needed /// for round-trip fidelity in constraint checking. Rejects NaN, Infinity, booleans, and nulls. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed pair: a validated value plus the original string representation it \ + was parsed from, kept for round-trip fidelity. Both fields are inherent \ + to what the type is, and callers construct and destructure it directly" +)] pub struct FlexFloat(pub f64, pub Option); /// Reject NaN and Infinity float values. @@ -924,6 +943,7 @@ impl std::fmt::Display for FlexFloat { /// §2.3 JobIntParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobIntParameterDefinition { pub name: Identifier, pub description: Option, @@ -1066,6 +1086,7 @@ impl JobIntParameterDefinition { /// §2.4 JobFloatParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobFloatParameterDefinition { pub name: Identifier, pub description: Option, @@ -1240,6 +1261,7 @@ impl JobFloatParameterDefinition { /// §2.2 JobPathParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct JobPathParameterDefinition { pub name: Identifier, pub description: Option, diff --git a/crates/openjd-model/src/template/parse.rs b/crates/openjd-model/src/template/parse.rs index 8d038ad1..e12fddd1 100644 --- a/crates/openjd-model/src/template/parse.rs +++ b/crates/openjd-model/src/template/parse.rs @@ -19,6 +19,10 @@ use crate::types::{ /// Document format. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum DocumentType { Json, Yaml, @@ -298,6 +302,10 @@ pub fn decode_environment_template( // Both variants are large structs only used as return values, not stored in collections. #[allow(clippy::large_enum_variant)] #[derive(Debug)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum DecodedTemplate { Job(JobTemplate), Environment(EnvironmentTemplate), diff --git a/crates/openjd-model/src/template/step.rs b/crates/openjd-model/src/template/step.rs index 33649072..b9c1f7cf 100644 --- a/crates/openjd-model/src/template/step.rs +++ b/crates/openjd-model/src/template/step.rs @@ -16,6 +16,7 @@ use serde::Deserialize; /// Allows specifying a script interpreter directly instead of a full StepScript. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct SimpleAction { /// Let bindings evaluated once per task (requires EXPR extension). #[serde(rename = "let")] @@ -33,6 +34,7 @@ pub struct SimpleAction { /// §3 StepTemplate #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StepTemplate { pub name: String, pub description: Option, @@ -129,6 +131,7 @@ impl StepTemplate { /// §3.2 StepDependency #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StepDependency { pub depends_on: String, } @@ -136,6 +139,7 @@ pub struct StepDependency { /// §3.5 StepScript #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StepScript { #[serde(rename = "let")] pub let_bindings: Option>, diff --git a/crates/openjd-model/src/template/task_parameters.rs b/crates/openjd-model/src/template/task_parameters.rs index 23d1f6ec..55b4010a 100644 --- a/crates/openjd-model/src/template/task_parameters.rs +++ b/crates/openjd-model/src/template/task_parameters.rs @@ -13,6 +13,7 @@ use serde::Deserialize; #[derive(Debug, Clone, Deserialize)] #[serde(tag = "type")] #[allow(non_camel_case_types)] +#[non_exhaustive] pub enum TaskParameterDefinition { INT(IntTaskParameterDefinition), FLOAT(FloatTaskParameterDefinition), @@ -47,6 +48,7 @@ impl TaskParameterDefinition { /// Int range: either a list of values or a range expression string. #[derive(Debug, Clone)] +#[non_exhaustive] pub enum IntRange { List(Vec), Expression(FormatString), @@ -77,6 +79,7 @@ impl<'de> Deserialize<'de> for IntRange { /// Concrete types to avoid derive conflicts with FormatString. #[derive(Debug, Clone)] +#[non_exhaustive] pub enum StringRange { List(Vec), Expression(FormatString), @@ -103,6 +106,7 @@ impl<'de> Deserialize<'de> for StringRange { /// A float range list item: either a literal float or a format string. #[derive(Debug, Clone)] +#[non_exhaustive] pub enum FloatRangeItem { Float(f64), FormatString(FormatString), @@ -130,6 +134,7 @@ impl<'de> Deserialize<'de> for FloatRangeItem { } #[derive(Debug, Clone)] +#[non_exhaustive] pub enum FloatRange { List(Vec), Expression(FormatString), @@ -157,6 +162,7 @@ impl<'de> Deserialize<'de> for FloatRange { /// §3.4.1.1 IntTaskParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct IntTaskParameterDefinition { pub name: Identifier, pub range: IntRange, @@ -165,6 +171,7 @@ pub struct IntTaskParameterDefinition { /// §3.4.1.2 FloatTaskParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct FloatTaskParameterDefinition { pub name: Identifier, pub range: FloatRange, @@ -173,6 +180,7 @@ pub struct FloatTaskParameterDefinition { /// §3.4.1.3 StringTaskParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StringTaskParameterDefinition { pub name: Identifier, pub range: StringRange, @@ -181,6 +189,7 @@ pub struct StringTaskParameterDefinition { /// §3.4.1.4 PathTaskParameterDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct PathTaskParameterDefinition { pub name: Identifier, pub range: StringRange, @@ -189,6 +198,7 @@ pub struct PathTaskParameterDefinition { /// §3.4.1.5 ChunkIntTaskParameterDefinition (TASK_CHUNKING extension) #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ChunkIntTaskParameterDefinition { pub name: Identifier, pub range: IntRange, @@ -203,6 +213,10 @@ pub struct ChunkIntTaskParameterDefinition { /// - String containing `{{…}}` → `IntOrFormatString::FormatString(fs)` /// - Boolean/null → error #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum IntOrFormatString { Int(i64), FormatString(FormatString), @@ -262,6 +276,7 @@ impl<'de> Deserialize<'de> for IntOrFormatString { /// Chunks configuration for `CHUNK[INT]` parameters. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct ChunksDefinition { pub default_task_count: IntOrFormatString, pub target_runtime_seconds: Option, @@ -270,6 +285,10 @@ pub struct ChunksDefinition { #[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, serde::Serialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum RangeConstraint { Contiguous, Noncontiguous, @@ -278,6 +297,7 @@ pub enum RangeConstraint { /// §3.4 StepParameterSpaceDefinition #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] +#[non_exhaustive] pub struct StepParameterSpaceDefinition { pub task_parameter_definitions: Vec, pub combination: Option, diff --git a/crates/openjd-model/src/types.rs b/crates/openjd-model/src/types.rs index 2c8a7ba2..c67ace5a 100644 --- a/crates/openjd-model/src/types.rs +++ b/crates/openjd-model/src/types.rs @@ -40,6 +40,10 @@ impl fmt::Display for FileType { /// End-of-line mode for embedded files (FEATURE_BUNDLE_1). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum EndOfLine { Lf, Crlf, @@ -59,6 +63,10 @@ impl fmt::Display for EndOfLine { /// §2.2 PATH parameter objectType. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum ObjectType { File, Directory, @@ -76,6 +84,10 @@ impl fmt::Display for ObjectType { /// §2.2 PATH parameter dataFlow. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum DataFlow { None, In, @@ -314,6 +326,10 @@ impl fmt::Display for TaskParameterType { /// A processed job parameter value. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct JobParameterValue { pub param_type: JobParameterType, pub value: openjd_expr::ExprValue, @@ -321,6 +337,10 @@ pub struct JobParameterValue { /// A processed task parameter value. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct TaskParameterValue { pub param_type: TaskParameterType, pub value: openjd_expr::ExprValue, @@ -416,6 +436,7 @@ impl serde::Serialize for ModelExtension { /// /// Caller limits can only add restrictions, never relax spec-defined ones. #[derive(Debug, Clone, Default)] +#[non_exhaustive] pub struct CallerLimits { /// Maximum number of steps in a job template. pub max_step_count: Option, @@ -433,6 +454,69 @@ pub struct CallerLimits { pub max_template_size: Option, } +impl CallerLimits { + /// Start from "no additional restrictions" and set limits with the + /// `with_*` methods. + /// + /// `CallerLimits` is `#[non_exhaustive]` — new limits are added as the + /// specification grows — so other crates cannot use a struct literal. + /// This builder is the supported construction path. + /// + /// # Examples + /// + /// ``` + /// use openjd_model::CallerLimits; + /// + /// let limits = CallerLimits::new().with_max_step_count(50).with_max_task_count(10_000); + /// assert_eq!(limits.max_step_count, Some(50)); + /// ``` + pub fn new() -> Self { + Self::default() + } + + /// Set the maximum number of steps in a job template. + #[must_use] + pub fn with_max_step_count(mut self, v: usize) -> Self { + self.max_step_count = Some(v); + self + } + + /// Set the maximum number of environments (job + all step environments). + #[must_use] + pub fn with_max_env_count(mut self, v: usize) -> Self { + self.max_env_count = Some(v); + self + } + + /// Set the maximum total task count across all steps. + #[must_use] + pub fn with_max_task_count(mut self, v: u64) -> Self { + self.max_task_count = Some(v); + self + } + + /// Set the maximum JSON-encoded size of a step script, in bytes. + #[must_use] + pub fn with_max_step_script_size(mut self, v: usize) -> Self { + self.max_step_script_size = Some(v); + self + } + + /// Set the maximum JSON-encoded size of an environment, in bytes. + #[must_use] + pub fn with_max_environment_size(mut self, v: usize) -> Self { + self.max_environment_size = Some(v); + self + } + + /// Set the maximum total template document size, in bytes. + #[must_use] + pub fn with_max_template_size(mut self, v: usize) -> Self { + self.max_template_size = Some(v); + self + } +} + /// Model-side profile: the specification revision plus the set of /// enabled extensions that together describe what features a template /// or job may use. @@ -573,6 +657,7 @@ impl Default for ModelProfile { /// extensions) and the caller's policy overrides. When only the /// profile is needed, take `&ModelProfile` directly. #[derive(Debug, Clone)] +#[non_exhaustive] pub struct ValidationContext { pub profile: ModelProfile, pub caller_limits: CallerLimits, diff --git a/crates/openjd-model/tests/integration/test_caller_limits.rs b/crates/openjd-model/tests/integration/test_caller_limits.rs index 0fff9b55..710c910d 100644 --- a/crates/openjd-model/tests/integration/test_caller_limits.rs +++ b/crates/openjd-model/tests/integration/test_caller_limits.rs @@ -132,10 +132,7 @@ fn template_with_two_steps_param_spaces(range1: usize, range2: usize) -> String #[test] fn max_step_count_within_limit() { - let limits = CallerLimits { - max_step_count: Some(3), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_step_count(3); let v = yaml_val(&minimal_template(3)); let result = decode_job_template(v, None, &limits); assert!( @@ -147,10 +144,7 @@ fn max_step_count_within_limit() { #[test] fn max_step_count_exceeded() { - let limits = CallerLimits { - max_step_count: Some(2), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_step_count(2); let v = yaml_val(&minimal_template(3)); let err = decode_job_template(v, None, &limits).unwrap_err(); let msg = err.to_string(); @@ -185,10 +179,7 @@ fn decode_job_template_without_limits_still_works() { #[test] fn max_env_count_within_limit() { - let limits = CallerLimits { - max_env_count: Some(5), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_env_count(5); // 2 job envs + 2 step envs = 4 total let v = yaml_val(&template_with_envs(2, 2)); let result = decode_job_template(v, None, &limits); @@ -201,10 +192,7 @@ fn max_env_count_within_limit() { #[test] fn max_env_count_exceeded() { - let limits = CallerLimits { - max_env_count: Some(3), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_env_count(3); // 2 job envs + 2 step envs = 4 total let v = yaml_val(&template_with_envs(2, 2)); let err = decode_job_template(v, None, &limits).unwrap_err(); @@ -217,10 +205,7 @@ fn max_env_count_exceeded() { #[test] fn max_env_count_counts_job_and_step_envs_together() { - let limits = CallerLimits { - max_env_count: Some(4), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_env_count(4); // Exactly 4 total (2 + 2) — should pass let v = yaml_val(&template_with_envs(2, 2)); let result = decode_job_template(v, None, &limits); @@ -233,10 +218,7 @@ fn max_env_count_counts_job_and_step_envs_together() { #[test] fn max_task_count_within_limit() { - let limits = CallerLimits { - max_task_count: Some(100), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_task_count(100); let v = yaml_val(&template_with_param_space(50)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); let params = preprocess_job_parameters( @@ -261,10 +243,7 @@ fn max_task_count_within_limit() { #[test] fn max_task_count_exceeded_single_step() { - let limits = CallerLimits { - max_task_count: Some(10), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_task_count(10); let v = yaml_val(&template_with_param_space(20)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); let params = preprocess_job_parameters( @@ -290,10 +269,7 @@ fn max_task_count_exceeded_single_step() { #[test] fn max_task_count_exceeded_across_steps() { - let limits = CallerLimits { - max_task_count: Some(15), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_task_count(15); // 10 + 10 = 20 total tasks let v = yaml_val(&template_with_two_steps_param_spaces(10, 10)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); @@ -341,10 +317,7 @@ fn max_task_count_none_means_no_limit() { #[test] fn max_task_count_step_without_param_space_counts_as_one() { - let limits = CallerLimits { - max_task_count: Some(5), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_task_count(5); // 3 steps with no parameter space = 3 tasks total let v = yaml_val(&minimal_template(3)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); @@ -377,10 +350,7 @@ fn max_task_count_counts_tasks_not_chunks() { // 100 tasks chunked into 1 chunk (defaultTaskCount=100). // The iterator would report len()=1 with default chunking, // but the actual task count is 100 and should exceed a limit of 50. - let limits = CallerLimits { - max_task_count: Some(50), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_task_count(50); let range: Vec = (0..100).map(|i| i.to_string()).collect(); let tmpl = format!( r#"{{ @@ -431,10 +401,7 @@ fn max_task_count_counts_tasks_not_chunks() { fn max_template_size_within_limit() { use openjd_model::template::parse::{document_string_to_object, DocumentType}; let doc = r#"{"specificationVersion": "jobtemplate-2023-09", "name": "T", "steps": [{"name": "S", "script": {"actions": {"onRun": {"command": "echo"}}}}]}"#; - let limits = CallerLimits { - max_template_size: Some(10000), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_template_size(10000); let result = document_string_to_object(doc, DocumentType::Json, &limits); assert!(result.is_ok(), "Within limit: {:?}", result.err()); } @@ -443,10 +410,7 @@ fn max_template_size_within_limit() { fn max_template_size_exceeded() { use openjd_model::template::parse::{document_string_to_object, DocumentType}; let doc = r#"{"specificationVersion": "jobtemplate-2023-09", "name": "T", "steps": [{"name": "S", "script": {"actions": {"onRun": {"command": "echo"}}}}]}"#; - let limits = CallerLimits { - max_template_size: Some(10), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_template_size(10); let err = document_string_to_object(doc, DocumentType::Json, &limits).unwrap_err(); let msg = err.to_string(); assert!( @@ -461,10 +425,7 @@ fn max_template_size_exceeded() { #[test] fn max_step_script_size_within_limit() { - let limits = CallerLimits { - max_step_script_size: Some(100_000), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_step_script_size(100_000); let v = yaml_val(&minimal_template(1)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); let params = preprocess_job_parameters( @@ -485,10 +446,7 @@ fn max_step_script_size_within_limit() { #[test] fn max_step_script_size_exceeded() { - let limits = CallerLimits { - max_step_script_size: Some(1), // impossibly small - ..Default::default() - }; + let limits = CallerLimits::new().with_max_step_script_size(1); let v = yaml_val(&minimal_template(1)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); let params = preprocess_job_parameters( @@ -518,10 +476,7 @@ fn max_step_script_size_exceeded() { #[test] fn max_environment_size_within_limit() { - let limits = CallerLimits { - max_environment_size: Some(100_000), - ..Default::default() - }; + let limits = CallerLimits::new().with_max_environment_size(100_000); let v = yaml_val(&template_with_envs(1, 0)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); let params = preprocess_job_parameters( @@ -542,10 +497,7 @@ fn max_environment_size_within_limit() { #[test] fn max_environment_size_exceeded() { - let limits = CallerLimits { - max_environment_size: Some(1), // impossibly small - ..Default::default() - }; + let limits = CallerLimits::new().with_max_environment_size(1); let v = yaml_val(&template_with_envs(1, 0)); let jt = decode_job_template(v, None, &CallerLimits::default()).unwrap(); let params = preprocess_job_parameters( @@ -590,11 +542,9 @@ fn caller_limits_default_is_all_none() { #[test] fn multiple_caller_limits_all_checked() { - let limits = CallerLimits { - max_step_count: Some(1), - max_env_count: Some(0), - ..Default::default() - }; + let limits = CallerLimits::new() + .with_max_step_count(1) + .with_max_env_count(0); // 2 steps + 1 env — both limits exceeded let v = yaml_val(&template_with_envs(1, 0)); // This template has 1 step and 1 job env — step count is fine (1), but env count (1) > 0 diff --git a/crates/openjd-model/tests/integration/test_template_public_api.rs b/crates/openjd-model/tests/integration/test_template_public_api.rs index 7a11e5ba..33e08f9b 100644 --- a/crates/openjd-model/tests/integration/test_template_public_api.rs +++ b/crates/openjd-model/tests/integration/test_template_public_api.rs @@ -221,6 +221,8 @@ fn step_template_full_surface() { CancelationMode::Terminate | CancelationMode::DeferredMode { .. } => { panic!("expected NotifyThenTerminate") } + #[allow(unreachable_patterns)] + _ => panic!("unexpected cancelation mode variant"), } let ef: &EmbeddedFile = &script.embedded_files.as_ref().unwrap()[0]; @@ -605,6 +607,8 @@ fn task_parameter_definition_int_variant_field_access() { assert_eq!(nums, vec![1, 2, 3]); } IntRange::Expression(_) => panic!("expected List, got Expression"), + #[allow(unreachable_patterns)] + _ => panic!("unexpected IntRange variant"), } } other => panic!("expected INT, got {other:?}"), @@ -639,6 +643,8 @@ fn task_parameter_definition_int_range_expression() { assert_eq!(fs.raw(), "1-10:2"); } IntRange::List(_) => panic!("expected Expression, got List"), + #[allow(unreachable_patterns)] + _ => panic!("unexpected IntRange variant"), }, _ => unreachable!(), } @@ -679,6 +685,8 @@ fn task_parameter_definition_float_variant_field_access() { assert!(matches!(items[2], FloatRangeItem::Float(f) if f == 3.5)); } FloatRange::Expression(_) => panic!("expected List"), + #[allow(unreachable_patterns)] + _ => panic!("unexpected FloatRange variant"), } } _ => unreachable!(), @@ -717,6 +725,8 @@ fn task_parameter_definition_string_path_variants() { assert_eq!(strs, vec!["red", "blue"]); } StringRange::Expression(_) => panic!("expected List"), + #[allow(unreachable_patterns)] + _ => panic!("unexpected StringRange variant"), } } _ => unreachable!(), @@ -730,6 +740,8 @@ fn task_parameter_definition_string_path_variants() { assert_eq!(strs, vec!["/tmp/a", "/tmp/b"]); } StringRange::Expression(_) => panic!("expected List"), + #[allow(unreachable_patterns)] + _ => panic!("unexpected StringRange variant"), } } _ => unreachable!(), diff --git a/crates/openjd-sessions/Cargo.toml b/crates/openjd-sessions/Cargo.toml index 61fd0617..9668b1d6 100644 --- a/crates/openjd-sessions/Cargo.toml +++ b/crates/openjd-sessions/Cargo.toml @@ -92,3 +92,6 @@ windows = { version = "0.62", features = [ "Win32_Security_Authorization", "Win32_System_Threading", ] } + +[lints] +workspace = true diff --git a/crates/openjd-sessions/src/action.rs b/crates/openjd-sessions/src/action.rs index ca944dc6..d9167739 100644 --- a/crates/openjd-sessions/src/action.rs +++ b/crates/openjd-sessions/src/action.rs @@ -14,6 +14,12 @@ /// assert_ne!(state, ActionState::Failed); /// ``` #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect( + clippy::exhaustive_enums, + reason = "runtime state machine: consumers match on these to react, and a new state \ + must be a compile error rather than a silently ignored `_` arm. Not \ + extension-gated." +)] pub enum ActionState { Running, Success, @@ -36,6 +42,12 @@ impl std::fmt::Display for ActionState { /// A parsed openjd stdout message from a running action. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_enums, + reason = "runtime state machine: consumers match on these to react, and a new state \ + must be a compile error rather than a silently ignored `_` arm. Not \ + extension-gated." +)] pub enum ActionMessage { /// `openjd_progress: ` Progress(f64), @@ -71,6 +83,10 @@ impl std::fmt::Display for ActionMessage { /// Result of running an action. #[derive(Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct ActionResult { pub state: ActionState, pub exit_code: Option, diff --git a/crates/openjd-sessions/src/action_status.rs b/crates/openjd-sessions/src/action_status.rs index a60c552f..08ec56c9 100644 --- a/crates/openjd-sessions/src/action_status.rs +++ b/crates/openjd-sessions/src/action_status.rs @@ -9,6 +9,10 @@ use std::time::SystemTime; /// Status of the currently running or most recently completed action. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct ActionStatus { pub state: ActionState, pub progress: Option, diff --git a/crates/openjd-sessions/src/embedded_files.rs b/crates/openjd-sessions/src/embedded_files.rs index 2044e6ba..b920c7d5 100644 --- a/crates/openjd-sessions/src/embedded_files.rs +++ b/crates/openjd-sessions/src/embedded_files.rs @@ -23,6 +23,7 @@ use crate::session_user::SessionUser; /// Scope for embedded file symbol table entries. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] pub enum EmbeddedFilesScope { Step, Env, diff --git a/crates/openjd-sessions/src/runner/mod.rs b/crates/openjd-sessions/src/runner/mod.rs index 71f1becb..0d6b91ba 100644 --- a/crates/openjd-sessions/src/runner/mod.rs +++ b/crates/openjd-sessions/src/runner/mod.rs @@ -40,6 +40,12 @@ use crate::subprocess::{run_subprocess, SubprocessConfig, SubprocessResult}; /// assert!(matches!(method, CancelMethod::NotifyThenTerminate { .. })); /// ``` #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_enums, + reason = "runtime state machine: consumers match on these to react, and a new state \ + must be a compile error rather than a silently ignored `_` arm. Not \ + extension-gated." +)] pub enum CancelMethod { /// Immediately terminate via SIGKILL. Terminate, @@ -60,6 +66,12 @@ impl std::fmt::Display for CancelMethod { /// State of a script runner. #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect( + clippy::exhaustive_enums, + reason = "runtime state machine: consumers match on these to react, and a new state \ + must be a compile error rather than a silently ignored `_` arm. Not \ + extension-gated." +)] pub enum ScriptRunnerState { Ready, Running, diff --git a/crates/openjd-sessions/src/session.rs b/crates/openjd-sessions/src/session.rs index c4e79ef2..98efbe1a 100644 --- a/crates/openjd-sessions/src/session.rs +++ b/crates/openjd-sessions/src/session.rs @@ -66,6 +66,12 @@ use crate::session_user::SessionUser; /// assert_eq!(format!("{state}"), "READY"); /// ``` #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[expect( + clippy::exhaustive_enums, + reason = "runtime state machine: consumers match on these to react, and a new state \ + must be a compile error rather than a silently ignored `_` arm. Not \ + extension-gated." +)] pub enum SessionState { Ready, Running, @@ -93,6 +99,13 @@ pub type EnvironmentIdentifier = String; pub type SessionCallbackType = Box; /// Configuration for creating a new Session. +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration assembled by struct literal (the \ + CLI and downstream callers build it directly), not a spec-mirroring \ + growth axis. Marking it would force `Default` plus field mutation \ + at every call site. See specs/non-exhaustive-policy.md." +)] pub struct SessionConfig { pub session_id: String, pub job_parameter_values: JobParameterValues, @@ -2133,11 +2146,12 @@ impl Session { .iter() .map(|rule| { serde_json::json!({ - "source_path_format": match rule.source_path_format { - openjd_expr::path_mapping::PathFormat::Posix => "POSIX", - openjd_expr::path_mapping::PathFormat::Windows => "WINDOWS", - openjd_expr::path_mapping::PathFormat::Uri => "URI", - }, + // `PathFormat` serializes to its canonical UPPERCASE + // transport name ("POSIX"/"WINDOWS"/"URI"), so defer to + // its `Serialize` impl rather than a hand-rolled match: + // `PathFormat` is `#[non_exhaustive]` and a future + // variant then needs no change here. + "source_path_format": rule.source_path_format, "source_path": &rule.source_path, "destination_path": &rule.destination_path, }) diff --git a/crates/openjd-sessions/src/session_user.rs b/crates/openjd-sessions/src/session_user.rs index 5bb38f4f..47b16d59 100644 --- a/crates/openjd-sessions/src/session_user.rs +++ b/crates/openjd-sessions/src/session_user.rs @@ -19,6 +19,10 @@ pub trait SessionUser: Send + Sync + std::fmt::Debug { /// POSIX session user identity for cross-user execution via sudo. #[cfg(unix)] #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct PosixSessionUser { pub user: String, pub group: String, @@ -78,6 +82,7 @@ impl SessionUser for PosixSessionUser { /// Error for incorrect username or password. #[cfg(windows)] #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum BadCredentialsError { #[error("The username or password is incorrect.")] LogonFailure, diff --git a/crates/openjd-sessions/src/subprocess.rs b/crates/openjd-sessions/src/subprocess.rs index 79be9729..06d5babb 100644 --- a/crates/openjd-sessions/src/subprocess.rs +++ b/crates/openjd-sessions/src/subprocess.rs @@ -68,6 +68,10 @@ pub(crate) fn truncate_line(line: &str) -> &str { /// Result of running a subprocess action. #[derive(Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of a decidable concept" +)] pub struct SubprocessResult { pub state: ActionState, pub exit_code: Option, diff --git a/crates/openjd-sessions/src/tempdir.rs b/crates/openjd-sessions/src/tempdir.rs index 62b18591..b366a3d9 100644 --- a/crates/openjd-sessions/src/tempdir.rs +++ b/crates/openjd-sessions/src/tempdir.rs @@ -15,6 +15,10 @@ use crate::session_user::SessionUser; /// user to rename or delete files belonging to other users. This is a security risk /// for session working directories. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum StickyBitPolicy { /// Refuse to create the session if a parent directory is unsafe. /// This is the default — fail-closed is the secure choice. diff --git a/crates/openjd-sessions/src/win32.rs b/crates/openjd-sessions/src/win32.rs index 38c3e461..97dadfbb 100644 --- a/crates/openjd-sessions/src/win32.rs +++ b/crates/openjd-sessions/src/win32.rs @@ -244,6 +244,11 @@ use windows::Win32::System::Threading::{ }; /// Result of spawning a cross-user process. +#[expect( + clippy::exhaustive_structs, + reason = "fixed tuple of what a spawn returns; callers destructure it to take \ + ownership of the handles. See specs/non-exhaustive-policy.md." +)] pub struct SpawnedProcess { pub process_handle: HANDLE, pub pid: u32, diff --git a/crates/openjd-sessions/tests/integration/test_cross_user_windows.rs b/crates/openjd-sessions/tests/integration/test_cross_user_windows.rs index c5273f67..d2ff5935 100644 --- a/crates/openjd-sessions/tests/integration/test_cross_user_windows.rs +++ b/crates/openjd-sessions/tests/integration/test_cross_user_windows.rs @@ -1405,6 +1405,9 @@ fn test_with_password_wrong_password_returns_logon_failure() { Check `WindowsSessionUser::validate_credentials` — the \ ERROR_LOGON_FAILURE (0x8007052E) branch must remain reachable." ), + // `BadCredentialsError` is `#[non_exhaustive]`; a new variant + // must not silently satisfy this assertion. + Err(e) => panic!("expected LogonFailure, got {e:?}"), Ok(_) => panic!( "with_password({user:?}, wrong_password) must reject. \ Did the test password accidentally match?" @@ -1442,6 +1445,9 @@ fn test_with_password_nonexistent_user_returns_logon_failure() { (LogonUserW returns ERROR_LOGON_FAILURE for unknown accounts \ to avoid leaking account existence). got Other({msg:?})." ), + // `BadCredentialsError` is `#[non_exhaustive]`; a new variant + // must not silently satisfy this assertion. + Err(e) => panic!("expected LogonFailure, got {e:?}"), Ok(_) => panic!( "with_password({nonexistent_user:?}, ...) must reject — \ did the test pick a username that happens to exist?" diff --git a/crates/openjd-sessions/tests/integration/test_path_mapping.rs b/crates/openjd-sessions/tests/integration/test_path_mapping.rs index 42a1b54a..15736a18 100644 --- a/crates/openjd-sessions/tests/integration/test_path_mapping.rs +++ b/crates/openjd-sessions/tests/integration/test_path_mapping.rs @@ -7,19 +7,11 @@ use openjd_sessions::path_mapping::{PathFormat, PathMappingRule}; fn posix_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: src.to_string(), - destination_path: dst.to_string(), - } + PathMappingRule::new(PathFormat::Posix, src, dst) } fn windows_rule(src: &str, dst: &str) -> PathMappingRule { - PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: src.to_string(), - destination_path: dst.to_string(), - } + PathMappingRule::new(PathFormat::Windows, src, dst) } // === test_remaps: posix->posix === diff --git a/crates/openjd-sessions/tests/integration/test_path_mapping_materialize.rs b/crates/openjd-sessions/tests/integration/test_path_mapping_materialize.rs index 6eab7290..732fc851 100644 --- a/crates/openjd-sessions/tests/integration/test_path_mapping_materialize.rs +++ b/crates/openjd-sessions/tests/integration/test_path_mapping_materialize.rs @@ -18,11 +18,11 @@ fn fs(s: &str) -> FormatString { #[tokio::test] async fn test_path_mapping_file_created_with_rules() { let tmp = TempDir::new().unwrap(); - let rules = vec![PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/shared".into(), - destination_path: "/local/shared".into(), - }]; + let rules = vec![PathMappingRule::new( + PathFormat::Posix, + "/mnt/shared", + "/local/shared", + )]; let mut session = Session::new_for_test(tmp.path().to_path_buf()).with_path_mapping(rules); let script = StepScript { let_bindings: None, @@ -77,11 +77,7 @@ async fn test_path_mapping_file_created_empty_when_no_rules() { #[tokio::test] async fn test_has_path_mapping_rules_true() { let tmp = TempDir::new().unwrap(); - let rules = vec![PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/src".into(), - destination_path: "/dst".into(), - }]; + let rules = vec![PathMappingRule::new(PathFormat::Posix, "/src", "/dst")]; let mut session = Session::new_for_test(tmp.path().to_path_buf()).with_path_mapping(rules); let script = StepScript { let_bindings: None, @@ -129,16 +125,8 @@ async fn test_has_path_mapping_rules_false() { async fn test_path_mapping_multiple_rules() { let tmp = TempDir::new().unwrap(); let rules = vec![ - PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/mnt/a".into(), - destination_path: "/local/a".into(), - }, - PathMappingRule { - source_path_format: PathFormat::Windows, - source_path: "C:\\share".into(), - destination_path: "/local/share".into(), - }, + PathMappingRule::new(PathFormat::Posix, "/mnt/a", "/local/a"), + PathMappingRule::new(PathFormat::Windows, "C:\\share", "/local/share"), ]; let mut session = Session::new_for_test(tmp.path().to_path_buf()).with_path_mapping(rules); let script = StepScript { diff --git a/crates/openjd-sessions/tests/integration/test_session.rs b/crates/openjd-sessions/tests/integration/test_session.rs index 1ba3f8da..b17bb484 100644 --- a/crates/openjd-sessions/tests/integration/test_session.rs +++ b/crates/openjd-sessions/tests/integration/test_session.rs @@ -2357,26 +2357,15 @@ async fn test_extend_path_mapping_rules_appends_and_sorts() { use openjd_expr::path_mapping::{PathFormat, PathMappingRule}; let tmp = TempDir::new().unwrap(); - let mut s = - Session::new_for_test(tmp.path().to_path_buf()).with_path_mapping(vec![PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/short".into(), - destination_path: "/s".into(), - }]); + let mut s = Session::new_for_test(tmp.path().to_path_buf()).with_path_mapping(vec![ + PathMappingRule::new(PathFormat::Posix, "/short", "/s"), + ]); assert_eq!(s.path_mapping_rules().len(), 1); s.extend_path_mapping_rules(vec![ - PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/much/longer/path".into(), - destination_path: "/m".into(), - }, - PathMappingRule { - source_path_format: PathFormat::Posix, - source_path: "/med".into(), - destination_path: "/d".into(), - }, + PathMappingRule::new(PathFormat::Posix, "/much/longer/path", "/m"), + PathMappingRule::new(PathFormat::Posix, "/med", "/d"), ]); let rules = s.path_mapping_rules(); diff --git a/crates/openjd-sessions/tests/integration/test_session_scenarios.rs b/crates/openjd-sessions/tests/integration/test_session_scenarios.rs index b9dd5673..f7392aad 100644 --- a/crates/openjd-sessions/tests/integration/test_session_scenarios.rs +++ b/crates/openjd-sessions/tests/integration/test_session_scenarios.rs @@ -123,11 +123,7 @@ fn parse_path_mapping_rules(rules: &[serde_json::Value]) -> Vec "WINDOWS" => openjd_sessions::PathFormat::Windows, _ => return None, }; - Some(PathMappingRule { - source_path_format: format, - source_path: src.to_string(), - destination_path: dst.to_string(), - }) + Some(PathMappingRule::new(format, src, dst)) }) .collect() } @@ -185,9 +181,7 @@ async fn run_scenario(scenario_path: &Path) { .first() .map(|r| match r.source_path_format { openjd_sessions::PathFormat::Windows => openjd_expr::path_mapping::PathFormat::Windows, - openjd_sessions::PathFormat::Posix | openjd_sessions::PathFormat::Uri => { - openjd_expr::path_mapping::PathFormat::Posix - } + _ => openjd_expr::path_mapping::PathFormat::Posix, }) .unwrap_or(openjd_expr::path_mapping::PathFormat::Posix); diff --git a/crates/openjd-sessions/tests/integration/test_windows_permissions.rs b/crates/openjd-sessions/tests/integration/test_windows_permissions.rs index 3e092e4c..a960705a 100644 --- a/crates/openjd-sessions/tests/integration/test_windows_permissions.rs +++ b/crates/openjd-sessions/tests/integration/test_windows_permissions.rs @@ -283,6 +283,9 @@ fn test_tempdir_windows_nonvalid_principal_raises_error() { returns ERROR_LOGON_FAILURE for unknown accounts to avoid \ leaking account existence). got Other({msg:?})." ), + // `BadCredentialsError` is `#[non_exhaustive]`; a new variant + // must not silently satisfy this assertion. + Err(e) => panic!("expected LogonFailure, got {e:?}"), Ok(_) => panic!("Non-existent user should fail credential validation, but got Ok"), } } diff --git a/crates/openjd-snapshots/Cargo.toml b/crates/openjd-snapshots/Cargo.toml index c30a03c8..a80e9349 100644 --- a/crates/openjd-snapshots/Cargo.toml +++ b/crates/openjd-snapshots/Cargo.toml @@ -85,3 +85,6 @@ optional = true [features] bench = ["clap", "rand", "tempfile", "aws-config"] + +[lints] +workspace = true diff --git a/crates/openjd-snapshots/src/codec.rs b/crates/openjd-snapshots/src/codec.rs index c6c94da9..fd5965b4 100644 --- a/crates/openjd-snapshots/src/codec.rs +++ b/crates/openjd-snapshots/src/codec.rs @@ -42,12 +42,19 @@ use tracing::warn; /// is expected to change before any stable release (its /// `specificationVersion` strings carry the `beta-2025-12` tag). #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] pub enum ManifestFormat { V2023, V2025, } #[derive(Debug)] +#[expect( + clippy::exhaustive_enums, + reason = "closed cross-product of (absolute|relative) x (snapshot|diff); matching \ + every arm is the API, and a new arm would mean a new manifest kind that \ + callers must handle explicitly" +)] pub enum DecodedManifest { AbsSnapshot(AbsSnapshot), AbsSnapshotDiff(AbsSnapshotDiff), diff --git a/crates/openjd-snapshots/src/data_cache.rs b/crates/openjd-snapshots/src/data_cache.rs index 2c9a22e5..9627cb2b 100644 --- a/crates/openjd-snapshots/src/data_cache.rs +++ b/crates/openjd-snapshots/src/data_cache.rs @@ -12,6 +12,10 @@ use async_trait::async_trait; /// Result of a copy_from attempt. #[derive(Debug, PartialEq)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum CopyResult { /// Server-side copy completed (no data transited through client) ServerSideCopy, @@ -75,6 +79,10 @@ const DEFAULT_COPY_CONCURRENCY: usize = 64; /// to [`S3_MAX_SINGLE_COPY_BYTES`] (5 GiB), so the threshold is always capped /// at 5 GiB regardless of the configured value. #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct S3CopyConfig { /// Sources strictly larger than this use multipart `UploadPartCopy`. /// Defaults to 5 MiB — a performance cutoff, since parallel multipart copy @@ -281,6 +289,10 @@ pub trait RangeReadDataCache: AsyncDataCache { } /// Content-addressed storage backed by a local or network filesystem. +#[expect( + clippy::exhaustive_structs, + reason = "internal mechanics, publicly reachable but not a spec surface" +)] pub struct FileSystemDataCache { pub root_path: PathBuf, } diff --git a/crates/openjd-snapshots/src/hash.rs b/crates/openjd-snapshots/src/hash.rs index f21a8e80..e84de1f1 100644 --- a/crates/openjd-snapshots/src/hash.rs +++ b/crates/openjd-snapshots/src/hash.rs @@ -13,6 +13,10 @@ pub const WHOLE_FILE_CHUNK_SIZE: i64 = -1; pub const DEFAULT_S3_MULTIPART_PART_SIZE: usize = 32 * 1024 * 1024; #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[expect( + clippy::exhaustive_enums, + reason = "decidable logical concept whose variants are not expected to change" +)] pub enum HashAlgorithm { #[serde(rename = "xxh128")] Xxh128, diff --git a/crates/openjd-snapshots/src/manifest.rs b/crates/openjd-snapshots/src/manifest.rs index bcb4af8d..3c0b8ac4 100644 --- a/crates/openjd-snapshots/src/manifest.rs +++ b/crates/openjd-snapshots/src/manifest.rs @@ -158,6 +158,7 @@ fn is_root_path(path: &str) -> bool { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] +#[non_exhaustive] pub struct FileEntry { pub path: String, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -231,6 +232,7 @@ impl std::fmt::Display for FileEntry { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] +#[non_exhaustive] pub struct DirEntry { pub path: String, #[serde(default, skip_serializing_if = "is_false")] @@ -266,12 +268,28 @@ impl std::fmt::Display for DirEntry { // --- Marker types --- #[derive(Clone, Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "zero-field marker type; there is nothing to add" +)] pub struct Abs; #[derive(Clone, Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "zero-field marker type; there is nothing to add" +)] pub struct Rel; #[derive(Clone, Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "zero-field marker type; there is nothing to add" +)] pub struct Full; #[derive(Clone, Debug)] +#[expect( + clippy::exhaustive_structs, + reason = "zero-field marker type; there is nothing to add" +)] pub struct Diff; // --- Manifest --- @@ -648,6 +666,7 @@ impl Manifest { // --- SymlinkPolicy --- #[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] pub enum SymlinkPolicy { CollapseEscaping, CollapseAll, @@ -682,12 +701,20 @@ pub trait ManifestRef { // --- Enum wrappers --- #[derive(Debug)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum AbsManifest { Snapshot(AbsSnapshot), Diff(AbsSnapshotDiff), } #[derive(Debug)] +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum RelManifest { Snapshot(Snapshot), Diff(SnapshotDiff), @@ -753,6 +780,10 @@ impl_manifest_accessors!(RelManifest, Snapshot, SnapshotDiff); // --- ManifestEntry enum for filter operations --- +#[expect( + clippy::exhaustive_enums, + reason = "closed dichotomy: matching every arm is the API" +)] pub enum ManifestEntry<'a> { File(&'a FileEntry), Dir(&'a DirEntry), diff --git a/crates/openjd-snapshots/src/ops/cache_sync.rs b/crates/openjd-snapshots/src/ops/cache_sync.rs index 9dc7a4fd..8867b008 100644 --- a/crates/openjd-snapshots/src/ops/cache_sync.rs +++ b/crates/openjd-snapshots/src/ops/cache_sync.rs @@ -11,6 +11,10 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; #[derive(Default)] +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct CacheSyncOptions { pub max_workers: Option, pub max_memory_bytes: Option, @@ -18,11 +22,13 @@ pub struct CacheSyncOptions { } #[derive(Debug)] +#[non_exhaustive] pub struct CacheSyncResult { pub statistics: CacheSyncStatistics, } #[derive(Debug, Default, Clone)] +#[non_exhaustive] pub struct CacheSyncStatistics { pub total_objects: usize, pub total_bytes: u64, diff --git a/crates/openjd-snapshots/src/ops/collect.rs b/crates/openjd-snapshots/src/ops/collect.rs index 917542c1..2d11b113 100644 --- a/crates/openjd-snapshots/src/ops/collect.rs +++ b/crates/openjd-snapshots/src/ops/collect.rs @@ -13,6 +13,10 @@ use tracing::{debug, warn}; use walkdir::WalkDir; #[derive(Debug, Clone)] +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct CollectOptions { pub optional_filenames: Vec, pub symlink_policy: SymlinkPolicy, diff --git a/crates/openjd-snapshots/src/ops/diff.rs b/crates/openjd-snapshots/src/ops/diff.rs index 2bd13a35..ecc9e443 100644 --- a/crates/openjd-snapshots/src/ops/diff.rs +++ b/crates/openjd-snapshots/src/ops/diff.rs @@ -7,6 +7,10 @@ use std::collections::HashMap; /// Options controlling the DIFF operation ([`diff_snapshots`]). #[derive(Default)] +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct DiffOptions { /// Hash of the parent manifest to record in the diff's /// `parent_manifest_hash` field. The caller computes this from the diff --git a/crates/openjd-snapshots/src/ops/download.rs b/crates/openjd-snapshots/src/ops/download.rs index 63bd4268..050abc48 100644 --- a/crates/openjd-snapshots/src/ops/download.rs +++ b/crates/openjd-snapshots/src/ops/download.rs @@ -92,12 +92,17 @@ fn preallocate_file(path: &std::path::Path, size: u64) -> std::io::Result<()> { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] pub enum FileConflictResolution { Skip, Overwrite, CreateCopy, } +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct DownloadOptions { pub hash_cache: Option>, pub file_conflict_resolution: FileConflictResolution, @@ -123,12 +128,14 @@ impl Default for DownloadOptions { } #[derive(Debug)] +#[non_exhaustive] pub struct DownloadResult { pub manifest: AbsManifest, pub statistics: DownloadStatistics, } #[derive(Debug, Default, Clone)] +#[non_exhaustive] pub struct DownloadStatistics { pub total_files: usize, pub total_bytes: u64, diff --git a/crates/openjd-snapshots/src/ops/hash_op.rs b/crates/openjd-snapshots/src/ops/hash_op.rs index b94ae493..3a3fdf2a 100644 --- a/crates/openjd-snapshots/src/ops/hash_op.rs +++ b/crates/openjd-snapshots/src/ops/hash_op.rs @@ -13,6 +13,10 @@ use std::sync::{Arc, Mutex}; use tracing::debug; #[derive(Default)] +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct HashOptions { pub hash_cache: Option>, pub force_rehash: bool, @@ -22,6 +26,7 @@ pub struct HashOptions { } #[derive(Debug, Default, Clone)] +#[non_exhaustive] pub struct HashStatistics { pub total_files: usize, pub total_bytes: u64, @@ -47,6 +52,7 @@ pub struct HashStatistics { /// returns an [`AbsManifest`]. The default `M = AbsManifest` keeps the bare /// `HashResult` name usable as a shorthand for the enum-dispatch case. #[derive(Debug)] +#[non_exhaustive] pub struct HashResult { pub manifest: M, pub statistics: HashStatistics, diff --git a/crates/openjd-snapshots/src/ops/hash_upload.rs b/crates/openjd-snapshots/src/ops/hash_upload.rs index 4cf67140..7c1606ee 100644 --- a/crates/openjd-snapshots/src/ops/hash_upload.rs +++ b/crates/openjd-snapshots/src/ops/hash_upload.rs @@ -74,6 +74,10 @@ async fn dedup_upload( } #[derive(Default)] +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct HashUploadOptions { pub hash_cache: Option>, pub force_rehash: bool, @@ -84,12 +88,14 @@ pub struct HashUploadOptions { } #[derive(Debug)] +#[non_exhaustive] pub struct UploadResult { pub manifest: AbsManifest, pub statistics: UploadStatistics, } #[derive(Debug, Default, Clone)] +#[non_exhaustive] pub struct UploadStatistics { pub total_files: usize, pub total_bytes: u64, diff --git a/crates/openjd-snapshots/src/ops/partition.rs b/crates/openjd-snapshots/src/ops/partition.rs index 57187359..a44141a9 100644 --- a/crates/openjd-snapshots/src/ops/partition.rs +++ b/crates/openjd-snapshots/src/ops/partition.rs @@ -7,6 +7,10 @@ use crate::ops::subtree::{subtree_rel_snapshot, subtree_snapshot}; use crate::path_util::{is_absolute_path, normalize_path}; use tracing::debug; +#[expect( + clippy::exhaustive_structs, + reason = "caller-constructed configuration, not a spec-mirroring growth axis; see specs/non-exhaustive-policy.md" +)] pub struct PartitionOptions { pub roots: Option>, pub referenced_paths: Option>, diff --git a/crates/openjd-snapshots/tests/integration/test_compose.rs b/crates/openjd-snapshots/tests/integration/test_compose.rs index ed7b56e8..29b1cdec 100644 --- a/crates/openjd-snapshots/tests/integration/test_compose.rs +++ b/crates/openjd-snapshots/tests/integration/test_compose.rs @@ -47,10 +47,7 @@ fn hf(path: &str, hash: &str, size: u64, mtime: u64) -> FileEntry { } fn deleted_dir(path: &str) -> DirEntry { - DirEntry { - path: path.into(), - deleted: true, - } + DirEntry::deleted(path) } fn fpaths(m: &Snapshot) -> std::collections::HashSet { diff --git a/crates/openjd-snapshots/tests/integration/test_download.rs b/crates/openjd-snapshots/tests/integration/test_download.rs index 3be36d1a..b1c235c7 100644 --- a/crates/openjd-snapshots/tests/integration/test_download.rs +++ b/crates/openjd-snapshots/tests/integration/test_download.rs @@ -321,10 +321,9 @@ async fn diff_deletes_empty_directory() { std::fs::create_dir(&dir_to_delete).unwrap(); let manifest: AbsSnapshotDiff = Manifest::new(HashAlgorithm::Xxh128, DEFAULT_FILE_CHUNK_SIZE) - .with_dirs(vec![DirEntry { - path: dir_to_delete.to_string_lossy().to_string(), - deleted: true, - }]); + .with_dirs(vec![DirEntry::deleted( + dir_to_delete.to_string_lossy().to_string(), + )]); download_abs_manifest( &AbsManifest::Diff(manifest), dc.clone(), @@ -350,10 +349,7 @@ async fn non_empty_directory_not_deleted() { std::fs::write(dir.join("file.txt"), b"content").unwrap(); let manifest: AbsSnapshotDiff = Manifest::new(HashAlgorithm::Xxh128, DEFAULT_FILE_CHUNK_SIZE) - .with_dirs(vec![DirEntry { - path: dir.to_string_lossy().to_string(), - deleted: true, - }]); + .with_dirs(vec![DirEntry::deleted(dir.to_string_lossy().to_string())]); download_abs_manifest( &AbsManifest::Diff(manifest), dc.clone(), @@ -386,14 +382,8 @@ async fn deletion_order_children_before_parents() { file_in_child.to_string_lossy().to_string(), )]) .with_dirs(vec![ - DirEntry { - path: parent.to_string_lossy().to_string(), - deleted: true, - }, - DirEntry { - path: child.to_string_lossy().to_string(), - deleted: true, - }, + DirEntry::deleted(parent.to_string_lossy().to_string()), + DirEntry::deleted(child.to_string_lossy().to_string()), ]); download_abs_manifest( &AbsManifest::Diff(manifest), diff --git a/crates/openjd-snapshots/tests/integration/test_join.rs b/crates/openjd-snapshots/tests/integration/test_join.rs index 7cf87567..610d6f15 100644 --- a/crates/openjd-snapshots/tests/integration/test_join.rs +++ b/crates/openjd-snapshots/tests/integration/test_join.rs @@ -153,10 +153,7 @@ fn abs_symlink_targets_prefixed() { fn diff_preserves_deleted_markers() { let m = snap_diff( vec![FileEntry::deleted("old.txt")], - vec![DirEntry { - path: "old_dir".into(), - deleted: true, - }], + vec![DirEntry::deleted("old_dir")], ); let result = join_snapshot_diff_rel(&m, "prefix").unwrap(); assert_eq!(result.files[0].path, "prefix/old.txt"); diff --git a/crates/openjd-snapshots/tests/integration/test_manifest.rs b/crates/openjd-snapshots/tests/integration/test_manifest.rs index 97087a8d..d6746947 100644 --- a/crates/openjd-snapshots/tests/integration/test_manifest.rs +++ b/crates/openjd-snapshots/tests/integration/test_manifest.rs @@ -257,15 +257,10 @@ fn validate_accepts_positive_chunk_size() { #[test] fn deserialization_accepts_mismatched_paths_and_validate_rejects_them() { let abs: AbsSnapshot = - Manifest::new(HashAlgorithm::Xxh128, WHOLE_FILE_CHUNK_SIZE).with_files(vec![FileEntry { - path: "/absolute/path.txt".into(), - hash: Some("abc123".into()), - size: Some(100), - mtime: Some(1000), - chunk_hashes: None, - symlink_target: None, - runnable: false, - deleted: false, + Manifest::new(HashAlgorithm::Xxh128, WHOLE_FILE_CHUNK_SIZE).with_files(vec![{ + let mut e = FileEntry::file("/absolute/path.txt", 100, 1000); + e.hash = Some("abc123".into()); + e }]); let json = serde_json::to_string(&abs).unwrap(); diff --git a/crates/openjd-snapshots/tests/integration/test_round_trip.rs b/crates/openjd-snapshots/tests/integration/test_round_trip.rs index 12e89eb7..40699d87 100644 --- a/crates/openjd-snapshots/tests/integration/test_round_trip.rs +++ b/crates/openjd-snapshots/tests/integration/test_round_trip.rs @@ -275,14 +275,13 @@ async fn delete_via_diff_manifest() { keep_entry, FileEntry::deleted(remove_path.to_string_lossy().to_string()), ]) - .with_dirs(vec![openjd_snapshots::DirEntry { - path: work_dir + .with_dirs(vec![openjd_snapshots::DirEntry::deleted( + work_dir .path() .join("empty_dir") .to_string_lossy() .to_string(), - deleted: true, - }]); + )]); download_abs_manifest( &AbsManifest::Diff(diff), diff --git a/specs/model/public-api.md b/specs/model/public-api.md index bb86e804..d3eb6034 100644 --- a/specs/model/public-api.md +++ b/specs/model/public-api.md @@ -1234,3 +1234,21 @@ Enums that are marked `#[non_exhaustive]` today: they represent decidable logical concepts (newline mode, filesystem entity kind, data direction) whose sets of variants are not expected to change. + +The `clippy::exhaustive_enums` and `clippy::exhaustive_structs` lints +(configured in the workspace `Cargo.toml`) now make this an explicit, +reviewed decision for every public type. The full decision rule — and +why the `template::*` deserialize types are marked `#[non_exhaustive]` +while the `job::*` instantiated types stay closed — is documented in +[`specs/non-exhaustive-policy.md`](../non-exhaustive-policy.md). In +brief: `template::*` types (`JobTemplate`, `HostRequirements`, the +parameter-definition types, and their `UserInterface` companions) grow +by extension RFC and are gated by the decode-time allowlist, so they +are `#[non_exhaustive]`; `job::*` types are constructed and exhaustively +matched by the sessions runtime with no allowlist in front of them, so +a new field must be a compile error and they stay closed. Additional +enums now marked `#[non_exhaustive]`: `PathFormat`, `HostContext` +(openjd-expr), and the template growth-axis enums +(`JobParameterDefinition`, `TaskParameterDefinition`, `IntRange`, +`FloatRange`, `StringRange`, `IntOrFormatString`, and the template +`CancelationMode`). diff --git a/specs/non-exhaustive-policy.md b/specs/non-exhaustive-policy.md new file mode 100644 index 00000000..df7a1bcd --- /dev/null +++ b/specs/non-exhaustive-policy.md @@ -0,0 +1,107 @@ +# `#[non_exhaustive]` Policy + +## The rule + +Ask what a *new* variant or field means for a downstream consumer that has +not updated their code. + +| | | +|---|---| +| **`#[non_exhaustive]`** | Decode-time input the consumer **reads**. New properties arrive by extension RFC and are gated by the decode-time allowlist, so ignoring an unknown one is correct behavior. | +| **Closed** | Anything the consumer must **react to**, and anything that cannot grow. Silently ignoring a new case would be a bug, so a compile error on upgrade is the point. | + +Concretely: `template::*` is `#[non_exhaustive]`; `job::*`, the runtime +state machines, caller-built config, and decidable concepts stay closed. + +## Why the allowlist decides it + +Extensions are gated at decode time on a caller-supplied allowlist — see +`decode_extensions` in `crates/openjd-model/src/template/parse.rs`. A +consumer who never opts into an extension gets templates using it +**rejected at decode with a clear error**, and the new fields stay `None` +for them. + +That is what makes `#[non_exhaustive]` safe on `template::*`: the +protection a compile error would give is already provided at runtime, by a +switch the consumer controls. The cost of leaving those types closed — a +SemVer-major bump per RFC — is real, and for a data-model crate it +partitions the ecosystem, since a host and a plugin on different major +versions cannot exchange a `Job` at all. + +Where no allowlist stands in front of a change, that reasoning inverts. + +## `template::*` vs `job::*` + +The two families look alike; they are on opposite sides of the rule. + +- **`template::*`** (`JobTemplate`, `HostRequirements`, the + parameter-definition and `UserInterface` types) is the deserialize-time + model. Serde builds it from YAML/JSON, new properties are + allowlist-gated, and consumers read it. → **`#[non_exhaustive]`** + +- **`job::*`** (`Job`, `Step`, `Action`, `EnvironmentActions`, …) is the + instantiated model. Extensions are already resolved, so the allowlist no + longer applies, and the `openjd-sessions` runtime **constructs and + exhaustively matches** these to decide what to execute. A new field is + execution behavior the runner must handle — the `WRAP_ACTIONS` hook + fields on `EnvironmentActions` are the precedent — and a `_` arm that + swallowed it would mean the action silently never runs. → **closed** + +## Other closed categories + +- **Runtime state machines** (`ActionState`, `SessionState`, + `ScriptRunnerState`, `ActionMessage`, `CancelMethod`) — not + allowlist-gated; consumers match to react. A `_ => {}` that swallows a + new state is a correctness bug. +- **Caller-built configuration** (`SessionConfig`, the `openjd-snapshots` + `*Options`) — exists to be assembled by callers. Marking it would force + `Default` plus field mutation at every call site, since + functional-record-update cannot construct a non-exhaustive struct from + another crate. +- **Decidable concepts** — the variant set *is* the definition: + `EndOfLine`, `ObjectType`, `DataFlow` (newline mode, entity kind, data + direction); closed dichotomies like `PathElement` (Field | Index) and + `DecodedTemplate` (Job | Environment), where matching every arm is the + API; `DiagnosticSpan` (an offset and a length). +- **Single-field wrappers** (`FlexInt`, `Description`, `ExtensionName`) — + one validated value behind a distinct type. Callers construct and + destructure them directly, and a second field would make it not a + newtype. +- **Zero-field markers** (`Abs`, `Rel`, `Full`, `Diff`) — nothing to add. + +## Recording the decision + +Both lints are `deny`, so an undecided public type is a hard error under a +plain `cargo clippy` — it fails while the type is being written, not later +under CI's `-D warnings`. Every public type carries either +`#[non_exhaustive]` or an `#[expect]` naming which rule applies: + +```rust +#[expect( + clippy::exhaustive_enums, + reason = "runtime state machine: consumers match on these to react, and \ + a new state must be a compile error rather than a silently \ + ignored `_` arm. Not extension-gated." +)] +pub enum ActionState { /* … */ } +``` + +Prefer `#[expect]` over `#[allow]`: it is itself linted, so marking the +type `#[non_exhaustive]` later leaves an unfulfilled expectation that must +be removed. Use `#[allow]` only where the lint fires in some build +configurations but not others (`Identifier` is the one such case). + +`openjd-cli` (a binary) and `openjd-for-js` (`publish = false` WASM +bindings) are exempt at the crate level — neither exposes a public Rust +API. + +## Construction + +`#[non_exhaustive]` blocks cross-crate *literal construction* and +*exhaustive destructuring*. It does **not** block field reads or `..` +patterns — but note it *does* block `..Default::default()`, which is why +caller-built config stays closed. + +Types consumers legitimately build by hand get a constructor instead: +`PathMappingRule::new` and the `CallerLimits::with_*` builder exist for +that reason.