From efef2bc3d20a1e87ddf76c724320065f712e682e Mon Sep 17 00:00:00 2001 From: Jeremy Wall Date: Sun, 3 Feb 2019 09:58:38 -0600 Subject: [PATCH] FIX: Better error reporting for binary expressions. Closes #34 --- src/build/compile_test.rs | 125 +++++++++++++++++++++++++++++++++++++- src/build/ir.rs | 2 +- src/build/mod.rs | 43 ++++--------- src/tokenizer/mod.rs | 4 +- 4 files changed, 139 insertions(+), 35 deletions(-) diff --git a/src/build/compile_test.rs b/src/build/compile_test.rs index 9bc5f95..763f43d 100644 --- a/src/build/compile_test.rs +++ b/src/build/compile_test.rs @@ -181,7 +181,7 @@ fn test_assert_partial_tuple_compile_failures() { Regex::new(r"line: 1, column: 1").unwrap(), Regex::new(r"Expected Tuple \{ok=, desc=\}: at line: 1, column: 8") .unwrap(), - Regex::new(r"Expected \(\}\) Instead is \(\): at line: 1, column: 9").unwrap(), + Regex::new(r"Expected \(\}\) but got \(\): at line: 1, column: 9").unwrap(), ], ); } @@ -266,7 +266,7 @@ fn test_binary_operator_missing_operand_compile_failure() { } #[test] -fn test_binary_operator_wrong_type_on_rhs_compile_failure() { +fn test_binary_sum_operator_wrong_type_on_rhs_compile_failure() { assert_build_failure( "1 + \"foo\";", vec![ @@ -275,3 +275,124 @@ fn test_binary_operator_wrong_type_on_rhs_compile_failure() { ], ) } + +#[test] +fn test_binary_minus_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 - \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 5").unwrap(), + ], + ) +} + +#[test] +fn test_binary_mul_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 * \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 5").unwrap(), + ], + ) +} + +#[test] +fn test_binary_div_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 / \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 5").unwrap(), + ], + ) +} + +#[test] +fn test_binary_gt_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 > \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 5").unwrap(), + ], + ) +} + +#[test] +fn test_binary_lt_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 < \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 5").unwrap(), + ], + ) +} + +#[test] +fn test_binary_lteq_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 <= \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 6").unwrap(), + ], + ) +} + +#[test] +fn test_binary_gteq_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 >= \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 6").unwrap(), + ], + ) +} + +#[test] +fn test_binary_eqeq_operator_wrong_type_on_rhs_compile_failure() { + assert_build_failure( + "1 == \"foo\";", + vec![ + Regex::new(r"Expected Integer but got .foo.").unwrap(), + Regex::new(r"at line: 1, column: 6").unwrap(), + ], + ) +} + +#[test] +fn test_incomplete_tuple_compile_failure() { + assert_build_failure( + "{;", + vec![ + Regex::new(r"Expected \(\}\) but got \(;\)").unwrap(), + Regex::new(r"at line: 1, column: 2").unwrap(), + ], + ) +} + +#[test] +fn test_incomplete_tuple_key_value_missing_eq_compile_failure() { + assert_build_failure( + "{foo", + vec![ + Regex::new(r"Expected \(=\) but got \(\)").unwrap(), + Regex::new(r"at line: 1, column: 5").unwrap(), + ], + ) +} + +#[test] +fn test_incomplete_tuple_key_value_missing_value_compile_failure() { + assert_build_failure( + "{foo=", + vec![ + Regex::new(r"Expected Expression").unwrap(), + Regex::new(r"at line: 1, column: 6").unwrap(), + ], + ) +} diff --git a/src/build/ir.rs b/src/build/ir.rs index 36de510..cb2d9b7 100644 --- a/src/build/ir.rs +++ b/src/build/ir.rs @@ -113,7 +113,7 @@ impl Val { (&Val::Empty, _) => Ok(false), (_, &Val::Empty) => Ok(false), (me, tgt) => Err(error::BuildError::new( - format!("Types differ for {}, {}", me, tgt), + format!("Expected {} but got {}", me.type_name(), tgt), error::ErrorType::TypeFail, pos, )), diff --git a/src/build/mod.rs b/src/build/mod.rs index 643844a..fecf3e8 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -680,11 +680,7 @@ impl<'a> FileBuilder<'a> { } } Err(Box::new(error::BuildError::new( - format!( - "Incompatible types for numeric comparison {} with {}", - left.type_name(), - right.type_name() - ), + format!("Expected {} but got {}", left.type_name(), right,), error::ErrorType::TypeFail, pos.clone(), ))) @@ -708,11 +704,7 @@ impl<'a> FileBuilder<'a> { } } Err(Box::new(error::BuildError::new( - format!( - "Incompatible types for numeric comparison {} with {}", - left.type_name(), - right.type_name() - ), + format!("Expected {} but got {}", left.type_name(), right,), error::ErrorType::TypeFail, pos.clone(), ))) @@ -735,11 +727,7 @@ impl<'a> FileBuilder<'a> { } } Err(Box::new(error::BuildError::new( - format!( - "Incompatible types for numeric comparison {} with {}", - left.type_name(), - right.type_name() - ), + format!("Expected {} but got {}", left.type_name(), right), error::ErrorType::TypeFail, pos.clone(), ))) @@ -762,11 +750,7 @@ impl<'a> FileBuilder<'a> { } } Err(Box::new(error::BuildError::new( - format!( - "Incompatible types for numeric comparison {} with {}", - left.type_name(), - right.type_name() - ), + format!("Expected {} but got {}", left.type_name(), right,), error::ErrorType::TypeFail, pos.clone(), ))) @@ -879,7 +863,7 @@ impl<'a> FileBuilder<'a> { ) -> Result, Box> { // First we evaluate our right hand side so we have a something to search // inside for our left hand expression. - let right_pos = right.pos().clone(); + let right_pos = right.pos(); let right = self.eval_expr(right, scope)?; // presence checks are only valid for tuples and lists. if !(right.is_tuple() || right.is_list()) { @@ -889,14 +873,13 @@ impl<'a> FileBuilder<'a> { right.type_name() ), error::ErrorType::TypeFail, - right_pos, + right_pos.clone(), ))); } if let &Val::List(ref els) = right.as_ref() { - let left_pos = left.pos().clone(); let left = self.eval_expr(left, scope)?; for val in els.iter() { - if let Ok(b) = self.do_deep_equal(&left_pos, left.clone(), val.clone()) { + if let Ok(b) = self.do_deep_equal(right_pos, left.clone(), val.clone()) { if let &Val::Boolean(b) = b.as_ref() { if b { // We found a match @@ -985,12 +968,12 @@ impl<'a> FileBuilder<'a> { &BinaryExprType::Mul => self.multiply_vals(&def.pos, def.right.pos(), left, right), &BinaryExprType::Div => self.divide_vals(&def.pos, def.right.pos(), left, right), // Handle Comparison operators here - &BinaryExprType::Equal => self.do_deep_equal(&def.pos, left, right), - &BinaryExprType::GT => self.do_gt(&def.pos, left, right), - &BinaryExprType::LT => self.do_lt(&def.pos, left, right), - &BinaryExprType::GTEqual => self.do_gtequal(&def.pos, left, right), - &BinaryExprType::LTEqual => self.do_ltequal(&def.pos, left, right), - &BinaryExprType::NotEqual => self.do_not_deep_equal(&def.pos, left, right), + &BinaryExprType::Equal => self.do_deep_equal(def.right.pos(), left, right), + &BinaryExprType::GT => self.do_gt(&def.right.pos(), left, right), + &BinaryExprType::LT => self.do_lt(&def.right.pos(), left, right), + &BinaryExprType::GTEqual => self.do_gtequal(&def.right.pos(), left, right), + &BinaryExprType::LTEqual => self.do_ltequal(&def.right.pos(), left, right), + &BinaryExprType::NotEqual => self.do_not_deep_equal(&def.right.pos(), left, right), &BinaryExprType::REMatch => { self.eval_re_match(left, def.left.pos(), right, def.right.pos(), false) } diff --git a/src/tokenizer/mod.rs b/src/tokenizer/mod.rs index 8e35080..ea0b401 100644 --- a/src/tokenizer/mod.rs +++ b/src/tokenizer/mod.rs @@ -609,7 +609,7 @@ macro_rules! match_token { $i, TokenType::BAREWORD, $f, - format!("Not a BAREWORD ({})", $f), + format!("Expected BAREWORD but got ({})", $f), $h ) }; @@ -629,7 +629,7 @@ macro_rules! match_token { } } else { Result::Fail(Error::new( - format!("Expected {} Instead is ({})", $msg, tok.fragment), + format!("Expected {} but got ({})", $msg, tok.fragment), Box::new($i.clone()), )) }