From 62fdaa822baa5ce3a7191e621e47d2ad7d25a5da Mon Sep 17 00:00:00 2001
From: Markus Westerlind <mwesterlind@influxdata.com>
Date: Tue, 14 Jun 2022 19:10:49 +0200
Subject: [PATCH] refactor: Avoid checking the string message of a fatal error

---
 libflux/flux-core/src/ast/check/mod.rs   | 29 +++++++++++-------------
 libflux/flux-core/src/ast/check/tests.rs |  8 +++----
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/libflux/flux-core/src/ast/check/mod.rs b/libflux/flux-core/src/ast/check/mod.rs
index c8af245333..1bc8d5f510 100644
--- a/libflux/flux-core/src/ast/check/mod.rs
+++ b/libflux/flux-core/src/ast/check/mod.rs
@@ -26,12 +26,7 @@ pub fn check(node: walk::Node) -> Result<(), Errors<Error>> {
             let errors = &mut self.errors;
 
             if self.depth > MAX_DEPTH {
-                errors.push(located(
-                    n.base().location.clone(),
-                    ErrorKind {
-                        message: String::from("Program is nested to deep"),
-                    },
-                ));
+                errors.push(located(n.base().location.clone(), ErrorKind::NestedToDeep));
 
                 return false;
             }
@@ -40,7 +35,7 @@ pub fn check(node: walk::Node) -> Result<(), Errors<Error>> {
             for err in n.base().errors.iter() {
                 errors.push(located(
                     n.base().location.clone(),
-                    ErrorKind {
+                    ErrorKind::Message {
                         message: err.clone(),
                     },
                 ));
@@ -49,13 +44,13 @@ pub fn check(node: walk::Node) -> Result<(), Errors<Error>> {
             match n {
                 walk::Node::BadStmt(n) => errors.push(located(
                     n.base.location.clone(),
-                    ErrorKind {
+                    ErrorKind::Message {
                         message: format!("invalid statement: {}", n.text),
                     },
                 )),
                 walk::Node::BadExpr(n) if !n.text.is_empty() => errors.push(located(
                     n.base.location.clone(),
-                    ErrorKind {
+                    ErrorKind::Message {
                         message: format!("invalid expression: {}", n.text),
                     },
                 )),
@@ -70,7 +65,7 @@ pub fn check(node: walk::Node) -> Result<(), Errors<Error>> {
                                     if let PropertyKey::StringLit(s) = &p.key {
                                         errors.push(located(
                                             n.base.location.clone(),
-                                            ErrorKind {
+                                            ErrorKind::Message {
                                                 message: format!(
                                                     "string literal key {} must have a value",
                                                     s.value
@@ -88,7 +83,7 @@ pub fn check(node: walk::Node) -> Result<(), Errors<Error>> {
                     if has_implicit && has_explicit {
                         errors.push(located(
                             n.base.location.clone(),
-                            ErrorKind {
+                            ErrorKind::Message {
                                 message: String::from(
                                     "cannot mix implicit and explicit properties",
                                 ),
@@ -123,15 +118,17 @@ pub type Error = Located<ErrorKind>;
 
 /// An error that can be returned while checking the AST.
 #[derive(Error, Debug, PartialEq)]
-#[error("{}", message)]
-pub struct ErrorKind {
-    /// Error message.
-    pub message: String,
+#[allow(missing_docs)]
+pub enum ErrorKind {
+    #[error("Program is nested to deep")]
+    NestedToDeep,
+    #[error("{message}")]
+    Message { message: String },
 }
 
 impl ErrorKind {
     pub(crate) fn is_fatal(&self) -> bool {
-        self.message.contains("Program is nested to deep")
+        matches!(self, Self::NestedToDeep)
     }
 }
 
diff --git a/libflux/flux-core/src/ast/check/tests.rs b/libflux/flux-core/src/ast/check/tests.rs
index 3065ac26d3..7fb1c06ad2 100644
--- a/libflux/flux-core/src/ast/check/tests.rs
+++ b/libflux/flux-core/src/ast/check/tests.rs
@@ -21,7 +21,7 @@ fn test_object_check() {
             },
             source: Some(String::from("{c: 2, a}")),
         },
-        ErrorKind {
+        ErrorKind::Message {
             message: String::from("cannot mix implicit and explicit properties"),
         },
     )]));
@@ -39,7 +39,7 @@ fn test_bad_stmt() {
             end: Position { line: 3, column: 3 },
             source: Some(String::from("=")),
         },
-        ErrorKind {
+        ErrorKind::Message {
             message: String::from("invalid statement: ="),
         },
     )]));
@@ -60,7 +60,7 @@ fn test_bad_expr() {
             },
             source: Some(String::from("/")),
         },
-        ErrorKind {
+        ErrorKind::Message {
             message: String::from("invalid expression: invalid token for primary expression: DIV"),
         },
     )]));
@@ -131,6 +131,6 @@ fn test_check_collect_existing_error() {
     let got = check(walk::Node::File(&file)).unwrap_err();
     assert_eq!(3, got.len());
     for (i, err) in got.iter().enumerate() {
-        assert_eq!(err.error.message, format!("error {}", i + 1));
+        assert_eq!(err.error.to_string(), format!("error {}", i + 1));
     }
 }