Skip to content

Commit 29455ba

Browse files
committed
feat: robustness improvment
1 parent 71ec268 commit 29455ba

4 files changed

Lines changed: 94 additions & 37 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "exprimo"
3-
version = "0.6.1"
3+
version = "0.7.0"
44
edition = "2021"
55
license = "MIT"
66
authors = ["joshua.tracey08@gmail.com"]

src/lib.rs

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -279,18 +279,12 @@ impl Evaluator {
279279
match (left.clone(), right.clone()) {
280280
(Value::Number(l), Value::Number(r)) => {
281281
let sum = l.as_f64().unwrap() + r.as_f64().unwrap();
282-
Ok(Value::Number(serde_json::Number::from_f64(sum).unwrap()))
282+
Ok(self.f64_to_value(sum))
283283
}
284284
(Value::String(l), Value::String(r)) => Ok(Value::String(l + &r)),
285285
(Value::String(l), r) => Ok(Value::String(l + &self.value_to_string(&r))),
286286
(l, Value::String(r)) => Ok(Value::String(self.value_to_string(&l) + &r)),
287287
_ => {
288-
// Type coercion similar to JavaScript
289-
// This branch might need to use to_number if we want it to behave like JS '+' with mixed types that coerce to number first.
290-
// However, current implementation coerces to string.
291-
// If numeric conversion is desired for non-string/non-number types,
292-
// to_number should be used, and it returns EvaluationError.
293-
// For now, sticking to string concatenation for non-numeric types.
294288
let l_str = self.value_to_string(&left);
295289
let r_str = self.value_to_string(&right);
296290
Ok(Value::String(l_str + &r_str))
@@ -302,45 +296,28 @@ impl Evaluator {
302296
let l_num = self.to_number(&left)?;
303297
let r_num = self.to_number(&right)?;
304298
let result = l_num - r_num;
305-
Ok(Value::Number(
306-
serde_json::Number::from_f64(result)
307-
.unwrap_or_else(|| serde_json::Number::from_f64(0.0).unwrap()),
308-
))
299+
Ok(self.f64_to_value(result))
309300
}
310301

311302
fn multiply_values(&self, left: Value, right: Value) -> Result<Value, EvaluationError> {
312303
let l_num = self.to_number(&left)?;
313304
let r_num = self.to_number(&right)?;
314305
let result = l_num * r_num;
315-
Ok(Value::Number(
316-
serde_json::Number::from_f64(result)
317-
.unwrap_or_else(|| serde_json::Number::from_f64(0.0).unwrap()),
318-
))
306+
Ok(self.f64_to_value(result))
319307
}
320308

321309
fn divide_values(&self, left: Value, right: Value) -> Result<Value, EvaluationError> {
322310
let l_num = self.to_number(&left)?;
323311
let r_num = self.to_number(&right)?;
324-
// JavaScript behavior: division by zero returns Infinity, -Infinity, or NaN
325312
let result = l_num / r_num;
326-
Ok(Value::Number(
327-
serde_json::Number::from_f64(result).unwrap_or_else(|| {
328-
// Handle NaN, Infinity, -Infinity by converting to null
329-
// Note: serde_json doesn't support NaN/Infinity in Number type
330-
// We'll use a special representation
331-
serde_json::Number::from_f64(0.0).unwrap()
332-
}),
333-
))
313+
Ok(self.f64_to_value(result))
334314
}
335315

336316
fn modulo_values(&self, left: Value, right: Value) -> Result<Value, EvaluationError> {
337317
let l_num = self.to_number(&left)?;
338318
let r_num = self.to_number(&right)?;
339319
let result = l_num % r_num;
340-
Ok(Value::Number(
341-
serde_json::Number::from_f64(result)
342-
.unwrap_or_else(|| serde_json::Number::from_f64(0.0).unwrap()),
343-
))
320+
Ok(self.f64_to_value(result))
344321
}
345322

346323
fn compare_values<F>(
@@ -375,11 +352,11 @@ impl Evaluator {
375352
Some((_, UnaryOp::LogicalNot)) => Value::Bool(!self.to_boolean(&expr_value)?),
376353
Some((_, UnaryOp::Minus)) => {
377354
let num = self.to_number(&expr_value)?;
378-
Value::Number(serde_json::Number::from_f64(-num).unwrap())
355+
self.f64_to_value(-num)
379356
}
380357
Some((_, UnaryOp::Plus)) => {
381358
let num = self.to_number(&expr_value)?;
382-
Value::Number(serde_json::Number::from_f64(num).unwrap())
359+
self.f64_to_value(num)
383360
}
384361
_ => {
385362
return Err(EvaluationError::Node(NodeError {
@@ -684,7 +661,6 @@ impl Evaluator {
684661

685662
// Handle string literals with escape sequences
686663
if literal_str.starts_with('"') || literal_str.starts_with('\'') {
687-
let quote_char = literal_str.chars().next().unwrap();
688664
// Remove only the first and last character (the quotes)
689665
let unquoted = if literal_str.len() >= 2 {
690666
&literal_str[1..literal_str.len() - 1]
@@ -769,8 +745,28 @@ impl Evaluator {
769745
}
770746
}
771747

748+
fn f64_to_value(&self, num: f64) -> Value {
749+
if let Some(n) = serde_json::Number::from_f64(num) {
750+
Value::Number(n)
751+
} else if num.is_nan() {
752+
Value::Null
753+
} else if num.is_infinite() {
754+
// Represent Infinity as max f64 as checked in evaluate_by_name
755+
Value::Number(
756+
serde_json::Number::from_f64(if num.is_sign_positive() {
757+
f64::MAX
758+
} else {
759+
f64::MIN
760+
})
761+
.unwrap(),
762+
)
763+
} else {
764+
Value::Null
765+
}
766+
}
767+
772768
fn process_escape_sequences(&self, s: &str) -> String {
773-
let mut result = String::new();
769+
let mut result = String::with_capacity(s.len());
774770
let mut chars = s.chars();
775771

776772
while let Some(ch) = chars.next() {
@@ -784,8 +780,6 @@ impl Evaluator {
784780
'\'' => result.push('\''),
785781
'"' => result.push('"'),
786782
'0' => result.push('\0'),
787-
// For simplicity, we don't handle \uXXXX or \xXX here
788-
// Just pass through the escaped character
789783
_ => {
790784
result.push('\\');
791785
result.push(next_ch);
@@ -798,7 +792,6 @@ impl Evaluator {
798792
result.push(ch);
799793
}
800794
}
801-
802795
result
803796
}
804797

tests/robustness.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
use exprimo::Evaluator;
2+
use serde_json::Value;
3+
use std::collections::HashMap;
4+
5+
#[test]
6+
fn test_overflow_addition() {
7+
let evaluator = Evaluator::new(HashMap::new(), HashMap::new());
8+
// 1.7976931348623157e308 is approx f64::MAX
9+
let result = evaluator.evaluate("1.7976931348623157e308 + 1.7976931348623157e308");
10+
match result {
11+
Ok(Value::Number(n)) => {
12+
// Should be f64::MAX (Infinity approximation)
13+
assert!(n.as_f64().unwrap() > 1.0e308);
14+
}
15+
Ok(val) => panic!("Expected Number, got {:?}", val),
16+
Err(e) => panic!("Evaluation failed: {:?}", e),
17+
}
18+
}
19+
20+
#[test]
21+
fn test_divide_by_zero() {
22+
let evaluator = Evaluator::new(HashMap::new(), HashMap::new());
23+
let result = evaluator.evaluate("1 / 0");
24+
match result {
25+
Ok(Value::Number(n)) => {
26+
// Should be f64::MAX (Infinity approximation) or Infinity
27+
// Currently it might be returning 0.0 based on previous analysis, we want it to be MAX
28+
let val = n.as_f64().unwrap();
29+
assert!(val > 1.0e308 || val.is_infinite());
30+
}
31+
Ok(val) => panic!("Expected Number, got {:?}", val),
32+
Err(e) => panic!("Evaluation failed: {:?}", e),
33+
}
34+
}
35+
36+
#[test]
37+
fn test_nan_creation() {
38+
let evaluator = Evaluator::new(HashMap::new(), HashMap::new());
39+
// 0/0 triggers NaN
40+
let result = evaluator.evaluate("0 / 0");
41+
match result {
42+
Ok(Value::Null) => {} // This is what we WANT (null for NaN)
43+
Ok(Value::Number(n)) => {
44+
// Current behavior might be 0.0
45+
println!("Got number: {:?}", n);
46+
// Verify it is NOT 0.0 if we want to fail before fix (but let's just assert our desired behavior)
47+
// We want it to be Null
48+
panic!("Expected Null for NaN, got Number: {:?}", n);
49+
}
50+
Ok(val) => panic!("Expected Null, got {:?}", val),
51+
Err(e) => panic!("Evaluation failed: {:?}", e),
52+
}
53+
}
54+
55+
#[test]
56+
fn test_escape_sequence() {
57+
let evaluator = Evaluator::new(HashMap::new(), HashMap::new());
58+
let result = evaluator.evaluate("'\\n'");
59+
match result {
60+
Ok(Value::String(s)) => assert_eq!(s, "\n"),
61+
Ok(val) => panic!("Expected String, got {:?}", val),
62+
Err(e) => panic!("Evaluation failed: {:?}", e),
63+
}
64+
}

0 commit comments

Comments
 (0)