From 233af8ca8df241e00606241a38b8aaed5e86871c Mon Sep 17 00:00:00 2001 From: Evan Robertson Date: Thu, 9 Apr 2026 21:39:40 -0400 Subject: [PATCH] feat(parser): add PUBLISH/SUBSCRIBE/UNSUBSCRIBE and DEFINE EVENT parsing Implement full parsing support for ABL's event system: - Named event statements: PUBLISH, SUBSCRIBE, UNSUBSCRIBE - Class event declaration: DEFINE EVENT with SIGNATURE VOID(...) - 7 new keywords via codegen (PUBLISH, SUBSCRIBE, UNSUBSCRIBE, ANYWHERE, EVENT, SIGNATURE, RUN-PROCEDURE) - Hand-rolled event-name parser to avoid function-call ambiguity - Shared parse_run_arguments() helper extracted for reuse - SubscribeTarget enum for required IN/ANYWHERE choice - 19 new tests covering all syntax variants + disambiguation Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 4 +- crates/oxabl_ast/src/statement.rs | 62 +++ crates/oxabl_lexer/build.rs | 7 + crates/oxabl_lexer/src/kind.rs | 14 + crates/oxabl_parser/src/parser/mod.rs | 8 + crates/oxabl_parser/src/parser/statements.rs | 261 ++++++++++- crates/oxabl_parser/src/parser/tests.rs | 398 ++++++++++++++++- ...2026-04-09-publish-subscribe-brainstorm.md | 172 ++++++++ ...eat-publish-subscribe-event-system-plan.md | 408 ++++++++++++++++++ resources/keyword_overrides.toml | 33 ++ 10 files changed, 1361 insertions(+), 6 deletions(-) create mode 100644 docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md create mode 100644 docs/plans/2026-04-09-001-feat-publish-subscribe-event-system-plan.md diff --git a/CLAUDE.md b/CLAUDE.md index 24f9267..d3ebd25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -96,14 +96,14 @@ Defines AST nodes for the parser. Key types: Parses ABL source code into an AST. Key capabilities: - **Expression parsing** with proper operator precedence (ternary → or → and → comparison → additive → multiplicative → unary → postfix → primary) -- **Statement parsing**: DEFINE VARIABLE/VAR/PARAMETER/TEMP-TABLE/BUFFER/PROPERTY/STREAM/FRAME, DO blocks (with counting loops), IF/THEN/ELSE, REPEAT, FOR EACH, FIND, CASE, PROCEDURE, FUNCTION, RUN, DISPLAY (with STREAM clause), MESSAGE, ASSIGN, CREATE, DELETE, RELEASE, VALIDATE, BUFFER-COPY, BUFFER-COMPARE, INPUT/OUTPUT/INPUT-OUTPUT stream I/O (FROM/TO/THROUGH/CLOSE), CATCH/FINALLY/THROW, LEAVE, NEXT, RETURN +- **Statement parsing**: DEFINE VARIABLE/VAR/PARAMETER/TEMP-TABLE/BUFFER/PROPERTY/STREAM/FRAME/EVENT, DO blocks (with counting loops), IF/THEN/ELSE, REPEAT, FOR EACH, FIND, CASE, PROCEDURE, FUNCTION, RUN, DISPLAY (with STREAM clause), MESSAGE, ASSIGN, CREATE, DELETE, RELEASE, VALIDATE, BUFFER-COPY, BUFFER-COMPARE, INPUT/OUTPUT/INPUT-OUTPUT stream I/O (FROM/TO/THROUGH/CLOSE), CATCH/FINALLY/THROW, PUBLISH/SUBSCRIBE/UNSUBSCRIBE, LEAVE, NEXT, RETURN - **OO-ABL**: CLASS (with ABSTRACT/FINAL, INHERITS, IMPLEMENTS), INTERFACE, METHOD (with access modifiers, STATIC/ABSTRACT/OVERRIDE), DEFINE PROPERTY (auto and computed GET/SET), CONSTRUCTOR, DESTRUCTOR, USING - **Postfix operations**: Method calls (object:method()), member access (object.member), array access (arr[i]), field access (table.field) - **Function calls** with argument lists - **Preprocessor**: &IF/&ELSEIF/&ELSE/&ENDIF at statement, expression, and data type levels via generic `PreprocIf`, &SCOPED-DEFINE/&GLOBAL-DEFINE with `PreprocEnd` lexer token, &UNDEFINE, &MESSAGE, `{&variable}` references - **Error recovery** via `parse_program()` with synchronization on period boundaries -Not yet implemented: DATASET, PUBLISH/SUBSCRIBE, ON triggers. +Not yet implemented: ON triggers. ### Code Generation (`oxabl_codegen`) diff --git a/crates/oxabl_ast/src/statement.rs b/crates/oxabl_ast/src/statement.rs index 18f39d4..040f405 100644 --- a/crates/oxabl_ast/src/statement.rs +++ b/crates/oxabl_ast/src/statement.rs @@ -453,6 +453,55 @@ pub enum Statement { source_buffers: Vec, }, + /// PUBLISH event-name [FROM publisher-handle] [(args...)]. + Publish { + /// Event name — string literal or character expression. + event_name: Expression, + /// Optional FROM publisher-handle. + from_handle: Option, + /// Arguments passed to subscribers (reuses RunArgument). + arguments: Vec, + }, + + /// SUBSCRIBE [PROCEDURE subscriber-handle] [TO] event-name {IN handle | ANYWHERE} + /// [RUN-PROCEDURE handler-name] [NO-ERROR]. + Subscribe { + /// Optional PROCEDURE subscriber-handle. + subscriber: Option, + /// Event name — string literal or character expression. + event_name: Expression, + /// IN publisher-handle or ANYWHERE (required). + target: SubscribeTarget, + /// Optional RUN-PROCEDURE handler name. + run_procedure: Option, + /// Whether NO-ERROR was specified. + no_error: bool, + }, + + /// UNSUBSCRIBE [PROCEDURE subscriber-handle] [TO] {event-name | ALL} [IN publisher-handle]. + Unsubscribe { + /// Optional PROCEDURE subscriber-handle. + subscriber: Option, + /// Event name, or None if ALL was specified. + event_name: Option, + /// Optional IN publisher-handle. + in_handle: Option, + }, + + /// DEFINE [access] [STATIC] [ABSTRACT] EVENT event-name SIGNATURE VOID (params...). + DefineEvent { + /// Access modifier (defaults to PUBLIC). + access: AccessModifier, + /// Whether STATIC was specified. + is_static: bool, + /// Whether ABSTRACT was specified. + is_abstract: bool, + /// Event name. + name: Identifier, + /// Signature parameters (reuses DefineParameter via Vec). + parameters: Vec, + }, + /// INPUT/OUTPUT/INPUT-OUTPUT stream I/O statement. StreamIo { direction: StreamDirection, @@ -803,3 +852,16 @@ pub struct HandlePassingOptions { pub bind: bool, pub by_value: bool, } + +// ============================================================================= +// Event system types +// ============================================================================= + +/// Target for SUBSCRIBE — where to listen for events. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SubscribeTarget { + /// Subscribe to events from a specific publisher handle. + InHandle(Expression), + /// Subscribe to events from any publisher. + Anywhere, +} diff --git a/crates/oxabl_lexer/build.rs b/crates/oxabl_lexer/build.rs index 76af0e0..934db8e 100644 --- a/crates/oxabl_lexer/build.rs +++ b/crates/oxabl_lexer/build.rs @@ -62,6 +62,7 @@ fn main() { "analyze", "and", "any", + "anywhere", "append", "apply", "as", @@ -208,6 +209,7 @@ fn main() { "error status", "escape", "etime", + "event", "event procedure", "except", "exclusive-lock", @@ -464,6 +466,7 @@ fn main() { "provers", "proversion", "public", + "publish", "put", "put-byte", "put-key-val", @@ -502,6 +505,7 @@ fn main() { "revoke", "rowid", "run", + "run-procedure", "save", "sax-comple", "sax-complete", @@ -540,6 +544,7 @@ fn main() { "shared", "show-stat", "show-stats", + "signature", "single-run", "skip", "skip-deleted-record", @@ -550,6 +555,7 @@ fn main() { "stream", "stream-handle", "stream-io", + "subscribe", "system-dialog", "table", "table-handle", @@ -584,6 +590,7 @@ fn main() { "union", "unique", "unix", + "unsubscribe", "up", "update", "use-index", diff --git a/crates/oxabl_lexer/src/kind.rs b/crates/oxabl_lexer/src/kind.rs index 7fcd6e2..0e9e0f6 100644 --- a/crates/oxabl_lexer/src/kind.rs +++ b/crates/oxabl_lexer/src/kind.rs @@ -156,6 +156,9 @@ pub enum Kind { BufferCompare, Close, ParentIdRelation, + Publish, + Subscribe, + Unsubscribe, // Functions Accum, @@ -316,6 +319,10 @@ pub enum Kind { TableHandle, Bind, ByValue, + Anywhere, + Event, + Signature, + RunProcedure, // Phrases Editing, @@ -762,6 +769,7 @@ pub fn match_keyword(s: &str) -> Option { "displ" => Some(Kind::Display), "entry" => Some(Kind::Entry), "etime" => Some(Kind::Etime), + "event" => Some(Kind::Event), "false" => Some(Kind::KwFalse), "fetch" => Some(Kind::Fetch), "field" => Some(Kind::Field), @@ -989,6 +997,7 @@ pub fn match_keyword(s: &str) -> Option { "promsgs" => Some(Kind::Promsgs), "propath" => Some(Kind::Propath), "provers" => Some(Kind::Proversion), + "publish" => Some(Kind::Publish), "putbyte" => Some(Kind::Putbyte), "r-index" => Some(Kind::RIndex), "readkey" => Some(Kind::Readkey), @@ -1018,6 +1027,7 @@ pub fn match_keyword(s: &str) -> Option { "abstract" => Some(Kind::Abstract), "accumula" => Some(Kind::Accumulate), "ambiguou" => Some(Kind::Ambiguous), + "anywhere" => Some(Kind::Anywhere), "ascendin" => Some(Kind::Ascending), "attr spa" => Some(Kind::AttrSpace), "attr-spa" => Some(Kind::AttrSpace), @@ -1192,7 +1202,9 @@ pub fn match_keyword(s: &str) -> Option { "screen-io" => Some(Kind::ScreenIo), "setuserid" => Some(Kind::Setuserid), "show-stat" => Some(Kind::ShowStats), + "signature" => Some(Kind::Signature), "stream-io" => Some(Kind::StreamIo), + "subscribe" => Some(Kind::Subscribe), "underline" => Some(Kind::Underline), "unformatt" => Some(Kind::Unformatted), "use-index" => Some(Kind::UseIndex), @@ -1349,6 +1361,7 @@ pub fn match_keyword(s: &str) -> Option { "thread-safe" => Some(Kind::ThreadSafe), "transaction" => Some(Kind::Transaction), "unformatted" => Some(Kind::Unformatted), + "unsubscribe" => Some(Kind::Unsubscribe), "widget-pool" => Some(Kind::WidgetPool), _ => None, }, @@ -1432,6 +1445,7 @@ pub fn match_keyword(s: &str) -> Option { "put-key-value" => Some(Kind::PutKeyValue), "query-off-end" => Some(Kind::QueryOffEnd), "rcode-informa" => Some(Kind::RcodeInformation), + "run-procedure" => Some(Kind::RunProcedure), "sax-write-tag" => Some(Kind::SaxWriteTag), "search-targer" => Some(Kind::SearchTarger), "stream-handle" => Some(Kind::StreamHandle), diff --git a/crates/oxabl_parser/src/parser/mod.rs b/crates/oxabl_parser/src/parser/mod.rs index 39f0957..5b88934 100644 --- a/crates/oxabl_parser/src/parser/mod.rs +++ b/crates/oxabl_parser/src/parser/mod.rs @@ -271,6 +271,14 @@ impl<'a> Parser<'a> { | Kind::ByValue | Kind::Query | Kind::Reposition + // Event system keywords (unreserved) + | Kind::Publish + | Kind::Subscribe + | Kind::Unsubscribe + | Kind::Anywhere + | Kind::Event + | Kind::Signature + | Kind::RunProcedure ) } diff --git a/crates/oxabl_parser/src/parser/statements.rs b/crates/oxabl_parser/src/parser/statements.rs index 36851bd..3bebf08 100644 --- a/crates/oxabl_parser/src/parser/statements.rs +++ b/crates/oxabl_parser/src/parser/statements.rs @@ -3,6 +3,7 @@ //! Handles DEFINE VARIABLE, VAR, ASSIGN, assignments, DO blocks (with counting loops), //! IF/THEN/ELSE, REPEAT, FOR EACH, FIND, CASE, PROCEDURE, FUNCTION, RUN, DISPLAY, //! MESSAGE, CLASS, INTERFACE, METHOD, PROPERTY, CONSTRUCTOR, DESTRUCTOR, USING, +//! PUBLISH, SUBSCRIBE, UNSUBSCRIBE, DEFINE EVENT, //! LEAVE, NEXT, and RETURN statements. use oxabl_ast::{ @@ -10,8 +11,8 @@ use oxabl_ast::{ DataSourceBuffer, DataSourceKeys, DisplayItem, Expression, FieldTypeSource, FindType, HandleParamKind, HandlePassingOptions, Identifier, IndexField, LockType, ParameterDirection, ParameterType, ParentIdRelation, PreprocIf, RunArgument, RunTarget, SortDirection, Span, - Statement, StreamDirection, StreamOperation, TempTableField, TempTableIndex, UseIndex, - WhenBranch, XmlSerializeOptions, + Statement, StreamDirection, StreamOperation, SubscribeTarget, TempTableField, TempTableIndex, + UseIndex, WhenBranch, XmlSerializeOptions, }; use oxabl_lexer::Kind; @@ -61,6 +62,9 @@ pub(crate) fn can_start_statement(kind: Kind) -> bool { | Kind::Input | Kind::Output | Kind::InputOutput + | Kind::Publish + | Kind::Subscribe + | Kind::Unsubscribe ) } @@ -126,6 +130,17 @@ impl Parser<'_> { return self.parse_run_statement(); } + // Event system statements + if self.check(Kind::Publish) { + return self.parse_publish_statement(); + } + if self.check(Kind::Subscribe) { + return self.parse_subscribe_statement(); + } + if self.check(Kind::Unsubscribe) { + return self.parse_unsubscribe_statement(); + } + // PROCEDURE statement if self.check(Kind::Procedure) { return self.parse_procedure(); @@ -307,10 +322,27 @@ impl Parser<'_> { access }; + // Check for ABSTRACT (used by DEFINE EVENT) + let is_abstract = if self.check(Kind::Abstract) { + self.advance(); + true + } else { + false + }; + if self.check(Kind::Property) { return self.parse_define_property(access.unwrap_or(AccessModifier::Public), is_static); } + // DEFINE EVENT + if self.check(Kind::Event) { + return self.parse_define_event( + access.unwrap_or(AccessModifier::Public), + is_static, + is_abstract, + ); + } + // Parse SERIALIZABLE / NON-SERIALIZABLE (dataset-specific, before DATASET keyword) let serializable = self.check(Kind::Serializable) && { self.advance(); @@ -368,7 +400,7 @@ impl Parser<'_> { } else { return Err(ParseError { message: - "Expected VARIABLE, VAR, TEMP-TABLE, BUFFER, STREAM, FRAME, DATASET, or DATA-SOURCE after DEFINE" + "Expected VARIABLE, VAR, TEMP-TABLE, BUFFER, STREAM, FRAME, DATASET, DATA-SOURCE, or EVENT after DEFINE" .to_string(), span: Span { start: self.peek().start as u32, @@ -2806,6 +2838,229 @@ impl Parser<'_> { }) } + // ── Event system parsing ──────────────────────────────────────── + + /// Parse event name: string literal, VALUE(expr), or bare identifier. + /// + /// Uses a hand-rolled approach matching `parse_run_statement` to avoid + /// `parse_primary()` or `parse_expression()` consuming parenthesized + /// arguments as a function call. + fn parse_event_name(&mut self) -> ParseResult { + if self.check(Kind::Value) { + // VALUE(expr) + self.advance(); + self.expect_kind(Kind::LeftParen, "Expected '(' after VALUE")?; + let expr = self.parse_expression()?; + self.expect_kind(Kind::RightParen, "Expected ')' after VALUE expression")?; + Ok(expr) + } else if self.check(Kind::StringLiteral) { + // String literal event name — safe to use parse_primary (no function-call promotion) + self.parse_primary() + } else if Self::can_be_identifier(self.peek().kind) || self.check(Kind::Identifier) { + // Bare identifier — do NOT use parse_primary() which promotes identifier( to function call. + // Instead, parse just the identifier and return it as an Expression::Identifier. + let ident = self.parse_identifier()?; + Ok(Expression::Identifier(ident)) + } else { + Err(ParseError { + message: "Expected event name (string literal, identifier, or VALUE expression)" + .to_string(), + span: self.current_span(), + }) + } + } + + /// Parse RUN-style parenthesized arguments: `(INPUT x, OUTPUT y, ...)`. + fn parse_run_arguments(&mut self) -> ParseResult> { + if !self.check(Kind::LeftParen) { + return Ok(Vec::new()); + } + self.advance(); // consume ( + + let mut args = Vec::new(); + if !self.check(Kind::RightParen) { + loop { + let direction = match self.peek().kind { + Kind::Input => { + self.advance(); + ParameterDirection::Input + } + Kind::Output => { + self.advance(); + ParameterDirection::Output + } + Kind::InputOutput => { + self.advance(); + ParameterDirection::InputOutput + } + _ => ParameterDirection::Input, + }; + + let expression = self.parse_expression()?; + args.push(RunArgument { + direction, + expression, + }); + + if !self.check(Kind::Comma) { + break; + } + self.advance(); // consume comma + } + } + + self.expect_kind(Kind::RightParen, "Expected ')' after arguments")?; + Ok(args) + } + + /// PUBLISH event-name [FROM publisher-handle] [(args...)]. + fn parse_publish_statement(&mut self) -> ParseResult { + self.advance(); // consume PUBLISH + + let event_name = self.parse_event_name()?; + + let from_handle = if self.check(Kind::From) { + self.advance(); + Some(self.parse_expression()?) + } else { + None + }; + + let arguments = self.parse_run_arguments()?; + + self.expect_kind(Kind::Period, "Expected '.' after PUBLISH statement")?; + + Ok(Statement::Publish { + event_name, + from_handle, + arguments, + }) + } + + /// SUBSCRIBE [PROCEDURE subscriber-handle] [TO] event-name {IN handle | ANYWHERE} + /// [RUN-PROCEDURE handler-name] [NO-ERROR]. + fn parse_subscribe_statement(&mut self) -> ParseResult { + self.advance(); // consume SUBSCRIBE + + // Optional PROCEDURE subscriber-handle + let subscriber = if self.check(Kind::Procedure) { + self.advance(); + Some(self.parse_expression()?) + } else { + None + }; + + // Optional TO noise word + if self.check(Kind::To) { + self.advance(); + } + + let event_name = self.parse_event_name()?; + + // Required: IN handle or ANYWHERE + let target = if self.check(Kind::KwIn) { + self.advance(); + SubscribeTarget::InHandle(self.parse_expression()?) + } else if self.check(Kind::Anywhere) { + self.advance(); + SubscribeTarget::Anywhere + } else { + return Err(ParseError { + message: "Expected IN or ANYWHERE after event name in SUBSCRIBE".to_string(), + span: self.current_span(), + }); + }; + + // Optional RUN-PROCEDURE handler-name + let run_procedure = if self.check(Kind::RunProcedure) { + self.advance(); + Some(self.parse_identifier()?) + } else { + None + }; + + let no_error = self.parse_no_error(); + self.expect_kind(Kind::Period, "Expected '.' after SUBSCRIBE statement")?; + + Ok(Statement::Subscribe { + subscriber, + event_name, + target, + run_procedure, + no_error, + }) + } + + /// UNSUBSCRIBE [PROCEDURE subscriber-handle] [TO] {event-name | ALL} [IN publisher-handle]. + fn parse_unsubscribe_statement(&mut self) -> ParseResult { + self.advance(); // consume UNSUBSCRIBE + + // Optional PROCEDURE subscriber-handle + let subscriber = if self.check(Kind::Procedure) { + self.advance(); + Some(self.parse_expression()?) + } else { + None + }; + + // Optional TO noise word + if self.check(Kind::To) { + self.advance(); + } + + // event-name or ALL + let event_name = if self.check(Kind::All) { + self.advance(); + None + } else { + Some(self.parse_event_name()?) + }; + + // Optional IN publisher-handle + let in_handle = if self.check(Kind::KwIn) { + self.advance(); + Some(self.parse_expression()?) + } else { + None + }; + + self.expect_kind(Kind::Period, "Expected '.' after UNSUBSCRIBE statement")?; + + Ok(Statement::Unsubscribe { + subscriber, + event_name, + in_handle, + }) + } + + /// DEFINE [access] [STATIC] [ABSTRACT] EVENT event-name SIGNATURE VOID (params...). + fn parse_define_event( + &mut self, + access: AccessModifier, + is_static: bool, + is_abstract: bool, + ) -> ParseResult { + self.advance(); // consume EVENT + + let name = self.parse_identifier()?; + + self.expect_kind(Kind::Signature, "Expected SIGNATURE after event name")?; + self.expect_kind(Kind::Void, "Expected VOID after SIGNATURE")?; + + // Parse parameter list using existing helper + let parameters = self.parse_parenthesized_params()?; + + self.expect_kind(Kind::Period, "Expected '.' after DEFINE EVENT statement")?; + + Ok(Statement::DefineEvent { + access, + is_static, + is_abstract, + name, + parameters, + }) + } + /// Helper: consume NO-ERROR if present. fn parse_no_error(&mut self) -> bool { if self.check(Kind::NoError) { diff --git a/crates/oxabl_parser/src/parser/tests.rs b/crates/oxabl_parser/src/parser/tests.rs index 5151d97..22efb14 100644 --- a/crates/oxabl_parser/src/parser/tests.rs +++ b/crates/oxabl_parser/src/parser/tests.rs @@ -3,7 +3,8 @@ use oxabl_ast::{ AccessModifier, BooleanLiteral, BufferTarget, CreateTarget, CreateTargetKind, DataSourceKeys, DataType, DecimalLiteral, Expression, FieldTypeSource, FindType, HandleParamKind, Identifier, IntegerLiteral, Literal, LockType, ParameterDirection, ParameterType, RunTarget, SortDirection, - Span, Statement, StreamDirection, StreamOperation, StringLiteral, UnknownLiteral, WhenBranch, + Span, Statement, StreamDirection, StreamOperation, StringLiteral, SubscribeTarget, + UnknownLiteral, WhenBranch, }; use oxabl_lexer::tokenize; use rust_decimal::Decimal; @@ -7147,3 +7148,398 @@ fn parse_define_buffer_xml_options() { _ => panic!("Expected DefineBuffer"), } } + +// ── PUBLISH tests ─────────────────────────────────────────────────── + +#[test] +fn parse_publish_string_literal() { + let source = r#"PUBLISH "NewCustomer"."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Publish { + event_name, + from_handle, + arguments, + } => { + assert!(matches!( + event_name, + Expression::Literal(Literal::String(_)) + )); + assert!(from_handle.is_none()); + assert!(arguments.is_empty()); + } + _ => panic!("Expected Publish statement"), + } +} + +#[test] +fn parse_publish_with_from() { + let source = r#"PUBLISH "NewCustomer" FROM hProc."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Publish { + from_handle, + arguments, + .. + } => { + assert!(from_handle.is_some()); + assert!(arguments.is_empty()); + } + _ => panic!("Expected Publish statement"), + } +} + +#[test] +fn parse_publish_with_params() { + let source = r#"PUBLISH "NewCustomer" (INPUT cName)."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Publish { + arguments, + from_handle, + .. + } => { + assert!(from_handle.is_none()); + assert_eq!(arguments.len(), 1); + assert_eq!(arguments[0].direction, ParameterDirection::Input); + } + _ => panic!("Expected Publish statement"), + } +} + +#[test] +fn parse_publish_expression_event() { + let source = "PUBLISH cEventName."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Publish { + event_name, + arguments, + .. + } => { + match &event_name { + Expression::Identifier(id) => assert_eq!(id.name, "cEventName"), + _ => panic!("Expected identifier event name"), + } + assert!(arguments.is_empty()); + } + _ => panic!("Expected Publish statement"), + } +} + +// ── SUBSCRIBE tests ───────────────────────────────────────────────── + +#[test] +fn parse_subscribe_anywhere() { + let source = r#"SUBSCRIBE TO "NewCustomer" ANYWHERE."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Subscribe { + subscriber, + target, + run_procedure, + no_error, + .. + } => { + assert!(subscriber.is_none()); + assert_eq!(target, SubscribeTarget::Anywhere); + assert!(run_procedure.is_none()); + assert!(!no_error); + } + _ => panic!("Expected Subscribe statement"), + } +} + +#[test] +fn parse_subscribe_in_handle() { + let source = r#"SUBSCRIBE TO "NewCustomer" IN hPub."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Subscribe { target, .. } => { + assert!(matches!(target, SubscribeTarget::InHandle(_))); + } + _ => panic!("Expected Subscribe statement"), + } +} + +#[test] +fn parse_subscribe_with_procedure() { + let source = r#"SUBSCRIBE PROCEDURE hSub TO "NewCustomer" IN hPub."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Subscribe { + subscriber, target, .. + } => { + assert!(subscriber.is_some()); + assert!(matches!(target, SubscribeTarget::InHandle(_))); + } + _ => panic!("Expected Subscribe statement"), + } +} + +#[test] +fn parse_subscribe_with_run_procedure() { + let source = r#"SUBSCRIBE TO "NewCustomer" IN hPub RUN-PROCEDURE MyHandler."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Subscribe { run_procedure, .. } => { + assert_eq!(run_procedure.as_ref().unwrap().name, "MyHandler"); + } + _ => panic!("Expected Subscribe statement"), + } +} + +#[test] +fn parse_subscribe_no_to() { + let source = r#"SUBSCRIBE "NewCustomer" ANYWHERE."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Subscribe { target, .. } => { + assert_eq!(target, SubscribeTarget::Anywhere); + } + _ => panic!("Expected Subscribe statement"), + } +} + +// ── UNSUBSCRIBE tests ─────────────────────────────────────────────── + +#[test] +fn parse_unsubscribe_event() { + let source = r#"UNSUBSCRIBE TO "NewCustomer"."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Unsubscribe { + subscriber, + event_name, + in_handle, + } => { + assert!(subscriber.is_none()); + assert!(event_name.is_some()); + assert!(in_handle.is_none()); + } + _ => panic!("Expected Unsubscribe statement"), + } +} + +#[test] +fn parse_unsubscribe_all() { + let source = "UNSUBSCRIBE TO ALL."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Unsubscribe { + event_name, + in_handle, + .. + } => { + assert!(event_name.is_none()); + assert!(in_handle.is_none()); + } + _ => panic!("Expected Unsubscribe statement"), + } +} + +#[test] +fn parse_unsubscribe_with_procedure() { + let source = r#"UNSUBSCRIBE PROCEDURE hSub TO "NewCustomer"."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Unsubscribe { subscriber, .. } => { + assert!(subscriber.is_some()); + } + _ => panic!("Expected Unsubscribe statement"), + } +} + +#[test] +fn parse_unsubscribe_with_in_handle() { + let source = r#"UNSUBSCRIBE TO "NewCustomer" IN hPub."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Unsubscribe { in_handle, .. } => { + assert!(in_handle.is_some()); + } + _ => panic!("Expected Unsubscribe statement"), + } +} + +// ── DEFINE EVENT tests ────────────────────────────────────────────── + +#[test] +fn parse_define_event_minimal() { + let source = "DEFINE EVENT MyEvent SIGNATURE VOID ()."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::DefineEvent { + access, + is_static, + is_abstract, + name, + parameters, + } => { + assert_eq!(access, AccessModifier::Public); + assert!(!is_static); + assert!(!is_abstract); + assert_eq!(name.name, "MyEvent"); + assert!(parameters.is_empty()); + } + _ => panic!("Expected DefineEvent statement"), + } +} + +#[test] +fn parse_define_event_abstract() { + let source = "DEFINE PROTECTED ABSTRACT EVENT MyEvent SIGNATURE VOID ()."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::DefineEvent { + access, + is_abstract, + name, + .. + } => { + assert_eq!(access, AccessModifier::Protected); + assert!(is_abstract); + assert_eq!(name.name, "MyEvent"); + } + _ => panic!("Expected DefineEvent statement"), + } +} + +#[test] +fn parse_define_event_multiple_params() { + let source = "DEFINE PUBLIC EVENT MyEvent SIGNATURE VOID (INPUT p1 AS INTEGER, INPUT p2 AS CHARACTER, OUTPUT p3 AS LOGICAL)."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::DefineEvent { + parameters, name, .. + } => { + assert_eq!(name.name, "MyEvent"); + assert_eq!(parameters.len(), 3); + // Verify first param + match ¶meters[0] { + Statement::DefineParameter { + direction, + param_type, + } => { + assert_eq!(*direction, ParameterDirection::Input); + match param_type { + ParameterType::Variable { + name, data_type, .. + } => { + assert_eq!(name.name, "p1"); + assert_eq!(*data_type, DataType::Integer); + } + _ => panic!("Expected Variable param type"), + } + } + _ => panic!("Expected DefineParameter"), + } + // Verify third param is OUTPUT + match ¶meters[2] { + Statement::DefineParameter { direction, .. } => { + assert_eq!(*direction, ParameterDirection::Output); + } + _ => panic!("Expected DefineParameter"), + } + } + _ => panic!("Expected DefineEvent statement"), + } +} + +// ── Integration tests ─────────────────────────────────────────────── + +#[test] +fn parse_publish_in_do_block() { + let source = r#"DO: + PUBLISH "NewCustomer" (INPUT cName). +END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Do { body, .. } => { + assert_eq!(body.len(), 1); + assert!(matches!(body[0], Statement::Publish { .. })); + } + _ => panic!("Expected Do statement"), + } +} + +#[test] +fn parse_define_event_in_interface() { + let source = "INTERFACE IObservable: + DEFINE PUBLIC EVENT OnChange SIGNATURE VOID (INPUT pValue AS CHARACTER). +END INTERFACE."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Interface { body, name, .. } => { + assert_eq!(name.name, "IObservable"); + assert_eq!(body.len(), 1); + assert!(matches!(body[0], Statement::DefineEvent { .. })); + } + _ => panic!("Expected Interface statement"), + } +} + +// ── Negative / disambiguation test ────────────────────────────────── + +#[test] +fn parse_publish_event_name_not_function_call() { + // Verify that `myEvent (INPUT x)` is parsed as event name `myEvent` + // with argument list `(INPUT x)`, NOT as function call `myEvent(INPUT x)`. + let source = "PUBLISH myEvent (INPUT x)."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Publish { + event_name, + arguments, + .. + } => { + // Event name should be a plain identifier, not a function call + match &event_name { + Expression::Identifier(id) => assert_eq!(id.name, "myEvent"), + _ => panic!("Expected identifier event name, got {:?}", event_name), + } + // Arguments should be parsed as PUBLISH arguments + assert_eq!(arguments.len(), 1); + assert_eq!(arguments[0].direction, ParameterDirection::Input); + } + _ => panic!("Expected Publish statement"), + } +} diff --git a/docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md b/docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md new file mode 100644 index 0000000..2a541c6 --- /dev/null +++ b/docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md @@ -0,0 +1,172 @@ +# Brainstorm: PUBLISH/SUBSCRIBE Event System + +**Date:** 2026-04-09 +**Status:** Complete +**Author:** Claude + Evan + +## What We're Building + +Full parsing support for ABL's event system, covering both: + +1. **Named events** (procedure-based statements): `PUBLISH`, `SUBSCRIBE`, `UNSUBSCRIBE` +2. **Class events** (OO-ABL): `DEFINE EVENT` with `SIGNATURE VOID(...)` parameter definitions + +Class event method calls (`:Publish()`, `:Subscribe()`, `:Unsubscribe()`) are syntactically regular method calls and are already handled by existing postfix parsing. No new parsing is needed for those. + +## Why This Approach + +### Unified AST with Shared Event Parameters + +Named events and class events both deal with event names and parameter lists. Rather than duplicating parameter representations, we use a shared `EventParameter` type across: + +- `PUBLISH` statement parameters +- `DEFINE EVENT SIGNATURE VOID(...)` parameter definitions + +This reduces duplication, makes future semantic validation easier (checking that a PUBLISH call matches a DEFINE EVENT signature), and keeps the AST consistent. + +### Parse Fully, Validate Later + +The parser captures complete syntax structure including SIGNATURE parameter types, modes (INPUT/OUTPUT/INPUT-OUTPUT), and names. Cross-reference validation (e.g., "does this Publish() call match the event's defined signature?") is deferred to a future semantic analysis pass — standard compiler architecture. + +## Key Decisions + +- **Scope**: Named events (PUBLISH/SUBSCRIBE/UNSUBSCRIBE) + class events (DEFINE EVENT). Full suite. +- **UNSUBSCRIBE**: Included in this effort, not deferred. +- **AST design**: Unified — shared `EventParameter` type across named and class event constructs. +- **Class event methods**: `:Publish()`, `:Subscribe()`, `:Unsubscribe()` are already parseable as method calls via existing postfix parsing. No special-casing needed. +- **Validation**: Full syntax parsed into AST. Semantic cross-reference validation deferred to a future pass. +- **Keywords to add**: `PUBLISH`, `SUBSCRIBE`, `UNSUBSCRIBE`, `ANYWHERE`, `EVENT` (if not present), `SIGNATURE`, `VOID`. Existing keywords reused: `FROM`, `TO`, `IN`, `ALL`, `NO-ERROR`, `PROCEDURE`, `RUN-PROCEDURE`. + +## Syntax Reference + +### PUBLISH (named event) + +``` +PUBLISH event-name + [ FROM publisher-handle ] + [ ( parameter [ , parameter ] ... ) ]. +``` + +- `event-name`: quoted string or character expression +- `FROM publisher-handle`: procedure/widget handle (defaults to THIS-PROCEDURE) +- Parameters: INPUT, OUTPUT, INPUT-OUTPUT (same syntax as RUN statement) +- Implicit NO-ERROR behavior + +### SUBSCRIBE (named event) + +``` +SUBSCRIBE [ PROCEDURE subscriber-handle ] [ TO ] event-name + { IN publisher-handle | ANYWHERE } + [ RUN-PROCEDURE local-internal-procedure ] + [ NO-ERROR ]. +``` + +- `PROCEDURE subscriber-handle`: optional, defaults to THIS-PROCEDURE +- `TO`: optional noise word +- `IN publisher-handle` or `ANYWHERE`: mutually exclusive, required +- `RUN-PROCEDURE`: names the handler internal procedure (defaults to event name) + +### UNSUBSCRIBE (named event) + +``` +UNSUBSCRIBE [ PROCEDURE subscriber-handle ] + [ TO ] { event-name | ALL } + [ IN publisher-handle ]. +``` + +- `event-name` or `ALL`: which subscriptions to cancel +- `IN publisher-handle`: limit to specific publisher +- Implicit NO-ERROR behavior + +### DEFINE EVENT (class event) + +``` +DEFINE [ access-mode ] [ STATIC | ABSTRACT ] + EVENT event-name + SIGNATURE VOID ( [ parameter-definition [ , ... ] ] ). +``` + +- Access modes: PRIVATE, PROTECTED, PUBLIC (etc.) +- STATIC or ABSTRACT modifiers +- SIGNATURE always VOID return type +- Parameter definitions follow standard ABL parameter syntax + +## Proposed AST Nodes + +### Named Event Statements + +``` +Statement::Publish { + event: Expression, // event name (string literal or expression) + from_handle: Option, // FROM publisher-handle + arguments: Vec, // parameter list +} + +Statement::Subscribe { + subscriber: Option, // PROCEDURE subscriber-handle + event: Expression, // event name + target: SubscribeTarget, // IN handle | ANYWHERE + run_procedure: Option, // RUN-PROCEDURE name + no_error: bool, +} + +Statement::Unsubscribe { + subscriber: Option, // PROCEDURE subscriber-handle + event: UnsubscribeEvent, // event-name | ALL + in_handle: Option, // IN publisher-handle +} + +enum SubscribeTarget { + InHandle(Expression), + Anywhere, +} + +enum UnsubscribeEvent { + Named(Expression), + All, +} +``` + +### Class Event Definition + +``` +Statement::DefineEvent { + name: Identifier, + access: Option, + is_static: bool, + is_abstract: bool, + signature: Vec, // shared type +} +``` + +### Shared Type + +``` +struct EventParameter { + mode: ParameterMode, // INPUT, OUTPUT, INPUT-OUTPUT + name: Identifier, + data_type: DataType, +} + +enum ParameterMode { + Input, + Output, + InputOutput, +} +``` + +## Implementation Touchpoints + +1. **keyword_overrides.toml** — Add PUBLISH, SUBSCRIBE, UNSUBSCRIBE, ANYWHERE, EVENT, SIGNATURE, VOID + run codegen +2. **oxabl_ast/src/statement.rs** — Add Publish, Subscribe, Unsubscribe, DefineEvent variants + supporting types +3. **oxabl_parser/src/parser/statements.rs** — Add to `can_start_statement()` dispatch + implement parse functions +4. **oxabl_parser/src/parser/statements.rs** — Extend `parse_define_statement()` to handle DEFINE EVENT +5. **Tests** — Comprehensive test cases for all syntax variants + +## Open Questions + +None — all questions resolved during brainstorming. + +## Next Steps + +Run `/ce:plan` to create a detailed implementation plan. diff --git a/docs/plans/2026-04-09-001-feat-publish-subscribe-event-system-plan.md b/docs/plans/2026-04-09-001-feat-publish-subscribe-event-system-plan.md new file mode 100644 index 0000000..1cb0648 --- /dev/null +++ b/docs/plans/2026-04-09-001-feat-publish-subscribe-event-system-plan.md @@ -0,0 +1,408 @@ +--- +title: "feat: Add PUBLISH/SUBSCRIBE/UNSUBSCRIBE event system and DEFINE EVENT" +type: feat +status: completed +date: 2026-04-09 +origin: docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md +--- + +# feat: Add PUBLISH/SUBSCRIBE/UNSUBSCRIBE Event System and DEFINE EVENT + +## Enhancement Summary + +**Deepened on:** 2026-04-09 +**Sections enhanced:** 6 +**Review agents used:** architecture-strategist, pattern-recognition-specialist, performance-oracle, code-simplicity-reviewer, software-architect, ABL syntax researcher + +### Key Improvements +1. Fixed critical `parse_primary()` inaccuracy — must use hand-rolled event-name parser (parse_primary would still consume function calls) +2. Eliminated redundant `all: bool` in Unsubscribe — use `Option` alone or an enum +3. Changed `run_procedure: Option` to `Option` for span tracking consistency +4. Confirmed interface body parsing needs NO changes (parse_interface already delegates to parse_statement) +5. Confirmed ANYWHERE is NOT valid in UNSUBSCRIBE syntax +6. Discovered event names can be full character expressions per ABL docs — impacts parsing strategy + +### New Considerations Discovered +- `parse_primary()` CANNOT be used for event names — it promotes `identifier(` to function calls (lines 396-398 of expressions.rs), which is the exact bug we're trying to avoid +- `EventSignatureParam` can be eliminated by reusing `Vec` with `DefineParameter` variants (matching how Method handles parameters) +- RUN-PROCEDURE is confirmed as a single hyphenated keyword token in the ABL keyword index +- SUBSCRIBE without IN or ANYWHERE is a compile error per ABL docs (braces = required choice) + +--- + +## Overview + +Add full parsing support for ABL's event system: the named-event statements (`PUBLISH`, `SUBSCRIBE`, `UNSUBSCRIBE`) and the class-level `DEFINE EVENT` declaration with `SIGNATURE VOID(...)`. Class event method calls (`:Publish()`, `:Subscribe()`, `:Unsubscribe()`) are already handled by existing postfix parsing and need no changes. + +This completes one of the three remaining parser gaps listed in CLAUDE.md: "DATASET, PUBLISH/SUBSCRIBE, ON triggers." + +(see brainstorm: `docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md`) + +## Problem Statement / Motivation + +Real-world ABL code uses PUBLISH/SUBSCRIBE for inter-procedure event communication. Without parser support, any ABL file containing these statements fails to parse, blocking downstream tooling (formatting, linting, analysis). + +## Proposed Solution + +### Phase 1: Keywords and Codegen + +Add new keywords to `resources/keyword_overrides.toml` and regenerate. + +**New keywords:** + +| Keyword | `keyword_type` | Notes | +|---------|----------------|-------| +| `PUBLISH` | `Statement` | Top-level statement | +| `SUBSCRIBE` | `Statement` | Top-level statement | +| `UNSUBSCRIBE` | `Statement` | Top-level statement | +| `ANYWHERE` | `Option` | SUBSCRIBE clause only (NOT valid in UNSUBSCRIBE) | +| `EVENT` | `Option` | DEFINE EVENT subtype | +| `SIGNATURE` | `Option` | DEFINE EVENT signature clause | +| `RUN-PROCEDURE` | `Option` | SUBSCRIBE handler clause — single hyphenated keyword token (confirmed in ABL keyword index, line 25734 of abl_keyword_index.html) | + +**Not needed:** `VOID` (already exists as `Kind::Void`), `FROM`/`TO`/`IN`/`ALL`/`NO-ERROR`/`PROCEDURE` (already exist). + +No abbreviations — full keywords required. Confirmed: none of these keywords have abbreviations in the ABL keyword index (all show `–` in the abbreviation column). + +Add a section header comment `# EVENT SYSTEM KEYWORDS` before the new entries to match the organizational convention in the file. + +After editing `keyword_overrides.toml`, run `cargo run -p oxabl_codegen` to regenerate `crates/oxabl_lexer/src/kind.rs`. + +**File:** `resources/keyword_overrides.toml` + +### Research Insights: Keyword Performance Impact + +Adding 7 new keywords to `match_keyword()` has negligible performance impact. The function uses length-dispatched matching — new keywords spread across 6 different length buckets (5, 7, 8, 9, 9, 11, 13). Each bucket is a flat string comparison optimized to a jump table by the compiler. No abbreviations means exactly 7 new match arms total (vs. 4x multiplier if abbreviations were used). The `[u8; 64]` stack buffer is unaffected — all keywords are well under 64 bytes. + +### Phase 2: AST Node Definitions + +Add new variants and types to `crates/oxabl_ast/src/statement.rs`. + +**Design correction from brainstorm:** The brainstorm proposed a unified `EventParameter` type, but analysis revealed this conflates two different things: + +- **PUBLISH arguments** are runtime values (direction + expression) — identical to `RunArgument`. Reuse `RunArgument`. +- **DEFINE EVENT SIGNATURE parameters** are type definitions (direction + name + data type) — reuse existing `DefineParameter` via `Vec` (matching how `Method` already handles its parameters at line 296 of statement.rs). This eliminates the need for a separate `EventSignatureParam` struct. + +#### New Statement Variants + +```rust +/// PUBLISH event-name [FROM publisher-handle] [(args...)]. +Publish { + /// Event name — string literal or character expression. + event_name: Expression, + /// Optional FROM publisher-handle. + from_handle: Option, + /// Arguments passed to subscribers (reuses RunArgument). + arguments: Vec, +}, + +/// SUBSCRIBE [PROCEDURE subscriber-handle] [TO] event-name {IN handle | ANYWHERE} +/// [RUN-PROCEDURE handler-name] [NO-ERROR]. +Subscribe { + /// Optional PROCEDURE subscriber-handle. + subscriber: Option, + /// Event name — string literal or character expression. + event_name: Expression, + /// IN publisher-handle or ANYWHERE (required — omitting both is a compile error). + target: SubscribeTarget, + /// Optional RUN-PROCEDURE handler name (Identifier preserves span info). + run_procedure: Option, + /// Whether NO-ERROR was specified. + no_error: bool, +}, + +/// UNSUBSCRIBE [PROCEDURE subscriber-handle] [TO] {event-name | ALL} [IN publisher-handle]. +Unsubscribe { + /// Optional PROCEDURE subscriber-handle. + subscriber: Option, + /// Event name, or None if ALL was specified. + event_name: Option, + /// Optional IN publisher-handle. + in_handle: Option, +}, + +/// DEFINE [access] [STATIC] [ABSTRACT] EVENT event-name SIGNATURE VOID (params...). +DefineEvent { + /// Access modifier (defaults to PUBLIC). + access: AccessModifier, + /// Whether STATIC was specified. + is_static: bool, + /// Whether ABSTRACT was specified. + is_abstract: bool, + /// Event name. + name: Identifier, + /// Signature parameters — reuses DefineParameter via Vec, matching Method pattern. + parameters: Vec, +}, +``` + +#### New Supporting Type + +```rust +/// Target for SUBSCRIBE — where to listen for events. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SubscribeTarget { + /// Subscribe to events from a specific publisher handle. + InHandle(Expression), + /// Subscribe to events from any publisher. + Anywhere, +} +``` + +**File:** `crates/oxabl_ast/src/statement.rs` + +### Research Insights: AST Design + +**Changes from original plan based on review feedback:** + +1. **`run_procedure` changed from `Option` to `Option`** — Every named reference in the AST uses `Identifier` (which carries a `Span`). Using bare `String` loses source location information needed for diagnostics, go-to-definition, and formatting. Zero additional heap cost; gains span tracking. (Architecture, Pattern, Performance reviewers unanimous) + +2. **Eliminated redundant `all: bool` from Unsubscribe** — The original `(event_name: Option, all: bool)` pair allowed impossible states (all=true with event_name=Some). Now using just `event_name: Option` where None means ALL was specified. The doc comment makes the semantics clear. (Code-simplicity, Pattern reviewers) + +3. **Eliminated `EventSignatureParam` struct** — Reusing `Vec` with `DefineParameter` variants matches how `Method` (line 296) and `Constructor` (line 500) already handle parameter lists. The `no_undo` field on `DefineParameter` is set to `false` for event params. This avoids a parallel type hierarchy (YAGNI). (Code-simplicity reviewer) + +4. **`SubscribeTarget` enum kept** — justified because IN/ANYWHERE are mutually exclusive AND required. An enum with two variants perfectly encodes this constraint. Using `Option + bool` would allow 4 states when only 2 are valid. (Code-simplicity reviewer confirmed: keep it) + +### Phase 3: Parser Implementation + +**File:** `crates/oxabl_parser/src/parser/statements.rs` + +#### 3a. Statement Dispatch + +Add to `can_start_statement()` (around line 22): +```rust +Kind::Publish | Kind::Subscribe | Kind::Unsubscribe +``` + +Add dispatch entries in `parse_statement()` (near the RUN/DISPLAY/MESSAGE block around line 122): +```rust +if self.check(Kind::Publish) { + return self.parse_publish_statement(); +} +if self.check(Kind::Subscribe) { + return self.parse_subscribe_statement(); +} +if self.check(Kind::Unsubscribe) { + return self.parse_unsubscribe_statement(); +} +``` + +### Research Insights: Dispatch Performance + +Adding 3 new `if self.check(Kind::...)` to `parse_statement()` (already 30+ branches) is negligible — each `check()` is a single integer comparison on a `Copy + Eq` enum. The branch predictor handles sequential if-checks well. Similarly, adding 3 variants to `can_start_statement()` (a `matches!()` macro compiled to a bitmask) and 7 variants to `can_be_identifier()` have zero measurable impact. + +#### 3b. PUBLISH Parsing + +``` +PUBLISH event-name [FROM publisher-handle] [(INPUT arg, ...)]. +``` + +**Critical: Event-name expression parsing.** ABL docs describe event-name as "a quoted string or a character expression," which technically includes function calls. However, `parse_primary()` CANNOT be used here — it promotes `identifier(` into function calls (line 396-398 of expressions.rs), which would cause `PUBLISH myFunc (INPUT x).` to be misparsed as "publish the result of myFunc(INPUT x)". + +**Use a hand-rolled event-name parser** matching the RUN statement approach (`parse_run_statement()`, lines 1195-1214). The RUN statement does NOT use `parse_primary()` — it explicitly checks for: +1. `Kind::Value` → parse VALUE(expr) +2. `Kind::StringLiteral` → extract string +3. Otherwise → parse a bare name (identifier) + +The event-name parser should follow the same three-way pattern. This prevents the parenthesized argument list from being consumed as part of a function call expression. + +**Consider factoring out the RUN argument parsing loop** (lines 1217-1256 of statements.rs) into a shared helper like `parse_parenthesized_arguments()` rather than duplicating it in `parse_publish_statement()`. This prevents drift between the two implementations. + +Implementation outline: +1. Advance past `Kind::Publish` +2. Parse event name via hand-rolled parser (string literal, VALUE(expr), or bare identifier) +3. If `Kind::From`, advance and parse expression (publisher handle) +4. If `Kind::LeftParen`, parse argument list using shared RUN argument pattern (`RunArgument` with direction + expression) +5. Expect period + +**Note:** PUBLISH has implicit NO-ERROR behavior per ABL docs — no `no_error` field needed. + +#### 3c. SUBSCRIBE Parsing + +``` +SUBSCRIBE [PROCEDURE subscriber-handle] [TO] event-name {IN handle | ANYWHERE} + [RUN-PROCEDURE handler-name] [NO-ERROR]. +``` + +Implementation outline: +1. Advance past `Kind::Subscribe` +2. If `Kind::Procedure`, advance and parse expression (subscriber handle) +3. If `Kind::To`, advance (optional noise word) +4. Parse event name (hand-rolled parser — same as PUBLISH) +5. **Required:** If `Kind::In`, advance and parse expression (publisher handle) → `SubscribeTarget::InHandle`. If `Kind::Anywhere`, advance → `SubscribeTarget::Anywhere`. Otherwise, emit error: "Expected IN or ANYWHERE after event name in SUBSCRIBE" (confirmed: omitting both is a compile error per ABL docs) +6. If `Kind::RunProcedure`, advance and parse identifier (handler name) → `Option` +7. Parse optional NO-ERROR via `parse_no_error()` +8. Expect period + +**Disambiguation note:** If the token after SUBSCRIBE is `Kind::Procedure`, it means the optional subscriber clause is present. This works because `PROCEDURE` is a reserved keyword and cannot be an event name. No lookahead ambiguity. + +#### 3d. UNSUBSCRIBE Parsing + +``` +UNSUBSCRIBE [PROCEDURE subscriber-handle] [TO] {event-name | ALL} [IN publisher-handle]. +``` + +Implementation outline: +1. Advance past `Kind::Unsubscribe` +2. If `Kind::Procedure`, advance and parse expression (subscriber handle) +3. If `Kind::To`, advance (optional noise word) +4. If `Kind::All`, advance and set `event_name = None`. Otherwise, parse event name expression. +5. If `Kind::In`, advance and parse expression (publisher handle) +6. Expect period + +**Note:** UNSUBSCRIBE has implicit NO-ERROR behavior — no `no_error` field. ANYWHERE is NOT valid in UNSUBSCRIBE (confirmed against ABL reference). The `IN` clause is optional in UNSUBSCRIBE (unlike SUBSCRIBE where IN/ANYWHERE is required). To cancel an ANYWHERE subscription, simply omit the `IN` clause. + +#### 3e. DEFINE EVENT Parsing + +Extend `parse_define_statement()` to handle `Kind::Event` after the access modifier, STATIC, and ABSTRACT checks. + +``` +DEFINE [access] [STATIC] [ABSTRACT] EVENT event-name SIGNATURE VOID (params...). +``` + +Implementation outline: +1. In `parse_define_statement()`, after parsing access modifier and STATIC flag, also check for `Kind::Abstract` (advance if present, set `is_abstract = true`) +2. Add `Kind::Event` check → `parse_define_event(access, is_static, is_abstract)` +3. In `parse_define_event()`: + - Advance past `Kind::Event` + - Parse identifier (event name) + - Expect `Kind::Signature` + - Expect `Kind::Void` + - Expect `Kind::LeftParen` + - Parse parameter list: loop of `[direction] name AS data-type`, comma-separated, producing `DefineParameter` statements with `no_undo: false` + - Expect `Kind::RightParen` + - Expect period + +**Update error message** on the default branch of `parse_define_statement()` (line ~319) to include "EVENT" in the expected keyword list. + +**ABSTRACT pass-through behavior:** The `is_abstract` flag is parsed but only semantically meaningful for DEFINE EVENT. For all other DEFINE subtypes (PROPERTY, VARIABLE, TEMP-TABLE, etc.), `is_abstract` is silently ignored — consistent with how `access` and `is_static` are already silently dropped for VARIABLE/TEMP-TABLE (noted at lines 290-292). Validation that abstract events only appear in abstract classes/interfaces is deferred to semantic analysis. + +**Strict ordering:** ABL syntax specifies STATIC before ABSTRACT. The plan follows strict order (access → STATIC → ABSTRACT → subtype), consistent with how `parse_define_statement` already handles access → STATIC. This differs from `parse_method()` which uses a flexible-order loop, but strict ordering is appropriate for DEFINE since only EVENT uses ABSTRACT here. + +#### 3f. `can_be_identifier()` Updates + +**File:** `crates/oxabl_parser/src/parser/mod.rs` (line ~177) + +Add new unreserved keywords so they can still be used as identifiers in other contexts (ABL allows this): +```rust +Kind::Publish | Kind::Subscribe | Kind::Unsubscribe | Kind::Anywhere | +Kind::Event | Kind::Signature | Kind::RunProcedure +``` + +Without this, `DEFINE VARIABLE subscribe AS INTEGER.` would fail. + +#### 3g. Interface and Class Body Updates + +**No changes needed.** `parse_interface()` (line 1834) delegates to `self.parse_statement()` in a loop, which already dispatches `Kind::Define` to `parse_define_statement()`. Once the DEFINE EVENT path is added there, `DEFINE EVENT` will automatically work inside interface bodies (and class bodies, which use the same pattern). Confirmed by architecture and system design reviewers. + +### Phase 4: Tests + +**File:** `crates/oxabl_parser/src/parser/tests.rs` + +Follow the established test pattern: create `source`, tokenize, create parser, call `parse_statement()`, match on the expected variant, assert fields. Use `..` for unasserted fields. + +#### PUBLISH Tests (4 tests) +- `parse_publish_string_literal` — `PUBLISH "NewCustomer".` +- `parse_publish_with_from` — `PUBLISH "NewCustomer" FROM hProc.` +- `parse_publish_with_params` — `PUBLISH "NewCustomer" (INPUT cName).` +- `parse_publish_expression_event` — `PUBLISH cEventName.` + +#### SUBSCRIBE Tests (5 tests) +- `parse_subscribe_anywhere` — `SUBSCRIBE TO "NewCustomer" ANYWHERE.` +- `parse_subscribe_in_handle` — `SUBSCRIBE TO "NewCustomer" IN hPub.` +- `parse_subscribe_with_procedure` — `SUBSCRIBE PROCEDURE hSub TO "NewCustomer" IN hPub.` +- `parse_subscribe_with_run_procedure` — `SUBSCRIBE TO "NewCustomer" IN hPub RUN-PROCEDURE "MyHandler".` +- `parse_subscribe_no_to` — `SUBSCRIBE "NewCustomer" ANYWHERE.` (TO is optional) + +#### UNSUBSCRIBE Tests (4 tests) +- `parse_unsubscribe_event` — `UNSUBSCRIBE TO "NewCustomer".` +- `parse_unsubscribe_all` — `UNSUBSCRIBE TO ALL.` +- `parse_unsubscribe_with_procedure` — `UNSUBSCRIBE PROCEDURE hSub TO "NewCustomer".` +- `parse_unsubscribe_with_in_handle` — `UNSUBSCRIBE TO "NewCustomer" IN hPub.` + +#### DEFINE EVENT Tests (3 tests) +- `parse_define_event_minimal` — `DEFINE EVENT MyEvent SIGNATURE VOID ().` +- `parse_define_event_abstract` — `DEFINE PROTECTED ABSTRACT EVENT MyEvent SIGNATURE VOID ().` +- `parse_define_event_multiple_params` — `DEFINE PUBLIC EVENT MyEvent SIGNATURE VOID (INPUT p1 AS INTEGER, INPUT p2 AS CHARACTER, OUTPUT p3 AS LOGICAL).` + +#### Integration Tests (2 tests) +- `parse_publish_in_do_block` — PUBLISH inside a DO block +- `parse_define_event_in_interface` — DEFINE EVENT inside an INTERFACE body (validates no interface changes needed) + +#### Negative Test (1 test) +- `parse_publish_event_name_not_function_call` — `PUBLISH myEvent (INPUT x).` — verify `myEvent` is the event name and `(INPUT x)` is parsed as PUBLISH arguments, NOT as `myEvent(INPUT x)` function call + +### Research Insights: Test Optimization + +Reduced from 27 to 19 tests based on simplicity review. Tests cut: +- **Combination tests** (`parse_publish_with_from_and_params`, `parse_subscribe_full`, `parse_unsubscribe_full`) — if individual clauses work, their combination is mechanical +- **Already-covered tests** (`parse_publish_with_output_param` — OUTPUT direction tested by RUN tests; `parse_subscribe_with_no_error` — NO-ERROR tested elsewhere; `parse_unsubscribe_no_to` — same as SUBSCRIBE's no_to) +- **Modifier tests** (`parse_define_event_public`, `parse_define_event_static`) — access modifiers and STATIC are parsed by shared `parse_define_statement()` dispatch, already covered by Property/Method tests + +Added negative test for the critical event-name parsing ambiguity. + +### Phase 5: Update CLAUDE.md and Benchmarks + +1. **CLAUDE.md** — Update the parser capabilities list: + - Add PUBLISH/SUBSCRIBE/UNSUBSCRIBE and DEFINE EVENT to the "Statement parsing" bullet + - Remove "PUBLISH/SUBSCRIBE" from the "Not yet implemented" list + +2. **Benchmark fixtures** — Add PUBLISH/SUBSCRIBE/DEFINE EVENT examples to the parser benchmark fixture file (if one exists) so CodSpeed catches regressions. Event parsing is not a hot path, so this is lower priority — defer if no fixture file is trivially appendable. + +## Technical Considerations + +### Event-Name Expression Parsing (Critical) + +The most important implementation detail: event names in PUBLISH/SUBSCRIBE/UNSUBSCRIBE must be parsed with a **hand-rolled event-name parser**, NOT `parse_primary()` and NOT `parse_expression()`. + +- `parse_expression()` would greedily consume `(...)` as a function call +- `parse_primary()` ALSO promotes `identifier(` to function calls (line 396-398 of expressions.rs) — using it would produce the exact misparsing we're trying to avoid +- The RUN statement (`parse_run_statement()`, lines 1195-1214) faces the same problem and solves it with explicit three-way checks: VALUE(expr), string literal, or bare name + +**ABL docs say event names are "a character expression"**, which technically includes function calls. However, parsing a full expression here is impractical due to the `(` ambiguity with PUBLISH's argument list. The hand-rolled approach covers the common cases (string literals, identifiers, VALUE(expr)) and matches the RUN statement precedent. If a user writes `PUBLISH funcCall()`, the parser will treat `funcCall` as the event name and `()` as an empty argument list — semantically different but syntactically unambiguous. This is the same tradeoff the RUN statement makes. + +### Type Reuse + +- PUBLISH arguments → `Vec` (existing type) +- DEFINE EVENT signature params → `Vec` with `DefineParameter` variants (matching Method pattern, `no_undo: false`) +- Parameter direction → `ParameterDirection` (existing enum) +- Access modifiers → `AccessModifier` (existing enum) +- Handler names → `Identifier` (existing type, preserves span) + +### Known Gaps (Deferred) + +- **DELEGATE form**: `DEFINE EVENT OnClick DELEGATE System.EventHandler.` — deferred per brainstorm decision +- **Semantic validation**: Cross-reference checking (Publish args match DEFINE EVENT signature) deferred to future semantic analysis pass +- **ON triggers**: Related event mechanism, tracked separately +- **Full expression event names**: Event names like `funcCall()` would be misparsed as event `funcCall` + empty argument list due to `(` ambiguity — documented limitation, matches RUN statement behavior + +## Acceptance Criteria + +- [x] `cargo run -p oxabl_codegen` generates Kind variants for all new keywords +- [x] PUBLISH parses all syntax variants (string event, expression event, FROM, parameters) +- [x] SUBSCRIBE parses all variants (PROCEDURE, TO, IN handle, ANYWHERE, RUN-PROCEDURE, NO-ERROR) +- [x] UNSUBSCRIBE parses all variants (event name, ALL, PROCEDURE, IN handle) +- [x] DEFINE EVENT parses with access modifiers, STATIC, ABSTRACT, and SIGNATURE VOID parameters +- [x] DEFINE EVENT works inside CLASS and INTERFACE bodies (no special-casing needed) +- [x] New keywords work as identifiers in non-event contexts (`can_be_identifier`) +- [x] Negative test confirms event name is not consumed as function call +- [x] All tests pass (`cargo test -p oxabl_parser`) +- [x] `cargo clippy` and `cargo fmt` pass +- [x] CLAUDE.md updated to reflect new capabilities + +## Dependencies & Risks + +- **Codegen dependency**: Keywords must be generated before parser work begins. Mechanical and low-risk. +- **ABSTRACT in DEFINE**: Currently `parse_define_statement()` checks for STATIC but not ABSTRACT. Need to add ABSTRACT parsing to the define dispatch — small scope increase (5 lines) but necessary for DEFINE EVENT. The `is_abstract` flag is silently ignored for non-EVENT define subtypes, consistent with existing behavior. +- **Interface body parsing**: Confirmed — NO changes needed. `parse_interface()` already delegates to `parse_statement()`. +- **Shared argument parsing**: Consider extracting RUN's parenthesized argument loop into a shared helper to avoid code duplication in PUBLISH parsing. This is optional but reduces drift risk. + +## Sources & References + +- **Origin brainstorm:** [docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md](docs/brainstorms/2026-04-09-publish-subscribe-brainstorm.md) — Key decisions: unified AST approach, parse-fully-validate-later, DELEGATE deferred, full UNSUBSCRIBE included +- **ABL docs:** https://docs.progress.com/bundle/openedge-develop-abl-applications/page/PUBLISHSUBSCRIBE-example.html +- **ABL reference (PUBLISH):** https://docs-be.progress.com/bundle/abl-reference/page/PUBLISH-statement.html +- **ABL reference (SUBSCRIBE):** https://docs-be.progress.com/bundle/abl-reference/page/SUBSCRIBE-statement.html +- **ABL reference (UNSUBSCRIBE):** https://docs-be.progress.com/bundle/abl-reference/page/UNSUBSCRIBE-statement.html +- **Similar patterns:** `crates/oxabl_parser/src/parser/statements.rs:1195` (RUN statement parsing), `crates/oxabl_ast/src/statement.rs:117` (RUN AST node), `crates/oxabl_ast/src/statement.rs:504` (ParameterDirection) +- **Institutional learning:** `docs/solutions/performance-issues/heap-allocation-in-keyword-matching.md` — always dispatch on Kind variants, never string comparison diff --git a/resources/keyword_overrides.toml b/resources/keyword_overrides.toml index 4d97912..551490b 100644 --- a/resources/keyword_overrides.toml +++ b/resources/keyword_overrides.toml @@ -633,6 +633,39 @@ keyword_type = "Statement" name = "DATASET-HANDLE" keyword_type = "Type" +# ============================================================================= +# EVENT SYSTEM KEYWORDS +# Keywords needed for PUBLISH/SUBSCRIBE/UNSUBSCRIBE statements and DEFINE EVENT. +# ============================================================================= + +[[add]] +name = "PUBLISH" +keyword_type = "Statement" + +[[add]] +name = "SUBSCRIBE" +keyword_type = "Statement" + +[[add]] +name = "UNSUBSCRIBE" +keyword_type = "Statement" + +[[add]] +name = "ANYWHERE" +keyword_type = "Option" + +[[add]] +name = "EVENT" +keyword_type = "Option" + +[[add]] +name = "SIGNATURE" +keyword_type = "Option" + +[[add]] +name = "RUN-PROCEDURE" +keyword_type = "Option" + # ============================================================================= # OVERRIDES # Correct misclassifications or add missing info