From 5ee70bd769cab81c1c9909e9bb103596567d4eea Mon Sep 17 00:00:00 2001
From: Joscha <joscha@plugh.de>
Date: Mon, 6 Dec 2021 22:18:57 +0100
Subject: [PATCH] Implement proper errors for expression evaluation

... the hard way
---
 src/eval/command/formula.rs | 161 ++++++++++++++++++++++++++++++------
 src/files/commands.rs       |  50 ++++-------
 src/files/parse.rs          |  39 +++++----
 src/files/primitives.rs     |  11 ++-
 4 files changed, 186 insertions(+), 75 deletions(-)

diff --git a/src/eval/command/formula.rs b/src/eval/command/formula.rs
index 509b1fa..55c0daa 100644
--- a/src/eval/command/formula.rs
+++ b/src/eval/command/formula.rs
@@ -1,7 +1,7 @@
 use chrono::{Datelike, NaiveDate};
 
-use crate::files::commands::{self, Command, Expr, Var};
-use crate::files::primitives::{Spanned, Time, Weekday};
+use crate::files::commands::{self, Command};
+use crate::files::primitives::{Span, Spanned, Time, Weekday};
 
 use super::super::command::CommandState;
 use super::super::date::Dates;
@@ -20,18 +20,34 @@ fn i2b(i: i64) -> bool {
     i != 0
 }
 
