Detect duplicate variable declarations correctly

Later I'll probably want to make it so that you can explicitly override
the value of a declared variable

schala-lang/language/src/eval.rs

@@ -483,8 +483,14 @@ impl<'a> State<'a> {
           _ => unreachable!(),
         },
         SymbolSpec::RecordConstructor { .. } => return Err(format!("This shouldn't be a record!")),
+        SymbolSpec::Binding => match self.values.lookup(&name) {
+          Some(Binding { val, .. }) => val.clone(),
+          None => return Err(format!("Symbol {} exists in symbol table but not in evaluator table", name))
+        }
       },
-      /* see if it's an ordinary variable TODO make variables go in symbol table */
+      //TODO ideally this should be returning a runtime error if this is ever None, but it's not
+      //handling all bindings correctly yet
+      //None => return Err(format!("Couldn't find value {}", name)),
       None => match self.values.lookup(&name) {
         Some(Binding { val, .. }) => val.clone(),
         None => return Err(format!("Couldn't find value {}", name)),

schala-lang/language/src/symbol_table.rs

@@ -76,7 +76,8 @@ pub enum SymbolSpec {
   },
   RecordConstructor {
     fields: HashMap<Rc<String>, Rc<String>>
-  }
+  },
+  Binding
 }
 
 impl fmt::Display for SymbolSpec {
@@ -86,6 +87,7 @@ impl fmt::Display for SymbolSpec {
       Func(type_names) => write!(f, "Func({:?})", type_names),
       DataConstructor { index, type_name, type_args } => write!(f, "DataConstructor(idx: {})({:?} -> {})", index, type_args, type_name),
       RecordConstructor { fields: _fields } => write!(f, "RecordConstructor( <fields> )"),
+      Binding => write!(f, "Binding"),
     }
   }
 }
@@ -95,18 +97,14 @@ impl SymbolTable {
    * later */
 
   pub fn add_top_level_symbols(&mut self, ast: &ast::AST) -> Result<(), String> {
-    let mut seen_identifiers = HashMap::new();
     let mut scope_name_stack = Vec::new();
-    self.add_symbols_from_scope(&ast.0, &mut scope_name_stack, &mut seen_identifiers)
+    self.add_symbols_from_scope(&ast.0, &mut scope_name_stack)
   }
 
-  fn add_symbols_from_scope<'a>(&'a mut self,
-    statements: &Vec<Meta<Statement>>,
-    scope_name_stack: &mut Vec<Rc<String>>,
-    seen_identifiers: &mut SymbolTrackTable) -> Result<(), String> {
+  fn add_symbols_from_scope<'a>(&'a mut self, statements: &Vec<Meta<Statement>>, scope_name_stack: &mut Vec<Rc<String>>) -> Result<(), String> {
     use self::ast::Declaration::*;
 
-    fn check_symbol(table: &mut SymbolTrackTable, name: &Rc<String>) -> Result<(), String> {
+    fn insert_and_check_duplicate_symbol(table: &mut SymbolTrackTable, name: &Rc<String>) -> Result<(), String> {
       match table.entry(name.clone()) {
         Entry::Occupied(o) => {
           let line_number = o.get(); //TODO make this actually work
@@ -120,24 +118,33 @@ impl SymbolTable {
       }
     }
 
+    let mut seen_identifiers: SymbolTrackTable = HashMap::new();
+
     for meta in statements.iter() {
       let statement = meta.node();
       if let Statement::Declaration(decl) = statement {
         match decl {
           FuncSig(ref signature) => {
-            check_symbol(seen_identifiers, &signature.name)?;
+            insert_and_check_duplicate_symbol(&mut seen_identifiers, &signature.name)?;
             self.add_function_signature(signature, scope_name_stack)?
           }
           FuncDecl(ref signature, ref body) => {
-            check_symbol(seen_identifiers, &signature.name)?;
+            insert_and_check_duplicate_symbol(&mut seen_identifiers, &signature.name)?;
             self.add_function_signature(signature, scope_name_stack)?;
-            let mut subscope_seen_identifiers = HashMap::new();
             scope_name_stack.push(signature.name.clone());
-            let output = self.add_symbols_from_scope(body, scope_name_stack, &mut subscope_seen_identifiers);
+            let output = self.add_symbols_from_scope(body, scope_name_stack);
             let _ = scope_name_stack.pop();
             output?
           },
-          TypeDecl { name, body, mutable } => self.add_type_decl(name, body, mutable, scope_name_stack)?,
+          TypeDecl { name, body, mutable } => {
+            insert_and_check_duplicate_symbol(&mut seen_identifiers, &name.name)?;
+            self.add_type_decl(name, body, mutable, scope_name_stack)?
+          },
+          Binding { name, .. } => {
+            insert_and_check_duplicate_symbol(&mut seen_identifiers, name)?;
+            let symbol = Symbol { name: name.clone(), spec: SymbolSpec::Binding };
+            self.add_new_symbol(name, scope_name_stack, symbol);
+          }
           _ => ()
         }
       }
@@ -263,6 +270,55 @@ mod symbol_table_tests {
     assert!(output.contains("Duplicate"))
   }
 
+  #[test]
+  fn no_duplicates_2() {
+    let source = r#"
+    let a = 20;
+    let q = 39;
+    let a = 30;
+    "#;
+    let mut symbol_table = SymbolTable::new();
+    let ast = quick_ast(source);
+    let output = symbol_table.add_top_level_symbols(&ast).unwrap_err();
+    assert!(output.contains("Duplicate"))
+  }
+
+  #[test]
+  fn no_duplicates_3() {
+    let source = r#"
+    fn a() {
+      let a = 20
+      let b = 40
+      a + b
+    }
+
+    fn q() {
+      let x = 30
+      let x = 33
+    }
+    "#;
+    let mut symbol_table = SymbolTable::new();
+    let ast = quick_ast(source);
+    let output = symbol_table.add_top_level_symbols(&ast).unwrap_err();
+    assert!(output.contains("Duplicate"))
+  }
+
+  fn dont_falsely_detect_duplicates() {
+    let source = r#"
+    let a = 20;
+    fn some_func() {
+      let a = 40;
+      77
+    }
+    let q = 39;
+    "#;
+    let mut symbol_table = SymbolTable::new();
+    let ast = quick_ast(source);
+    symbol_table.add_top_level_symbols(&ast).unwrap();
+    assert!(symbol_table.lookup_by_path(&rc!(a), &vec![]).is_some());
+    assert!(symbol_table.lookup_by_path(&rc!(a), &vec![rc!(some_func)]).is_some());
+  }
+
   #[test]
   fn enclosing_scopes() {
     let source = r#"