diff --git a/crates/workshop-rs/src/parser.rs b/crates/workshop-rs/src/parser.rs index ab83ced..fbc3471 100644 --- a/crates/workshop-rs/src/parser.rs +++ b/crates/workshop-rs/src/parser.rs @@ -1431,7 +1431,7 @@ impl Parser<'_> { if !is_event_player { // Object/member assignments use the same value grammar as member // reads (`receiver.member` and `receiver.member[index]`). Keep - // this source-level form distinct from catalog actions: the + // this native member-assignment form distinct from catalog actions: the // receiver and member are dynamic Workshop values, not a builtin // identity. Global and Event Player assignments are handled by // their dedicated variable paths above and below. diff --git a/crates/workshop-rs/src/validate.rs b/crates/workshop-rs/src/validate.rs index db862cf..1ccdfc4 100644 --- a/crates/workshop-rs/src/validate.rs +++ b/crates/workshop-rs/src/validate.rs @@ -140,7 +140,18 @@ fn validate_action( validate_value(program, catalog, *player, errors); validate_value(program, catalog, *value, errors); } - wir::Action::AssignMember { target, value, .. } => { + wir::Action::AssignMember { + target, + value, + span, + .. + } => { + if !is_member_assignment_target(program, *target) { + errors.push(WorkshopError::Malformed { + message: "AssignMember target must be a memberAccess value".to_string(), + span: *span, + }); + } validate_value(program, catalog, *target, errors); validate_value(program, catalog, *value, errors); } @@ -203,6 +214,16 @@ fn validate_action( } } +fn is_member_assignment_target(program: &wir::Program, target: wir::ValueId) -> bool { + match program.values.get(target) { + Some(wir::ValueNode { + value: wir::Value::Call { name, args }, + .. + }) if name == "memberAccess" => (2..=3).contains(&args.len()), + _ => false, + } +} + fn validate_value( program: &wir::Program, catalog: &Catalog, diff --git a/crates/workshop-rs/src/wir/mod.rs b/crates/workshop-rs/src/wir/mod.rs index 7520cde..1f93d12 100644 --- a/crates/workshop-rs/src/wir/mod.rs +++ b/crates/workshop-rs/src/wir/mod.rs @@ -351,9 +351,9 @@ pub enum Action { /// The exact span of the modified variable identifier. target_span: Option, }, - /// Assignment to a dynamic Workshop object member, optionally through an - /// indexed `memberAccess` value. This is source semantics, not a builtin - /// catalog action; the emitter preserves the member-assignment syntax. + /// Assignment to a canonical Workshop member-access target, optionally + /// indexed. This is not a builtin catalog action; the emitter preserves + /// the native member-assignment syntax. AssignMember { target: ValueId, op: Option, diff --git a/crates/workshop-rs/tests/parser.rs b/crates/workshop-rs/tests/parser.rs index 9bd5885..419ed4c 100644 --- a/crates/workshop-rs/tests/parser.rs +++ b/crates/workshop-rs/tests/parser.rs @@ -249,7 +249,7 @@ fn every_corpus_workshop_text_parses_to_valid_wir() { } #[test] -fn member_assignment_lowers_to_source_semantic_wir() { +fn member_assignment_lowers_to_canonical_wir() { let text = r#"rule ("member") { event { Ongoing - Global; } actions { All Players(All Teams).abilityHUD[17] = True; Global.botOrisaChild.botDoesUniqueBehaviour = False; diff --git a/crates/workshop-rs/tests/wir_expansion.rs b/crates/workshop-rs/tests/wir_expansion.rs index 8c9cdcc..e02e927 100644 --- a/crates/workshop-rs/tests/wir_expansion.rs +++ b/crates/workshop-rs/tests/wir_expansion.rs @@ -50,6 +50,163 @@ fn member_access_has_a_canonical_shape_contract() { ); } +#[test] +fn indexed_members_and_native_break_controls_have_one_canonical_contract() { + let catalog = catalog(); + let locale = Locale::new("en-US"); + let source = r#" + variables { global: 0: values } + rule ("interop primitives") { + event { Ongoing - Global; } + actions { + Global.values[1] = 2; + Event Player.values[0] = 3; + If(True); + Event Player.payload.member[0] = 4; + Skip If(True, 1); + While(True); + Break; + Continue; + End; + Else; + Skip(2); + End; + } + } + "#; + let program = workshop_rs::parser::parse_with_context(source, &catalog, &locale, &catalog) + .expect("native indexed/member and control-flow forms parse"); + program + .validate() + .expect("canonical WIR is structurally valid"); + validate::validate_canonical_ids(&program, &catalog).expect("catalog ids resolve"); + + let rule = program.rules.iter().next().expect("rule"); + assert!(matches!( + program.actions.get(rule.actions[0]), + Some(Action::Call { name, args, .. }) + if name == "setGlobalVariableAtIndex" + && args.len() == 3 + && matches!(program.values.get(args[0]).map(|node| &node.value), Some(Value::GlobalVariable(_))) + )); + assert!(matches!( + program.actions.get(rule.actions[1]), + Some(Action::Call { name, args, .. }) + if name == "setPlayerVariableAtIndex" + && args.len() == 3 + && matches!(program.values.get(args[0]).map(|node| &node.value), Some(Value::PlayerVariable { .. })) + )); + + let Action::If { + branches, + else_body: Some(else_body), + .. + } = program.actions.get(rule.actions[2]).expect("if action") + else { + panic!("expected an if with an else branch"); + }; + let member_action = program + .actions + .get(branches[0].body[0]) + .expect("member assignment"); + let Action::AssignMember { target, .. } = member_action else { + panic!("expected canonical member assignment"); + }; + assert!(matches!( + program.values.get(*target).map(|node| &node.value), + Some(Value::Call { name, args }) + if name == "memberAccess" + && args.len() == 3 + && matches!(program.values.get(args[1]).map(|node| &node.value), Some(Value::String(member)) if member == "member") + )); + assert!(matches!( + program.actions.get(branches[0].body[1]), + Some(Action::Call { name, args, .. }) if name == "skipIf" && args.len() == 2 + )); + let Action::While { body, .. } = program.actions.get(branches[0].body[2]).expect("while") + else { + panic!("expected nested while"); + }; + assert!(matches!( + program.actions.get(body[0]), + Some(Action::Call { name, args, .. }) if name == "break" && args.is_empty() + )); + assert!(matches!( + program.actions.get(body[1]), + Some(Action::Call { name, args, .. }) if name == "continue" && args.is_empty() + )); + assert!(matches!( + program.actions.get(else_body[0]), + Some(Action::Call { name, args, .. }) if name == "skip" && args.len() == 1 + )); + + let emitted = workshop_rs::emitter::emit(&program, &catalog, &locale).expect("emits"); + let reparsed = workshop_rs::parser::parse_with_context(&emitted, &catalog, &locale, &catalog) + .expect("emitted native controls reparse"); + assert!(workshop_rs::roundtrip::equivalent(&program, &reparsed)); + assert_eq!( + emitted, + workshop_rs::emitter::emit(&reparsed, &catalog, &locale).expect("re-emits") + ); +} + +#[test] +fn assign_member_rejects_non_lvalue_member_access_targets() { + let catalog = catalog(); + let mut program = wir::Program::default(); + let receiver = program + .values + .push(ValueNode::new(Value::EventPlayer, None)); + let member = program + .values + .push(ValueNode::new(Value::String("payload".into()), None)); + let member_access = program.values.push(ValueNode::new( + Value::Call { + name: "memberAccess".into(), + args: vec![receiver, member], + }, + None, + )); + let index = program.values.push(ValueNode::new( + Value::Number { + value: 1.0, + text: "1".into(), + }, + None, + )); + let target = program.values.push(ValueNode::new( + Value::Call { + name: "valueInArray".into(), + args: vec![member_access, index], + }, + None, + )); + let value = program.values.push(ValueNode::new(Value::Bool(true), None)); + let action = program.actions.push(Action::AssignMember { + target, + op: None, + value, + span: None, + }); + program.rules.push(wir::Rule { + name: "invalid member target".into(), + span: None, + name_span: None, + disabled: false, + event: Event::Global, + conditions: vec![], + actions: vec![action], + }); + + let error = validate::validate_canonical_ids(&program, &catalog) + .expect_err("AssignMember must require a memberAccess target"); + assert!( + error + .to_string() + .contains("AssignMember target must be a memberAccess value") + ); +} + fn span(file: workshop_rs::ids::Id, line: u32, col: u32, end_col: u32) -> Span { Span::new(file, Position::new(line, col), Position::new(line, end_col)) } diff --git a/docs/implementation-role.md b/docs/implementation-role.md index 4fdf028..994aef3 100644 --- a/docs/implementation-role.md +++ b/docs/implementation-role.md @@ -54,3 +54,33 @@ implementation. This keeps the shared Workshop contract useful without turning it into a union of every frontend/compiler's internal model. + +## Indexed members and control-flow lowering contract + +The canonical WIR contract for the interoperability surface in +`wrightkit/workshop-rs#123` is deliberately composed from native Workshop +identities: + +- indexing a declared global or player variable uses the catalog-backed + `set*VariableAtIndex` and `modify*VariableAtIndex` action calls; +- a member read is the `memberAccess(receiver, "member"[, index])` value call, + and `AssignMember` accepts only that canonical member-access value as its + assignment target without assigning it dictionary, object, or container type + semantics; +- native `Break`, `Continue`, `Skip`, and `Skip If` actions remain generic + catalog action calls and can occur inside the existing structured `If`, + `While`, and `For` actions. + +There are no `Dictionary`, `Switch`, or `Break` WIR nodes. OPY dictionary and +switch lowering remains an `opy-rs` concern: a source form is consumable only +after it is lowered to the native indexed/member or control-flow calls above. +If no lossless lowering exists, `opy-rs` must report its explicit integration +boundary rather than extending this Workshop contract with source-language +carriers. + +This classifies the remaining interoperability probes without using OPY syntax +as Workshop evidence: the dictionary-literal probe is an OPY-owned lowering +gap unless it folds to one of the native indexed/member forms, while the +nested and multiple switch-break probes have the required native `Break`, +`Skip`, and `Skip If` primitives but still require `opy-rs` to prove a lossless +source-to-Workshop lowering for their control-flow shape.