refactor: replace BinOp string with typed enum in Expr AST

Previously Expr::BinOp(String, ...) accepted any string as an operator.
Invalid operators (e.g. "diagonal") would compile fine and silently
return CellValue::Empty at eval time.

Now BinOp is an enum with variants Add/Sub/Mul/Div/Pow/Eq/Ne/Lt/Gt/Le/Ge.
The parser produces enum variants directly; the evaluator pattern-matches
exhaustively with no fallback branch. An invalid operator is now a
compile error at the call site, and the compiler ensures every variant
is handled in both eval_expr and eval_bool.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Ed L
2026-03-24 00:20:08 -07:00
parent 5434a60cc4
commit 599f1adcbd
4 changed files with 60 additions and 34 deletions

View File

@ -9,6 +9,25 @@ pub enum AggFunc {
Count,
}
/// Arithmetic and comparison operators used in binary expressions.
/// Having an enum (rather than a raw String) means the parser must
/// produce a valid operator; invalid operators are caught at parse
/// time rather than silently returning Empty at eval time.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub enum BinOp {
Add,
Sub,
Mul,
Div,
Pow,
Eq,
Ne,
Lt,
Gt,
Le,
Ge,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Filter {
pub category: String,
@ -19,7 +38,7 @@ pub struct Filter {
pub enum Expr {
Number(f64),
Ref(String),
BinOp(String, Box<Expr>, Box<Expr>),
BinOp(BinOp, Box<Expr>, Box<Expr>),
UnaryMinus(Box<Expr>),
Agg(AggFunc, Box<Expr>, Option<Filter>),
If(Box<Expr>, Box<Expr>, Box<Expr>),

View File

@ -1,5 +1,5 @@
pub mod parser;
pub mod ast;
pub use ast::{AggFunc, Expr, Formula};
pub use ast::{AggFunc, BinOp, Expr, Formula};
pub use parser::parse_formula;

View File

@ -1,6 +1,6 @@
use anyhow::{anyhow, Result};
use super::ast::{AggFunc, Expr, Filter, Formula};
use super::ast::{AggFunc, BinOp, Expr, Filter, Formula};
/// Parse a formula string like "Profit = Revenue - Cost"
/// or "Tax = Revenue * 0.08 WHERE Region = \"East\""
@ -161,13 +161,13 @@ fn parse_add_sub(tokens: &[Token], pos: &mut usize) -> Result<Expr> {
let mut left = parse_mul_div(tokens, pos)?;
while *pos < tokens.len() {
let op = match &tokens[*pos] {
Token::Plus => "+",
Token::Minus => "-",
Token::Plus => BinOp::Add,
Token::Minus => BinOp::Sub,
_ => break,
};
*pos += 1;
let right = parse_mul_div(tokens, pos)?;
left = Expr::BinOp(op.to_string(), Box::new(left), Box::new(right));
left = Expr::BinOp(op, Box::new(left), Box::new(right));
}
Ok(left)
}
@ -176,13 +176,13 @@ fn parse_mul_div(tokens: &[Token], pos: &mut usize) -> Result<Expr> {
let mut left = parse_pow(tokens, pos)?;
while *pos < tokens.len() {
let op = match &tokens[*pos] {
Token::Star => "*",
Token::Slash => "/",
Token::Star => BinOp::Mul,
Token::Slash => BinOp::Div,
_ => break,
};
*pos += 1;
let right = parse_pow(tokens, pos)?;
left = Expr::BinOp(op.to_string(), Box::new(left), Box::new(right));
left = Expr::BinOp(op, Box::new(left), Box::new(right));
}
Ok(left)
}
@ -192,7 +192,7 @@ fn parse_pow(tokens: &[Token], pos: &mut usize) -> Result<Expr> {
if *pos < tokens.len() && tokens[*pos] == Token::Caret {
*pos += 1;
let exp = parse_unary(tokens, pos)?;
return Ok(Expr::BinOp("^".to_string(), Box::new(base), Box::new(exp)));
return Ok(Expr::BinOp(BinOp::Pow, Box::new(base), Box::new(exp)));
}
Ok(base)
}
@ -298,30 +298,30 @@ fn parse_comparison(tokens: &[Token], pos: &mut usize) -> Result<Expr> {
let left = parse_add_sub(tokens, pos)?;
if *pos >= tokens.len() { return Ok(left); }
let op = match &tokens[*pos] {
Token::Eq => "=",
Token::Ne => "!=",
Token::Lt => "<",
Token::Gt => ">",
Token::Le => "<=",
Token::Ge => ">=",
Token::Eq => BinOp::Eq,
Token::Ne => BinOp::Ne,
Token::Lt => BinOp::Lt,
Token::Gt => BinOp::Gt,
Token::Le => BinOp::Le,
Token::Ge => BinOp::Ge,
_ => return Ok(left),
};
*pos += 1;
let right = parse_add_sub(tokens, pos)?;
Ok(Expr::BinOp(op.to_string(), Box::new(left), Box::new(right)))
Ok(Expr::BinOp(op, Box::new(left), Box::new(right)))
}
#[cfg(test)]
mod tests {
use super::parse_formula;
use crate::formula::{Expr, AggFunc};
use crate::formula::{AggFunc, BinOp, Expr};
#[test]
fn parse_simple_subtraction() {
let f = parse_formula("Profit = Revenue - Cost", "Measure").unwrap();
assert_eq!(f.target, "Profit");
assert_eq!(f.target_category, "Measure");
assert!(matches!(f.expr, Expr::BinOp(ref op, _, _) if op == "-"));
assert!(matches!(f.expr, Expr::BinOp(BinOp::Sub, _, _)));
}
#[test]