From cf0897e1cb62b76bf4281a5eb706ea853ee643bb Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 23 Mar 2026 15:16:47 -0400 Subject: [PATCH] Attractor spec hunks 22-23: add parse_literal and quoted string support to condition parser Add parse_literal() that strips surrounding double-quotes from condition literal values, so `outcome="success"` and `outcome=success` behave identically. Update the tokenizer to handle "..." as single tokens (including spaces and escaped characters). Add BareLiteral to the grammar comment per spec Section 10. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-graphviz/src/condition.rs | 131 ++++++++++++++++++-- lib/crates/fabro-workflows/src/condition.rs | 53 ++++++++ 2 files changed, 175 insertions(+), 9 deletions(-) diff --git a/lib/crates/fabro-graphviz/src/condition.rs b/lib/crates/fabro-graphviz/src/condition.rs index f9265f7ce..f44a132d9 100644 --- a/lib/crates/fabro-graphviz/src/condition.rs +++ b/lib/crates/fabro-graphviz/src/condition.rs @@ -2,13 +2,15 @@ /// /// Grammar: /// ```text -/// Expr ::= OrExpr -/// OrExpr ::= AndExpr ('||' AndExpr)* -/// AndExpr ::= UnaryExpr ('&&' UnaryExpr)* -/// UnaryExpr ::= '!' UnaryExpr | Clause -/// Clause ::= Key Op Literal | Key (bare key = truthy) -/// Op ::= '=' | '!=' | '>' | '<' | '>=' | '<=' -/// | 'contains' | 'matches' +/// Expr ::= OrExpr +/// OrExpr ::= AndExpr ('||' AndExpr)* +/// AndExpr ::= UnaryExpr ('&&' UnaryExpr)* +/// UnaryExpr ::= '!' UnaryExpr | Clause +/// Clause ::= Key Op Literal | Key (bare key = truthy) +/// Op ::= '=' | '!=' | '>' | '<' | '>=' | '<=' +/// | 'contains' | 'matches' +/// Literal ::= String | Integer | Boolean | BareLiteral +/// BareLiteral ::= [A-Za-z_][A-Za-z0-9_.:-]* /// ``` use crate::error::GraphvizError; @@ -140,9 +142,27 @@ fn tokenize(input: &str) -> Result, GraphvizError> { _ => {} } + // Quoted string: consume `"..."` as a single word token + if chars[i] == '"' { + let start = i; + i += 1; // skip opening quote + while i < len && chars[i] != '"' { + if chars[i] == '\\' && i + 1 < len { + i += 1; // skip escaped char + } + i += 1; + } + if i < len { + i += 1; // skip closing quote + } + let word: String = chars[start..i].iter().collect(); + tokens.push(Token::Word(word)); + continue; + } + // Word: everything up to whitespace or operator char let start = i; - while i < len && !chars[i].is_whitespace() && !is_op_char(chars[i]) { + while i < len && !chars[i].is_whitespace() && !is_op_char(chars[i]) && chars[i] != '"' { i += 1; } if i == start { @@ -286,7 +306,7 @@ impl Parser { // Value: must be a Word let value = match self.advance() { - Some(Token::Word(w)) => w, + Some(Token::Word(w)) => parse_literal(&w), Some(other) => { return Err(GraphvizError::Parse(format!( "expected value after operator, got {other:?}" @@ -315,6 +335,19 @@ impl Parser { } } +/// Strip surrounding double-quotes from a literal value. +/// Bare values pass through unchanged. +fn parse_literal(raw: &str) -> String { + let trimmed = raw.trim(); + if trimmed.len() >= 2 && trimmed.starts_with('"') && trimmed.ends_with('"') { + trimmed[1..trimmed.len() - 1] + .replace("\\\"", "\"") + .replace("\\\\", "\\") + } else { + trimmed.to_string() + } +} + fn parse_expression(expr: &str) -> Result { let tokens = tokenize(expr)?; if tokens.is_empty() { @@ -457,4 +490,84 @@ mod tests { fn parse_not() { assert!(parse_condition("!x=y").is_ok()); } + + // ----------------------------------------------------------------------- + // parse_literal: quote stripping + // ----------------------------------------------------------------------- + + #[test] + fn parse_literal_bare_value() { + assert_eq!(parse_literal("success"), "success"); + } + + #[test] + fn parse_literal_strips_quotes() { + assert_eq!(parse_literal("\"success\""), "success"); + } + + #[test] + fn parse_literal_unescapes_inner_quotes() { + assert_eq!(parse_literal(r#""say \"hello\"""#), r#"say "hello""#); + } + + #[test] + fn parse_literal_single_quote_not_stripped() { + // Only double-quotes are stripped + assert_eq!(parse_literal("\"partial"), "\"partial"); + } + + #[test] + fn parse_literal_empty_quoted_string() { + assert_eq!(parse_literal("\"\""), ""); + } + + // ----------------------------------------------------------------------- + // Quoted string values in conditions + // ----------------------------------------------------------------------- + + #[test] + fn quoted_value_stripped_in_clause() { + let expr = parse_expression(r#"outcome="success""#).unwrap(); + assert_eq!( + expr, + ConditionExpr::Clause(Clause { + key: "outcome".to_string(), + op: Op::Eq, + value: "success".to_string(), + }) + ); + } + + #[test] + fn bare_and_quoted_values_produce_same_clause() { + let bare = parse_expression("outcome=success").unwrap(); + let quoted = parse_expression(r#"outcome="success""#).unwrap(); + assert_eq!(bare, quoted); + } + + #[test] + fn quoted_value_with_not_eq() { + let expr = parse_expression(r#"outcome!="fail""#).unwrap(); + assert_eq!( + expr, + ConditionExpr::Clause(Clause { + key: "outcome".to_string(), + op: Op::NotEq, + value: "fail".to_string(), + }) + ); + } + + #[test] + fn quoted_value_with_spaces() { + let expr = parse_expression(r#"context.msg="hello world""#).unwrap(); + assert_eq!( + expr, + ConditionExpr::Clause(Clause { + key: "context.msg".to_string(), + op: Op::Eq, + value: "hello world".to_string(), + }) + ); + } } diff --git a/lib/crates/fabro-workflows/src/condition.rs b/lib/crates/fabro-workflows/src/condition.rs index 13745f624..b79c2af79 100644 --- a/lib/crates/fabro-workflows/src/condition.rs +++ b/lib/crates/fabro-workflows/src/condition.rs @@ -579,4 +579,57 @@ mod tests { &context )); } + + // ----------------------------------------------------------------------- + // Phase 6: Quoted literal values (spec parse_literal) + // ----------------------------------------------------------------------- + + #[test] + fn quoted_value_matches_bare_value() { + let outcome = make_outcome(StageStatus::Success); + let context = Context::new(); + assert!(evaluate_condition( + r#"outcome="success""#, + &outcome, + &context + )); + assert!(!evaluate_condition(r#"outcome="fail""#, &outcome, &context)); + } + + #[test] + fn quoted_not_eq_matches() { + let outcome = make_outcome(StageStatus::Success); + let context = Context::new(); + assert!(evaluate_condition(r#"outcome!="fail""#, &outcome, &context)); + assert!(!evaluate_condition( + r#"outcome!="success""#, + &outcome, + &context + )); + } + + #[test] + fn quoted_context_value() { + let outcome = make_outcome(StageStatus::Success); + let context = Context::new(); + context.set("env", serde_json::json!("production")); + assert!(evaluate_condition( + r#"context.env="production""#, + &outcome, + &context + )); + } + + #[test] + fn quoted_and_bare_equivalent_in_compound() { + let outcome = make_outcome(StageStatus::Success); + let context = Context::new(); + context.set("ready", serde_json::json!("true")); + // Mix bare and quoted in a compound expression + assert!(evaluate_condition( + r#"outcome=success && context.ready="true""#, + &outcome, + &context + )); + } }