From 6dfffdcac671142290109f13294499ba14e8fac7 Mon Sep 17 00:00:00 2001 From: Evan Robertson Date: Thu, 9 Apr 2026 22:15:30 -0400 Subject: [PATCH] feat(parser): add ON trigger and TRIGGER PROCEDURE statement parsing Add full parsing support for ABL's ON statement (all 3 forms) and the TRIGGER PROCEDURE statement, completing the last major parser gap. ON statement forms: - UI/developer event triggers with comma-separated events/widgets, OR clause chaining, ANYWHERE, IN FRAME/IN BROWSE qualifiers, and trigger actions (DO block, single statement, REVERT, PERSISTENT RUN) - Database event triggers (CREATE/DELETE/FIND/WRITE/ASSIGN) with NEW/OLD BUFFER referencing, OLD VALUE, and OVERRIDE - Key remapping (ON key-label key-function) TRIGGER PROCEDURE for schema triggers: - All 5 standard events plus REPLICATION-CREATE/DELETE/WRITE - WRITE with optional NEW/OLD BUFFER clauses - ASSIGN with both OF table.field and NEW VALUE variable forms New AST types: OnKind, OnAction, OnEventClause, DbTriggerEvent, TriggerReferencing, WidgetRef, WidgetQualifier, TriggerAssignParam. 29 new tests (406 total parser tests), all passing. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 6 +- crates/oxabl_ast/src/statement.rs | 154 ++++ crates/oxabl_lexer/build.rs | 3 + crates/oxabl_lexer/src/kind.rs | 6 + crates/oxabl_parser/src/parser/mod.rs | 8 + crates/oxabl_parser/src/parser/statements.rs | 552 ++++++++++++++- crates/oxabl_parser/src/parser/tests.rs | 573 ++++++++++++++- .../2026-04-09-on-triggers-brainstorm.md | 66 ++ ...feat-on-triggers-trigger-procedure-plan.md | 663 ++++++++++++++++++ resources/keyword_overrides.toml | 20 + 10 files changed, 2039 insertions(+), 12 deletions(-) create mode 100644 docs/brainstorms/2026-04-09-on-triggers-brainstorm.md create mode 100644 docs/plans/2026-04-09-002-feat-on-triggers-trigger-procedure-plan.md diff --git a/CLAUDE.md b/CLAUDE.md index d3ebd25..98cb347 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/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 +- **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, ON (UI/developer event triggers with IN FRAME/IN BROWSE, database event triggers, key remapping), TRIGGER PROCEDURE, 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: ON triggers. +Not yet implemented: DO/FOR/REPEAT block-header ON phrases (ON ERROR UNDO, ON ENDKEY UNDO). ### Code Generation (`oxabl_codegen`) @@ -170,4 +170,4 @@ cargo bench -p oxabl_common --bench source_map_bench - `oxabl_lexer`: MVP complete with 43 tests - `oxabl_common/source_map`: Implemented with 10 tests - `oxabl_ast`: Implemented with expressions, statements, and data types -- `oxabl_parser`: Actively developed with 330 tests; parses expressions, control flow, variable declarations, functions, procedures, temp-tables, error handling, OO-ABL (CLASS, METHOD, PROPERTY, INTERFACE), preprocessor directives, stream I/O, and frame definitions +- `oxabl_parser`: Actively developed with 406 tests; parses expressions, control flow, variable declarations, functions, procedures, temp-tables, error handling, OO-ABL (CLASS, METHOD, PROPERTY, INTERFACE), preprocessor directives, stream I/O, frame definitions, ON triggers (UI events, database events, key remapping), and TRIGGER PROCEDURE diff --git a/crates/oxabl_ast/src/statement.rs b/crates/oxabl_ast/src/statement.rs index 040f405..44f876a 100644 --- a/crates/oxabl_ast/src/statement.rs +++ b/crates/oxabl_ast/src/statement.rs @@ -509,6 +509,36 @@ pub enum Statement { operation: StreamOperation, }, + /// ON trigger statement -- event handlers for UI, database, and key events. + /// + /// ```abl + /// ON CHOOSE OF btnOk IN FRAME f1 DO: /* ... */ END. + /// ON WRITE OF Customer NEW BUFFER bNew OLD BUFFER bOld DO: /* ... */ END. + /// ON F1 HELP. + /// ``` + On { kind: OnKind }, + + /// TRIGGER PROCEDURE FOR event OF table [NEW/OLD clauses]. + /// + /// Declares a schema trigger -- always the first statement in a trigger procedure file. + /// + /// ```abl + /// TRIGGER PROCEDURE FOR WRITE OF Customer + /// NEW BUFFER bNew OLD BUFFER bOld. + /// ``` + TriggerProcedure { + /// The trigger event (CREATE, DELETE, FIND, WRITE, ASSIGN, or REPLICATION-*). + event: DbTriggerEvent, + /// The target table (or table.field for ASSIGN OF form). + target: Identifier, + /// NEW/OLD BUFFER referencing (WRITE triggers). + referencing: TriggerReferencing, + /// NEW VALUE variable definition (ASSIGN triggers, mutually exclusive with OF form). + new_value: Option, + /// OLD VALUE variable definition (ASSIGN NEW VALUE form). + old_value_param: Option, + }, + /// Leave statement - exit innermost loop Leave, @@ -865,3 +895,127 @@ pub enum SubscribeTarget { /// Subscribe to events from any publisher. Anywhere, } + +// ============================================================================= +// ON trigger types +// ============================================================================= + +/// Discriminant for the different forms of the ON statement. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OnKind { + /// UI/developer event trigger (includes "WEB-NOTIFY" ANYWHERE form): + /// ON event-list OF widget-list [OR event-list OF widget-list]... [ANYWHERE] + /// { trigger-block | REVERT | PERSISTENT RUN proc [(args)] } + UiEvent { + /// Event/widget clauses -- at least one, chained via OR. + /// Empty when ANYWHERE is used standalone (e.g., ON "WEB-NOTIFY" ANYWHERE). + clauses: Vec, + /// Whether ANYWHERE was specified. + anywhere: bool, + /// The trigger action. + action: OnAction, + }, + /// Database event trigger: + /// ON CREATE|DELETE|FIND|WRITE|ASSIGN OF table [referencing] [OVERRIDE] + /// { trigger-block | REVERT } + DbEvent { + /// The database event. + event: DbTriggerEvent, + /// The table (or table.field for ASSIGN) the trigger is on. + target: Identifier, + /// NEW/OLD BUFFER/VALUE referencing phrases. + referencing: TriggerReferencing, + /// Whether OVERRIDE was specified. + is_override: bool, + /// The trigger action (block or REVERT). + action: OnAction, + }, + /// Key remapping: ON key-label key-function. + KeyRemap { + /// The key label (e.g., F1, CTRL-X) -- any identifier. + key_label: Identifier, + /// The key function (e.g., HELP, ENDKEY, GO) -- any identifier. + key_function: Identifier, + }, +} + +/// A single event/widget-list clause in a UI ON trigger. +/// +/// `ON CHOOSE, ENTRY OF btnOk IN FRAME f1, btnCancel` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OnEventClause { + /// Comma-separated event names (identifiers, including keywords like LEAVE/ENTRY). + pub events: Vec, + /// Comma-separated widget references with optional frame/browse qualifiers. + pub widgets: Vec, +} + +/// The action taken by an ON trigger. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OnAction { + /// A trigger block -- either a single statement or DO...END block. + Block(Box), + /// REVERT -- removes the trigger. + Revert, + /// PERSISTENT RUN procedure [(args)]. + PersistentRun { + procedure: Identifier, + arguments: Vec, + }, +} + +/// Database trigger event types. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DbTriggerEvent { + Create, + Delete, + Find, + Write, + Assign, + ReplicationCreate, + ReplicationDelete, + ReplicationWrite, +} + +/// Referencing phrase for database triggers (NEW/OLD BUFFER for WRITE, OLD VALUE for ASSIGN). +/// Shared between ON db-event triggers and TRIGGER PROCEDURE. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct TriggerReferencing { + /// NEW [BUFFER] alias (WRITE triggers). + pub new_buffer: Option, + /// OLD [BUFFER] alias (WRITE triggers). + pub old_buffer: Option, + /// OLD [VALUE] alias (ASSIGN triggers in ON statement). + pub old_value: Option, +} + +/// Widget reference in an ON trigger, with optional frame/browse qualifier. +/// +/// `btnOk IN FRAME main-frame` or `col1 IN BROWSE brw1` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WidgetRef { + pub name: Identifier, + pub qualifier: Option, +} + +/// Optional qualification for a widget reference. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WidgetQualifier { + /// IN FRAME frame-name + InFrame(Identifier), + /// IN BROWSE browse-name + InBrowse(Identifier), +} + +/// A variable-like parameter for TRIGGER PROCEDURE FOR ASSIGN NEW VALUE form. +/// +/// ```abl +/// TRIGGER PROCEDURE FOR ASSIGN +/// NEW VALUE newVal AS CHARACTER +/// OLD VALUE oldVal AS CHARACTER. +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TriggerAssignParam { + pub name: Identifier, + pub data_type: DataType, +} diff --git a/crates/oxabl_lexer/build.rs b/crates/oxabl_lexer/build.rs index 934db8e..5d05ca7 100644 --- a/crates/oxabl_lexer/build.rs +++ b/crates/oxabl_lexer/build.rs @@ -93,6 +93,7 @@ fn main() { "blank", "blob", "break", + "browse", "buffer", "buffer-compare", "buffer-copy", @@ -111,6 +112,7 @@ fn main() { "char", "character", "check", + "choose", "chr", "class", "clear", @@ -203,6 +205,7 @@ fn main() { "enable", "encode", "end", + "endkey", "entry", "eq", "error stat", diff --git a/crates/oxabl_lexer/src/kind.rs b/crates/oxabl_lexer/src/kind.rs index 0e9e0f6..18abc97 100644 --- a/crates/oxabl_lexer/src/kind.rs +++ b/crates/oxabl_lexer/src/kind.rs @@ -323,6 +323,9 @@ pub enum Kind { Event, Signature, RunProcedure, + Choose, + Endkey, + Browse, // Phrases Editing, @@ -832,11 +835,13 @@ pub fn match_keyword(s: &str) -> Option { "availa" => Some(Kind::Available), "backgr" => Some(Kind::Background), "begins" => Some(Kind::Begins), + "browse" => Some(Kind::Browse), "buffer" => Some(Kind::Buffer), "can do" => Some(Kind::CanDo), "can-do" => Some(Kind::CanDo), "center" => Some(Kind::Centered), "charac" => Some(Kind::Character), + "choose" => Some(Kind::Choose), "create" => Some(Kind::Create), "cursor" => Some(Kind::Cursor), "dbname" => Some(Kind::Dbname), @@ -852,6 +857,7 @@ pub fn match_keyword(s: &str) -> Option { "displa" => Some(Kind::Display), "enable" => Some(Kind::Enable), "encode" => Some(Kind::Encode), + "endkey" => Some(Kind::Endkey), "escape" => Some(Kind::Escape), "except" => Some(Kind::Except), "exists" => Some(Kind::Exists), diff --git a/crates/oxabl_parser/src/parser/mod.rs b/crates/oxabl_parser/src/parser/mod.rs index 5b88934..97df4f8 100644 --- a/crates/oxabl_parser/src/parser/mod.rs +++ b/crates/oxabl_parser/src/parser/mod.rs @@ -279,6 +279,14 @@ impl<'a> Parser<'a> { | Kind::Event | Kind::Signature | Kind::RunProcedure + // ON trigger keywords (unreserved) + | Kind::Trigger + | Kind::Triggers + | Kind::Persistent + | Kind::Revert + | Kind::Choose + | Kind::Endkey + | Kind::Browse ) } diff --git a/crates/oxabl_parser/src/parser/statements.rs b/crates/oxabl_parser/src/parser/statements.rs index 3bebf08..04b04b9 100644 --- a/crates/oxabl_parser/src/parser/statements.rs +++ b/crates/oxabl_parser/src/parser/statements.rs @@ -8,11 +8,12 @@ use oxabl_ast::{ AccessModifier, AssignPair, BufferTarget, CreateTarget, CreateTargetKind, DataRelation, - DataSourceBuffer, DataSourceKeys, DisplayItem, Expression, FieldTypeSource, FindType, - HandleParamKind, HandlePassingOptions, Identifier, IndexField, LockType, ParameterDirection, - ParameterType, ParentIdRelation, PreprocIf, RunArgument, RunTarget, SortDirection, Span, - Statement, StreamDirection, StreamOperation, SubscribeTarget, TempTableField, TempTableIndex, - UseIndex, WhenBranch, XmlSerializeOptions, + DataSourceBuffer, DataSourceKeys, DbTriggerEvent, DisplayItem, Expression, FieldTypeSource, + FindType, HandleParamKind, HandlePassingOptions, Identifier, IndexField, LockType, OnAction, + OnEventClause, OnKind, ParameterDirection, ParameterType, ParentIdRelation, PreprocIf, + RunArgument, RunTarget, SortDirection, Span, Statement, StreamDirection, StreamOperation, + SubscribeTarget, TempTableField, TempTableIndex, TriggerAssignParam, TriggerReferencing, + UseIndex, WhenBranch, WidgetQualifier, WidgetRef, XmlSerializeOptions, }; use oxabl_lexer::Kind; @@ -65,6 +66,8 @@ pub(crate) fn can_start_statement(kind: Kind) -> bool { | Kind::Publish | Kind::Subscribe | Kind::Unsubscribe + | Kind::On + | Kind::Trigger ) } @@ -141,6 +144,19 @@ impl Parser<'_> { return self.parse_unsubscribe_statement(); } + // ON triggers + // NOTE: ON in block headers (DO ON ERROR UNDO) is consumed inside + // parse_do_statement/parse_for_each/parse_repeat_statement. + // Any ON reaching here is always a trigger statement. + if self.check(Kind::On) { + return self.parse_on_statement(); + } + + // TRIGGER PROCEDURE + if self.check(Kind::Trigger) { + return self.parse_trigger_procedure(); + } + // PROCEDURE statement if self.check(Kind::Procedure) { return self.parse_procedure(); @@ -3326,4 +3342,530 @@ impl Parser<'_> { let expression = self.parse_expression()?; Ok(Statement::PreprocMessage { expression }) } + + // ========================================================================= + // ON triggers and TRIGGER PROCEDURE + // ========================================================================= + + /// Parse an ON statement, disambiguating between the 3 forms: + /// 1. UI/developer event trigger: ON event-list OF widget-list ... + /// 2. Database event trigger: ON CREATE/DELETE/FIND/WRITE/ASSIGN OF table ... + /// 3. Key remapping: ON key-label key-function. + fn parse_on_statement(&mut self) -> ParseResult { + self.advance(); // consume ON + + // Check for string literal event name (e.g., ON "WEB-NOTIFY" ANYWHERE ...) + // Parsed as UiEvent with the string as the sole event name. + if self.check(Kind::StringLiteral) { + return self.parse_on_ui_event(); + } + + // Check for DB events: CREATE/DELETE/FIND/WRITE/ASSIGN followed by OF (not Comma). + // If followed by Comma, it's a UI event with multiple event names. + if self.is_db_event_kind(self.peek().kind) && self.check_at(1, Kind::Of) { + return self.parse_on_db_event(); + } + + // Check for key remapping: ON . + // Two tokens followed by a period, with no OF or comma. + // UI events always have OF or comma after the event list. + // Key labels/functions can be any keyword (HELP, ENDKEY, GO, etc.). + let next_kind = self.peek_at(1).kind; + if !matches!(next_kind, Kind::Of | Kind::Comma | Kind::Eof | Kind::Period) + && self.check_at(2, Kind::Period) + { + return self.parse_on_key_remap(); + } + + // Default: UI/developer event trigger + self.parse_on_ui_event() + } + + /// Parse a UI/developer event trigger: + /// ON event-list [OF widget-list] [OR event-list OF widget-list]... [ANYWHERE] + /// { trigger-block | REVERT | PERSISTENT RUN proc [(args)] } + fn parse_on_ui_event(&mut self) -> ParseResult { + let mut clauses = Vec::new(); + let mut anywhere = false; + + loop { + let events = self.parse_trigger_event_list()?; + + // ANYWHERE without OF — standalone form + if self.check(Kind::Anywhere) && !self.check_at(1, Kind::Of) { + anywhere = true; + self.advance(); + break; + } + + // OF widget-list + if self.check(Kind::Of) { + self.advance(); + let widgets = self.parse_widget_ref_list()?; + clauses.push(OnEventClause { events, widgets }); + } else if clauses.is_empty() && events.is_empty() { + return Err(ParseError { + message: "Expected event name or ANYWHERE after ON".to_string(), + span: self.current_span(), + }); + } + + // Check for OR to chain another event/widget clause + if self.check(Kind::Or) { + self.advance(); + } else { + break; + } + } + + // Check for trailing ANYWHERE (after widget list) + if self.check(Kind::Anywhere) { + anywhere = true; + self.advance(); + } + + let action = self.parse_trigger_action()?; + Ok(Statement::On { + kind: OnKind::UiEvent { + clauses, + anywhere, + action, + }, + }) + } + + /// Parse a database event trigger: + /// ON CREATE|DELETE|FIND|WRITE|ASSIGN OF table [referencing] [OVERRIDE] + /// { trigger-block | REVERT } + fn parse_on_db_event(&mut self) -> ParseResult { + let event = self.parse_db_event_kind()?; + self.expect_kind(Kind::Of, "Expected OF after database event")?; + let target = self.parse_dotted_name()?; + + let mut referencing = TriggerReferencing::default(); + + // Parse optional referencing phrases for WRITE + if event == DbTriggerEvent::Write { + if self.check(Kind::New) { + self.advance(); + // Optional BUFFER keyword + if self.check(Kind::Buffer) { + self.advance(); + } + referencing.new_buffer = Some(self.parse_identifier()?); + } + if self.check(Kind::Old) { + self.advance(); + if self.check(Kind::Buffer) { + self.advance(); + } + referencing.old_buffer = Some(self.parse_identifier()?); + } + } + + // Parse optional OLD VALUE for ASSIGN + if event == DbTriggerEvent::Assign && self.check(Kind::Old) { + self.advance(); + if self.check(Kind::Value) { + self.advance(); + } + referencing.old_value = Some(self.parse_identifier()?); + } + + let is_override = if self.check(Kind::Override) { + self.advance(); + true + } else { + false + }; + + let action = self.parse_trigger_action()?; + Ok(Statement::On { + kind: OnKind::DbEvent { + event, + target, + referencing, + is_override, + action, + }, + }) + } + + /// Parse key remapping: ON key-label key-function. + /// Key labels and functions can be any identifier, including reserved keywords + /// like HELP, ENDKEY, GO, RETURN, STOP, ERROR, END, TAB, HOME, CLEAR, etc. + fn parse_on_key_remap(&mut self) -> ParseResult { + let key_label = self.parse_any_keyword_as_identifier()?; + let key_function = self.parse_any_keyword_as_identifier()?; + self.expect_kind(Kind::Period, "Expected '.' after key remapping")?; + Ok(Statement::On { + kind: OnKind::KeyRemap { + key_label, + key_function, + }, + }) + } + + /// Parse a trigger action: REVERT, PERSISTENT RUN, DO block, or single statement. + fn parse_trigger_action(&mut self) -> ParseResult { + // REVERT + if self.check(Kind::Revert) { + self.advance(); + self.expect_kind(Kind::Period, "Expected '.' after REVERT")?; + return Ok(OnAction::Revert); + } + + // PERSISTENT RUN procedure [(args)] + if self.check(Kind::Persistent) { + self.advance(); + self.expect_kind(Kind::Run, "Expected RUN after PERSISTENT")?; + let procedure = self.parse_identifier()?; + let arguments = if self.check(Kind::LeftParen) { + self.parse_persistent_run_args()? + } else { + Vec::new() + }; + self.expect_kind(Kind::Period, "Expected '.' after PERSISTENT RUN")?; + return Ok(OnAction::PersistentRun { + procedure, + arguments, + }); + } + + // DO...END block + if self.check(Kind::Do) { + let block = self.parse_do_statement()?; + return Ok(OnAction::Block(Box::new(block))); + } + + // Single statement (terminates with its own period) + let stmt = self.parse_statement()?; + Ok(OnAction::Block(Box::new(stmt))) + } + + /// Parse PERSISTENT RUN arguments: (INPUT expr, ...). + /// Simplified: just parses comma-separated expressions inside parens. + fn parse_persistent_run_args(&mut self) -> ParseResult> { + self.expect_kind(Kind::LeftParen, "Expected '(' for arguments")?; + let mut args = Vec::new(); + while !self.check(Kind::RightParen) && !self.at_end() { + // Skip optional INPUT keyword (only INPUT is valid for PERSISTENT RUN) + if self.check(Kind::Input) { + self.advance(); + } + args.push(self.parse_expression()?); + if !self.check(Kind::RightParen) { + self.expect_kind(Kind::Comma, "Expected ',' between arguments")?; + } + } + self.expect_kind(Kind::RightParen, "Expected ')' after arguments")?; + Ok(args) + } + + /// Parse a comma-separated list of event names (identifiers). + /// Accepts keywords that double as event names (LEAVE, ENTRY, CREATE, DELETE, etc.). + fn parse_trigger_event_list(&mut self) -> ParseResult> { + let mut events = Vec::new(); + loop { + if self.can_be_event_name() { + events.push(self.parse_event_name_identifier()?); + } else { + break; + } + if !self.check(Kind::Comma) { + break; + } + self.advance(); // consume comma + } + Ok(events) + } + + /// Check if the current token can be an event name in an ON trigger. + /// Event names include regular identifiers, `can_be_identifier()` keywords, + /// and reserved keywords that double as event names (LEAVE, ENTRY, HELP, etc.). + fn can_be_event_name(&self) -> bool { + let kind = self.peek().kind; + kind == Kind::Identifier + || kind == Kind::StringLiteral + || Self::can_be_identifier(kind) + || matches!( + kind, + Kind::Leave + | Kind::Entry + | Kind::Create + | Kind::Delete + | Kind::Close + | Kind::Write + | Kind::Help + | Kind::GoOn + | Kind::ErrorStatus + | Kind::ValueChanged + ) + } + + /// Parse a single event name identifier, including reserved keywords + /// that serve as event names in ON trigger context. + fn parse_event_name_identifier(&mut self) -> ParseResult { + let token = self.peek(); + let kind = token.kind; + + if kind == Kind::StringLiteral { + // String literal event name (e.g., "WEB-NOTIFY") + let name = self.source[token.start..token.end].to_string(); + let ident = Identifier { + name, + span: Span { + start: token.start as u32, + end: token.end as u32, + }, + }; + self.advance(); + Ok(ident) + } else if kind == Kind::Identifier || Self::can_be_identifier(kind) { + self.parse_identifier() + } else if matches!( + kind, + Kind::Leave + | Kind::Entry + | Kind::Create + | Kind::Delete + | Kind::Close + | Kind::Write + | Kind::Help + | Kind::GoOn + | Kind::ErrorStatus + | Kind::ValueChanged + ) { + // Reserved keywords that are also valid event names + let name = self.source[token.start..token.end].to_string(); + let ident = Identifier { + name, + span: Span { + start: token.start as u32, + end: token.end as u32, + }, + }; + self.advance(); + Ok(ident) + } else { + Err(ParseError { + message: "Expected event name".to_string(), + span: self.current_span(), + }) + } + } + + /// Parse a comma-separated list of widget references, each with optional IN FRAME/BROWSE. + fn parse_widget_ref_list(&mut self) -> ParseResult> { + let mut refs = Vec::new(); + loop { + let name = self.parse_identifier()?; + let qualifier = if self.check(Kind::KwIn) { + self.advance(); + if self.check(Kind::Frame) { + self.advance(); + Some(WidgetQualifier::InFrame(self.parse_identifier()?)) + } else if self.check(Kind::Browse) { + self.advance(); + Some(WidgetQualifier::InBrowse(self.parse_identifier()?)) + } else { + None + } + } else { + None + }; + refs.push(WidgetRef { name, qualifier }); + if !self.check(Kind::Comma) { + break; + } + self.advance(); // consume comma + } + Ok(refs) + } + + /// Parse any token as an identifier, even reserved keywords. + /// Used for key labels and key functions where any keyword is valid. + fn parse_any_keyword_as_identifier(&mut self) -> ParseResult { + let token = self.peek(); + if token.kind == Kind::Eof { + return Err(ParseError { + message: "Expected identifier".to_string(), + span: self.current_span(), + }); + } + let name = self.source[token.start..token.end].to_string(); + let ident = Identifier { + name, + span: Span { + start: token.start as u32, + end: token.end as u32, + }, + }; + self.advance(); + Ok(ident) + } + + /// Check if a Kind is a database trigger event keyword. + fn is_db_event_kind(&self, kind: Kind) -> bool { + matches!( + kind, + Kind::Create | Kind::Delete | Kind::Find | Kind::Write | Kind::Assign + ) + } + + /// Parse a database event keyword and return the corresponding DbTriggerEvent. + fn parse_db_event_kind(&mut self) -> ParseResult { + let kind = self.peek().kind; + let event = match kind { + Kind::Create => DbTriggerEvent::Create, + Kind::Delete => DbTriggerEvent::Delete, + Kind::Find => DbTriggerEvent::Find, + Kind::Write => DbTriggerEvent::Write, + Kind::Assign => DbTriggerEvent::Assign, + _ => { + return Err(ParseError { + message: "Expected database event (CREATE, DELETE, FIND, WRITE, or ASSIGN)" + .to_string(), + span: self.current_span(), + }); + } + }; + self.advance(); + Ok(event) + } + + /// Parse a dotted name like `table.field` for ASSIGN OF targets. + /// Returns an Identifier whose name contains the full dotted form. + fn parse_dotted_name(&mut self) -> ParseResult { + let first = self.parse_identifier()?; + // Check if a period follows and the next token is an identifier-like token + // (not a statement-starting keyword or end of input). + // This distinguishes `Customer.Name` from `Customer.` (statement terminator). + if self.check(Kind::Period) { + let next_kind = self.peek_at(1).kind; + if next_kind == Kind::Identifier || Self::can_be_identifier(next_kind) { + self.advance(); // consume period + let second = self.parse_identifier()?; + let name = format!("{}.{}", first.name, second.name); + let span = Span { + start: first.span.start, + end: second.span.end, + }; + return Ok(Identifier { name, span }); + } + } + Ok(first) + } + + /// Parse a TRIGGER PROCEDURE statement: + /// TRIGGER PROCEDURE FOR event OF table [NEW/OLD clauses]. + fn parse_trigger_procedure(&mut self) -> ParseResult { + self.advance(); // consume TRIGGER + self.expect_kind(Kind::Procedure, "Expected PROCEDURE after TRIGGER")?; + self.expect_kind(Kind::KwFor, "Expected FOR after TRIGGER PROCEDURE")?; + + // Parse event kind — also check for REPLICATION-* events + let event = if self.peek().kind == Kind::Identifier { + let token = self.peek(); + let text = &self.source[token.start..token.end]; + if text.eq_ignore_ascii_case("replication-create") { + self.advance(); + DbTriggerEvent::ReplicationCreate + } else if text.eq_ignore_ascii_case("replication-delete") { + self.advance(); + DbTriggerEvent::ReplicationDelete + } else if text.eq_ignore_ascii_case("replication-write") { + self.advance(); + DbTriggerEvent::ReplicationWrite + } else { + self.parse_db_event_kind()? + } + } else { + self.parse_db_event_kind()? + }; + + // ASSIGN has two mutually exclusive forms + if event == DbTriggerEvent::Assign { + if self.check(Kind::Of) { + // OF table.field form + self.advance(); + let target = self.parse_dotted_name()?; + self.expect_kind(Kind::Period, "Expected '.' after TRIGGER PROCEDURE")?; + return Ok(Statement::TriggerProcedure { + event, + target, + referencing: TriggerReferencing::default(), + new_value: None, + old_value_param: None, + }); + } else { + // NEW VALUE form + self.expect_kind(Kind::New, "Expected NEW or OF after ASSIGN")?; + if self.check(Kind::Value) { + self.advance(); + } + let new_value = self.parse_trigger_assign_param()?; + let old_value_param = if self.check(Kind::Old) { + self.advance(); + if self.check(Kind::Value) { + self.advance(); + } + Some(self.parse_trigger_assign_param()?) + } else { + None + }; + self.expect_kind(Kind::Period, "Expected '.' after TRIGGER PROCEDURE")?; + // Use a placeholder target for the NEW VALUE form + let target = Identifier { + name: String::new(), + span: self.current_span(), + }; + return Ok(Statement::TriggerProcedure { + event, + target, + referencing: TriggerReferencing::default(), + new_value: Some(new_value), + old_value_param, + }); + } + } + + self.expect_kind(Kind::Of, "Expected OF after event")?; + let target = self.parse_dotted_name()?; + + let mut referencing = TriggerReferencing::default(); + if event == DbTriggerEvent::Write { + if self.check(Kind::New) { + self.advance(); + if self.check(Kind::Buffer) { + self.advance(); + } + referencing.new_buffer = Some(self.parse_identifier()?); + } + if self.check(Kind::Old) { + self.advance(); + if self.check(Kind::Buffer) { + self.advance(); + } + referencing.old_buffer = Some(self.parse_identifier()?); + } + } + + self.expect_kind(Kind::Period, "Expected '.' after TRIGGER PROCEDURE")?; + Ok(Statement::TriggerProcedure { + event, + target, + referencing, + new_value: None, + old_value_param: None, + }) + } + + /// Parse a TRIGGER PROCEDURE ASSIGN parameter: name AS type. + fn parse_trigger_assign_param(&mut self) -> ParseResult { + let name = self.parse_identifier()?; + self.expect_kind(Kind::KwAs, "Expected AS after parameter name")?; + let data_type = self.parse_data_type()?; + Ok(TriggerAssignParam { name, data_type }) + } } diff --git a/crates/oxabl_parser/src/parser/tests.rs b/crates/oxabl_parser/src/parser/tests.rs index 22efb14..a27026b 100644 --- a/crates/oxabl_parser/src/parser/tests.rs +++ b/crates/oxabl_parser/src/parser/tests.rs @@ -1,10 +1,10 @@ use super::*; 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, SubscribeTarget, - UnknownLiteral, WhenBranch, + DataType, DbTriggerEvent, DecimalLiteral, Expression, FieldTypeSource, FindType, + HandleParamKind, Identifier, IntegerLiteral, Literal, LockType, OnAction, OnKind, + ParameterDirection, ParameterType, RunTarget, SortDirection, Span, Statement, StreamDirection, + StreamOperation, StringLiteral, SubscribeTarget, UnknownLiteral, WhenBranch, WidgetQualifier, }; use oxabl_lexer::tokenize; use rust_decimal::Decimal; @@ -7543,3 +7543,568 @@ fn parse_publish_event_name_not_function_call() { _ => panic!("Expected Publish statement"), } } + +// ── ON trigger tests ──────────────────────────────────────────────── + +#[test] +fn parse_on_choose_of_button() { + let source = r#"ON CHOOSE OF btnOk DO: MESSAGE "clicked". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: + OnKind::UiEvent { + clauses, + anywhere, + action, + }, + } => { + assert!(!anywhere); + assert_eq!(clauses.len(), 1); + assert_eq!(clauses[0].events.len(), 1); + assert_eq!(clauses[0].events[0].name, "CHOOSE"); + assert_eq!(clauses[0].widgets.len(), 1); + assert_eq!(clauses[0].widgets[0].name.name, "btnOk"); + assert!(clauses[0].widgets[0].qualifier.is_none()); + assert!(matches!(action, OnAction::Block(_))); + } + _ => panic!("Expected On UiEvent statement, got {:?}", stmt), + } +} + +#[test] +fn parse_on_multiple_events() { + let source = r#"ON CHOOSE, ENTRY OF btnOk DO: MESSAGE "hi". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { clauses, .. }, + } => { + assert_eq!(clauses[0].events.len(), 2); + assert_eq!(clauses[0].events[0].name, "CHOOSE"); + assert_eq!(clauses[0].events[1].name, "ENTRY"); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_multiple_widgets() { + let source = r#"ON CHOOSE OF btn1, btn2 DO: MESSAGE "hi". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { clauses, .. }, + } => { + assert_eq!(clauses[0].widgets.len(), 2); + assert_eq!(clauses[0].widgets[0].name.name, "btn1"); + assert_eq!(clauses[0].widgets[1].name.name, "btn2"); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_widget_in_frame() { + let source = r#"ON CHOOSE OF btnOk IN FRAME main-frame DO: MESSAGE "hi". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { clauses, .. }, + } => { + assert_eq!(clauses[0].widgets.len(), 1); + assert_eq!(clauses[0].widgets[0].name.name, "btnOk"); + match &clauses[0].widgets[0].qualifier { + Some(WidgetQualifier::InFrame(frame)) => assert_eq!(frame.name, "main-frame"), + other => panic!("Expected InFrame qualifier, got {:?}", other), + } + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_or_clause() { + let source = r#"ON CHOOSE OF btn1 OR ENTRY OF fill1 DO: MESSAGE "hi". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { clauses, .. }, + } => { + assert_eq!(clauses.len(), 2); + assert_eq!(clauses[0].events[0].name, "CHOOSE"); + assert_eq!(clauses[0].widgets[0].name.name, "btn1"); + assert_eq!(clauses[1].events[0].name, "ENTRY"); + assert_eq!(clauses[1].widgets[0].name.name, "fill1"); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_anywhere() { + let source = r#"ON CHOOSE ANYWHERE DO: MESSAGE "hi". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { + clauses, anywhere, .. + }, + } => { + assert!(anywhere); + assert!(clauses.is_empty()); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_anywhere_with_widgets() { + let source = r#"ON CHOOSE OF btn1 ANYWHERE DO: MESSAGE "hi". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { + clauses, anywhere, .. + }, + } => { + assert!(anywhere); + assert_eq!(clauses.len(), 1); + assert_eq!(clauses[0].widgets[0].name.name, "btn1"); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_single_statement() { + let source = r#"ON CHOOSE OF btnOk MESSAGE "clicked"."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { action, .. }, + } => { + assert!(matches!(action, OnAction::Block(_))); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_revert() { + let source = "ON CHOOSE OF btnOk REVERT."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { action, .. }, + } => { + assert!(matches!(action, OnAction::Revert)); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_persistent_run() { + let source = "ON CHOOSE OF btnOk PERSISTENT RUN myProc."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { action, .. }, + } => match action { + OnAction::PersistentRun { + procedure, + arguments, + } => { + assert_eq!(procedure.name, "myProc"); + assert!(arguments.is_empty()); + } + other => panic!("Expected PersistentRun, got {:?}", other), + }, + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_persistent_run_with_args() { + let source = "ON CHOOSE OF btnOk PERSISTENT RUN myProc (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::On { + kind: OnKind::UiEvent { action, .. }, + } => match action { + OnAction::PersistentRun { arguments, .. } => { + assert_eq!(arguments.len(), 1); + } + other => panic!("Expected PersistentRun, got {:?}", other), + }, + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_leave_event_name() { + let source = r#"ON LEAVE OF fill1 DO: MESSAGE "left". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { clauses, .. }, + } => { + assert_eq!(clauses[0].events[0].name, "LEAVE"); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +#[test] +fn parse_on_web_notify() { + let source = r#"ON "WEB-NOTIFY" ANYWHERE DO: MESSAGE "notify". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::UiEvent { anywhere, .. }, + } => { + assert!(anywhere); + } + _ => panic!("Expected On UiEvent statement"), + } +} + +// ── ON database event tests ───────────────────────────────────────── + +#[test] +fn parse_on_create_of_table() { + let source = r#"ON CREATE OF Customer DO: MESSAGE "created". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: + OnKind::DbEvent { + event, + target, + is_override, + .. + }, + } => { + assert_eq!(event, DbTriggerEvent::Create); + assert_eq!(target.name, "Customer"); + assert!(!is_override); + } + _ => panic!("Expected On DbEvent statement"), + } +} + +#[test] +fn parse_on_write_with_buffers() { + let source = + r#"ON WRITE OF Customer NEW BUFFER bNew OLD BUFFER bOld DO: MESSAGE "wrote". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::DbEvent { + event, referencing, .. + }, + } => { + assert_eq!(event, DbTriggerEvent::Write); + assert_eq!(referencing.new_buffer.as_ref().unwrap().name, "bNew"); + assert_eq!(referencing.old_buffer.as_ref().unwrap().name, "bOld"); + } + _ => panic!("Expected On DbEvent statement"), + } +} + +#[test] +fn parse_on_assign_of_field() { + let source = r#"ON ASSIGN OF Customer.Name DO: MESSAGE "assigned". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::DbEvent { event, target, .. }, + } => { + assert_eq!(event, DbTriggerEvent::Assign); + assert_eq!(target.name, "Customer.Name"); + } + _ => panic!("Expected On DbEvent statement"), + } +} + +#[test] +fn parse_on_assign_old_value() { + let source = r#"ON ASSIGN OF Customer.Name OLD VALUE oldName DO: MESSAGE "changed". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::DbEvent { referencing, .. }, + } => { + assert_eq!(referencing.old_value.as_ref().unwrap().name, "oldName"); + } + _ => panic!("Expected On DbEvent statement"), + } +} + +#[test] +fn parse_on_write_override() { + let source = r#"ON WRITE OF Customer OVERRIDE DO: MESSAGE "overridden". END."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::DbEvent { is_override, .. }, + } => { + assert!(is_override); + } + _ => panic!("Expected On DbEvent statement"), + } +} + +#[test] +fn parse_on_db_revert() { + let source = "ON WRITE OF Customer REVERT."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: OnKind::DbEvent { action, .. }, + } => { + assert!(matches!(action, OnAction::Revert)); + } + _ => panic!("Expected On DbEvent statement"), + } +} + +// ── ON key remap tests ────────────────────────────────────────────── + +#[test] +fn parse_on_key_remap() { + let source = "ON F1 HELP."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::On { + kind: + OnKind::KeyRemap { + key_label, + key_function, + }, + } => { + assert_eq!(key_label.name, "F1"); + assert_eq!(key_function.name, "HELP"); + } + _ => panic!("Expected On KeyRemap statement"), + } +} + +// ── TRIGGER PROCEDURE tests ───────────────────────────────────────── + +#[test] +fn parse_trigger_procedure_create() { + let source = "TRIGGER PROCEDURE FOR CREATE OF Customer."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::TriggerProcedure { event, target, .. } => { + assert_eq!(event, DbTriggerEvent::Create); + assert_eq!(target.name, "Customer"); + } + _ => panic!("Expected TriggerProcedure statement"), + } +} + +#[test] +fn parse_trigger_procedure_write() { + let source = "TRIGGER PROCEDURE FOR WRITE OF Customer NEW BUFFER bNew OLD BUFFER bOld."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::TriggerProcedure { + event, referencing, .. + } => { + assert_eq!(event, DbTriggerEvent::Write); + assert_eq!(referencing.new_buffer.as_ref().unwrap().name, "bNew"); + assert_eq!(referencing.old_buffer.as_ref().unwrap().name, "bOld"); + } + _ => panic!("Expected TriggerProcedure statement"), + } +} + +#[test] +fn parse_trigger_procedure_write_no_buffer_keyword() { + let source = "TRIGGER PROCEDURE FOR WRITE OF Customer NEW bNew OLD bOld."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::TriggerProcedure { referencing, .. } => { + assert_eq!(referencing.new_buffer.as_ref().unwrap().name, "bNew"); + assert_eq!(referencing.old_buffer.as_ref().unwrap().name, "bOld"); + } + _ => panic!("Expected TriggerProcedure statement"), + } +} + +#[test] +fn parse_trigger_procedure_assign_of() { + let source = "TRIGGER PROCEDURE FOR ASSIGN OF Customer.Name."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::TriggerProcedure { + event, + target, + new_value, + .. + } => { + assert_eq!(event, DbTriggerEvent::Assign); + assert_eq!(target.name, "Customer.Name"); + assert!(new_value.is_none()); + } + _ => panic!("Expected TriggerProcedure statement"), + } +} + +#[test] +fn parse_trigger_procedure_assign_new_value() { + let source = "TRIGGER PROCEDURE FOR ASSIGN NEW VALUE newVal AS CHARACTER."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::TriggerProcedure { + event, + new_value, + old_value_param, + .. + } => { + assert_eq!(event, DbTriggerEvent::Assign); + let nv = new_value.unwrap(); + assert_eq!(nv.name.name, "newVal"); + assert_eq!(nv.data_type, DataType::Character); + assert!(old_value_param.is_none()); + } + _ => panic!("Expected TriggerProcedure statement"), + } +} + +#[test] +fn parse_trigger_procedure_assign_new_old_value() { + let source = + "TRIGGER PROCEDURE FOR ASSIGN NEW VALUE newVal AS CHARACTER OLD VALUE oldVal AS CHARACTER."; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::TriggerProcedure { + new_value, + old_value_param, + .. + } => { + let nv = new_value.unwrap(); + assert_eq!(nv.name.name, "newVal"); + assert_eq!(nv.data_type, DataType::Character); + let ov = old_value_param.unwrap(); + assert_eq!(ov.name.name, "oldVal"); + assert_eq!(ov.data_type, DataType::Character); + } + _ => panic!("Expected TriggerProcedure statement"), + } +} + +// ── ON trigger integration tests ──────────────────────────────────── + +#[test] +fn parse_on_trigger_in_procedure() { + let source = r#"PROCEDURE myProc: + ON CHOOSE OF btnOk DO: + MESSAGE "clicked". + END. +END PROCEDURE."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Procedure { name, body } => { + assert_eq!(name.name, "myProc"); + assert_eq!(body.len(), 1); + assert!(matches!(body[0], Statement::On { .. })); + } + _ => panic!("Expected Procedure statement"), + } +} + +#[test] +fn parse_on_trigger_inside_do_block() { + let source = r#"DO: + ON CHOOSE OF btnOk DO: + MESSAGE "clicked". + END. +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::On { .. })); + } + _ => panic!("Expected Do statement"), + } +} + +#[test] +fn parse_on_trigger_in_class_body() { + let source = r#"CLASS MyApp.MyClass: + ON CHOOSE OF btnOk DO: + MESSAGE "clicked". + END. +END CLASS."#; + let tokens = tokenize(source); + let mut parser = Parser::new(&tokens, source); + let stmt = parser.parse_statement().expect("Expected a statement"); + match stmt { + Statement::Class { body, .. } => { + assert_eq!(body.len(), 1); + assert!(matches!(body[0], Statement::On { .. })); + } + _ => panic!("Expected Class statement"), + } +} diff --git a/docs/brainstorms/2026-04-09-on-triggers-brainstorm.md b/docs/brainstorms/2026-04-09-on-triggers-brainstorm.md new file mode 100644 index 0000000..18ca336 --- /dev/null +++ b/docs/brainstorms/2026-04-09-on-triggers-brainstorm.md @@ -0,0 +1,66 @@ +# Brainstorm: ON Triggers & TRIGGER PROCEDURE + +**Date:** 2026-04-09 +**Status:** Complete + +## What We're Building + +Full parsing support for ABL's ON statement (all 4 forms) and the TRIGGER PROCEDURE statement. These are event handlers for UI events, database events, key remapping, and web notifications, plus schema-level trigger procedure declarations. + +### ON Statement Forms + +1. **UI/Developer event triggers** -- `ON event-list OF widget-list [ANYWHERE] trigger-block` +2. **Database event triggers** -- `ON CREATE|DELETE|FIND|WRITE|ASSIGN OF table [referencing-phrase] [OVERRIDE] trigger-block` +3. **Key remapping** -- `ON key-label key-function` +4. **SpeedScript web notify** -- `ON "WEB-NOTIFY" ANYWHERE trigger-block` + +### TRIGGER PROCEDURE Statement + +File-level declaration for schema triggers: +- `TRIGGER PROCEDURE FOR CREATE|DELETE|FIND OF table` +- `TRIGGER PROCEDURE FOR WRITE OF table [NEW [BUFFER] name] [OLD [BUFFER] name]` +- `TRIGGER PROCEDURE FOR ASSIGN OF table.field` (OF form) +- `TRIGGER PROCEDURE FOR ASSIGN NEW [VALUE] var {AS type | LIKE field} ...` (NEW VALUE form) + +## Why This Approach + +### AST Design: Statement::On + OnKind enum + separate Statement::TriggerProcedure + +- **Single `Statement::On` variant** with an `OnKind` enum distinguishing UI event, DB event, key remap, and web notify. Keeps the Statement enum slim while capturing all forms. +- **Separate `Statement::TriggerProcedure`** because it's semantically distinct -- a file-level declaration, not an event handler. Different dispatch keyword (`TRIGGER` vs `ON`). + +### Trigger Body: Match IF/THEN pattern + +- Check for `DO` keyword -- if present, parse full block via `parse_block_body()`; otherwise parse a single statement + period. +- Reuse `parse_block_body()` as-is so ON trigger DO blocks get CATCH/FINALLY support for free. + +### Disambiguation: Lookahead after ON + +- `ON` appears both as a trigger statement (`ON CHOOSE OF btn`) and in block headers (`DO ON ERROR UNDO`). +- Peek at next token(s) to distinguish: DB events (CREATE/DELETE/WRITE/FIND/ASSIGN) and known UI event names vs block-header keywords (ERROR/ENDKEY/STOP/QUIT). +- Similar pattern to how `Kind::Input` already uses lookahead for stream I/O vs parameter direction. + +## Key Decisions + +1. **Scope**: All 4 ON forms + TRIGGER PROCEDURE in one pass +2. **AST**: `Statement::On { kind: OnKind, ... }` + `Statement::TriggerProcedure { ... }` as separate variants +3. **Block parsing**: Reuse `parse_block_body()` for DO blocks (gets CATCH/FINALLY for free) +4. **Trigger body**: Single statement OR DO...END block, matching IF/THEN pattern +5. **Disambiguation**: Lookahead on token(s) after ON to distinguish trigger vs block header +6. **New keywords**: Add event names and clause keywords to `keyword_overrides.toml`, regenerate via codegen + +## Implementation Checklist (for planning phase) + +- [ ] Add new keywords to `keyword_overrides.toml` (event names, ANYWHERE, OVERRIDE, REVERT, PERSISTENT, TRIGGER, PROCEDURE) +- [ ] Regenerate lexer via `cargo run -p oxabl_codegen` +- [ ] Define `OnKind` enum and `Statement::On` variant in `oxabl_ast` +- [ ] Define `Statement::TriggerProcedure` variant in `oxabl_ast` +- [ ] Add `Kind::On` to `can_start_statement()` and `Kind::Trigger` dispatch +- [ ] Implement `parse_on_statement()` with lookahead disambiguation +- [ ] Implement `parse_trigger_procedure()` +- [ ] Add comprehensive tests for all forms +- [ ] Update CLAUDE.md parser capabilities + +## Open Questions + +None -- all key decisions resolved during brainstorm. diff --git a/docs/plans/2026-04-09-002-feat-on-triggers-trigger-procedure-plan.md b/docs/plans/2026-04-09-002-feat-on-triggers-trigger-procedure-plan.md new file mode 100644 index 0000000..7daed55 --- /dev/null +++ b/docs/plans/2026-04-09-002-feat-on-triggers-trigger-procedure-plan.md @@ -0,0 +1,663 @@ +--- +title: "feat: Add ON trigger and TRIGGER PROCEDURE statement parsing" +type: feat +status: completed +date: 2026-04-09 +origin: docs/brainstorms/2026-04-09-on-triggers-brainstorm.md +--- + +# feat: Add ON Trigger and TRIGGER PROCEDURE Statement Parsing + +## Enhancement Summary + +**Deepened on:** 2026-04-09 +**Sections enhanced:** 6 +**Review agents used:** architecture-strategist, code-simplicity-reviewer, pattern-recognition-specialist, performance-oracle, FDM4/ABL syntax validator, spec-flow-analyzer + +### Key Improvements +1. Fixed incorrect disambiguation safety claim -- `parse_do_statement()` does NOT consume ON in block headers today; the dispatch is safe trivially because that construct isn't implemented yet +2. Widget references need `IN FRAME`/`IN BROWSE` qualifiers -- `Vec` is insufficient for real ABL code; added `WidgetRef` struct +3. Merged `OnKind::WebNotify` into `UiEvent` (YAGNI -- syntactically identical to UI event with ANYWHERE) +4. Extracted shared `TriggerReferencing` struct for NEW/OLD BUFFER fields used by both `DbEvent` and `TriggerProcedure` +5. Fixed type mismatch: `old_value` on `TriggerProcedure` must be `Option`, not `Option` +6. Added REPLICATION-CREATE/DELETE/WRITE to `DbTriggerEvent` for TRIGGER PROCEDURE completeness +7. Key remapping disambiguation: `is_key_label()`/`is_key_function()` should accept any identifier (not restrictive lists), since `ON .` with no OF is always key remapping + +### New Considerations Discovered +- `Kind::Leave`, `Kind::Create`, `Kind::Delete`, etc. double as event names in ON triggers -- the event parser must accept statement keywords via `can_be_identifier()` or a dedicated helper +- ASSIGN OF targets use `table.field` dotted names -- verify `parse_identifier()` handles this or use expression parsing +- OR keyword serves dual role (logical operator vs event chain separator) -- context resolves this naturally since OR appears between widget-list and next event-list +- ON triggers are valid inside CLASS bodies -- add test for this +- Trigger phrase (inline `TRIGGERS: ... END TRIGGERS.` in widget definitions) is out of scope but noted as future gap + +--- + +## Overview + +Add full parsing support for ABL's ON statement (all 4 forms) and the TRIGGER PROCEDURE statement. These are event handlers for UI events, database events, key remapping, and web notifications, plus schema-level trigger procedure declarations. + +This completes the last remaining parser gap listed in CLAUDE.md: "ON triggers." + +(see brainstorm: `docs/brainstorms/2026-04-09-on-triggers-brainstorm.md`) + +## Problem Statement / Motivation + +Real-world ABL code uses ON triggers extensively for UI event handling, database triggers, and key remapping. Without parser support, any ABL file containing these constructs 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 needed:** + +| Keyword | `keyword_type` | Notes | +|---------|----------------|-------| +| `CHOOSE` | `Option` | UI event name (not in Kind yet) | +| `ENDKEY` | `Option` | Key function / event name (not in Kind yet) | + +**Keywords that already exist in Kind (no codegen changes):** + +- `Kind::On` (line 108), `Kind::Trigger` (line 330), `Kind::Anywhere` (line 322) +- `Kind::Persistent` (line 257), `Kind::Override` (line 295), `Kind::Revert` (line 531) +- `Kind::Assign` (line 80), `Kind::Create`, `Kind::Delete` +- `Kind::Find`, `Kind::Write` (line 584), `Kind::GoOn` (line 466) +- `Kind::Entry` (line 96), `Kind::ValueChanged` (line 572), `Kind::ErrorStatus` (line 422) + +After adding new keywords: `cargo run -p oxabl_codegen` + +### Phase 2: AST Types + +**File:** `crates/oxabl_ast/src/statement.rs` + +#### 2a. Shared Types + +Extract referencing fields shared by `OnKind::DbEvent` and `Statement::TriggerProcedure`: + +```rust +/// Referencing phrase for database triggers (NEW/OLD BUFFER for WRITE, OLD VALUE for ASSIGN). +/// Shared between ON db-event triggers and TRIGGER PROCEDURE. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct TriggerReferencing { + /// NEW [BUFFER] alias (WRITE triggers). + pub new_buffer: Option, + /// OLD [BUFFER] alias (WRITE triggers). + pub old_buffer: Option, + /// OLD [VALUE] alias (ASSIGN triggers in ON statement). + pub old_value: Option, +} + +/// Database trigger event types. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DbTriggerEvent { + Create, + Delete, + Find, + Write, + Assign, + /// Replication events (TRIGGER PROCEDURE only). + ReplicationCreate, + ReplicationDelete, + ReplicationWrite, +} + +/// Widget reference in an ON trigger, with optional frame/browse qualifier. +/// +/// `btnOk IN FRAME main-frame` or `col1 IN BROWSE brw1` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WidgetRef { + pub name: Identifier, + pub qualifier: Option, +} + +/// Optional qualification for a widget reference. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum WidgetQualifier { + /// IN FRAME frame-name + InFrame(Identifier), + /// IN BROWSE browse-name + InBrowse(Identifier), +} +``` + +#### 2b. ON Statement + +Add `Statement::On` with an `OnKind` enum to distinguish the 3 forms (WebNotify merged into UiEvent per simplicity review): + +```rust +/// ON trigger statement -- event handlers for UI, database, and key events. +/// +/// ```abl +/// ON CHOOSE OF btnOk IN FRAME f1 DO: /* ... */ END. +/// ON WRITE OF Customer NEW BUFFER bNew OLD BUFFER bOld DO: /* ... */ END. +/// ON F1 HELP. +/// ON "WEB-NOTIFY" ANYWHERE DO: /* ... */ END. +/// ``` +On { + kind: OnKind, +}, +``` + +```rust +/// Discriminant for the different forms of the ON statement. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OnKind { + /// UI/developer event trigger (includes "WEB-NOTIFY" ANYWHERE form): + /// ON event-list OF widget-list [OR event-list OF widget-list]... [ANYWHERE] + /// { trigger-block | REVERT | PERSISTENT RUN proc [(args)] } + UiEvent { + /// Event/widget clauses -- at least one, chained via OR. + /// Empty when ANYWHERE is used standalone (e.g., ON "WEB-NOTIFY" ANYWHERE). + clauses: Vec, + /// Whether ANYWHERE was specified. + anywhere: bool, + /// The trigger action. + action: OnAction, + }, + /// Database event trigger: + /// ON CREATE|DELETE|FIND|WRITE|ASSIGN OF table [referencing] [OVERRIDE] + /// { trigger-block | REVERT } + DbEvent { + /// The database event. + event: DbTriggerEvent, + /// The table (or table.field for ASSIGN) the trigger is on. + target: Identifier, + /// NEW/OLD BUFFER/VALUE referencing phrases. + referencing: TriggerReferencing, + /// Whether OVERRIDE was specified. + is_override: bool, + /// The trigger action (block or REVERT). + action: OnAction, + }, + /// Key remapping: ON key-label key-function. + KeyRemap { + /// The key label (e.g., F1, CTRL-X) -- any identifier. + key_label: Identifier, + /// The key function (e.g., HELP, ENDKEY, GO) -- any identifier. + key_function: Identifier, + }, +} +``` + +```rust +/// A single event/widget-list clause in a UI ON trigger. +/// +/// `ON CHOOSE, ENTRY OF btnOk IN FRAME f1, btnCancel` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OnEventClause { + /// Comma-separated event names (identifiers, including keywords like LEAVE/ENTRY). + pub events: Vec, + /// Comma-separated widget references with optional frame/browse qualifiers. + pub widgets: Vec, +} + +/// The action taken by an ON trigger. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OnAction { + /// A trigger block -- either a single statement or DO...END block. + Block(Box), + /// REVERT -- removes the trigger. + Revert, + /// PERSISTENT RUN procedure [(args)]. + PersistentRun { + procedure: Identifier, + arguments: Vec, + }, +} +``` + +#### 2c. TRIGGER PROCEDURE Statement + +Separate `Statement::TriggerProcedure` variant (see brainstorm: semantically distinct from ON): + +```rust +/// TRIGGER PROCEDURE FOR event OF table [NEW/OLD clauses]. +/// +/// Declares a schema trigger -- always the first statement in a trigger procedure file. +/// +/// ```abl +/// TRIGGER PROCEDURE FOR WRITE OF Customer +/// NEW BUFFER bNew OLD BUFFER bOld. +/// ``` +TriggerProcedure { + /// The trigger event (CREATE, DELETE, FIND, WRITE, ASSIGN, or REPLICATION-*). + event: DbTriggerEvent, + /// The target table (or table.field for ASSIGN OF form). + target: Identifier, + /// NEW/OLD BUFFER referencing (WRITE triggers). + referencing: TriggerReferencing, + /// NEW VALUE variable definition (ASSIGN triggers, mutually exclusive with OF form). + new_value: Option, + /// OLD VALUE variable definition (ASSIGN NEW VALUE form). + old_value_param: Option, +}, +``` + +```rust +/// A variable-like parameter for TRIGGER PROCEDURE FOR ASSIGN NEW VALUE form. +/// +/// ```abl +/// TRIGGER PROCEDURE FOR ASSIGN +/// NEW VALUE newVal AS CHARACTER +/// OLD VALUE oldVal AS CHARACTER. +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TriggerAssignParam { + pub name: Identifier, + pub data_type: DataType, +} +``` + +**Simplification note:** `TriggerAssignParam` reduced to just `(name, data_type)` fields. The ABL grammar technically allows INITIAL, NO-UNDO, COLUMN-LABEL, FORMAT, LABEL on these params, but they are virtually never used in real trigger declarations. Promote to a richer struct later if needed. + +#### 2d. Import Updates + +Add new types to `crates/oxabl_ast/src/lib.rs` re-exports: +`DbTriggerEvent`, `OnAction`, `OnEventClause`, `OnKind`, `TriggerAssignParam`, `TriggerReferencing`, `WidgetQualifier`, `WidgetRef` + +### Phase 3: Parser Implementation + +**File:** `crates/oxabl_parser/src/parser/statements.rs` + +#### 3a. Statement Dispatch + +Add to `can_start_statement()`: +```rust +| Kind::On +| Kind::Trigger +``` + +Add to `parse_statement()` dispatch chain (before the expression/assignment fallback at line 263): +```rust +// ON triggers +if self.check(Kind::On) { + return self.parse_on_statement(); +} + +// TRIGGER PROCEDURE +if self.check(Kind::Trigger) { + return self.parse_trigger_procedure(); +} +``` + +#### 3b. Disambiguation Strategy + +The `ON` keyword appears in two contexts: +1. **Trigger statement** (top-level): `ON CHOOSE OF btn DO: ... END.` +2. **Block header phrase**: `DO ON ERROR UNDO: ... END.` + +**Current state:** `parse_do_statement()`, `parse_for_each()`, and `parse_repeat_statement()` do NOT currently parse `ON ERROR UNDO` phrases at all. There is no `Kind::On` handling anywhere in the parser today. Therefore, any `ON` that reaches `parse_statement()` is always a trigger statement -- the dispatch is safe because block-header ON simply isn't implemented yet. + +**Forward-compatibility invariant:** When `DO ON ERROR UNDO` support is added in the future, it MUST consume the `ON` inside the block header parser (e.g., `parse_do_statement()`) before `parse_statement()` is reached. Add a code comment at the dispatch site documenting this invariant. + +Within `parse_on_statement()`, the 3 forms are distinguished by: + +``` +After consuming ON: + 1. String literal → UiEvent with ANYWHERE (e.g., ON "WEB-NOTIFY" ANYWHERE ...) + 2. DB event keyword (CREATE/DELETE/FIND/WRITE/ASSIGN) + peek(1) is OF → DbEvent + BUT: if peek(1) is Comma, fall through to UiEvent (e.g., ON CREATE, DELETE OF table) + 3. Any identifier + peek(1) is NOT OF/Comma → KeyRemap (ON .) + 4. Everything else → UiEvent (event name list, expects OF for widget list) +``` + +**Key insight for KeyRemap:** `ON .` with no `OF` or comma is always key remapping, because single-statement UI triggers require `OF` between event and widget. No restrictive key-label/key-function lists needed -- accept any identifier pair. This avoids string comparison. + +#### 3c. parse_on_statement() Implementation + +``` +fn parse_on_statement() -> ParseResult: + advance() // consume ON + + // Check for string literal event name (e.g., "WEB-NOTIFY") + // Parsed as UiEvent with no clauses and ANYWHERE + if check(Kind::String): + // fall through to UI event parsing (string is the event name) + return parse_on_ui_event_from_string() + + // Check for DB events: CREATE/DELETE/FIND/WRITE/ASSIGN followed by OF (not Comma) + if is_db_event_kind(current) && check_at(1, Kind::Of): + return parse_on_db_event() + + // Check for key remapping: identifier followed by identifier, no OF/Comma + // ON . + if !check_at(1, Kind::Of) && !check_at(1, Kind::Comma) + && check_at(1, /* is identifier-like */) && check_at(2, Kind::Period): + return parse_on_key_remap() + + // Default: UI/developer event trigger + return parse_on_ui_event() +``` + +#### 3d. parse_on_ui_event() -- UI/Developer Events + +``` +fn parse_on_ui_event() -> ParseResult: + clauses = vec![] + anywhere = false + + loop: + events = parse_event_name_list() // comma-separated, accepts keywords as identifiers + // Check for ANYWHERE without OF (standalone) + if check(Kind::Anywhere): + anywhere = true + advance() + break + expect(Kind::Of) + widgets = parse_widget_ref_list() // comma-separated, each with optional IN FRAME/BROWSE + clauses.push(OnEventClause { events, widgets }) + + // Check for OR to chain another clause + if !check(Kind::KwOr): + break + advance() // consume OR + + // Check for trailing ANYWHERE (after widget list) + if check(Kind::Anywhere): + anywhere = true + advance() + + action = parse_trigger_action() + return Statement::On { kind: OnKind::UiEvent { clauses, anywhere, action } } +``` + +**Event name list termination:** Comma-separated identifiers, terminated by `OF`, `ANYWHERE`, or any non-identifier/non-comma token. + +**Widget ref list parsing:** +``` +fn parse_widget_ref_list() -> ParseResult>: + refs = vec![] + loop: + name = parse_identifier() + qualifier = None + if check(Kind::In): + advance() + if check(Kind::Frame): + advance(); qualifier = Some(WidgetQualifier::InFrame(parse_identifier())) + elif check(Kind::Browse): + advance(); qualifier = Some(WidgetQualifier::InBrowse(parse_identifier())) + refs.push(WidgetRef { name, qualifier }) + if !check(Kind::Comma): break + advance() // consume comma + return refs +``` + +**Widget list termination:** Ends at `OR`, `ANYWHERE`, `DO`, `Colon`, `REVERT`, `PERSISTENT`, or period (i.e., anything that isn't a comma or identifier/IN). + +#### 3e. parse_event_name_list() -- Accepts Keywords as Event Names + +``` +fn parse_event_name_list() -> ParseResult>: + // Event names include keywords like LEAVE, ENTRY, CREATE that double as statement keywords. + // Accept any identifier-like token or known event keyword. + names = vec![] + loop: + names.push(parse_event_name_identifier()) // uses extended can_be_identifier + statement keywords + if !check(Kind::Comma): break + advance() + return names +``` + +The `parse_event_name_identifier()` helper should accept: +- Any token that passes `can_be_identifier()` +- Plus statement keywords that are also event names: `Kind::Leave`, `Kind::Create`, `Kind::Delete`, `Kind::Find`, `Kind::Close` + +#### 3f. parse_on_db_event() -- Database Events + +``` +fn parse_on_db_event() -> ParseResult: + event = parse_db_event_kind() // CREATE|DELETE|FIND|WRITE|ASSIGN + expect(Kind::Of) + target = parse_identifier() // table or table.field (dotted name) + + referencing = TriggerReferencing::default() + + // Parse optional referencing phrases + if event == WRITE: + if check(Kind::KwNew): advance(); eat optional BUFFER; referencing.new_buffer = Some(parse_identifier()) + if check(Kind::Old): advance(); eat optional BUFFER; referencing.old_buffer = Some(parse_identifier()) + if event == ASSIGN: + if check(Kind::Old): advance(); eat optional VALUE; referencing.old_value = Some(parse_identifier()) + + is_override = false + if check(Kind::Override): + is_override = true + advance() + + action = parse_trigger_action() + return Statement::On { kind: OnKind::DbEvent { event, target, referencing, is_override, action } } +``` + +**ASSIGN OF target:** The target is `table.field` (e.g., `Customer.Name`). This is a dotted name that `parse_identifier()` may not handle. Options: +- Use `parse_expression()` and extract the identifier from the resulting member-access expression +- Or parse the first identifier, check for period + identifier (taking care not to confuse the field separator `.` with the statement terminator `.`), and concatenate + +The safest approach is to parse the identifier, then if a period follows and the next token is an identifier-like token (not a keyword that starts a statement), consume the period and next identifier to build a dotted name. This mirrors how CATCH blocks parse dotted class names (lines 2114-2126 of statements.rs). + +#### 3g. parse_trigger_action() -- Shared Trigger Body + +Follows the IF/THEN pattern (see brainstorm). **Must check REVERT and PERSISTENT before single-statement fallthrough** to avoid misparse: + +``` +fn parse_trigger_action() -> ParseResult: + if check(Kind::Revert): + advance() + expect(Kind::Period) + return Ok(OnAction::Revert) + + if check(Kind::Persistent): + advance() + expect(Kind::Run) + procedure = parse_identifier() + arguments = if check(Kind::LeftParen): parse_argument_list() else vec![] + expect(Kind::Period) + return Ok(OnAction::PersistentRun { procedure, arguments }) + + // Trigger block: DO...END or single statement + if check(Kind::Do): + block = parse_do_statement() // already handles colon, block body, END, period + return Ok(OnAction::Block(Box::new(block))) + + // Single statement (terminates with its own period) + stmt = parse_statement() + return Ok(OnAction::Block(Box::new(stmt))) +``` + +#### 3h. parse_trigger_procedure() -- TRIGGER PROCEDURE + +``` +fn parse_trigger_procedure() -> ParseResult: + advance() // consume TRIGGER + expect(Kind::Procedure) + expect(Kind::KwFor) + + event = parse_db_event_kind() // CREATE|DELETE|FIND|WRITE|ASSIGN|REPLICATION-* + + // ASSIGN has two mutually exclusive forms + if event == ASSIGN: + if check(Kind::Of): + advance() + target = parse_identifier() // table.field (dotted name) + expect(Kind::Period) + return Ok(Statement::TriggerProcedure { event, target, referencing: default(), new_value: None, old_value_param: None }) + else: + // NEW VALUE form + expect(Kind::KwNew) + eat optional VALUE + new_value = parse_trigger_assign_param() + old_value_param = if check(Kind::Old): + advance(); eat optional VALUE; Some(parse_trigger_assign_param()) + else None + expect(Kind::Period) + // target is empty/placeholder for NEW VALUE form + return Ok(Statement::TriggerProcedure { event, target: placeholder, new_value: Some(new_value), old_value_param, referencing: default() }) + + expect(Kind::Of) + target = parse_identifier() + + referencing = TriggerReferencing::default() + if event == WRITE: + if check(Kind::KwNew): advance(); eat optional BUFFER; referencing.new_buffer = Some(parse_identifier()) + if check(Kind::Old): advance(); eat optional BUFFER; referencing.old_buffer = Some(parse_identifier()) + + expect(Kind::Period) + return Ok(Statement::TriggerProcedure { event, target, referencing, new_value: None, old_value_param: None }) +``` + +#### 3i. parse_trigger_assign_param() + +``` +fn parse_trigger_assign_param() -> ParseResult: + name = parse_identifier() + // AS type or LIKE field + data_type = parse_data_type() // reuse existing data type parser + return Ok(TriggerAssignParam { name, data_type }) +``` + +#### 3j. Import Updates + +Add new AST types to the import block at the top of `statements.rs`: +```rust +use oxabl_ast::{ + ..., DbTriggerEvent, OnAction, OnEventClause, OnKind, + TriggerAssignParam, TriggerReferencing, WidgetQualifier, WidgetRef, +}; +``` + +#### 3k. `can_be_identifier()` Updates + +**File:** `crates/oxabl_parser/src/parser/mod.rs` + +Add these keywords to `can_be_identifier()` under a new `// ON trigger keywords (unreserved)` section: + +```rust +// ON trigger keywords (unreserved) +| Kind::Trigger +| Kind::Anywhere +| Kind::Persistent +| Kind::Revert +| Kind::Override +| Kind::Choose +| Kind::Endkey +``` + +Note: `Kind::On` is reserved in ABL and should NOT be in `can_be_identifier()`. + +### Phase 4: Tests + +**File:** `crates/oxabl_parser/src/parser/tests.rs` + +Add a new test section using box-drawing header style: +```rust +// ── ON trigger tests ──────────────────────────────────────────────── +``` + +**UI Event form:** +- `parse_on_choose_of_button` -- minimal: `ON CHOOSE OF btnOk DO: MESSAGE "clicked". END.` +- `parse_on_multiple_events` -- comma-separated events: `ON CHOOSE, ENTRY OF btnOk DO: ... END.` +- `parse_on_multiple_widgets` -- comma-separated widgets: `ON CHOOSE OF btn1, btn2 DO: ... END.` +- `parse_on_widget_in_frame` -- frame qualifier: `ON CHOOSE OF btnOk IN FRAME main-frame DO: ... END.` +- `parse_on_or_clause` -- OR chaining: `ON CHOOSE OF btn1 OR ENTRY OF fill1 DO: ... END.` +- `parse_on_anywhere` -- ANYWHERE standalone: `ON CHOOSE ANYWHERE DO: ... END.` +- `parse_on_anywhere_with_widgets` -- ANYWHERE after widget list: `ON CHOOSE OF btn1 ANYWHERE DO: ... END.` +- `parse_on_single_statement` -- no DO block: `ON CHOOSE OF btnOk MESSAGE "clicked".` +- `parse_on_revert` -- REVERT action: `ON CHOOSE OF btnOk REVERT.` +- `parse_on_persistent_run` -- PERSISTENT RUN: `ON CHOOSE OF btnOk PERSISTENT RUN myProc.` +- `parse_on_persistent_run_with_args` -- with arguments: `ON CHOOSE OF btnOk PERSISTENT RUN myProc (INPUT x).` +- `parse_on_leave_event_name` -- statement keyword as event: `ON LEAVE OF fill1 DO: ... END.` +- `parse_on_web_notify` -- string event: `ON "WEB-NOTIFY" ANYWHERE DO: ... END.` + +**Database Event form:** +- `parse_on_create_of_table` -- `ON CREATE OF Customer DO: ... END.` +- `parse_on_write_with_buffers` -- NEW/OLD BUFFER: `ON WRITE OF Customer NEW BUFFER bNew OLD BUFFER bOld DO: ... END.` +- `parse_on_assign_of_field` -- dotted name: `ON ASSIGN OF Customer.Name DO: ... END.` +- `parse_on_assign_old_value` -- `ON ASSIGN OF Customer.Name OLD VALUE oldName DO: ... END.` +- `parse_on_write_override` -- OVERRIDE: `ON WRITE OF Customer OVERRIDE DO: ... END.` +- `parse_on_db_revert` -- `ON WRITE OF Customer REVERT.` + +**Key Remap form:** +- `parse_on_key_remap` -- `ON F1 HELP.` + +**TRIGGER PROCEDURE:** +- `parse_trigger_procedure_create` -- `TRIGGER PROCEDURE FOR CREATE OF Customer.` +- `parse_trigger_procedure_write` -- `TRIGGER PROCEDURE FOR WRITE OF Customer NEW BUFFER bNew OLD BUFFER bOld.` +- `parse_trigger_procedure_write_no_buffer_keyword` -- `TRIGGER PROCEDURE FOR WRITE OF Customer NEW bNew OLD bOld.` +- `parse_trigger_procedure_assign_of` -- `TRIGGER PROCEDURE FOR ASSIGN OF Customer.Name.` +- `parse_trigger_procedure_assign_new_value` -- `TRIGGER PROCEDURE FOR ASSIGN NEW VALUE newVal AS CHARACTER.` +- `parse_trigger_procedure_assign_new_old_value` -- `TRIGGER PROCEDURE FOR ASSIGN NEW VALUE newVal AS CHARACTER OLD VALUE oldVal AS CHARACTER.` + +**Integration:** +- `parse_on_trigger_in_procedure` -- ON trigger inside a PROCEDURE body +- `parse_on_trigger_inside_do_block` -- ON trigger nested in a DO block (not confused with DO ON ERROR) +- `parse_on_trigger_in_class_body` -- ON trigger inside a CLASS body (valid per ABL spec) + +### Phase 5: Update CLAUDE.md + +Add to the parser capabilities list in CLAUDE.md: +- `ON` (UI/developer event triggers with IN FRAME/IN BROWSE, database event triggers, key remapping) +- `TRIGGER PROCEDURE` (schema trigger declarations including REPLICATION events) + +Update the "Not yet implemented" line to remove "ON triggers." + +## Technical Considerations + +### Disambiguation Safety + +**Current state:** `parse_do_statement()`, `parse_for_each()`, and `parse_repeat_statement()` do NOT handle `ON ERROR UNDO` block-header phrases. There is no `Kind::On` handling anywhere in the parser crate today. Therefore, the `Kind::On` dispatch in `parse_statement()` is safe -- it will only match trigger statements. + +**Forward-compatibility invariant:** When `DO ON ERROR UNDO` support is added, it MUST consume the `ON` inside the block header parser before `parse_statement()` is reached. Add a code comment at the dispatch site documenting this. + +Within `parse_on_statement()`, the 3 forms are distinguishable by: +1. **String literal** or event names + OF/ANYWHERE --> UiEvent +2. **DB event keyword + OF** (no comma after) --> DbEvent +3. **Two identifiers + period** (no OF/comma) --> KeyRemap + +### Performance + +- All dispatch uses `Kind` enum matching (O(1)), no string comparison +- New keywords go through `keyword_overrides.toml` + codegen per CLAUDE.md guidance +- `OnAction::Block` boxes the statement to prevent recursive size inflation +- `Statement::On` will NOT inflate the Statement enum (OnKind ~200 bytes, well under DefineDataset's ~352 bytes) +- `Vec` allocation is acceptable on this cold path (parsed once per ON statement) +- For the `"WEB-NOTIFY"` string check: use `atom!("WEB-NOTIFY")` if the atom exists, otherwise byte comparison on this rare cold path is fine + +### Error Recovery + +`Kind::On` and `Kind::Trigger` in `can_start_statement()` allows the error recovery in `parse_program()` to synchronize on these keywords when a preceding statement is malformed. + +### Known Future Gaps + +- **Trigger phrase** (`TRIGGERS: ON ... END TRIGGERS.` inline in widget definitions) -- separate construct, not in scope +- **PERSISTENT RUN IN handle** -- the trigger phrase supports `IN handle` on PERSISTENT RUN; standalone ON may also need this +- **TRIGGER PROCEDURE ASSIGN** -- full syntax supports LIKE, COLUMN-LABEL, FORMAT, LABEL on params; deferred to keep TriggerAssignParam simple + +## Acceptance Criteria + +- [ ] All 3 ON statement forms parse correctly (UiEvent, DbEvent, KeyRemap) +- [ ] "WEB-NOTIFY" ANYWHERE parses as UiEvent with no clauses +- [ ] TRIGGER PROCEDURE parses all 5 standard event types (CREATE/DELETE/FIND/WRITE/ASSIGN) +- [ ] TRIGGER PROCEDURE ASSIGN handles both OF and NEW VALUE forms +- [ ] Widget references support IN FRAME and IN BROWSE qualifiers +- [ ] UI triggers support comma-separated events and widgets +- [ ] UI triggers support OR clause chaining +- [ ] ANYWHERE works both standalone and after widget list +- [ ] REVERT action parses for both UI and DB forms +- [ ] PERSISTENT RUN with optional arguments parses +- [ ] Single-statement triggers (no DO block) work +- [ ] DO block triggers get CATCH/FINALLY for free via `parse_block_body()` +- [ ] Event names that are also statement keywords (LEAVE, CREATE, etc.) parse correctly +- [ ] ASSIGN OF targets with dotted table.field names parse correctly +- [ ] ON inside block headers (DO ON ERROR) is NOT broken by this change +- [ ] ON triggers inside CLASS bodies parse correctly +- [ ] All tests pass (existing + new) +- [ ] `cargo clippy -D warnings` passes +- [ ] CLAUDE.md updated + +## Sources & References + +- **Origin brainstorm:** [docs/brainstorms/2026-04-09-on-triggers-brainstorm.md](docs/brainstorms/2026-04-09-on-triggers-brainstorm.md) -- key decisions: single On variant + OnKind enum, separate TriggerProcedure, reuse parse_block_body(), IF/THEN pattern for trigger bodies +- **Template pattern:** [docs/plans/2026-04-09-001-feat-publish-subscribe-event-system-plan.md](docs/plans/2026-04-09-001-feat-publish-subscribe-event-system-plan.md) -- same implementation flow (codegen -> AST -> parser -> tests -> CLAUDE.md) +- **ABL ON statement:** https://docs.progress.com/bundle/abl-reference/page/ON-statement.html +- **ABL TRIGGER PROCEDURE:** https://docs.progress.com/bundle/abl-reference/page/TRIGGER-PROCEDURE-statement.html +- **Institutional learning:** `docs/solutions/performance-issues/heap-allocation-in-keyword-matching.md` -- always use `keyword_overrides.toml` + codegen for new keywords, never string comparison in parser diff --git a/resources/keyword_overrides.toml b/resources/keyword_overrides.toml index 551490b..1f5ffc9 100644 --- a/resources/keyword_overrides.toml +++ b/resources/keyword_overrides.toml @@ -666,6 +666,26 @@ keyword_type = "Option" name = "RUN-PROCEDURE" keyword_type = "Option" +# ============================================================================= +# ON TRIGGER KEYWORDS +# Keywords needed for ON statement and TRIGGER PROCEDURE parsing. +# ============================================================================= + +# CHOOSE is a UI event name (unreserved) +[[add]] +name = "CHOOSE" +keyword_type = "Option" + +# ENDKEY is a key function / event name (unreserved) +[[add]] +name = "ENDKEY" +keyword_type = "Option" + +# BROWSE is needed for IN BROWSE widget qualifiers in ON triggers +[[add]] +name = "BROWSE" +keyword_type = "Option" + # ============================================================================= # OVERRIDES # Correct misclassifications or add missing info