Merge pull request RustPython#1341 from RustPython/coolreader18/method-scopes

Fix class vs method scopes

Author: coolreader18

compiler/src/symboltable.rs

@@ -61,7 +61,7 @@ impl SymbolTable {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Copy, PartialEq)]
 pub enum SymbolTableType {
     Module,
     Class,
@@ -179,24 +179,24 @@ fn analyze_symbol_table(symbol_table: &mut SymbolTable) -> SymbolTableResult {
 /// build symbol table structure. It will mark variables
 /// as local variables for example.
 #[derive(Default)]
-struct SymbolTableAnalyzer {
-    tables: Vec<SymbolTable>,
+struct SymbolTableAnalyzer<'a> {
+    tables: Vec<(&'a mut IndexMap<String, Symbol>, SymbolTableType)>,
 }
 
-impl SymbolTableAnalyzer {
-    fn analyze_symbol_table(&mut self, symbol_table: &mut SymbolTable) -> SymbolTableResult {
-        // Store a copy to determine the parent.
-        // TODO: this should be improved to resolve this clone action.
-        self.tables.push(symbol_table.clone());
+impl<'a> SymbolTableAnalyzer<'a> {
+    fn analyze_symbol_table(&mut self, symbol_table: &'a mut SymbolTable) -> SymbolTableResult {
+        let symbols = &mut symbol_table.symbols;
+        let sub_tables = &mut symbol_table.sub_tables;
 
+        self.tables.push((symbols, symbol_table.typ));
         // Analyze sub scopes:
-        for sub_table in &mut symbol_table.sub_tables {
+        for sub_table in sub_tables {
             self.analyze_symbol_table(sub_table)?;
         }
-        self.tables.pop();
+        let (symbols, _) = self.tables.pop().unwrap();
 
         // Analyze symbols:
-        for symbol in symbol_table.symbols.values_mut() {
+        for symbol in symbols.values_mut() {
             self.analyze_symbol(symbol)?;
         }
 
@@ -207,11 +207,11 @@ impl SymbolTableAnalyzer {
         match symbol.scope {
             SymbolScope::Nonlocal => {
                 // check if name is defined in parent table!
-                let parent_symbol_table: Option<&SymbolTable> = self.tables.last();
+                let parent_symbol_table = self.tables.last();
                 // symbol.table.borrow().parent.clone();
 
-                if let Some(table) = parent_symbol_table {
-                    if !table.symbols.contains_key(&symbol.name) {
+                if let Some((symbols, _)) = parent_symbol_table {
+                    if !symbols.contains_key(&symbol.name) {
                         return Err(SymbolTableError {
                             error: format!("no binding for nonlocal '{}' found", symbol.name),
                             location: Default::default(),
@@ -241,12 +241,10 @@ impl SymbolTableAnalyzer {
                 } else {
                     // Interesting stuff about the __class__ variable:
                     // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object
-                    let found_in_outer_scope = (symbol.name == "__class__")
-                        || self
-                            .tables
-                            .iter()
-                            .skip(1)
-                            .any(|t| t.symbols.contains_key(&symbol.name));
+                    let found_in_outer_scope = symbol.name == "__class__"
+                        || self.tables.iter().skip(1).any(|(symbols, typ)| {
+                            *typ != SymbolTableType::Class && symbols.contains_key(&symbol.name)
+                        });
 
                     if found_in_outer_scope {
                         // Symbol is in some outer scope.
@@ -387,7 +385,6 @@ impl SymbolTableBuilder {
                 keywords,
                 decorator_list,
             } => {
-                self.register_name(name, SymbolUsage::Assigned)?;
                 self.enter_scope(name, SymbolTableType::Class, statement.location.row());
                 self.scan_statements(body)?;
                 self.leave_scope();
@@ -396,6 +393,7 @@ impl SymbolTableBuilder {
                     self.scan_expression(&keyword.value, &ExpressionContext::Load)?;
                 }
                 self.scan_expressions(decorator_list, &ExpressionContext::Load)?;
+                self.register_name(name, SymbolUsage::Assigned)?;
             }
             Expression { expression } => {
                 self.scan_expression(expression, &ExpressionContext::Load)?

tests/snippets/class.py

@@ -164,3 +164,28 @@ class T5(int):
 assert str(super(int, T5(5))) == "<super: <class 'int'>, <T5 object>>"
 
 #assert str(super(type, None)) == "<super: <class 'type'>, NULL>"
+
+a = 1
+class A:
+    a = 2
+    def b():
+        assert a == 1
+    b()
+    assert a == 2
+A.b()
+
+# TODO: uncomment once free vars/cells are working
+# The symboltable sees that b() is referring to a in the nested scope,
+# so it marks it as non local. When it's executed, it walks up the scopes
+# and still finds the a from the class scope.
+# a = 1
+# def nested_scope():
+#     a = 2
+#     class A:
+#         a = 3
+#         def b():
+#             assert a == 2
+#         b()
+#         assert a == 3
+#     A.b()
+# nested_scope()