Fix signature of function to load types from a directory

Taking &mut self makes more sense when wanting to populate from a list.

Also we should return a list of diagnostics for each file that we loaded
or an error if we couldn't load the file (for example due to permission
problems, etc.).

This way the caller can choose to show diagnostics, ignore them, etc.

sixtyfps_compiler/Cargo.toml

@@ -29,6 +29,7 @@ codemap = { version = "0.1", optional = true }
 quote = { version = "1.0", optional = true }
 proc-macro2 = { version = "1.0.17", optional = true }
 lyon = { version = "0.15.8", features = ["svg"] }
+thiserror = "1"
 
 
 [dev-dependencies]

sixtyfps_compiler/diagnostics.rs

@@ -30,7 +30,7 @@ pub struct CompilerDiagnostic {
     pub span: Span,
 }
 
-#[derive(Default, Debug, Clone)]
+#[derive(Default, Debug, Clone, PartialEq)]
 pub struct Diagnostics {
     pub inner: Vec<CompilerDiagnostic>,
     pub current_path: std::path::PathBuf,
@@ -133,22 +133,6 @@ impl Diagnostics {
         self
     }
 
-    pub fn check_errors(self) -> std::io::Result<Self> {
-        if !self.has_error() {
-            return Ok(self);
-        }
-        #[cfg(feature = "display-diagnostics")]
-        return Err(std::io::Error::new(std::io::ErrorKind::Other, self.diagnostics_as_string()));
-        #[cfg(not(feature = "display-diagnostics"))]
-        return Err(std::io::Error::new(
-            std::io::ErrorKind::Other,
-            format!(
-                "Error compiling {} but diagnostics were disabled in the compiler",
-                self.current_path.to_string_lossy()
-            ),
-        ));
-    }
-
     #[cfg(feature = "proc_macro_span")]
     /// Will convert the diagnostics that only have offsets to the actual span
     pub fn map_offsets_to_span(&mut self, span_map: &[crate::parser::Token]) {

sixtyfps_compiler/lib.rs

@@ -41,6 +41,13 @@ mod passes {
     pub mod unique_id;
 }
 
+#[derive(thiserror::Error, Debug)]
+#[error("Error loading {path} from disk: {source}")]
+pub struct FileLoadError {
+    path: std::path::PathBuf,
+    source: std::io::Error,
+}
+
 #[derive(Default)]
 /// CompilationConfiguration allows configuring different aspects of the compiler.
 pub struct CompilerConfiguration {

sixtyfps_compiler/typeregister.rs

@@ -1,3 +1,5 @@
+use crate::diagnostics::Diagnostics;
+use crate::FileLoadError;
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::{fmt::Display, rc::Rc};
 
@@ -434,39 +436,49 @@ impl TypeRegister {
         self.types.insert(comp.id.clone(), Type::Component(comp));
     }
 
+    /// Loads the .60 file and adds it to the type registry. An error is returned if there were I/O problems,
+    /// otherwise the diagnostics collected during the parsing are returned.
     pub fn add_type_from_source<P: AsRef<std::path::Path>>(
         &mut self,
         path: P,
-    ) -> std::io::Result<()> {
+    ) -> std::io::Result<Diagnostics> {
         let (syntax_node, diag) = crate::parser::parse_file(&path)?;
 
-        let diag = diag.check_errors()?;
-
         // For the time being .60 files added to a type registry cannot depend on other .60 files.
         let (doc, diag) = crate::compile_syntax_node(syntax_node, diag);
 
-        diag.check_errors()?;
+        if !doc.root_component.id.is_empty() {
             self.add(doc.root_component);
-        Ok(())
         }
 
-    pub fn new_from_library<P: AsRef<std::path::Path>>(
+        Ok(diag)
+    }
+
+    /// Adds all .60 files from the specified directory to the type registry. For each file a result is
+    /// included in the returned vector that either contains the diagnostics encountered during the parsing
+    /// (if any) or an I/O error if it occured. If there was a problem reading the directory, then an I/O error
+    /// is returned.
+    pub fn add_from_directory<P: AsRef<std::path::Path>>(
+        &mut self,
         directory: P,
-        parent_registry: &Rc<TypeRegister>,
+    ) -> std::io::Result<Vec<Result<Diagnostics, FileLoadError>>> {
-    ) -> std::io::Result<Rc<TypeRegister>> {
+        Ok(std::fs::read_dir(directory)?
-        let mut result =
+            .filter_map(Result::ok)
-            TypeRegister { parent_registry: Some(parent_registry.clone()), ..Default::default() };
+            .filter_map(|entry| {
-
-        for entry in std::fs::read_dir(directory)? {
-            let entry = entry?;
                 let path = entry.path();
-            if path.is_file() && path.extension().unwrap_or_default() == std::ffi::OsStr::new("60")
+                if path.is_file()
+                    && path.extension().unwrap_or_default() == std::ffi::OsStr::new("60")
                 {
-                result.add_type_from_source(path)?;
+                    Some(path)
+                } else {
+                    None
                 }
-        }
+            })
-
+            .map(|path| {
-        Ok(Rc::new(result))
+                self.add_type_from_source(&path)
+                    .map_err(|ioerr| FileLoadError { path, source: ioerr })
+            })
+            .collect())
     }
 
     pub fn property_animation_type_for_property(&self, property_type: Type) -> Type {
@@ -503,11 +515,14 @@ fn test_registry_from_library() {
     let test_source_path: std::path::PathBuf =
         [env!("CARGO_MANIFEST_DIR"), "tests"].iter().collect();
 
-    let result = TypeRegister::new_from_library(test_source_path, &global_types);
+    let mut local_types = TypeRegister::new(&global_types);
+    let result = local_types.add_from_directory(test_source_path);
 
     assert!(result.is_ok());
-
+    let file_load_status_list = result.unwrap();
-    let local_types = result.unwrap();
+    assert_eq!(file_load_status_list.len(), 1);
+    assert!(file_load_status_list[0].is_ok());
+    assert!(!file_load_status_list[0].as_ref().unwrap().has_error());
 
     assert_ne!(local_types.lookup("PublicType"), Type::Invalid);
     assert_eq!(local_types.lookup("HiddenInternalType"), Type::Invalid);