Add check for nonlocal on top level.

windelbouwman · windelbouwman · commit ce8ea805ae8a · 2019-04-21T19:40:14.000+02:00
diff --git a/tests/snippets/global_nonlocal.py b/tests/snippets/global_nonlocal.py
@@ -11,6 +11,7 @@ def b():
 b()
 assert a == 4
 
+
 def x():
     def y():
         nonlocal b
@@ -21,3 +22,37 @@ def y():
 
 res = x()
 assert res == 3, str(res)
+
+# Invalid syntax:
+src = """
+b = 2
+global b
+"""
+
+try:
+    exec(src)
+except Exception as ex:
+    # print(ex)
+    pass
+else:
+    raise RuntimeError('Must raise')
+
+# Invalid syntax:
+
+src = """
+nonlocal c
+"""
+
+try:
+    exec(src)
+except Exception as ex:
+    # print(ex)
+    pass
+else:
+    raise RuntimeError('Must raise')
+
+
+# class X:
+#     nonlocal c
+#     c = 2
+
diff --git a/vm/src/compile.rs b/vm/src/compile.rs
@@ -41,17 +41,17 @@ pub fn compile(
     match mode {
         Mode::Exec => {
             let ast = parser::parse_program(source)?;
-            let symbol_table = make_symbol_table(&ast);
+            let symbol_table = make_symbol_table(&ast)?;
             compiler.compile_program(&ast, symbol_table)
         }
         Mode::Eval => {
             let statement = parser::parse_statement(source)?;
-            let symbol_table = statements_to_symbol_table(&statement);
+            let symbol_table = statements_to_symbol_table(&statement)?;
             compiler.compile_statement_eval(&statement, symbol_table)
         }
         Mode::Single => {
             let ast = parser::parse_program(source)?;
-            let symbol_table = make_symbol_table(&ast);
+            let symbol_table = make_symbol_table(&ast)?;
             compiler.compile_program_single(&ast, symbol_table)
         }
     }?;
@@ -1776,7 +1776,7 @@ mod tests {
         compiler.source_path = Some("source_path".to_string());
         compiler.push_new_code_object("<module>".to_string());
         let ast = parser::parse_program(&source.to_string()).unwrap();
-        let symbol_scope = make_symbol_table(&ast);
+        let symbol_scope = make_symbol_table(&ast).unwrap();
         compiler.compile_program(&ast, symbol_scope).unwrap();
         compiler.pop_code_object()
     }
diff --git a/vm/src/error.rs b/vm/src/error.rs
@@ -29,6 +29,7 @@ pub enum CompileErrorType {
     ExpectExpr,
     /// Parser error
     Parse(ParseError),
+    SyntaxError(String),
     /// Multiple `*` detected
     StarArgs,
     /// Break statement outside of loop.
@@ -46,6 +47,7 @@ impl fmt::Display for CompileError {
             CompileErrorType::Delete(target) => write!(f, "can't delete {}", target),
             CompileErrorType::ExpectExpr => write!(f, "Expecting expression, got statement"),
             CompileErrorType::Parse(err) => write!(f, "{}", err),
+            CompileErrorType::SyntaxError(err) => write!(f, "{}", err),
             CompileErrorType::StarArgs => write!(f, "Two starred expressions in assignment"),
             CompileErrorType::InvalidBreak => write!(f, "'break' outside loop"),
             CompileErrorType::InvalidContinue => write!(f, "'continue' outside loop"),
diff --git a/vm/src/symboltable.rs b/vm/src/symboltable.rs
@@ -5,23 +5,27 @@ Then the compiler can use the symbol table to generate proper
 load and store instructions for names.
 */
 
+use crate::error::{CompileError, CompileErrorType};
 use rustpython_parser::ast;
+use rustpython_parser::lexer::Location;
 use std::collections::HashMap;
 
-pub fn make_symbol_table(program: &ast::Program) -> SymbolScope {
+pub fn make_symbol_table(program: &ast::Program) -> Result<SymbolScope, SymbolTableError> {
     let mut builder = SymbolTableBuilder::new();
     builder.enter_scope();
-    builder.scan_program(program).unwrap();
+    builder.scan_program(program)?;
     assert!(builder.scopes.len() == 1);
-    builder.scopes.pop().unwrap()
+    Ok(builder.scopes.pop().unwrap())
 }
 
-pub fn statements_to_symbol_table(statements: &[ast::LocatedStatement]) -> SymbolScope {
+pub fn statements_to_symbol_table(
+    statements: &[ast::LocatedStatement],
+) -> Result<SymbolScope, SymbolTableError> {
     let mut builder = SymbolTableBuilder::new();
     builder.enter_scope();
-    builder.scan_statements(statements).unwrap();
+    builder.scan_statements(statements)?;
     assert!(builder.scopes.len() == 1);
-    builder.scopes.pop().unwrap()
+    Ok(builder.scopes.pop().unwrap())
 }
 
 #[derive(Debug)]
@@ -43,7 +47,22 @@ pub struct SymbolScope {
     pub sub_scopes: Vec<SymbolScope>,
 }
 
-type SymbolTableResult = Result<(), String>;
+#[derive(Debug)]
+pub struct SymbolTableError {
+    error: String,
+    location: Location,
+}
+
+impl From<SymbolTableError> for CompileError {
+    fn from(error: SymbolTableError) -> Self {
+        CompileError {
+            error: CompileErrorType::SyntaxError(error.error),
+            location: error.location,
+        }
+    }
+}
+
+type SymbolTableResult = Result<(), SymbolTableError>;
 
 impl SymbolScope {
     pub fn new() -> Self {
@@ -85,7 +104,7 @@ impl SymbolTableBuilder {
         self.scopes.push(scope);
     }
 
-    pub fn leave_scope(&mut self) {
+    fn leave_scope(&mut self) {
         // Pop scope and add to subscopes of parent scope.
         let scope = self.scopes.pop().unwrap();
         self.scopes.last_mut().unwrap().sub_scopes.push(scope);
@@ -445,17 +464,37 @@ impl SymbolTableBuilder {
     }
 
     fn register_name(&mut self, name: &str, role: SymbolRole) -> SymbolTableResult {
+        let scope_depth = self.scopes.len();
         let current_scope = self.scopes.last_mut().unwrap();
+        let location = Default::default();
         if let Some(_old_role) = current_scope.symbols.get(name) {
             // Role already set..
             // debug!("TODO: {:?}", old_role);
             match role {
-                SymbolRole::Global => return Err("global must appear first".to_string()),
+                SymbolRole::Global => {
+                    return Err(SymbolTableError {
+                        error: format!("name '{}' is used prior to global declaration", name),
+                        location,
+                    })
+                }
                 _ => {
                     // Ok?
                 }
             }
         } else {
+            match role {
+                SymbolRole::Nonlocal => {
+                    if scope_depth < 2 {
+                        return Err(SymbolTableError {
+                            error: format!("name '{}' is used prior to global declaration", name),
+                            location,
+                        });
+                    }
+                }
+                _ => {
+                    // Ok!
+                }
+            }
             current_scope.symbols.insert(name.to_string(), role);
         }
         Ok(())
