From 208e5fdbf493089dbb76b3321a80d249ce52b7e4 Mon Sep 17 00:00:00 2001
From: Stephen Chung <schungx@live.com>
Date: Fri, 16 Apr 2021 13:28:36 +0800
Subject: [PATCH] New switch errors.

---
 CHANGELOG.md       |  2 +-
 src/parse_error.rs |  3 +++
 src/parser.rs      |  7 +++++++
 tests/switch.rs    | 50 +++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a6b27309..bff3b748 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,7 +18,7 @@ Breaking changes
 ----------------
 
 * Negative index to an array or string yields the appropriate element/character counting from the _end_.
-* The default `_` case of a `switch` statement now must be the last case, together with a new error variant `EvalAltResult::WrongSwitchDefaultCase`.
+* The default `_` case of a `switch` statement now must be the last case, together with two new error variants: `EvalAltResult::WrongSwitchDefaultCase` and `EvalAltResult::WrongSwitchCaseCondition`.
 * `ModuleResolver` trait methods take an additional parameter `source_path` that contains the path of the current environment. This is to facilitate loading other script files always from the current directory.
 * `FileModuleResolver` now resolves relative paths under the source path if there is no base path set.
 * `FileModuleResolver::base_path` now returns `Option<&str>` which is `None` if there is no base path set.
diff --git a/src/parse_error.rs b/src/parse_error.rs
index bce23876..a7b75cba 100644
--- a/src/parse_error.rs
+++ b/src/parse_error.rs
@@ -117,6 +117,8 @@ pub enum ParseErrorType {
     DuplicatedSwitchCase,
     /// The default case of a `switch` statement is not the last.
     WrongSwitchDefaultCase,
+    /// The case condition of a `switch` statement is not appropriate.
+    WrongSwitchCaseCondition,
     /// Missing a property name for custom types and maps.
     ///
     /// Never appears under the `no_object` feature.
@@ -198,6 +200,7 @@ impl ParseErrorType {
             Self::DuplicatedProperty(_) => "Duplicated property in object map literal",
             Self::DuplicatedSwitchCase => "Duplicated switch case",
             Self::WrongSwitchDefaultCase => "Default switch case is not the last",
+            Self::WrongSwitchCaseCondition => "Default switch case cannot have condition",
             Self::PropertyExpected => "Expecting name of a property",
             Self::VariableExpected => "Expecting name of a variable",
             Self::Reserved(_) => "Invalid use of reserved keyword",
diff --git a/src/parser.rs b/src/parser.rs
index b94683c3..d376d7f2 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -829,6 +829,13 @@ fn parse_switch(
             (Token::Underscore, pos) if def_stmt.is_none() => {
                 def_pos = *pos;
                 eat_token(input, Token::Underscore);
+
+                let (if_clause, if_pos) = match_token(input, Token::If);
+
+                if if_clause {
+                    return Err(PERR::WrongSwitchCaseCondition.into_err(if_pos));
+                }
+
                 (None, None)
             }
             (Token::Underscore, pos) => return Err(PERR::DuplicatedSwitchCase.into_err(*pos)),
diff --git a/tests/switch.rs b/tests/switch.rs
index 38ddebbf..db0d8e17 100644
--- a/tests/switch.rs
+++ b/tests/switch.rs
@@ -1,4 +1,4 @@
-use rhai::{Engine, EvalAltResult, Scope, INT};
+use rhai::{Engine, EvalAltResult, ParseErrorType, Scope, INT};
 
 #[test]
 fn test_switch() -> Result<(), Box<EvalAltResult>> {
@@ -67,6 +67,28 @@ fn test_switch() -> Result<(), Box<EvalAltResult>> {
     Ok(())
 }
 
+#[test]
+fn test_switch_errors() -> Result<(), Box<EvalAltResult>> {
+    let engine = Engine::new();
+
+    assert!(matches!(
+        *engine
+            .compile("switch x { 1 => 123, 1 => 42 }")
+            .expect_err("should error")
+            .0,
+        ParseErrorType::DuplicatedSwitchCase
+    ));
+    assert!(matches!(
+        *engine
+            .compile("switch x { _ => 123, 1 => 42 }")
+            .expect_err("should error")
+            .0,
+        ParseErrorType::WrongSwitchDefaultCase
+    ));
+
+    Ok(())
+}
+
 #[test]
 fn test_switch_condition() -> Result<(), Box<EvalAltResult>> {
     let engine = Engine::new();
@@ -103,6 +125,32 @@ fn test_switch_condition() -> Result<(), Box<EvalAltResult>> {
         9
     );
 
+    assert!(matches!(
+        *engine
+            .compile(
+                r"
+                    switch x {
+                        21 if x < 40 => 1,
+                        21 if x == 10 => 10,
+                        0 if x < 100 => 2,
+                        1 => 3,
+                        _ => 9
+                    }
+                "
+            )
+            .expect_err("should error")
+            .0,
+        ParseErrorType::DuplicatedSwitchCase
+    ));
+
+    assert!(matches!(
+        *engine
+            .compile("switch x { 1 => 123, _ if true => 42 }")
+            .expect_err("should error")
+            .0,
+        ParseErrorType::WrongSwitchCaseCondition
+    ));
+
     Ok(())
 }