From 57dc4afe701065472b40515dd0880a60d8ebab0b Mon Sep 17 00:00:00 2001 From: Liam Date: Fri, 31 Jul 2026 23:38:31 -0400 Subject: [PATCH] Consolidate error types --- Cargo.lock | 11 +++++---- Cargo.toml | 1 + crates/val-wasm/src/lib.rs | 12 +++++----- src/arithmetic_error.rs | 20 ---------------- src/builtins.rs | 10 ++++---- src/error.rs | 48 +++++++++++++++++++++++++++++--------- src/evaluator.rs | 6 ++--- src/lib.rs | 20 ++++------------ src/number.rs | 42 ++++++++++++++++++--------------- src/value.rs | 25 ++++---------------- tests/integration.rs | 6 ++--- 11 files changed, 94 insertions(+), 107 deletions(-) delete mode 100644 src/arithmetic_error.rs diff --git a/Cargo.lock b/Cargo.lock index c0f3c0d..e4845da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -966,22 +966,22 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.17" +version = "2.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f63587ca0f12b72a0600bcba1d40081f830876000bb46dd2337a3051618f4fc8" +checksum = "09a43598840e33d5b0331f38c5e30d13bb11c11210a4b58f0d9b18a5a5eefcd9" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "2.0.17" +version = "2.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3ff15c8ecd7de3849db632e14d18d2571fa09dfc5ed93479bc4485c7a517c913" +checksum = "43cbfe0cf76104d42a574802844187e84a305e531ed54455f11fbde0f10541cd" dependencies = [ "proc-macro2", "quote", - "syn 2.0.117", + "syn 3.0.3", ] [[package]] @@ -1041,6 +1041,7 @@ dependencies = [ "rug", "rustyline", "tempfile", + "thiserror", "unindent", ] diff --git a/Cargo.toml b/Cargo.toml index ff2ab6f..43ec936 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ chumsky = { version = "0.13.0", features = ["pratt"] } clap = { version = "4.5.60", features = ["derive"] } gmp-mpfr-sys = { version = "~1.7", default-features = false, optional = true } rug = { version = "1.30.0", default-features = false, features = ["float", "rational", "std"] } +thiserror = "2.0.19" [target.'cfg(not(target_family = "wasm"))'.dependencies] dirs = "6.0.0" diff --git a/crates/val-wasm/src/lib.rs b/crates/val-wasm/src/lib.rs index 030e097..b7179ba 100644 --- a/crates/val-wasm/src/lib.rs +++ b/crates/val-wasm/src/lib.rs @@ -44,8 +44,8 @@ pub fn parse(input: &str) -> Result { .into_iter() .map(|error| ValError { kind: ErrorKind::Parser, - message: error.message, - range: converter.convert(Range::from(error.span)), + message: error.to_string(), + range: converter.convert(Range::from(error.span())), }) .collect::>(), ) @@ -71,8 +71,8 @@ pub fn evaluate(input: &str) -> Result { Err(error) => Err( to_value(&[ValError { kind: ErrorKind::Evaluator, - message: error.message, - range: converter.convert(Range::from(error.span)), + message: error.to_string(), + range: converter.convert(Range::from(error.span())), }]) .unwrap(), ), @@ -84,8 +84,8 @@ pub fn evaluate(input: &str) -> Result { .into_iter() .map(|error| ValError { kind: ErrorKind::Parser, - message: error.message, - range: converter.convert(Range::from(error.span)), + message: error.to_string(), + range: converter.convert(Range::from(error.span())), }) .collect::>(), ) diff --git a/src/arithmetic_error.rs b/src/arithmetic_error.rs deleted file mode 100644 index 51cdf9b..0000000 --- a/src/arithmetic_error.rs +++ /dev/null @@ -1,20 +0,0 @@ -use super::*; - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum ArithmeticError { - DivisionByZero, - ModuloByZero, - ZeroToNegativePower, -} - -impl Display for ArithmeticError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - f.write_str(match self { - Self::DivisionByZero => "Division by zero", - Self::ModuloByZero => "Modulo by zero", - Self::ZeroToNegativePower => "Zero cannot be raised to a negative power", - }) - } -} - -impl std::error::Error for ArithmeticError {} diff --git a/src/builtins.rs b/src/builtins.rs index 3ceef04..b37af6a 100644 --- a/src/builtins.rs +++ b/src/builtins.rs @@ -269,7 +269,7 @@ fn acsc<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { let reciprocal = Number::from(1_i64) .div(argument, payload.config) - .map_err(|error| Error::new(payload.span, error.to_string()))?; + .map_err(|error| error.with_span(payload.span))?; Ok(Value::Number(reciprocal.asin(payload.config))) } @@ -304,7 +304,7 @@ fn asec<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { let reciprocal = Number::from(1_i64) .div(argument, payload.config) - .map_err(|error| Error::new(payload.span, error.to_string()))?; + .map_err(|error| error.with_span(payload.span))?; Ok(Value::Number(reciprocal.acos(payload.config))) } @@ -403,7 +403,7 @@ fn cot<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { Number::from(1_i64) .div(&tan, payload.config) .map(Value::Number) - .map_err(|error| Error::new(payload.span, error.to_string())) + .map_err(|error| error.with_span(payload.span)) } fn csc<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { @@ -421,7 +421,7 @@ fn csc<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { Number::from(1_i64) .div(&sin, payload.config) .map(Value::Number) - .map_err(|error| Error::new(payload.span, error.to_string())) + .map_err(|error| error.with_span(payload.span)) } fn e<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { @@ -775,7 +775,7 @@ fn sec<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { Number::from(1_i64) .div(&cos, payload.config) .map(Value::Number) - .map_err(|error| Error::new(payload.span, error.to_string())) + .map_err(|error| error.with_span(payload.span)) } fn sin<'a>(payload: &BuiltinFunctionPayload<'a>) -> Result, Error> { diff --git a/src/error.rs b/src/error.rs index 96bf1d8..72317c4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,36 +1,62 @@ use super::*; -#[derive(Debug, PartialEq)] -pub struct Error { - pub message: String, - pub span: Span, +#[derive(Debug, PartialEq, thiserror::Error)] +pub enum Error { + #[error("division by zero")] + DivisionByZero, + #[error("invalid decimal")] + InvalidDecimal, + #[error("{0}")] + Message(String), + #[error("modulo by zero")] + ModuloByZero, + #[error("{error}")] + Spanned { error: Box, span: Span }, + #[error("zero cannot be raised to a negative power")] + ZeroToNegativePower, } impl Error { pub fn new(span: Span, message: impl Into) -> Self { - Self { - message: message.into(), - span, - } + Self::Message(message.into()).with_span(span) } #[must_use] pub fn report<'a>(&self, id: &'a str) -> Report<'a, (&'a str, Range)> { - let span_range = self.span.into_range(); + let span_range = self.span().into_range(); let mut report = Report::build( ReportKind::Custom("error", Color::Red), (id, span_range.clone()), ) .with_config(ariadne::Config::new().with_index_type(IndexType::Byte)) - .with_message(&self.message); + .with_message(self.to_string()); report = report.with_label( Label::new((id, span_range)) - .with_message(&self.message) + .with_message(self.to_string()) .with_color(Color::Red), ); report.finish() } + + #[must_use] + pub fn span(&self) -> Span { + match self { + Self::Spanned { span, .. } => *span, + _ => Span::from(0..0), + } + } + + #[must_use] + pub fn with_span(self, span: Span) -> Self { + match self { + Self::Spanned { error, .. } => Self::Spanned { error, span }, + error => Self::Spanned { + error: Box::new(error), + span, + }, + } + } } diff --git a/src/evaluator.rs b/src/evaluator.rs index 23102c5..494181d 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -174,7 +174,7 @@ impl<'a> Evaluator<'a> { lhs_num .div(rhs_num, self.environment.config) .map(Value::Number) - .map_err(|error| Error::new(rhs.1, error.to_string())) + .map_err(|error| error.with_span(rhs.1)) } Expression::BinaryOp(BinaryOp::Equal, lhs, rhs) => Ok(Value::Boolean( self.evaluate_expression(lhs)? == self.evaluate_expression(rhs)?, @@ -246,7 +246,7 @@ impl<'a> Evaluator<'a> { lhs_num .rem(rhs_num, self.environment.config) .map(Value::Number) - .map_err(|error| Error::new(rhs.1, error.to_string())) + .map_err(|error| error.with_span(rhs.1)) } Expression::BinaryOp(BinaryOp::Multiply, lhs, rhs) => Ok(Value::Number( self.evaluate_expression(lhs)?.number(lhs.1)?.mul( @@ -269,7 +269,7 @@ impl<'a> Evaluator<'a> { lhs_num .pow(rhs_num, self.environment.config) .map(Value::Number) - .map_err(|error| Error::new(rhs.1, error.to_string())) + .map_err(|error| error.with_span(rhs.1)) } Expression::BinaryOp(BinaryOp::Subtract, lhs, rhs) => Ok(Value::Number( self.evaluate_expression(lhs)?.number(lhs.1)?.sub( diff --git a/src/lib.rs b/src/lib.rs index b117ba9..bbdb9b0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,21 +28,12 @@ use { }; pub use crate::{ - arithmetic_error::ArithmeticError, - builtin::Builtin, - builtin_arity::BuiltinArity, + builtin::Builtin, builtin_arity::BuiltinArity, builtin_function::BuiltinFunction, - builtin_function_payload::BuiltinFunctionPayload, - completion::Completion, - config::Config, - environment::Environment, - error::Error, - evaluator::Evaluator, - function::Function, - number::{Number, ParseDecimalError}, - parser::parse, - rounding_mode::RoundingMode, - value::Value, + builtin_function_payload::BuiltinFunctionPayload, completion::Completion, + config::Config, environment::Environment, error::Error, evaluator::Evaluator, + function::Function, number::Number, parser::parse, + rounding_mode::RoundingMode, value::Value, }; pub type Span = SimpleSpan; @@ -50,7 +41,6 @@ pub type Spanned = (T, Span); type Result = std::result::Result; -mod arithmetic_error; pub mod ast; mod builtin; mod builtin_arity; diff --git a/src/number.rs b/src/number.rs index 9462e03..ce3c74f 100644 --- a/src/number.rs +++ b/src/number.rs @@ -1,8 +1,5 @@ use super::*; -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct ParseDecimalError; - #[derive(Clone, Debug)] pub enum Number { Approx(Float), @@ -120,14 +117,14 @@ impl Number { /// # Errors /// - /// Returns [`ArithmeticError::DivisionByZero`] if `rhs` is zero. + /// Returns [`Error::DivisionByZero`] if `rhs` is zero. pub fn div( &self, rhs: &Self, config: Config, - ) -> std::result::Result { + ) -> std::result::Result { if rhs.is_zero() { - Err(ArithmeticError::DivisionByZero) + Err(Error::DivisionByZero) } else if let (Self::Exact(lhs), Self::Exact(rhs)) = (self, rhs) { Ok(Self::Exact((lhs / rhs).complete())) } else { @@ -219,15 +216,15 @@ impl Number { /// # Errors /// - /// Returns [`ArithmeticError::ZeroToNegativePower`] if `self` is zero and + /// Returns [`Error::ZeroToNegativePower`] if `self` is zero and /// `rhs` is negative. pub fn pow( &self, rhs: &Self, config: Config, - ) -> std::result::Result { + ) -> std::result::Result { if self.is_zero() && rhs.is_negative() { - return Err(ArithmeticError::ZeroToNegativePower); + return Err(Error::ZeroToNegativePower); } match (self, rhs) { @@ -246,14 +243,14 @@ impl Number { /// # Errors /// - /// Returns [`ArithmeticError::ModuloByZero`] if `rhs` is zero. + /// Returns [`Error::ModuloByZero`] if `rhs` is zero. pub fn rem( &self, rhs: &Self, config: Config, - ) -> std::result::Result { + ) -> std::result::Result { if rhs.is_zero() { - return Err(ArithmeticError::ModuloByZero); + return Err(Error::ModuloByZero); } Ok(self.sub(&self.div(rhs, config)?.floor().mul(rhs, config), config)) @@ -445,7 +442,7 @@ impl PartialOrd for Number { } impl TryFrom<&str> for Number { - type Error = ParseDecimalError; + type Error = Error; fn try_from(s: &str) -> std::result::Result { let s = s.trim(); @@ -459,14 +456,14 @@ impl TryFrom<&str> for Number { let (integer, fraction) = s.split_once('.').unwrap_or((s, "")); if integer.is_empty() && fraction.is_empty() { - return Err(ParseDecimalError); + return Err(Error::InvalidDecimal); } let mut numerator = Integer::from(0); for b in integer.bytes().chain(fraction.bytes()) { if !b.is_ascii_digit() { - return Err(ParseDecimalError); + return Err(Error::InvalidDecimal); } numerator *= 10; @@ -478,7 +475,7 @@ impl TryFrom<&str> for Number { } let exponent = - u32::try_from(fraction.len()).map_err(|_| ParseDecimalError)?; + u32::try_from(fraction.len()).map_err(|_| Error::InvalidDecimal)?; Ok(Self::Exact(Rational::from(( numerator, @@ -563,6 +560,13 @@ mod tests { ); } + #[test] + fn invalid_decimal_returns_error() { + for value in [".", "foo"] { + assert_eq!(Number::try_from(value), Err(Error::InvalidDecimal)); + } + } + #[test] fn list_indexes_integer() { assert_eq!( @@ -607,17 +611,17 @@ mod tests { assert_eq!( Number::from(1_i64).div(&zero, Config::default()), - Err(ArithmeticError::DivisionByZero) + Err(Error::DivisionByZero) ); assert_eq!( Number::from(1_i64).rem(&zero, Config::default()), - Err(ArithmeticError::ModuloByZero) + Err(Error::ModuloByZero) ); assert_eq!( zero.pow(&Number::from(-1_i64), Config::default()), - Err(ArithmeticError::ZeroToNegativePower) + Err(Error::ZeroToNegativePower) ); } diff --git a/src/value.rs b/src/value.rs index f496853..d7efaa2 100644 --- a/src/value.rs +++ b/src/value.rs @@ -15,10 +15,7 @@ impl<'a> Value<'a> { if let Value::Boolean(x) = self { Ok(*x) } else { - Err(Error { - span, - message: format!("'{self}' is not a boolean"), - }) + Err(Error::new(span, format!("'{self}' is not a boolean"))) } } @@ -47,10 +44,7 @@ impl<'a> Value<'a> { pub(crate) fn into_list(self, span: Span) -> Result>, Error> { match self { Value::List(x) => Ok(x), - value => Err(Error { - span, - message: format!("'{value}' is not a list"), - }), + value => Err(Error::new(span, format!("'{value}' is not a list"))), } } @@ -58,10 +52,7 @@ impl<'a> Value<'a> { if let Value::List(x) = self { Ok(x) } else { - Err(Error { - span, - message: format!("'{self}' is not a list"), - }) + Err(Error::new(span, format!("'{self}' is not a list"))) } } @@ -69,10 +60,7 @@ impl<'a> Value<'a> { if let Value::Number(x) = self { Ok(x) } else { - Err(Error { - span, - message: format!("'{self}' is not a number"), - }) + Err(Error::new(span, format!("'{self}' is not a number"))) } } @@ -80,10 +68,7 @@ impl<'a> Value<'a> { if let Value::String(x) = self { Ok(x.as_ref()) } else { - Err(Error { - span, - message: format!("'{self}' is not a string"), - }) + Err(Error::new(span, format!("'{self}' is not a string"))) } } diff --git a/tests/integration.rs b/tests/integration.rs index 2eb69a7..ec4dd6c 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -981,7 +981,7 @@ fn division_by_zero() -> Result { Test::new()? .program("println(5 / 0)") .expected_status(1) - .expected_stderr(Contains("Division by zero")) + .expected_stderr(Contains("division by zero")) .run() } @@ -2477,7 +2477,7 @@ fn modulo_by_zero() -> Result { Test::new()? .program("println(5 % 0)") .expected_status(1) - .expected_stderr(Contains("Modulo by zero")) + .expected_stderr(Contains("modulo by zero")) .run() } @@ -2955,7 +2955,7 @@ fn power() -> Result { Test::new()? .program("println(0 ^ -1)") .expected_status(1) - .expected_stderr(Contains("Zero cannot be raised to a negative power")) + .expected_stderr(Contains("zero cannot be raised to a negative power")) .run() }