Cleanup for binary operators, especially around `null`

This cleans up a lot of logic when dealing with binary operators:

 * `==` now deals with boolean types, not just numbers and strings.
 * `==` now evaluates to `false` if the types of the operands don't match, including `null`.
 * `!=` is implemented in terms of `==`.
 * The right hand operand of `&&` and `||` operators is evaluated conditionally, matching how JS implementation works.
 * `null` is now falsey (was truthy!)
This commit is contained in:
jhugman 2022-04-19 15:34:56 +00:00 коммит произвёл GitHub
Родитель cc3706294f 6eb0810aa3
Коммит e3f3e0e704
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
1 изменённых файлов: 237 добавлений и 10 удалений

Просмотреть файл

@ -57,7 +57,7 @@ impl Truthy for Value {
fn is_truthy(&self) -> bool {
match self {
Value::Bool(b) => *b,
Value::Null => true,
Value::Null => false,
Value::Number(f) => f.as_f64().unwrap() != 0.0,
Value::String(s) => !s.is_empty(),
// It would be better if these depended on the contents of the
@ -210,11 +210,7 @@ impl<'a> Evaluator<'a> {
left,
right,
operation,
} => {
let left = self.eval_ast(*left, context)?;
let right = self.eval_ast(*right, context)?;
Self::apply_op(operation, left, right)
}
} => self.eval_op(operation, left, right, context),
Expression::Transform {
name,
subject,
@ -259,8 +255,47 @@ impl<'a> Evaluator<'a> {
}
}
fn eval_op<'b>(
&self,
operation: OpCode,
left: Box<Expression>,
right: Box<Expression>,
context: &Context,
) -> Result<'b, Value> {
let left = self.eval_ast(*left, context)?;
// We want to delay evaluating the right hand side in the cases of AND and OR.
let eval_right = || self.eval_ast(*right, context);
Ok(match operation {
OpCode::Or => {
if left.is_truthy() {
left
} else {
eval_right()?
}
}
OpCode::And => {
if left.is_truthy() {
eval_right()?
} else {
left
}
}
_ => Self::apply_op(operation, left, eval_right()?)?,
})
}
fn apply_op<'b>(operation: OpCode, left: Value, right: Value) -> Result<'b, Value> {
match (operation, left, right) {
(OpCode::NotEqual, a, b) => {
// Implement NotEquals as the inverse of Equals.
let value = Self::apply_op(OpCode::Equal, a, b)?;
let equality = value
.as_bool()
.unwrap_or_else(|| unreachable!("Equality always returns a bool"));
Ok(value!(!equality))
}
(OpCode::And, a, b) => Ok(if a.is_truthy() { b } else { a }),
(OpCode::Or, a, b) => Ok(if a.is_truthy() { a } else { b }),
@ -280,7 +315,7 @@ impl<'a> Evaluator<'a> {
OpCode::LessEqual => value!(left <= right),
OpCode::GreaterEqual => value!(left >= right),
OpCode::Equal => value!((left - right).abs() < EPSILON),
OpCode::NotEqual => value!((left - right).abs() > EPSILON),
OpCode::NotEqual => value!((left - right).abs() >= EPSILON),
OpCode::In => value!(false),
OpCode::And | OpCode::Or => {
unreachable!("Covered by previous case in parent match")
@ -288,10 +323,37 @@ impl<'a> Evaluator<'a> {
})
}
(OpCode::Add, Value::String(a), Value::String(b)) => Ok(value!(format!("{}{}", a, b))),
(OpCode::In, Value::String(a), Value::String(b)) => Ok(value!(b.contains(&a))),
(op, Value::String(a), Value::String(b)) => match op {
OpCode::Equal => Ok(value!(a == b)),
OpCode::Add => Ok(value!(format!("{}{}", a, b))),
OpCode::In => Ok(value!(b.contains(&a))),
OpCode::Less => Ok(value!(a < b)),
OpCode::Greater => Ok(value!(a > b)),
OpCode::LessEqual => Ok(value!(a <= b)),
OpCode::GreaterEqual => Ok(value!(a >= b)),
_ => Err(EvaluationError::InvalidBinaryOp {
operation,
left: value!(a),
right: value!(b),
}),
},
(OpCode::In, left, Value::Array(v)) => Ok(value!(v.contains(&left))),
(OpCode::Equal, Value::String(a), Value::String(b)) => Ok(value!(a == b)),
(OpCode::Equal, a, b) => match (a, b) {
// Number == Number is handled above
// String == String is handled above
(Value::Bool(a), Value::Bool(b)) => Ok(value!(a == b)),
(Value::Null, Value::Null) => Ok(value!(true)),
(Value::Array(a), Value::Array(b)) => Ok(value!(a == b)),
(Value::Object(a), Value::Object(b)) => Ok(value!(a == b)),
// If the types don't match, it's always false
_ => Ok(value!(false)),
},
(operation, left, right) => Err(EvaluationError::InvalidBinaryOp {
operation,
left,
@ -651,4 +713,169 @@ mod tests {
value!([{"bobo": 50, "fofo": 100}, {"bobo": 60, "baz": 90}])
);
}
#[test]
fn test_binary_op_eq_ne() {
let evaluator = Evaluator::new();
let context = value!({
"NULL": null,
"STRING": "string",
"BOOLEAN": true,
"NUMBER": 42,
"OBJECT": { "x": 1, "y": 2 },
"ARRAY": [ "string" ]
});
let test = |l: &str, r: &str, exp: bool| {
let expr = format!("{} == {}", l, r);
assert_eq!(
evaluator.eval_in_context(&expr, context.clone()).unwrap(),
value!(exp)
);
let expr = format!("{} != {}", l, r);
assert_eq!(
evaluator.eval_in_context(&expr, context.clone()).unwrap(),
value!(!exp)
);
};
test("STRING", "'string'", true);
test("NUMBER", "42", true);
test("BOOLEAN", "true", true);
test("NULL", "null", true);
test("OBJECT", "OBJECT", true);
test("ARRAY", "[ 'string' ]", true);
test("OBJECT", "{ 'x': 1, 'y': 2 }", false);
test("STRING", "NULL", false);
test("NUMBER", "NULL", false);
test("BOOLEAN", "NULL", false);
// test("NULL", "NULL", false);
test("OBJECT", "NULL", false);
test("ARRAY", "NULL", false);
// test("STRING", "STRING", false);
test("NUMBER", "STRING", false);
test("BOOLEAN", "STRING", false);
test("NULL", "STRING", false);
test("OBJECT", "STRING", false);
test("ARRAY", "STRING", false);
test("STRING", "NUMBER", false);
// test("NUMBER", "NUMBER", false);
test("BOOLEAN", "NUMBER", false);
test("NULL", "NUMBER", false);
test("OBJECT", "NUMBER", false);
test("ARRAY", "NUMBER", false);
test("STRING", "BOOLEAN", false);
test("NUMBER", "BOOLEAN", false);
// test("BOOLEAN", "BOOLEAN", false);
test("NULL", "BOOLEAN", false);
test("OBJECT", "BOOLEAN", false);
test("ARRAY", "BOOLEAN", false);
test("STRING", "OBJECT", false);
test("NUMBER", "OBJECT", false);
test("BOOLEAN", "OBJECT", false);
test("NULL", "OBJECT", false);
// test("OBJECT", "OBJECT", false);
test("ARRAY", "OBJECT", false);
test("STRING", "ARRAY", false);
test("NUMBER", "ARRAY", false);
test("BOOLEAN", "ARRAY", false);
test("NULL", "ARRAY", false);
test("OBJECT", "ARRAY", false);
// test("ARRAY", "ARRAY", false);
}
#[test]
fn test_binary_op_string_gt_lt_gte_lte() {
let evaluator = Evaluator::new();
let context = value!({
"A": "A string",
"B": "B string",
});
let test = |l: &str, r: &str, is_gt: bool| {
let expr = format!("{} > {}", l, r);
assert_eq!(
evaluator.eval_in_context(&expr, context.clone()).unwrap(),
value!(is_gt)
);
let expr = format!("{} <= {}", l, r);
assert_eq!(
evaluator.eval_in_context(&expr, context.clone()).unwrap(),
value!(!is_gt)
);
// we test equality in another test
let expr = format!("{} == {}", l, r);
let is_eq = evaluator
.eval_in_context(&expr, context.clone())
.unwrap()
.as_bool()
.unwrap();
if is_eq {
let expr = format!("{} >= {}", l, r);
assert_eq!(
evaluator.eval_in_context(&expr, context.clone()).unwrap(),
value!(true)
);
} else {
let expr = format!("{} < {}", l, r);
assert_eq!(
evaluator.eval_in_context(&expr, context.clone()).unwrap(),
value!(!is_gt)
);
}
};
test("A", "B", false);
test("B", "A", true);
test("A", "A", false);
}
#[test]
fn test_lazy_eval_binary_op_and_or() {
let evaluator = Evaluator::new();
// error is a missing transform
let res = evaluator.eval("42 || 0|error");
assert!(res.is_ok());
assert_eq!(res.unwrap(), value!(42.0));
let res = evaluator.eval("false || 0|error");
assert!(res.is_err());
let res = evaluator.eval("42 && 0|error");
assert!(res.is_err());
let res = evaluator.eval("false && 0|error");
assert!(res.is_ok());
assert_eq!(res.unwrap(), value!(false));
}
#[test]
fn test_lazy_eval_trinary_op() {
let evaluator = Evaluator::new();
// error is a missing transform
let res = evaluator.eval("true ? 42 : 0|error");
assert!(res.is_ok());
assert_eq!(res.unwrap(), value!(42.0));
let res = evaluator.eval("true ? 0|error : 42");
assert!(res.is_err());
let res = evaluator.eval("true ? 0|error : 42");
assert!(res.is_err());
let res = evaluator.eval("false ? 0|error : 42");
assert!(res.is_ok());
assert_eq!(res.unwrap(), value!(42.0));
}
}