From 610fbfeb08b12f9c36a4d4f7632623e46ff50b64 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 28 Aug 2026 15:33:18 +0800 Subject: [PATCH 1/2] fix: move catalog validation and settings lowering to OPY owner --- Cargo.lock | 1 + crates/opy-compiler/src/lib.rs | 82 +++++++++++++++++++++++++++++----- crates/opy-rs/Cargo.toml | 1 + crates/opy-rs/src/lower.rs | 64 +++++++++++++++++--------- 4 files changed, 116 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7ffd937..a090fd0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -399,6 +399,7 @@ dependencies = [ "opy-macro-js", "serde", "serde_json", + "workshop-rs", ] [[package]] diff --git a/crates/opy-compiler/src/lib.rs b/crates/opy-compiler/src/lib.rs index 079f02d..5c042a9 100644 --- a/crates/opy-compiler/src/lib.rs +++ b/crates/opy-compiler/src/lib.rs @@ -188,7 +188,6 @@ impl Compiler { pub fn compile_hir(&self, hir: &hir::Program) -> Result { let mut lowering = Lowering::new(self, hir)?; lowering.copy_files()?; - lowering.reject_unsupported_metadata()?; lowering.lower_declarations()?; lowering.lower_rules()?; @@ -228,6 +227,72 @@ pub struct CompilationArtifact { pub catalog_identity: CatalogIdentity, } +fn convert_settings(settings: opy_rs::hir::Settings) -> workshop_rs::settings::Settings { + workshop_rs::settings::Settings { + span: settings.span.map(convert_settings_span), + children: settings + .children + .into_iter() + .map(convert_settings_node) + .collect(), + } +} + +fn convert_settings_node(node: opy_rs::hir::SettingsNode) -> workshop_rs::settings::SettingsNode { + use opy_rs::hir::SettingsNode as SourceNode; + use workshop_rs::settings::{SettingsListElement, SettingsNode as TargetNode}; + + match node { + SourceNode::Group { + name, + children, + span, + } => TargetNode::Group { + name, + children: children.into_iter().map(convert_settings_node).collect(), + span: span.map(convert_settings_span), + }, + SourceNode::Number { name, value, span } => TargetNode::Number { + name, + value, + span: span.map(convert_settings_span), + }, + SourceNode::Bool { name, value, span } => TargetNode::Bool { + name, + value, + span: span.map(convert_settings_span), + }, + SourceNode::String { name, value, span } => TargetNode::String { + name, + value, + span: span.map(convert_settings_span), + }, + SourceNode::List { + name, + elements, + span, + } => TargetNode::List { + name, + elements: elements + .into_iter() + .map(|element| SettingsListElement { + value: element.value, + span: element.span.map(convert_settings_span), + }) + .collect(), + span: span.map(convert_settings_span), + }, + } +} + +fn convert_settings_span(span: HirSpan) -> WorkshopSpan { + WorkshopSpan::new( + workshop_rs::source::FileId::from_index(span.file as usize), + WorkshopPosition::new(span.start.line, span.start.col), + WorkshopPosition::new(span.end.line, span.end.col), + ) +} + struct Lowering<'a> { compiler: &'a Compiler, hir: &'a hir::Program, @@ -268,6 +333,11 @@ impl<'a> Lowering<'a> { } fn copy_files(&mut self) -> Result<(), IntegrationError> { + self.wir.settings = self + .hir + .settings + .clone() + .map(|settings| convert_settings(settings)); for file in &self.hir.files { if self.files.contains_key(&file.id) { return Err(IntegrationError::new( @@ -283,16 +353,6 @@ impl<'a> Lowering<'a> { Ok(()) } - fn reject_unsupported_metadata(&self) -> Result<(), IntegrationError> { - if let Some(settings) = &self.hir.settings { - return Err(self.unsupported( - "custom-game settings lowering is outside #46", - settings.span, - )); - } - Ok(()) - } - fn lower_declarations(&mut self) -> Result<(), IntegrationError> { let (implicit_globals, implicit_players) = implicit_default_variables(self.hir); for declaration in &self.hir.declarations { diff --git a/crates/opy-rs/Cargo.toml b/crates/opy-rs/Cargo.toml index 503649c..bbe5c0b 100644 --- a/crates/opy-rs/Cargo.toml +++ b/crates/opy-rs/Cargo.toml @@ -13,3 +13,4 @@ workspace = true opy-macro-js = { path = "../opy-macro-js", version = "0.1.1" } serde = { workspace = true, features = ["derive"] } serde_json.workspace = true +workshop-rs.workspace = true diff --git a/crates/opy-rs/src/lower.rs b/crates/opy-rs/src/lower.rs index c4c610d..4115886 100644 --- a/crates/opy-rs/src/lower.rs +++ b/crates/opy-rs/src/lower.rs @@ -40,6 +40,7 @@ use crate::diag::{OpyError, OpyResult, Span}; use crate::manifest::{ Function, FunctionContext, FunctionKind, Manifest, Param, ParamDefault, ReceiverCategory, }; +use workshop_rs::catalog::Catalog; /// The protocol envelope this frontend produces. const PROTOCOL_NAME: &str = "wright/opy-hir"; @@ -69,6 +70,8 @@ struct Lowerer { allow_dict_literal: bool, /// The authoritative builtin semantic table (issue #109). manifest: &'static Manifest, + /// The canonical Workshop catalog linked by the manifest. + catalog: Catalog, errors: Vec, } @@ -96,6 +99,15 @@ pub fn lower_with_preprocessing( )); } }; + let catalog = match Catalog::builtin() { + Ok(catalog) => catalog, + Err(error) => { + return Err(OpyError::new( + "catalog-error", + format!("cannot load the Workshop catalog: {error}"), + )); + } + }; let mut lowerer = Lowerer { globals: HashSet::new(), players: HashSet::new(), @@ -105,6 +117,7 @@ pub fn lower_with_preprocessing( locals: Vec::new(), allow_dict_literal: false, manifest, + catalog, errors: Vec::new(), }; lowerer.collect_symbols(program); @@ -1099,9 +1112,22 @@ impl Lowerer { // Builtin Workshop enum: the domain name is a declared OPY // signature identity (manifest `param.domain`); the member list // is Workshop-owned catalog content, so the member access - // resolves as an opaque identity without member validation. - // Member-existence checks are lowering-dependent (issue #8). + // resolves as an opaque identity after validating the member + // against the canonical Workshop catalog. if self.manifest.domain_identity(name) { + if let Some(domain) = self.catalog.enum_domain(name) + && !domain + .members + .iter() + .any(|candidate| candidate.member == *member) + { + self.error_at( + "unknown-enum-member", + format!("enum '{name}' has no member '{member}'"), + span, + ); + return HirExpr::Null { span: None }; + } return HirExpr::Enum { value_type: name.clone(), value: member.to_string(), @@ -2165,14 +2191,14 @@ mod tests { } #[test] - fn unknown_chase_time_reeval_member_resolves_as_an_opaque_enum_identity() { - // Member spellings are Workshop catalog content: a member access on - // a declared domain identity resolves as an opaque Enum node without - // member validation (lowering-dependent, #8). - let value = lowered_value( + fn unknown_chase_time_reeval_member_is_rejected_by_the_catalog() { + let error = crate::compile( "globalvar g\nrule \"r\":\n @Event global\n g = ChaseTimeReeval.NOPE\n", - ); - assert_enum(&value, "ChaseTimeReeval", "NOPE"); + "test.opy", + std::path::Path::new(""), + ) + .expect_err("unknown catalog member must be rejected"); + assert_eq!(error.code, "unknown-enum-member"); } #[test] @@ -2770,19 +2796,15 @@ mod tests { } #[test] - fn previously_rejected_enum_member_spellings_resolve_as_opaque_identities() { - // The KNOWN_ENUMS-era member allowlist is gone: Color.CYAN and - // DynamicEffect.SPARKLES are member spellings whose validity is - // Workshop-owned knowledge, so they resolve as opaque enum - // identities instead of failing (lowering-dependent, #8). - let value = - lowered_value("globalvar g\nrule \"r\":\n @Event global\n g = Color.CYAN\n"); - assert_enum(&value, "Color", "CYAN"); - - let value = lowered_value( + fn unknown_catalog_enum_members_are_rejected() { + for source in [ + "globalvar g\nrule \"r\":\n @Event global\n g = Color.CYAN\n", "globalvar g\nrule \"r\":\n @Event global\n g = DynamicEffect.SPARKLES\n", - ); - assert_enum(&value, "DynamicEffect", "SPARKLES"); + ] { + let error = crate::compile(source, "test.opy", std::path::Path::new("")) + .expect_err("unknown catalog member must be rejected"); + assert_eq!(error.code, "unknown-enum-member"); + } } #[test] From c02302c02f0815b510997107a502dbd215deea77 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 28 Aug 2026 15:40:22 +0800 Subject: [PATCH 2/2] fix: satisfy clippy in settings lowering --- crates/opy-compiler/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/opy-compiler/src/lib.rs b/crates/opy-compiler/src/lib.rs index 5c042a9..0c86e13 100644 --- a/crates/opy-compiler/src/lib.rs +++ b/crates/opy-compiler/src/lib.rs @@ -333,11 +333,7 @@ impl<'a> Lowering<'a> { } fn copy_files(&mut self) -> Result<(), IntegrationError> { - self.wir.settings = self - .hir - .settings - .clone() - .map(|settings| convert_settings(settings)); + self.wir.settings = self.hir.settings.clone().map(convert_settings); for file in &self.hir.files { if self.files.contains_key(&file.id) { return Err(IntegrationError::new(