From 89e50d9a1b601ca2991431a7955b2af21dc5861f Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sun, 27 Sep 2026 11:26:34 +0000 Subject: [PATCH 1/2] refactor!: remove duplicate public paths before 1.0 Resolve the duplicate-path decision in #315 with its proposed default: - settings::schema is now internal; its types and functions (including the previously unexported validate_catalog) are public only from settings directly. - actions no longer re-exports ActionLayout, ActionLayoutError, or action_width; they are public only from emitter, the path already used by opy-rs. - rules no longer re-exports validate_canonical_ids; it is public only from validate. - The root, program::, and per-domain Program model re-exports (actions::{Action, ModifyOp}, events::*, rules::{Program, Rule, Condition, Subroutine, Variable}, values::Value) are kept and documented as intentional in docs/compatibility-facades.md, along with the already-documented detect and signatures aliases. Verified with cargo semver-checks that only the three intended paths are flagged as removed, cargo test --workspace (all 231+ tests pass), and cargo clippy --workspace --all-targets --all-features -D warnings clean. --- crates/workshop-rs-cli/src/census.rs | 4 +--- crates/workshop-rs/src/actions/mod.rs | 8 ++++---- .../src/bin/workshop-catalog-gen.rs | 6 +++--- crates/workshop-rs/src/emitter.rs | 2 +- crates/workshop-rs/src/rules/mod.rs | 5 ++--- crates/workshop-rs/src/settings/mod.rs | 4 ++-- crates/workshop-rs/tests/action_layout.rs | 9 ++++----- crates/workshop-rs/tests/program_model.rs | 2 +- crates/workshop-rs/tests/public_api.rs | 13 +++++++------ docs/compatibility-facades.md | 19 +++++++++++++++++++ 10 files changed, 44 insertions(+), 28 deletions(-) diff --git a/crates/workshop-rs-cli/src/census.rs b/crates/workshop-rs-cli/src/census.rs index a48eb7c..b569b58 100644 --- a/crates/workshop-rs-cli/src/census.rs +++ b/crates/workshop-rs-cli/src/census.rs @@ -16,9 +16,7 @@ use crate::conformance::{ TestArtifact, }; use workshop_rs::catalog::{Catalog, CatalogEntry, EnumDomain, Kind, Locale}; -use workshop_rs::settings::schema::{ - self as settings_schema, SettingDefinition, SettingValueDomain, -}; +use workshop_rs::settings::{self as settings_schema, SettingDefinition, SettingValueDomain}; use workshop_rs::{WorkshopError, convert, emitter, parser, roundtrip}; #[derive(Clone, Copy)] diff --git a/crates/workshop-rs/src/actions/mod.rs b/crates/workshop-rs/src/actions/mod.rs index ba199e4..4edfc83 100644 --- a/crates/workshop-rs/src/actions/mod.rs +++ b/crates/workshop-rs/src/actions/mod.rs @@ -1,11 +1,12 @@ //! The Workshop action domain. //! //! Action data is modeled canonically in [`crate::Program`]. Action-specific -//! layout and element-count operations are re-exported here so contributors -//! can start from the domain rather than from an implementation phase. +//! layout and element-count operations live in [`crate::emitter`], the +//! Workshop-operations entry point; this domain module re-exports the model +//! types only. pub(crate) mod emitter; -mod layout; +pub(crate) mod layout; pub(crate) mod parser; pub(crate) mod validate; @@ -13,4 +14,3 @@ pub use crate::analysis::element_count::{ ElementCountError, ElementCountNode, ElementCountReport, ElementNodeKind, }; pub use crate::program::{Action, ModifyOp}; -pub use layout::{ActionLayout, ActionLayoutError, action_width}; diff --git a/crates/workshop-rs/src/bin/workshop-catalog-gen.rs b/crates/workshop-rs/src/bin/workshop-catalog-gen.rs index e8a3ff5..82e41b0 100644 --- a/crates/workshop-rs/src/bin/workshop-catalog-gen.rs +++ b/crates/workshop-rs/src/bin/workshop-catalog-gen.rs @@ -42,7 +42,7 @@ use std::path::{Path, PathBuf}; use std::process::ExitCode; use workshop_rs::catalog::{Catalog, Locale, build_canonical}; -use workshop_rs::settings::schema; +use workshop_rs::settings; /// The committed catalog data, relative to the workspace root (where CI and /// the documented pipeline commands run); `--file` overrides it. @@ -128,7 +128,7 @@ fn main() -> ExitCode { } return ExitCode::from(1); } - if let Err(errors) = schema::validate_catalog() { + if let Err(errors) = settings::validate_catalog() { for error in errors { eprintln!("workshop-catalog-gen: settings catalog: {error}"); } @@ -613,7 +613,7 @@ mod corpus { .collect(); let mut labels: Vec<(String, String)> = Vec::new(); let mut seen = std::collections::HashSet::new(); - let definitions: Vec<_> = schema::definitions().collect(); + let definitions: Vec<_> = settings::definitions().collect(); for definition in &definitions { if definition.path().ends_with(".enabled") { continue; diff --git a/crates/workshop-rs/src/emitter.rs b/crates/workshop-rs/src/emitter.rs index e7cec28..aa081fe 100644 --- a/crates/workshop-rs/src/emitter.rs +++ b/crates/workshop-rs/src/emitter.rs @@ -1,6 +1,6 @@ //! Public complete-program Workshop emission over [`crate::Program`]. -pub use crate::actions::{ActionLayout, ActionLayoutError, action_width}; +pub use crate::actions::layout::{ActionLayout, ActionLayoutError, action_width}; pub use crate::output::emitter::*; #[cfg(test)] diff --git a/crates/workshop-rs/src/rules/mod.rs b/crates/workshop-rs/src/rules/mod.rs index 1719d28..11e831f 100644 --- a/crates/workshop-rs/src/rules/mod.rs +++ b/crates/workshop-rs/src/rules/mod.rs @@ -1,8 +1,8 @@ //! The Workshop rule and declaration domain. //! //! Rule, variable, and subroutine data is modeled canonically in [`crate::Program`]. -//! Whole-program inspection and validation are available from this domain -//! entry point as well as their compatibility modules. +//! Whole-program inspection is available from this domain entry point; +//! canonical validation is at [`crate::validate::validate_canonical_ids`]. pub(crate) mod emitter; pub(crate) mod parser; @@ -12,4 +12,3 @@ pub use crate::analysis::semantic::{ IncompletenessKind, ResidualClassification, SemanticIssue, inspect, }; pub use crate::program::{Condition, Program, Rule, Subroutine, Variable}; -pub use validate::validate_canonical_ids; diff --git a/crates/workshop-rs/src/settings/mod.rs b/crates/workshop-rs/src/settings/mod.rs index 9247a77..0a65fad 100644 --- a/crates/workshop-rs/src/settings/mod.rs +++ b/crates/workshop-rs/src/settings/mod.rs @@ -12,7 +12,7 @@ pub(crate) mod emitter; pub(crate) mod parser; pub(crate) mod reconciliation; -pub mod schema; +pub(crate) mod schema; pub(crate) mod table; /// A segment of a path accepted by settings schema lookups. @@ -45,7 +45,7 @@ pub use schema::{ SettingEnumMember, SettingId, SettingIdentity, SettingOccurrence, SettingOperationError, SettingPresentation, SettingScope, SettingSource, SettingSourceEdit, SettingSourceKind, SettingTarget, SettingTargetKind, SettingValue, SettingValueDomain, TeamId, definition, - definitions, definitions_by_id, + definitions, definitions_by_id, validate_catalog, }; use crate::core::source::Span; diff --git a/crates/workshop-rs/tests/action_layout.rs b/crates/workshop-rs/tests/action_layout.rs index 2fbefdf..322c33a 100644 --- a/crates/workshop-rs/tests/action_layout.rs +++ b/crates/workshop-rs/tests/action_layout.rs @@ -1,6 +1,5 @@ -use workshop_rs::actions::{self, ActionLayoutError}; use workshop_rs::catalog::{Catalog, Locale}; -use workshop_rs::emitter; +use workshop_rs::emitter::{self, ActionLayoutError}; use workshop_rs::{Action, Event, Program, Rule, Value, Variable}; fn program_with_structured_actions() -> (Program, Vec) { @@ -67,7 +66,7 @@ fn structured_action_widths_count_native_expansion() { for (range, width) in [((0..5), 5), ((5..8), 3), ((8..11), 3), ((11..18), 7)] { assert_eq!( - actions::action_width(&program, &catalog, &locale, &actions[range]) + emitter::action_width(&program, &catalog, &locale, &actions[range]) .unwrap() .width, width @@ -92,7 +91,7 @@ fn layout_matches_canonical_emission_for_a_nested_sequence() { .lines() .filter(|line| !line.trim().is_empty()) .count(); - let layout = actions::action_width(&program, &catalog, &locale, &actions).unwrap(); + let layout = emitter::action_width(&program, &catalog, &locale, &actions).unwrap(); assert_eq!(layout.width, emitted_width); assert_eq!(layout.width, 19); } @@ -105,7 +104,7 @@ fn invalid_layout_requests_fail_as_invalid_programs() { let actions = rule.actions.clone(); program.rules.push(rule); - let error = actions::action_width( + let error = emitter::action_width( &program, &Catalog::builtin().unwrap(), &Locale::new("en-US"), diff --git a/crates/workshop-rs/tests/program_model.rs b/crates/workshop-rs/tests/program_model.rs index 8c8b85d..b991b56 100644 --- a/crates/workshop-rs/tests/program_model.rs +++ b/crates/workshop-rs/tests/program_model.rs @@ -116,7 +116,7 @@ fn independently_constructed_program_uses_the_same_operations() { program.validate().expect("structurally validates"); workshop_rs::validate::validate_canonical_ids(&program, &catalog).expect("catalog validates"); let layout = - workshop_rs::actions::action_width(&program, &catalog, &locale, &program.rules[0].actions) + workshop_rs::emitter::action_width(&program, &catalog, &locale, &program.rules[0].actions) .expect("lays out"); assert_eq!(layout.width, 1); let emitted = workshop_rs::emitter::emit(&program, &catalog, &locale).expect("emits"); diff --git a/crates/workshop-rs/tests/public_api.rs b/crates/workshop-rs/tests/public_api.rs index 089e685..6fc8ca0 100644 --- a/crates/workshop-rs/tests/public_api.rs +++ b/crates/workshop-rs/tests/public_api.rs @@ -1,7 +1,8 @@ use workshop_rs::catalog::{Catalog, Locale}; -use workshop_rs::settings::{PathPart, schema}; +use workshop_rs::settings::{self, PathPart}; use workshop_rs::{ - Action, Event, MappedText, Program, Rule, SourceMap, Value, emitter, parser, roundtrip, rules, + Action, Event, MappedText, Program, Rule, SourceMap, Value, emitter, parser, roundtrip, + validate, }; #[test] @@ -18,8 +19,8 @@ fn canonical_program_operations_cover_parse_validate_inspect_emit_and_roundtrip( program .validate() .expect("canonical program is structurally valid"); - rules::validate_canonical_ids(&program, &catalog) - .expect("canonical ids resolve through the public rule API"); + validate::validate_canonical_ids(&program, &catalog) + .expect("canonical ids resolve through the public validate API"); assert!(program.semantic_issues(&catalog).is_empty()); let emitted = emitter::emit(&program, &catalog, &locale).expect("canonical emission"); @@ -29,7 +30,7 @@ fn canonical_program_operations_cover_parse_validate_inspect_emit_and_roundtrip( #[test] fn settings_schema_exposes_enum_values_without_the_internal_table() { - let definition = schema::definition(&[ + let definition = settings::definition(&[ PathPart::Part("lobby"), PathPart::Part("enableMatchVoiceChat"), ]) @@ -73,7 +74,7 @@ fn catalog_actions_and_values_are_built_by_canonical_id() { )), ); program.validate().expect("catalog calls validate"); - rules::validate_canonical_ids(&program, &catalog).expect("catalog ids resolve"); + validate::validate_canonical_ids(&program, &catalog).expect("catalog ids resolve"); let emitted = emitter::emit(&program, &catalog, &locale).expect("catalog calls emit"); assert!( diff --git a/docs/compatibility-facades.md b/docs/compatibility-facades.md index 236ab9d..29c7dd9 100644 --- a/docs/compatibility-facades.md +++ b/docs/compatibility-facades.md @@ -29,6 +29,25 @@ parse-context contract shared by Workshop parsing and frontends that supply expected enum domains. Both are intentional public APIs, not compatibility-only facades; the catalog remains the sole source of signature data. +The canonical `Program` model types are also reachable from the domain module +that owns their concept, so contributors and consumers can start from either +the model or the domain: `actions::{Action, ModifyOp}`, `events::{Event, +EventTarget, EventTeam, PlayerEventKind}`, `rules::{Condition, Program, Rule, +Subroutine, Variable}`, and `values::Value` are the same items as their +crate-root and `program::` re-exports, not copies. `program`, the crate root, +and these domain modules are the discoverable Workshop domains described in +the crate's top-level docs; every one of these paths is an intentional public +API and a separate 1.x compatibility commitment. + +Every other public item is reachable through exactly one path. `actions` +re-exports only the `Program` model types above; action layout and +element-count operations (`ActionLayout`, `ActionLayoutError`, `action_width`) +are public only from `emitter`. `rules` re-exports only the `Program` model +types above; canonical validation (`validate_canonical_ids`) is public only +from `validate`. `settings::schema` is an internal module; its types and +functions (including `validate_catalog`) are public only from `settings` +directly. + ## Catalog-backed actions and values Catalog actions and values are built with `Action::call` and `Value::call`, From 57d003b7b3b0467b592aec2e2403bf07a4928153 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Sun, 27 Sep 2026 11:45:41 +0000 Subject: [PATCH 2/2] docs: fix compatibility-facades overclaim and stale ADR-0006 reference - actions and rules re-export more than just the Program model types (element-count analysis types and semantic-inspection types respectively); say so instead of claiming 'only'. - ADR-0006 still pointed at settings::schema, which is now internal; point at settings and the compatibility doc instead. Found by an independent review of #319. --- docs/adr/0006-settings-semantic-schema.md | 5 ++++- docs/compatibility-facades.md | 16 +++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/adr/0006-settings-semantic-schema.md b/docs/adr/0006-settings-semantic-schema.md index 7ccd14f..3be99eb 100644 --- a/docs/adr/0006-settings-semantic-schema.md +++ b/docs/adr/0006-settings-semantic-schema.md @@ -7,7 +7,10 @@ Query and edit API ergonomics are outside this decision. ## Decision -`workshop-rs` exposes typed setting facts through `settings::schema`. +`workshop-rs` exposes typed setting facts through `settings` (implemented in +the internal `settings::schema` module; see +[`docs/compatibility-facades.md`](../compatibility-facades.md) for the public +path). `SettingId` is an open, locale-independent identity for a Workshop setting concept. A concrete hero or ability display label is never required in that identity; hero and logical ability-slot information is represented by diff --git a/docs/compatibility-facades.md b/docs/compatibility-facades.md index 29c7dd9..adaebd1 100644 --- a/docs/compatibility-facades.md +++ b/docs/compatibility-facades.md @@ -40,13 +40,15 @@ the crate's top-level docs; every one of these paths is an intentional public API and a separate 1.x compatibility commitment. Every other public item is reachable through exactly one path. `actions` -re-exports only the `Program` model types above; action layout and -element-count operations (`ActionLayout`, `ActionLayoutError`, `action_width`) -are public only from `emitter`. `rules` re-exports only the `Program` model -types above; canonical validation (`validate_canonical_ids`) is public only -from `validate`. `settings::schema` is an internal module; its types and -functions (including `validate_catalog`) are public only from `settings` -directly. +re-exports the `Program` model types above and its own element-count analysis +types (`ElementCountError`, `ElementCountNode`, `ElementCountReport`, +`ElementNodeKind`); action layout and element-count operations (`ActionLayout`, +`ActionLayoutError`, `action_width`) are public only from `emitter`. `rules` +re-exports the `Program` model types above and its own inspection types +(`IncompletenessKind`, `ResidualClassification`, `SemanticIssue`, `inspect`); +canonical validation (`validate_canonical_ids`) is public only from +`validate`. `settings::schema` is an internal module; its types and functions +(including `validate_catalog`) are public only from `settings` directly. ## Catalog-backed actions and values