From 28c907d9deadf54161c95ea84a2b512be5a30355 Mon Sep 17 00:00:00 2001
From: KtorZ <matthias.benkort@gmail.com>
Date: Tue, 20 Dec 2022 23:21:56 +0100
Subject: [PATCH] Fix acceptance 021: allow registering type aliases in any
 order.

  This is the most intuitive thing I could come up with: since the
  problem is mainly due to the order in which we try declaring the
  aliases, then it suffices to simply try as much as we can, and retry
  on failure until there's no more failure.

  Note that it's important to detect cycles if we do such thing (which
  we can by noticing that a given iteration didn't make any progress).

  It works pretty well in the end and even allow us to define a new kind
  of type error should there be a cyclic definition.
---
 crates/lang/src/tipo/environment.rs | 71 ++++++++++++++++++++++++++++-
 crates/lang/src/tipo/error.rs       |  7 +++
 crates/lang/src/tipo/infer.rs       |  9 ++--
 3 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/crates/lang/src/tipo/environment.rs b/crates/lang/src/tipo/environment.rs
index d654993b..e7460f58 100644
--- a/crates/lang/src/tipo/environment.rs
+++ b/crates/lang/src/tipo/environment.rs
@@ -823,6 +823,73 @@ impl<'a> Environment<'a> {
 
     /// Iterate over a module, registering any new types created by the module into the typer
     pub fn register_types(
+        &mut self,
+        definitions: Vec<&'a UntypedDefinition>,
+        module: &String,
+        hydrators: &mut HashMap<String, Hydrator>,
+        names: &mut HashMap<&'a str, &'a Span>,
+    ) -> Result<(), Error> {
+        let known_types_before = names.keys().copied().collect::<Vec<_>>();
+
+        let mut error = None;
+        let mut remaining_definitions = vec![];
+
+        // in case we failed at registering a type-definition, we backtrack and
+        // try again until either of:
+        //
+        // (a) we do not make any more progress;
+        // (b) there's no more errors.
+        //
+        // This is because some definition, especially when combining type-aliases may depend on
+        // types that we haven't yet seen (because declared later in the module). In which case, it
+        // would suffice to register type in a different order. Thus instead of failing on the
+        // first error, we try to register as many types as we can, recursively until we've
+        // exhausted all the definitions or, until we no longer make any progress (which may signal
+        // a cycle).
+        for def in definitions {
+            if let Err(e) = self.register_type(def, module, hydrators, names) {
+                error = Some(e);
+                remaining_definitions.push(def);
+                if let Definition::TypeAlias(TypeAlias { alias, .. }) = def {
+                    names.remove(alias.as_str());
+                }
+            };
+        }
+
+        match error {
+            None => Ok(()),
+            Some(e) => {
+                let known_types_after = names.keys().copied().collect::<Vec<_>>();
+                if known_types_before == known_types_after {
+                    let cycle = remaining_definitions
+                        .into_iter()
+                        .filter_map(|def| match def {
+                            Definition::TypeAlias(TypeAlias {
+                                alias, location, ..
+                            }) => Some((alias.to_owned(), location.to_owned())),
+                            _ => None,
+                        })
+                        .collect::<Vec<(String, Span)>>();
+                    match cycle.first() {
+                        None => Err(e),
+                        Some((alias, location)) => {
+                            let mut types =
+                                cycle.iter().map(|def| def.0.clone()).collect::<Vec<_>>();
+                            types.push(alias.clone());
+                            Err(Error::CyclicTypeDefinitions {
+                                location: *location,
+                                types,
+                            })
+                        }
+                    }
+                } else {
+                    self.register_types(remaining_definitions, module, hydrators, names)
+                }
+            }
+        }
+    }
+
+    pub fn register_type(
         &mut self,
         def: &'a UntypedDefinition,
         module: &String,
@@ -885,11 +952,11 @@ impl<'a> Environment<'a> {
             }) => {
                 assert_unique_type_name(names, name, location)?;
 
-                // Register the paramerterised types
+                // Register the parameterised types
                 let mut hydrator = Hydrator::new();
                 let parameters = self.make_type_vars(args, location, &mut hydrator)?;
 
-                // Disallow creation of new types outside the paramerterised types
+                // Disallow creation of new types outside the parameterised types
                 hydrator.disallow_new_type_variables();
 
                 // Create the type that the alias resolves to
diff --git a/crates/lang/src/tipo/error.rs b/crates/lang/src/tipo/error.rs
index 19352b0b..40a230e2 100644
--- a/crates/lang/src/tipo/error.rs
+++ b/crates/lang/src/tipo/error.rs
@@ -271,6 +271,13 @@ pub enum Error {
         name: String,
     },
 
+    #[error("Cyclic type definition detected: {}\n", types.join(" -> "))]
+    CyclicTypeDefinitions {
+        #[label]
+        location: Span,
+        types: Vec<String>,
+    },
+
     #[error("Recursive type detected\n")]
     RecursiveType {
         #[label]
diff --git a/crates/lang/src/tipo/infer.rs b/crates/lang/src/tipo/infer.rs
index c6f33fb1..1abd2adc 100644
--- a/crates/lang/src/tipo/infer.rs
+++ b/crates/lang/src/tipo/infer.rs
@@ -48,9 +48,12 @@ impl UntypedModule {
 
         // Register types so they can be used in constructors and functions
         // earlier in the module.
-        for def in self.definitions() {
-            environment.register_types(def, &name, &mut hydrators, &mut type_names)?;
-        }
+        environment.register_types(
+            self.definitions.iter().collect(),
+            &name,
+            &mut hydrators,
+            &mut type_names,
+        )?;
 
         // Register values so they can be used in functions earlier in the module.
         for def in self.definitions() {