+#[derive(Debug, Clone, Copy)]
+pub enum Var {
+    JulianDay,
+    Year,
+    YearLength,
+    YearDay,
+    YearDayReverse,
+    YearWeek,
+    YearWeekReverse,
+    Month,
+    MonthLength,
+    MonthWeek,
+    MonthWeekReverse,
+    Day,
+    DayReverse,
+    IsoYear,
+    IsoYearLength,
+    IsoWeek,
+    Weekday,
+    Easter(Span),
+    IsWeekday,
+    IsWeekend,
+    IsLeapYear,
+}
+
 impl Var {
     fn eval(self, date: NaiveDate) -> Result<i64> {
         Ok(match self {
-            Var::True => 1,
-            Var::False => 0,
-            Var::Monday => 1,
-            Var::Tuesday => 2,
-            Var::Wednesday => 3,
-            Var::Thursday => 4,
-            Var::Friday => 5,
-            Var::Saturday => 6,
-            Var::Sunday => 7,
             Var::JulianDay => date.num_days_from_ce().into(),
             Var::Year => date.year().into(),
             Var::YearLength => util::year_length(date.year()).into(),
@@ -63,9 +79,9 @@ impl Var {
                 let wd: Weekday = date.weekday().into();
                 wd.num().into()
             }
-            Var::Easter => {
+            Var::Easter(span) => {
                 let e = computus::gregorian(date.year()).map_err(|e| Error::Easter {
-                    span: todo!(),
+                    span,
                     date,
                     msg: e,
                 })?;
@@ -84,33 +100,123 @@ impl Var {
     }
 }
 
+#[derive(Debug)]
+pub enum Expr {
+    Lit(i64),
+    Var(Var),
+    Neg(Box<Expr>),
+    Add(Box<Expr>, Box<Expr>),
+    Sub(Box<Expr>, Box<Expr>),
+    Mul(Box<Expr>, Box<Expr>),
+    Div(Box<Expr>, Box<Expr>, Span),
+    Mod(Box<Expr>, Box<Expr>, Span),
+    Eq(Box<Expr>, Box<Expr>),
+    Neq(Box<Expr>, Box<Expr>),
+    Lt(Box<Expr>, Box<Expr>),
+    Lte(Box<Expr>, Box<Expr>),
+    Gt(Box<Expr>, Box<Expr>),
+    Gte(Box<Expr>, Box<Expr>),
+    Not(Box<Expr>),
+    And(Box<Expr>, Box<Expr>),
+    Or(Box<Expr>, Box<Expr>),
+    Xor(Box<Expr>, Box<Expr>),
+}
+
+impl From<&Spanned<commands::Expr>> for Expr {
+    fn from(expr: &Spanned<commands::Expr>) -> Self {
+        fn conv(expr: &Spanned<commands::Expr>) -> Box<Expr> {
+            Box::new(expr.into())
+        }
+
+        match &expr.value {
+            commands::Expr::Lit(l) => Self::Lit(*l),
+            commands::Expr::Var(v) => match v {
+                commands::Var::True => Self::Lit(1),
+                commands::Var::False => Self::Lit(0),
+                commands::Var::Monday => Self::Lit(1),
+                commands::Var::Tuesday => Self::Lit(2),
+                commands::Var::Wednesday => Self::Lit(3),
+                commands::Var::Thursday => Self::Lit(4),
+                commands::Var::Friday => Self::Lit(5),
+                commands::Var::Saturday => Self::Lit(6),
+                commands::Var::Sunday => Self::Lit(7),
+                commands::Var::JulianDay => Self::Var(Var::JulianDay),
+                commands::Var::Year => Self::Var(Var::Year),
+                commands::Var::YearLength => Self::Var(Var::YearLength),
+                commands::Var::YearDay => Self::Var(Var::YearDay),
+                commands::Var::YearDayReverse => Self::Var(Var::YearDayReverse),
+                commands::Var::YearWeek => Self::Var(Var::YearWeek),
+                commands::Var::YearWeekReverse => Self::Var(Var::YearWeekReverse),
+                commands::Var::Month => Self::Var(Var::Month),
+                commands::Var::MonthLength => Self::Var(Var::MonthLength),
+                commands::Var::MonthWeek => Self::Var(Var::MonthWeek),
+                commands::Var::MonthWeekReverse => Self::Var(Var::MonthWeekReverse),
+                commands::Var::Day => Self::Var(Var::Day),
+                commands::Var::DayReverse => Self::Var(Var::DayReverse),
+                commands::Var::IsoYear => Self::Var(Var::IsoYear),
+                commands::Var::IsoYearLength => Self::Var(Var::IsoYearLength),
+                commands::Var::IsoWeek => Self::Var(Var::IsoWeek),
+                commands::Var::Weekday => Self::Var(Var::Weekday),
+                commands::Var::Easter => Self::Var(Var::Easter(expr.span)),
+                commands::Var::IsWeekday => Self::Var(Var::IsWeekday),
+                commands::Var::IsWeekend => Self::Var(Var::IsWeekend),
+                commands::Var::IsLeapYear => Self::Var(Var::IsLeapYear),
+            },
+            commands::Expr::Paren(i) => i.as_ref().into(),
+            commands::Expr::Neg(i) => Self::Neg(conv(i)),
+            commands::Expr::Add(a, b) => Self::Add(conv(a), conv(b)),
+            commands::Expr::Sub(a, b) => Self::Sub(conv(a), conv(b)),
+            commands::Expr::Mul(a, b) => Self::Mul(conv(a), conv(b)),
+            commands::Expr::Div(a, b) => Self::Div(conv(a), conv(b), expr.span),
+            commands::Expr::Mod(a, b) => Self::Mod(conv(a), conv(b), expr.span),
+            commands::Expr::Eq(a, b) => Self::Eq(conv(a), conv(b)),
+            commands::Expr::Neq(a, b) => Self::Neq(conv(a), conv(b)),
+            commands::Expr::Lt(a, b) => Self::Lt(conv(a), conv(b)),
+            commands::Expr::Lte(a, b) => Self::Lte(conv(a), conv(b)),
+            commands::Expr::Gt(a, b) => Self::Gt(conv(a), conv(b)),
+            commands::Expr::Gte(a, b) => Self::Gte(conv(a), conv(b)),
+            commands::Expr::Not(i) => Self::Not(conv(i)),
+            commands::Expr::And(a, b) => Self::And(conv(a), conv(b)),
+            commands::Expr::Or(a, b) => Self::Or(conv(a), conv(b)),
+            commands::Expr::Xor(a, b) => Self::Xor(conv(a), conv(b)),
+        }
+    }
+}
+
+impl From<Weekday> for Expr {
+    fn from(wd: Weekday) -> Self {
+        match wd {
+            Weekday::Monday => Self::Lit(1),
+            Weekday::Tuesday => Self::Lit(2),
+            Weekday::Wednesday => Self::Lit(3),
+            Weekday::Thursday => Self::Lit(4),
+            Weekday::Friday => Self::Lit(5),
+            Weekday::Saturday => Self::Lit(6),
+            Weekday::Sunday => Self::Lit(7),
+        }
+    }
+}
+
 impl Expr {
     fn eval(&self, date: NaiveDate) -> Result<i64> {
         Ok(match self {
             Expr::Lit(l) => *l,
             Expr::Var(v) => v.eval(date)?,
-            Expr::Paren(e) => e.eval(date)?,
             Expr::Neg(e) => -e.eval(date)?,
             Expr::Add(a, b) => a.eval(date)? + b.eval(date)?,
             Expr::Sub(a, b) => a.eval(date)? - b.eval(date)?,
             Expr::Mul(a, b) => a.eval(date)? * b.eval(date)?,
-            Expr::Div(a, b) => {
+            Expr::Div(a, b, span) => {
                 let b = b.eval(date)?;
                 if b == 0 {
-                    return Err(Error::DivByZero {
-                        span: todo!(),
-                        date,
-                    });
+                    return Err(Error::DivByZero { span: *span, date });
                 }
                 a.eval(date)?.div_euclid(b)
             }
-            Expr::Mod(a, b) => {
+            Expr::Mod(a, b, span) => {
                 let b = b.eval(date)?;
                 if b == 0 {
-                    return Err(Error::ModByZero {
-                        span: todo!(),
-                        date,
-                    });
+                    return Err(Error::ModByZero { span: *span, date });
                 }
                 a.eval(date)?.rem_euclid(b)
             }
@@ -137,7 +243,10 @@ pub struct FormulaSpec {
 
 impl From<&commands::FormulaSpec> for FormulaSpec {
     fn from(spec: &commands::FormulaSpec) -> Self {
-        let start: Expr = spec.start.as_ref().cloned().unwrap_or(Expr::Lit(1));
+        let start: Expr = match &spec.start {
+            Some(expr) => expr.into(),
+            None => Expr::Lit(1), // Always true
+        };
 
         let start_delta: Delta = spec
             .start_delta
@@ -169,7 +278,7 @@ impl From<&commands::WeekdaySpec> for FormulaSpec {
     fn from(spec: &commands::WeekdaySpec) -> Self {
         let start = Expr::Eq(
             Box::new(Expr::Var(Var::Weekday)),
-            Box::new(Expr::Var(spec.start.into())),
+            Box::new(spec.start.into()),
         );
 
         let mut end_delta = Delta::default();
diff --git a/src/files/commands.rs b/src/files/commands.rs
index 07e1d9b..dbdf755 100644
--- a/src/files/commands.rs
+++ b/src/files/commands.rs
@@ -201,49 +201,35 @@ impl Var {
     }
 }
 
-impl From<Weekday> for Var {
-    fn from(wd: Weekday) -> Self {
-        match wd {
-            Weekday::Monday => Self::Monday,
-            Weekday::Tuesday => Self::Tuesday,
-            Weekday::Wednesday => Self::Wednesday,
-            Weekday::Thursday => Self::Thursday,
-            Weekday::Friday => Self::Friday,
-            Weekday::Saturday => Self::Saturday,
-            Weekday::Sunday => Self::Sunday,
-        }
-    }
-}
-
 #[derive(Debug, Clone)]
 pub enum Expr {
     Lit(i64),
     Var(Var),
-    Paren(Box<Expr>),
+    Paren(Box<Spanned<Expr>>),
     // Integer-y operations
-    Neg(Box<Expr>),
-    Add(Box<Expr>, Box<Expr>),
-    Sub(Box<Expr>, Box<Expr>),
-    Mul(Box<Expr>, Box<Expr>),
-    Div(Box<Expr>, Box<Expr>),
-    Mod(Box<Expr>, Box<Expr>),
+    Neg(Box<Spanned<Expr>>),
+    Add(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Sub(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Mul(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Div(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Mod(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
     // Comparisons
-    Eq(Box<Expr>, Box<Expr>),
-    Neq(Box<Expr>, Box<Expr>),
-    Lt(Box<Expr>, Box<Expr>),
-    Lte(Box<Expr>, Box<Expr>),
-    Gt(Box<Expr>, Box<Expr>),
-    Gte(Box<Expr>, Box<Expr>),
+    Eq(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Neq(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Lt(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Lte(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Gt(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Gte(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
     // Boolean-y operations
-    Not(Box<Expr>),
-    And(Box<Expr>, Box<Expr>),
-    Or(Box<Expr>, Box<Expr>),
-    Xor(Box<Expr>, Box<Expr>),
+    Not(Box<Spanned<Expr>>),
+    And(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Or(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
+    Xor(Box<Spanned<Expr>>, Box<Spanned<Expr>>),
 }
 
 #[derive(Debug)]
 pub struct FormulaSpec {
-    pub start: Option<Expr>, // None: *
+    pub start: Option<Spanned<Expr>>, // None: *
     pub start_delta: Option<Delta>,
     pub start_time: Option<Time>,
     pub end_delta: Option<Delta>,
diff --git a/src/files/parse.rs b/src/files/parse.rs
index e3d1979..50fd490 100644
--- a/src/files/parse.rs
+++ b/src/files/parse.rs
@@ -350,42 +350,48 @@ fn parse_variable(p: Pair<'_, Rule>) -> Var {
     }
 }
 
-fn parse_unop_expr(p: Pair<'_, Rule>) -> Expr {
+fn parse_unop_expr(p: Pair<'_, Rule>) -> Spanned<Expr> {
     assert_eq!(p.as_rule(), Rule::unop_expr);
+    let span = (&p.as_span()).into();
+
     let mut p = p.into_inner();
     let p_op = p.next().unwrap();
     let p_expr = p.next().unwrap();
     assert_eq!(p.next(), None);
 
-    let expr = parse_expr(p_expr);
-    match p_op.as_rule() {
-        Rule::unop_neg => Expr::Neg(Box::new(expr)),
-        Rule::unop_not => Expr::Not(Box::new(expr)),
+    let inner = parse_expr(p_expr);
+    let expr = match p_op.as_rule() {
+        Rule::unop_neg => Expr::Neg(Box::new(inner)),
+        Rule::unop_not => Expr::Not(Box::new(inner)),
         _ => unreachable!(),
-    }
+    };
+    Spanned::new(span, expr)
 }
 
-fn parse_paren_expr(p: Pair<'_, Rule>) -> Expr {
+fn parse_paren_expr(p: Pair<'_, Rule>) -> Spanned<Expr> {
     assert_eq!(p.as_rule(), Rule::paren_expr);
+    let span = (&p.as_span()).into();
     let inner = parse_expr(p.into_inner().next().unwrap());
-    Expr::Paren(Box::new(inner))
+    Spanned::new(span, Expr::Paren(Box::new(inner)))
 }
 
-fn parse_term(p: Pair<'_, Rule>) -> Expr {
+fn parse_term(p: Pair<'_, Rule>) -> Spanned<Expr> {
     assert_eq!(p.as_rule(), Rule::term);
+    let span = (&p.as_span()).into();
     let p = p.into_inner().next().unwrap();
     match p.as_rule() {
-        Rule::number => Expr::Lit(parse_number(p).into()),
-        Rule::boolean => Expr::Var(parse_boolean(p)),
-        Rule::variable => Expr::Var(parse_variable(p)),
+        Rule::number => Spanned::new(span, Expr::Lit(parse_number(p).into())),
+        Rule::boolean => Spanned::new(span, Expr::Var(parse_boolean(p))),
+        Rule::variable => Spanned::new(span, Expr::Var(parse_variable(p))),
         Rule::unop_expr => parse_unop_expr(p),
         Rule::paren_expr => parse_paren_expr(p),
         _ => unreachable!(),
     }
 }
 
-fn parse_op(l: Expr, p: Pair<'_, Rule>, r: Expr) -> Expr {
-    match p.as_rule() {
+fn parse_op(l: Spanned<Expr>, p: Pair<'_, Rule>, r: Spanned<Expr>) -> Spanned<Expr> {
+    let span = l.span.join(r.span);
+    let expr = match p.as_rule() {
         // Integer-y operations
         Rule::op_add => Expr::Add(Box::new(l), Box::new(r)),
         Rule::op_sub => Expr::Sub(Box::new(l), Box::new(r)),
@@ -407,10 +413,11 @@ fn parse_op(l: Expr, p: Pair<'_, Rule>, r: Expr) -> Expr {
         Rule::op_xor => Expr::Xor(Box::new(l), Box::new(r)),
 
         _ => unreachable!(),
-    }
+    };
+    Spanned::new(span, expr)
 }
 
-fn parse_expr(p: Pair<'_, Rule>) -> Expr {
+fn parse_expr(p: Pair<'_, Rule>) -> Spanned<Expr> {
     assert_eq!(p.as_rule(), Rule::expr);
 
     fn op(rule: Rule) -> Operator<Rule> {
diff --git a/src/files/primitives.rs b/src/files/primitives.rs
index 2444e75..d498122 100644
--- a/src/files/primitives.rs
+++ b/src/files/primitives.rs
@@ -1,4 +1,4 @@
-use std::cmp::Ordering;
+use std::cmp::{self, Ordering};
 use std::fmt;
 
 #[derive(Debug, Clone, Copy)]
@@ -16,6 +16,15 @@ impl<'a> From<&pest::Span<'a>> for Span {
     }
 }
 
+impl Span {
+    pub fn join(self, other: Self) -> Self {
+        Self {
+            start: cmp::min(self.start, other.start),
+            end: cmp::max(self.end, other.end),
+        }
+    }
+}
+
 #[derive(Clone, Copy)]
 pub struct Spanned<T> {
     pub span: Span,