SymbolTable error refactoring

Cargo.lock

@@ -41,6 +41,12 @@ dependencies = [
  "nodrop",
 ]
 
+[[package]]
+name = "assert_matches"
+version = "1.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9"
+
 [[package]]
 name = "autocfg"
 version = "0.1.6"
@@ -831,6 +837,7 @@ dependencies = [
 name = "schala-lang"
 version = "0.1.0"
 dependencies = [
+ "assert_matches",
  "colored",
  "derivative",
  "ena",

schala-lang/language/Cargo.toml

@@ -14,6 +14,7 @@ stopwatch = "0.0.7"
 derivative = "1.0.3"
 colored = "1.8"
 radix_trie = "0.1.5"
+assert_matches = "1.5"
 
 schala-lang-codegen = { path = "../codegen" }
 schala-repl = { path = "../../schala-repl" }

schala-lang/language/src/error.rs

@@ -1,6 +1,7 @@
 use crate::parsing::ParseError;
 use crate::schala::{SourceReference, Stage};
 use crate::tokenizing::{Location, Token, TokenKind};
+use crate::symbol_table::SymbolError;
 use crate::typechecking::TypeError;
 
 pub struct SchalaError {
@@ -29,6 +30,19 @@ impl SchalaError {
         }
     }
 
+    pub(crate) fn from_symbol_table(symbol_errs: Vec<SymbolError>) -> Self {
+      //TODO this could be better
+      let errors = symbol_errs.into_iter().map(|_symbol_err| Error {
+        location: None,
+        text: Some("symbol table error".to_string()),
+        stage: Stage::Symbols
+      }).collect();
+      Self {
+        errors,
+        formatted_parse_error: None
+      }
+    }
+
     pub(crate) fn from_string(text: String, stage: Stage) -> Self {
         Self {
             formatted_parse_error: None,

schala-lang/language/src/schala.rs

@@ -81,7 +81,7 @@ impl Schala {
 
     //Perform all symbol table work
     self.symbol_table.borrow_mut().process_ast(&ast)
-      .map_err(|err| SchalaError::from_string(err, Stage::Symbols))?;
+      .map_err(|err| SchalaError::from_symbol_table(err))?;
 
     // Typechecking
     // TODO typechecking not working

schala-lang/language/src/symbol_table/mod.rs

@@ -54,9 +54,11 @@ enum Scope {
 
 #[allow(dead_code)]
 #[derive(Debug, Clone)]
-struct DuplicateName {
+pub enum SymbolError {
-  prev_name: FQSN,
+  DuplicateName {
-  location: Location
+    prev_name: FQSN,
+    location: Location
+  }
 }
 
 #[allow(dead_code)]
@@ -86,10 +88,10 @@ impl<K> NameTable<K> {
     Self { table: HashMap::new() }
   }
 
-  fn register(&mut self, name: FQSN, spec: NameSpec<K>) -> Result<(), DuplicateName> {
+  fn register(&mut self, name: FQSN, spec: NameSpec<K>) -> Result<(), SymbolError> {
     match self.table.entry(name.clone()) {
       Entry::Occupied(o) => {
-        Err(DuplicateName { prev_name: name, location: o.get().location })
+        Err(SymbolError::DuplicateName { prev_name: name, location: o.get().location })
       },
       Entry::Vacant(v) => {
         v.insert(spec);
@@ -132,13 +134,13 @@ impl SymbolTable {
   /// The main entry point into the symbol table. This will traverse the AST in several
   /// different ways and populate subtables with information that will be used further in the
   /// compilation process.
-  pub fn process_ast(&mut self, ast: &ast::AST) -> Result<(), String> {
+  pub fn process_ast(&mut self, ast: &ast::AST) -> Result<(), Vec<SymbolError>> {
 
     let errs = self.populate_name_tables(ast);
     if !errs.is_empty() {
-      return Err(format!("{:?}", errs));
+      return Err(errs);
     }
-    self.resolve_symbol_ids(ast)?;
+    self.resolve_symbol_ids(ast);
     Ok(())
   }
 
@@ -204,22 +206,21 @@ impl SymbolTable {
 
   /// Walks the AST, matching the ID of an identifier used in some expression to
   /// the corresponding Symbol.
-  fn resolve_symbol_ids(&mut self, ast: &ast::AST) -> Result<(), String> {
+  fn resolve_symbol_ids(&mut self, ast: &ast::AST) {
     let mut resolver = resolver::Resolver::new(self);
-    resolver.resolve(ast)?;
+    resolver.resolve(ast);
-    Ok(())
   }
 
   /// This function traverses the AST and adds symbol table entries for
   /// constants, functions, types, and modules defined within. This simultaneously
   /// checks for dupicate definitions (and returns errors if discovered), and sets
   /// up name tables that will be used by further parts of the compiler
-  fn populate_name_tables(&mut self, ast: &ast::AST) -> Vec<DuplicateName> {
+  fn populate_name_tables(&mut self, ast: &ast::AST) -> Vec<SymbolError> {
     let mut scope_stack = vec![];
     self.add_from_scope(ast.statements.as_ref(), &mut scope_stack)
   }
 
-  fn add_from_scope<'a>(&'a mut self, statements: &[Statement], scope_stack: &mut Vec<Scope>) -> Vec<DuplicateName> {
+  fn add_from_scope<'a>(&'a mut self, statements: &[Statement], scope_stack: &mut Vec<Scope>) -> Vec<SymbolError> {
     let mut errors = vec![];
 
     for statement in statements {
@@ -227,34 +228,35 @@ impl SymbolTable {
       let location = *location;
       if let Err(err) = self.add_single_statement(kind, location, &scope_stack) {
         errors.push(err);
+      } else { // If there's an error with a name, don't recurse into subscopes of that name
+        let recursive_errs = match kind {
+          StatementKind::Declaration(Declaration::FuncDecl(signature, body)) => {
+            let new_scope = Scope::Name(signature.name.clone());
+            scope_stack.push(new_scope);
+            let output = self.add_from_scope(body.as_ref(), scope_stack);
+            scope_stack.pop();
+            output
+          }
+          StatementKind::Module(ModuleSpecifier { name, contents }) => {
+            let new_scope = Scope::Name(name.clone());
+            scope_stack.push(new_scope);
+            let output = self.add_from_scope(contents.as_ref(), scope_stack);
+            scope_stack.pop();
+            output
+          }
+          StatementKind::Declaration(Declaration::TypeDecl { name, body, mutable }) => {
+            self.add_type_members(name, body, mutable, location, scope_stack)
+          }
+          _ => vec![]
+        };
+        errors.extend(recursive_errs.into_iter());
       }
-      let recursive_errs = match kind {
-        StatementKind::Declaration(Declaration::FuncDecl(signature, body)) => {
-          let new_scope = Scope::Name(signature.name.clone());
-          scope_stack.push(new_scope);
-          let output = self.add_from_scope(body.as_ref(), scope_stack);
-          scope_stack.pop();
-          output
-        }
-        StatementKind::Module(ModuleSpecifier { name, contents }) => {
-          let new_scope = Scope::Name(name.clone());
-          scope_stack.push(new_scope);
-          let output = self.add_from_scope(contents.as_ref(), scope_stack);
-          scope_stack.pop();
-          output
-        }
-        StatementKind::Declaration(Declaration::TypeDecl { name, body, mutable }) => {
-          self.add_type_members(name, body, mutable, location, scope_stack)
-        }
-        _ => vec![]
-      };
-      errors.extend(recursive_errs.into_iter());
     }
 
     errors
   }
 
-  fn add_single_statement(&mut self, kind: &StatementKind, location: Location, scope_stack: &Vec<Scope>) -> Result<(), DuplicateName> {
+  fn add_single_statement(&mut self, kind: &StatementKind, location: Location, scope_stack: &Vec<Scope>) -> Result<(), SymbolError> {
       match kind {
         StatementKind::Declaration(Declaration::FuncSig(signature)) => {
           let fq_function = FQSN::from_scope_stack(scope_stack.as_ref(), signature.name.clone());
@@ -298,7 +300,7 @@ impl SymbolTable {
       Ok(())
   }
 
-  fn add_type_members(&mut self, type_name: &TypeSingletonName, type_body: &TypeBody, _mutable: &bool, location: Location, scope_stack: &mut Vec<Scope>) -> Vec<DuplicateName> {
+  fn add_type_members(&mut self, type_name: &TypeSingletonName, type_body: &TypeBody, _mutable: &bool, location: Location, scope_stack: &mut Vec<Scope>) -> Vec<SymbolError> {
     let mut errors = vec![];
 
     let mut register = |fqsn: FQSN, spec: SymbolSpec| {

schala-lang/language/src/symbol_table/resolver.rs

@@ -16,9 +16,8 @@ impl<'a> Resolver<'a> {
     let name_scope_stack: ScopeStack<'a, Rc<String>, FQSNPrefix> = ScopeStack::new(None);
     Self { symbol_table, name_scope_stack }
   }
-  pub fn resolve(&mut self, ast: &AST) -> Result<(), String> {
+  pub fn resolve(&mut self, ast: &AST) {
     walk_ast(self, ast);
-    Ok(())
   }
 
   fn lookup_name_in_scope(&self, sym_name: &QualifiedName) -> FQSN {

schala-lang/language/src/symbol_table/test.rs

@@ -1,8 +1,9 @@
 #![cfg(test)]
 use super::*;
+use assert_matches::assert_matches;
 use crate::util::quick_ast;
 
-fn add_symbols(src: &str) -> (SymbolTable, Result<(), String>) {
+fn add_symbols(src: &str) -> (SymbolTable, Result<(), Vec<SymbolError>>) {
   let ast = quick_ast(src);
   let mut symbol_table = SymbolTable::new();
   let result = symbol_table.process_ast(&ast);
@@ -41,8 +42,11 @@ fn no_function_definition_duplicates() {
   fn a() { 3 }
   "#;
   let (_, output) = add_symbols(source);
-  //TODO test for right type of error
+  let errs = output.unwrap_err();
-  output.unwrap_err();
+  assert_matches!(&errs[..], [
+    SymbolError::DuplicateName { prev_name, ..}
+  ] if prev_name == &FQSN::from_strs(&["a"])
+  );
 }
 
 #[test]
@@ -52,13 +56,16 @@ fn no_variable_definition_duplicates() {
   let a = 20
   let q = 39
   let a = 30
+  let x = 34
   "#;
   let (_, output) = add_symbols(source);
-  let _output = output.unwrap_err();
+  let errs = output.unwrap_err();
-  /*
+
-  assert!(output.contains("Duplicate variable definition: a"));
+  assert_matches!(&errs[..], [
-  assert!(output.contains("already defined at 2"));
+    SymbolError::DuplicateName { prev_name: pn1, ..},
-  */
+    SymbolError::DuplicateName { prev_name: pn2, ..}
+  ] if pn1 == &FQSN::from_strs(&["a"]) && pn2 == &FQSN::from_strs(&["x"])
+  );
 }
 
 #[test]
@@ -77,8 +84,11 @@ fn no_variable_definition_duplicates_in_function() {
   }
   "#;
   let (_, output) = add_symbols(source);
-  let _err = output.unwrap_err();
+  let errs = output.unwrap_err();
-  //assert!(output.unwrap_err().contains("Duplicate variable definition: x"))
+  assert_matches!(&errs[..], [
+    SymbolError::DuplicateName { prev_name: pn1, ..},
+  ] if pn1 == &FQSN::from_strs(&["q", "x"])
+  );
 }
 
 #[test]
@@ -183,9 +193,16 @@ fn duplicate_modules() {
     }
 
     module a {
+      fn sarat() { 39 }
       fn foo() { 256.1 }
     }
     "#;
   let (_, output) = add_symbols(source);
-  let _output = output.unwrap_err();
+  let errs = output.unwrap_err();
+
+  assert_matches!(&errs[..], [
+    SymbolError::DuplicateName { prev_name: pn1, ..},
+  ] if pn1 == &FQSN::from_strs(&["a"])
+  );
+
 }