From f86d9b8ec63a563ee45aef7f09d4d8dda688439e Mon Sep 17 00:00:00 2001
From: Jeremy Wall <jeremy@marzhillstudios.com>
Date: Tue, 19 Mar 2019 19:47:21 -0500
Subject: [PATCH] FIX: Better error messages when importing.

If the path or directory doesn't exist show the invalid path in the
error message.

Fixes #37
---
 src/build/compile_test.rs | 11 +++++++
 src/build/mod.rs          | 64 +++++++++++++++++++++++++++++----------
 src/error.rs              |  4 +++
 3 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/src/build/compile_test.rs b/src/build/compile_test.rs
index 3b28519..31313e6 100644
--- a/src/build/compile_test.rs
+++ b/src/build/compile_test.rs
@@ -569,3 +569,14 @@ fn test_select_missed_case_boolean_no_default_compile_failure() {
         ],
     )
 }
+
+#[test]
+fn test_bad_import_path_compile_failure() {
+    assert_build_failure(
+        "let bad = import \"no/such/path.ucg\";",
+        vec![
+            Regex::new(r"OSError: Path not found").unwrap(),
+            Regex::new(r"at <eval> line: 1, column: 18").unwrap(),
+        ],
+    )
+}
diff --git a/src/build/mod.rs b/src/build/mod.rs
index 4b69aff..1d07f6e 100644
--- a/src/build/mod.rs
+++ b/src/build/mod.rs
@@ -213,7 +213,7 @@ impl<'a> FileBuilder<'a> {
     pub fn build<P: Into<PathBuf>>(&mut self, file: P) -> BuildResult {
         let file = file.into();
         self.working_dir = file.parent().unwrap().to_path_buf();
-        let mut f = File::open(&file)?;
+        let mut f = self.open_file(&Position::new(0, 0, 0), &file)?;
         let mut s = String::new();
         f.read_to_string(&mut s)?;
         let input = OffsetStrIter::new(&s).with_src_file(file.clone());
@@ -389,7 +389,15 @@ impl<'a> FileBuilder<'a> {
         } else {
             normalized = path;
         }
-        Ok(normalized.canonicalize()?)
+        match normalized.canonicalize() {
+            Ok(p) => Ok(p),
+            Err(e) => Err(error::BuildError::new(
+                format!("Path not found {}", normalized.to_string_lossy()),
+                error::ErrorType::OSError,
+            )
+            .wrap_cause(Box::new(e))
+            .to_boxed()),
+        }
     }
 
     fn eval_import(&self, def: &ImportDef) -> Result<Rc<Val>, Box<dyn Error>> {
@@ -423,7 +431,18 @@ impl<'a> FileBuilder<'a> {
             }
         }
         // Try a relative path first.
-        let normalized = self.find_file(&def.path.fragment, true)?;
+        let normalized = match self.find_file(&def.path.fragment, true) {
+            Ok(p) => p,
+            Err(e) => {
+                return Err(error::BuildError::with_pos(
+                    "No Such Import",
+                    error::ErrorType::ImportError,
+                    def.path.pos.clone(),
+                )
+                .wrap_cause(e)
+                .to_boxed());
+            }
+        };
         if self.detect_import_cycle(normalized.to_string_lossy().as_ref()) {
             return Err(error::BuildError::with_pos(
                 format!(
@@ -444,8 +463,18 @@ impl<'a> FileBuilder<'a> {
             Some(v) => v.clone(),
             None => {
                 let mut b = self.clone_builder();
-                b.build(&normalized)?;
-                b.get_outputs_as_val()
+                match b.build(&normalized) {
+                    Ok(_) => b.get_outputs_as_val(),
+                    Err(e) => {
+                        return Err(error::BuildError::with_pos(
+                            "Import failed",
+                            error::ErrorType::ImportError,
+                            def.pos.clone(),
+                        )
+                        .wrap_cause(e)
+                        .to_boxed());
+                    }
+                }
             }
         };
         let mut mut_assets_cache = self.assets.borrow_mut();
@@ -1679,6 +1708,19 @@ impl<'a> FileBuilder<'a> {
         Ok(ok)
     }
 
+    fn open_file<P: Into<PathBuf>>(&self, pos: &Position, path: P) -> Result<File, Box<dyn Error>> {
+        let path = path.into();
+        match File::open(&path) {
+            Ok(f) => Ok(f),
+            Err(e) => Err(error::BuildError::with_pos(
+                format!("Error opening file {} {}", path.to_string_lossy(), e),
+                error::ErrorType::TypeFail,
+                pos.clone(),
+            )
+            .to_boxed()),
+        }
+    }
+
     fn get_file_as_string(&self, pos: &Position, path: &str) -> Result<String, Box<dyn Error>> {
         let normalized = match self.find_file(path, false) {
             Ok(p) => p,
@@ -1691,17 +1733,7 @@ impl<'a> FileBuilder<'a> {
                 .to_boxed());
             }
         };
-        let mut f = match File::open(&normalized) {
-            Ok(f) => f,
-            Err(e) => {
-                return Err(error::BuildError::with_pos(
-                    format!("Error opening file {} {}", normalized.to_string_lossy(), e),
-                    error::ErrorType::TypeFail,
-                    pos.clone(),
-                )
-                .to_boxed());
-            }
-        };
+        let mut f = self.open_file(pos, normalized)?;
         let mut contents = String::new();
         f.read_to_string(&mut contents)?;
         Ok(contents)
diff --git a/src/error.rs b/src/error.rs
index aa53e63..cf100f0 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -31,10 +31,12 @@ pub enum ErrorType {
     BadArgLen,
     FormatError,
     IncludeError,
+    ImportError,
     ReservedWordError,
     // Parsing Errors
     ParseError,
     AssertError,
+    OSError,
     // User Defined Declarative Errors
     UserDefined,
 }
@@ -49,9 +51,11 @@ impl fmt::Display for ErrorType {
             &ErrorType::BadArgLen => "BadArgLen",
             &ErrorType::FormatError => "FormatError",
             &ErrorType::IncludeError => "IncludeError",
+            &ErrorType::ImportError => "ImportError",
             &ErrorType::ReservedWordError => "ReservedWordError",
             &ErrorType::ParseError => "ParseError",
             &ErrorType::AssertError => "AssertError",
+            &ErrorType::OSError => "OSError",
             &ErrorType::UserDefined => "UserDefined",
         };
         w.write_str(name)