Merge pull request #2 from Hirevo/feature/overlapping-fields

Added static checks for overlapping fields

som-interpreter/src/class.rs

@@ -39,20 +39,32 @@ pub struct Class {
 
 impl Class {
     /// Load up a class from its class definition from the AST.
-    pub fn from_class_def(defn: ClassDef) -> SOMRef<Class> {
-        let static_locals = defn
-            .static_locals
-            .iter()
-            .cloned()
-            .zip(std::iter::repeat(Value::Nil))
-            .collect();
-
-        let instance_locals = defn
-            .instance_locals
-            .iter()
-            .cloned()
-            .zip(std::iter::repeat(Value::Nil))
-            .collect();
+    pub fn from_class_def(defn: ClassDef) -> Result<SOMRef<Class>, String> {
+        let static_locals = {
+            let mut static_locals = IndexMap::new();
+            for field in defn.static_locals.iter() {
+                if static_locals.insert(field.clone(), Value::Nil).is_some() {
+                    return Err(format!(
+                        "{}: the field named '{}' is already defined in this class",
+                        defn.name, field,
+                    ));
+                }
+            }
+            static_locals
+        };
+
+        let instance_locals = {
+            let mut instance_locals = IndexMap::new();
+            for field in defn.instance_locals.iter() {
+                if instance_locals.insert(field.clone(), Value::Nil).is_some() {
+                    return Err(format!(
+                        "{}: the field named '{}' is already defined in this class",
+                        defn.name, field,
+                    ));
+                }
+            }
+            instance_locals
+        };
 
         let static_class = Rc::new(RefCell::new(Self {
             name: format!("{} class", defn.name),
@@ -117,7 +129,7 @@ impl Class {
         static_class.borrow_mut().methods = static_methods;
         instance_class.borrow_mut().methods = instance_methods;
 
-        instance_class
+        Ok(instance_class)
     }
 
     /// Get the class' name.

som-interpreter/src/universe.rs

@@ -1,4 +1,5 @@
 use std::cell::RefCell;
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::fs;
 use std::io;
@@ -248,7 +249,7 @@ impl Universe {
                 ));
             }
 
-            return Ok(Class::from_class_def(defn));
+            return Class::from_class_def(defn).map_err(Error::msg);
         }
 
         Err(anyhow!("could not find the '{}' system class", class_name))
@@ -295,9 +296,45 @@ impl Universe {
                 self.core.object_class.clone()
             };
 
-            let class = Class::from_class_def(defn);
+            let class = Class::from_class_def(defn).map_err(Error::msg)?;
             set_super_class(&class, &super_class, &self.core.metaclass_class);
 
+            fn has_duplicated_field(class: &SOMRef<Class>) -> Option<(String, (String, String))> {
+                let super_class_iterator = std::iter::successors(Some(class.clone()), |class| {
+                    class.borrow().super_class()
+                });
+                let mut map = HashMap::<String, String>::new();
+                for class in super_class_iterator {
+                    let class_name = class.borrow().name().to_string();
+                    for (field, _) in class.borrow().locals.iter() {
+                        let field_name = field.clone();
+                        match map.entry(field_name.clone()) {
+                            Entry::Occupied(entry) => {
+                                return Some((field_name, (class_name, entry.get().clone())))
+                            }
+                            Entry::Vacant(v) => {
+                                v.insert(class_name.clone());
+                            }
+                        }
+                    }
+                }
+                return None;
+            }
+
+            if let Some((field, (c1, c2))) = has_duplicated_field(&class) {
+                return Err(anyhow!(
+                    "the field named '{}' is defined more than once (by '{}' and '{}', where the latter inherits from the former)",
+                    field, c1, c2,
+                ));
+            }
+
+            if let Some((field, (c1, c2))) = has_duplicated_field(&class.borrow().class()) {
+                return Err(anyhow!(
+                    "the field named '{}' is defined more than once (by '{}' and '{}', where the latter inherits from the former)",
+                    field, c1, c2,
+                ));
+            }
+
             self.globals.insert(
                 class.borrow().name().to_string(),
                 Value::Class(class.clone()),
@@ -350,7 +387,7 @@ impl Universe {
             self.core.object_class.clone()
         };
 
-        let class = Class::from_class_def(defn);
+        let class = Class::from_class_def(defn).map_err(Error::msg)?;
         set_super_class(&class, &super_class, &self.core.metaclass_class);
 
         Ok(class)