Require exactly 0/1 for bools and only-ascending-or-descending comparison chains in the math expression parser

libraries/math-parser/src/ast.rs

@@ -17,10 +17,10 @@ pub enum BinaryOp {
 	Add,
 	Sub,
 	Mul,
-	/// Logical AND (nonzero treated as true, returns 1. or 0.)
+	/// Logical AND over operands that must each be exactly 0 or 1, returning 0 or 1.
 	And,
 	Div,
-	/// Logical OR (nonzero treated as true, returns 1. or 0.)
+	/// Logical OR over operands that must each be exactly 0 or 1, returning 0 or 1.
 	Or,
 	Modulo,
 	Pow,
@@ -32,6 +32,16 @@ pub enum BinaryOp {
 	Eq,
 }
 
+impl BinaryOp {
+	/// Whether a chain of comparisons reads in one direction: `<`/`<=`/`==` ascending, `>`/`>=`/`==` descending, or `!=` alone.
+	pub fn chain_in_one_direction(ops: &[BinaryOp]) -> bool {
+		let ascending = ops.iter().all(|op| matches!(op, BinaryOp::Lt | BinaryOp::Leq | BinaryOp::Eq));
+		let descending = ops.iter().all(|op| matches!(op, BinaryOp::Gt | BinaryOp::Geq | BinaryOp::Eq));
+		let distinct = ops.iter().all(|op| matches!(op, BinaryOp::Neq));
+		ascending || descending || distinct
+	}
+}
+
 #[derive(Debug, PartialEq, Clone, Copy)]
 pub enum UnaryOp {
 	Pos,
@@ -46,8 +56,27 @@ pub enum UnaryOp {
 pub enum Node {
 	Lit(Literal),
 	Var(String),
-	FnCall { name: String, expr: Vec<Node> },
-	BinOp { lhs: Box<Node>, op: BinaryOp, rhs: Box<Node> },
-	UnaryOp { expr: Box<Node>, op: UnaryOp },
-	Conditional { condition: Box<Node>, if_block: Box<Node>, else_block: Box<Node> },
+	FnCall {
+		name: String,
+		expr: Vec<Node>,
+	},
+	BinOp {
+		lhs: Box<Node>,
+		op: BinaryOp,
+		rhs: Box<Node>,
+	},
+	UnaryOp {
+		expr: Box<Node>,
+		op: UnaryOp,
+	},
+	/// A chain of two or more comparisons like `a < b < c`, each operator paired with the operand after it: one predicate over each adjacent pair, or over every pair for `!=`.
+	Comparison {
+		first: Box<Node>,
+		rest: Vec<(BinaryOp, Node)>,
+	},
+	Conditional {
+		condition: Box<Node>,
+		if_block: Box<Node>,
+		else_block: Box<Node>,
+	},
 }

libraries/math-parser/src/executer.rs

@@ -1,4 +1,4 @@
-use crate::ast::{BinaryOp, Literal, Node};
+use crate::ast::{BinaryOp, Literal, Node, UnaryOp};
 use crate::constants::{builtin_function, suffixed_function};
 use crate::context::{EvalContext, FunctionProvider, ValueProvider};
 use crate::lexer::Constant;
@@ -19,6 +19,9 @@ pub enum EvalError {
 	#[error("Unsupported operand types for operator")]
 	OperatorTypeError,
 
+	#[error("Logic requires values of exactly 0 (false) or 1 (true)")]
+	NotATruthValue,
+
 	#[error("Indeterminate result, like `0/0` or `∞ - ∞`")]
 	Indeterminate,
 
@@ -71,11 +74,47 @@ impl Node {
 			},
 
 			Node::BinOp { lhs, op, rhs } => match (lhs.eval(context)?, rhs.eval(context)?) {
-				(Value::Number(lhs), Value::Number(rhs)) => settle(Value::Number(lhs.binary_op(*op, rhs).ok_or(EvalError::OperatorTypeError)?)),
+				(Value::Number(lhs), Value::Number(rhs)) => {
+					// Logic rejects operands that aren't truth values, while the other operators reject operand types they don't support
+					let rejected = if matches!(op, BinaryOp::And | BinaryOp::Or) {
+						EvalError::NotATruthValue
+					} else {
+						EvalError::OperatorTypeError
+					};
+					settle(Value::Number(lhs.binary_op(*op, rhs).ok_or(rejected)?))
+				}
 			},
 			Node::UnaryOp { expr, op } => match expr.eval(context)? {
-				Value::Number(num) => settle(Value::Number(num.unary_op(*op).ok_or(EvalError::OperatorTypeError)?)),
+				Value::Number(num) => {
+					let rejected = if *op == UnaryOp::Not { EvalError::NotATruthValue } else { EvalError::OperatorTypeError };
+					settle(Value::Number(num.unary_op(*op).ok_or(rejected)?))
+				}
 			},
+			Node::Comparison { first, rest } => {
+				let Value::Number(first) = first.eval(context)?;
+				let rest = rest
+					.iter()
+					.map(|(op, operand)| operand.eval(context).map(|Value::Number(number)| (*op, number)))
+					.collect::<Result<Vec<(BinaryOp, Number)>, EvalError>>()?;
+
+				// A `!=` chain asserts every pair distinct, while the ordered chains assert each adjacent pair's relation; every pair is checked so an unsupported comparison errors regardless of the others
+				let holds = if rest.iter().all(|(op, _)| *op == BinaryOp::Neq) {
+					let numbers: Vec<Number> = std::iter::once(first).chain(rest.iter().map(|(_, number)| *number)).collect();
+					numbers
+						.iter()
+						.enumerate()
+						.all(|(index, a)| numbers[index + 1..].iter().all(|b| a.binary_op(BinaryOp::Neq, *b) == Some(Number::Real(1.))))
+				} else {
+					let mut holds = true;
+					let mut previous = first;
+					for (op, number) in rest {
+						holds &= previous.binary_op(op, number).ok_or(EvalError::OperatorTypeError)? == Number::Real(1.);
+						previous = number;
+					}
+					holds
+				};
+				Ok(Value::from_f64(holds as u8 as f64))
+			}
 			Node::Var(name) => {
 				let value = resolve_value(context, name).ok_or_else(|| EvalError::MissingValue(name.clone()))?;
 				canonical_host_value(name, value)
@@ -120,7 +159,9 @@ impl Node {
 			}
 			Node::Conditional { condition, if_block, else_block } => {
 				let Value::Number(number) = condition.eval(context)?;
-				if number.as_bool() { if_block.eval(context) } else { else_block.eval(context) }
+				let Some(condition) = number.as_bool() else { return Err(EvalError::NotATruthValue) };
+
+				if condition { if_block.eval(context) } else { else_block.eval(context) }
 			}
 		}
 	}

libraries/math-parser/src/lib.rs

@@ -108,6 +108,23 @@ mod tests {
 		}
 	}
 
+	#[test]
+	fn comparison_chains_stay_in_one_direction() {
+		for input in ["1 < 2 > 1", "1 < 2 != 3", "1 == 2 != 2"] {
+			let error = evaluate(input).unwrap_err().to_string();
+			let expected = "A comparison chain must read in one direction: all ascending (`<`, `<=`, `==`), all descending (`>`, `>=`, `==`), or all `!=`";
+			assert_eq!(error, format!("{expected} at 0..{}", input.len()), "`{input}`");
+		}
+	}
+
+	#[test]
+	fn logic_requires_truth_values() {
+		// A logical operand must be exactly 0 or 1, so a general number becomes a truth value only through a comparison
+		for input in ["!5", "2 && 1", "0.5 || 0", "if(3, 1, 0)", "1 && i", "if(sqrt(-1), 1, 2)"] {
+			assert!(matches!(evaluate(input).unwrap(), Err(EvalError::NotATruthValue)), "expected `{input}` to need a truth value");
+		}
+	}
+
 	#[test]
 	fn indeterminate_forms_are_errors() {
 		// No operation returns NaN: an indeterminate form is an evaluation error, as is a domain failure with no complex answer
@@ -469,7 +486,7 @@ mod tests {
 		if_arithmetic_false: "if(3*2-5, 1, 0)" => 1.,
 
 		// Nested arithmetic
-		if_complex_arithmetic: "if((5+3)*(2-1), 10, 20)" => 10.,
+		if_complex_arithmetic: "if((5+3)*(2-1) > 0, 10, 20)" => 10.,
 		if_with_division: "if(8/4-2 == 0, 15, 25)" => 15.,
 		if_with_division_ne: "if(8/4-2 ≠ 0, 15, 25)" => 25.,
 
@@ -483,7 +500,7 @@ mod tests {
 
 		// Logical NOT (prefix !)
 		logical_not_zero: "!0" => 1.,
-		logical_not_nonzero: "!5" => 0.,
+		logical_not_one: "!1" => 0.,
 		logical_not_expression: "!(2 - 2)" => 1.,
 
 		// Log / exp / pow / root
@@ -591,7 +608,7 @@ mod tests {
 		if_zero: "if(0.0, 1, 2)" => 2.,
 
 		// Complex nested expressions
-		if_nested_expr: "if((sqrt(16) + 2) * (sin(pi) + 1), 3 + 4 * 2, 5 - 2 / 1)" => 11.,
+		if_nested_expr: "if((sqrt(16) + 2) * (sin(pi) + 1) > 5, 3 + 4 * 2, 5 - 2 / 1)" => 11.,
 
 		// Overflow-safe evaluation
 		factorial_overflows_to_infinity: "171!" => f64::INFINITY,
@@ -605,10 +622,9 @@ mod tests {
 		root_negative_odd_reciprocal: "root(-8, -3)" => -0.5,
 		root_negative_even: "root(-4, 2)" => Complex::new(0., 2.),
 
-		// Logic and equality span real and complex operands
+		// Equality spans real and complex operands
 		mixed_equality: "1 == i" => 0.,
 		complex_equality: "i == i" => 1.,
-		mixed_and: "1 && i" => 1.,
 
 		// Value identity: a zero imaginary part or a signed zero never changes a result, so a result landing on the real line is real
 		value_identity_product: "sqrt(-4) * sqrt(-4)" => -4.,
@@ -742,6 +758,15 @@ mod tests {
 		not_inside_magnitude: "|!0|" => 1.,
 		not_inside_magnitude_with_or: "| !0 || 0 |" => 1.,
 
+		// Ordered chains are one predicate over adjacent pairs, like interval notation, rather than `(a < b) < c`, while `!=` chains require every pair to differ
+		chain_interval: "0 <= 0.5 < 1" => 1.,
+		chain_fails_on_one_pair: "1 < 2 < 2" => 0.,
+		chain_not_c_style: "3 < 2 < 1" => 0.,
+		chain_descending: "3 > 2 >= 2" => 1.,
+		chain_equality: "1 == 1 == 1" => 1.,
+		chain_distinct: "1 != 2 != 3" => 1.,
+		chain_distinct_all_pairs: "1 != 2 != 1" => 0.,
+
 		// Correctly rounded literals via std parsing
 		seventeen_digit_literal: "999999999999999999" => 1e18,
 		long_fraction_literal: "0.1111111111111111111111111111111111111111" => 1. / 9.,
@@ -819,5 +844,9 @@ mod tests {
 		assert_eq!(evaluate("300").unwrap().unwrap().as_u8(), None);
 		assert_eq!(evaluate("i").unwrap().unwrap().as_i64(), None);
 		assert_eq!(evaluate("inf").unwrap().unwrap().as_u64(), None);
+
+		// A truth value is exactly 0 or 1
+		assert_eq!(evaluate("2 > 1").unwrap().unwrap().as_bool(), Some(true));
+		assert_eq!(evaluate("0.5").unwrap().unwrap().as_bool(), None);
 	}
 }

libraries/math-parser/src/parser.rs

@@ -1,6 +1,6 @@
 use crate::ast::{BinaryOp, Literal, Node, UnaryOp};
 use crate::lexer::{Lexer, Span, Token};
-use chumsky::error::LabelError;
+use chumsky::error::{EmptyErr, LabelError};
 use chumsky::input::ValueInput;
 use chumsky::{Parser, prelude::*};
 use std::fmt;
@@ -23,6 +23,23 @@ impl fmt::Display for ParseError {
 
 impl std::error::Error for ParseError {}
 
+/// Builds a parse error from a plain message, for a failure that no "expected ..., found ..." phrasing describes.
+pub trait CustomError {
+	fn custom(span: Span, message: &'static str) -> Self;
+}
+
+impl CustomError for EmptyErr {
+	fn custom(_: Span, _: &'static str) -> Self {
+		EmptyErr::default()
+	}
+}
+
+impl<'src> CustomError for Rich<'src, Token<'src>, Span> {
+	fn custom(span: Span, message: &'static str) -> Self {
+		Rich::custom(span, message)
+	}
+}
+
 impl Node {
 	pub fn try_parse_from_str(src: &str) -> Result<Node, ParseError> {
 		// Parse with zero-cost errors first (several times faster), then re-parse invalid input with rich errors to build the messages
@@ -41,7 +58,7 @@ pub fn parser<'src, I, E>() -> impl Parser<'src, I, Node, E>
 where
 	I: ValueInput<'src, Token = Token<'src>, Span = Span>,
 	E: extra::ParserExtra<'src, I>,
-	E::Error: LabelError<'src, I, &'static str>,
+	E::Error: LabelError<'src, I, &'static str> + CustomError,
 {
 	recursive(|expr| {
 		let constant = select! { Token::Float(f) => Node::Lit(Literal::Float(f)) };
@@ -130,11 +147,33 @@ where
 			rhs: Box::new(rhs),
 		});
 
-		let cmp = add.clone().foldl(cmp_op.then(add).repeated(), |lhs: Node, (op, rhs)| Node::BinOp {
-			lhs: Box::new(lhs),
-			op,
-			rhs: Box::new(rhs),
-		});
+		// A chain like `0 <= x < 1` is one predicate over its adjacent pairs, not an implicit `(0 <= x) < 1` (which is only read that way when its parentheses are written out), and must read in one direction
+		let cmp = add
+			.clone()
+			.then(cmp_op.then(add).repeated().collect::<Vec<_>>())
+			.try_map(|(first, mut rest): (Node, Vec<(BinaryOp, Node)>), span| {
+				// A lone comparison is an ordinary binary operation
+				if rest.len() <= 1 {
+					return Ok(match rest.pop() {
+						Some((op, second)) => Node::BinOp {
+							lhs: Box::new(first),
+							op,
+							rhs: Box::new(second),
+						},
+						None => first,
+					});
+				}
+
+				let ops: Vec<BinaryOp> = rest.iter().map(|(op, _)| *op).collect();
+				if !BinaryOp::chain_in_one_direction(&ops) {
+					return Err(CustomError::custom(
+						span,
+						"A comparison chain must read in one direction: all ascending (`<`, `<=`, `==`), all descending (`>`, `>=`, `==`), or all `!=`",
+					));
+				}
+
+				Ok(Node::Comparison { first: Box::new(first), rest })
+			});
 
 		let and = cmp.clone().foldl(and_op.then(cmp).repeated(), |lhs, (op, rhs)| Node::BinOp {
 			lhs: Box::new(lhs),

libraries/math-parser/src/value.rs

@@ -38,6 +38,12 @@ impl Value {
 		self.as_real().map(|real| real as f32)
 	}
 
+	/// Reads the value as a truth value, or `None` unless it is exactly 0 or 1.
+	pub fn as_bool(&self) -> Option<bool> {
+		let Self::Number(number) = self;
+		number.as_bool()
+	}
+
 	integer_accessors! {
 		as_u8: u8,
 		as_u16: u16,
@@ -96,11 +102,12 @@ impl Number {
 		}
 	}
 
-	/// The value's truthiness for conditions and logic operators, where any nonzero number is true.
-	pub fn as_bool(self) -> bool {
+	/// The truth value of a logical operand, which must be exactly 0 or 1: any other number is not a truth value, so logic on it is an error rather than a guess.
+	pub fn as_bool(self) -> Option<bool> {
 		match self {
-			Number::Real(real) => real != 0.,
-			Number::Complex(complex) => complex != Complex::ZERO,
+			Number::Real(0.) => Some(false),
+			Number::Real(1.) => Some(true),
+			_ => None,
 		}
 	}
 
@@ -126,7 +133,7 @@ impl Number {
 		// Logic and equality work uniformly across real and complex operands
 		match op {
 			BinaryOp::And | BinaryOp::Or => {
-				let (lhs, rhs) = (self.as_bool(), other.as_bool());
+				let (Some(lhs), Some(rhs)) = (self.as_bool(), other.as_bool()) else { return None };
 				let result = if matches!(op, BinaryOp::And) { lhs && rhs } else { lhs || rhs };
 				return Some(Number::Real(result as u8 as f64));
 			}
@@ -218,7 +225,7 @@ impl Number {
 				Number::Real(real) => Number::Real(-real),
 				Number::Complex(complex) => Number::Complex(-complex),
 			}),
-			UnaryOp::Not => Some(Number::Real(!self.as_bool() as u8 as f64)),
+			UnaryOp::Not => self.as_bool().map(|boolean| Number::Real(!boolean as u8 as f64)),
 			UnaryOp::Magnitude => Some(Number::Real(match self {
 				Number::Real(real) => real.abs(),
 				Number::Complex(complex) => complex.norm(),