[red-knot] Remove Scope::name (#12137)

crates/red_knot_python_semantic/src/semantic_index.rs

@@ -307,8 +307,8 @@ mod tests {
     use ruff_db::vfs::{system_path_to_file, VfsFile};
 
     use crate::db::tests::TestDb;
-    use crate::semantic_index::symbol::{FileScopeId, ScopeKind, SymbolTable};
-    use crate::semantic_index::{root_scope, semantic_index, symbol_table};
+    use crate::semantic_index::symbol::{FileScopeId, FileSymbolId, Scope, ScopeKind, SymbolTable};
+    use crate::semantic_index::{root_scope, semantic_index, symbol_table, SemanticIndex};
 
     struct TestCase {
         db: TestDb,
@@ -440,12 +440,18 @@ y = 2
 
         let index = semantic_index(&db, file);
 
+        let root = index.symbol_table(FileScopeId::root());
         let scopes: Vec<_> = index.child_scopes(FileScopeId::root()).collect();
         assert_eq!(scopes.len(), 1);
 
         let (class_scope_id, class_scope) = scopes[0];
         assert_eq!(class_scope.kind(), ScopeKind::Class);
-        assert_eq!(class_scope.name(), "C");
+        assert_eq!(
+            class_scope
+                .defining_symbol()
+                .map(super::symbol::FileSymbolId::scoped_symbol_id),
+            root.symbol_id_by_name("C")
+        );
 
         let class_table = index.symbol_table(class_scope_id);
         assert_eq!(names(&class_table), vec!["x"]);
@@ -474,7 +480,12 @@ y = 2
 
         let (function_scope_id, function_scope) = scopes[0];
         assert_eq!(function_scope.kind(), ScopeKind::Function);
-        assert_eq!(function_scope.name(), "func");
+        assert_eq!(
+            function_scope
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("func")
+        );
 
         let function_table = index.symbol_table(function_scope_id);
         assert_eq!(names(&function_table), vec!["x"]);
@@ -509,9 +520,20 @@ def func():
         let (func_scope2_id, func_scope_2) = scopes[1];
 
         assert_eq!(func_scope_1.kind(), ScopeKind::Function);
-        assert_eq!(func_scope_1.name(), "func");
+
+        assert_eq!(
+            func_scope_1
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("func")
+        );
         assert_eq!(func_scope_2.kind(), ScopeKind::Function);
-        assert_eq!(func_scope_2.name(), "func");
+        assert_eq!(
+            func_scope_2
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("func")
+        );
 
         let func1_table = index.symbol_table(func_scope1_id);
         let func2_table = index.symbol_table(func_scope2_id);
@@ -546,7 +568,12 @@ def func[T]():
         let (ann_scope_id, ann_scope) = scopes[0];
 
         assert_eq!(ann_scope.kind(), ScopeKind::Annotation);
-        assert_eq!(ann_scope.name(), "func");
+        assert_eq!(
+            ann_scope
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("func")
+        );
         let ann_table = index.symbol_table(ann_scope_id);
         assert_eq!(names(&ann_table), vec!["T"]);
 
@@ -554,7 +581,12 @@ def func[T]():
         assert_eq!(scopes.len(), 1);
         let (func_scope_id, func_scope) = scopes[0];
         assert_eq!(func_scope.kind(), ScopeKind::Function);
-        assert_eq!(func_scope.name(), "func");
+        assert_eq!(
+            func_scope
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("func")
+        );
         let func_table = index.symbol_table(func_scope_id);
         assert_eq!(names(&func_table), vec!["x"]);
     }
@@ -578,7 +610,12 @@ class C[T]:
         assert_eq!(scopes.len(), 1);
         let (ann_scope_id, ann_scope) = scopes[0];
         assert_eq!(ann_scope.kind(), ScopeKind::Annotation);
-        assert_eq!(ann_scope.name(), "C");
+        assert_eq!(
+            ann_scope
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("C")
+        );
         let ann_table = index.symbol_table(ann_scope_id);
         assert_eq!(names(&ann_table), vec!["T"]);
         assert!(
@@ -590,10 +627,15 @@ class C[T]:
 
         let scopes: Vec<_> = index.child_scopes(ann_scope_id).collect();
         assert_eq!(scopes.len(), 1);
-        let (func_scope_id, func_scope) = scopes[0];
+        let (func_scope_id, class_scope) = scopes[0];
 
-        assert_eq!(func_scope.kind(), ScopeKind::Class);
-        assert_eq!(func_scope.name(), "C");
+        assert_eq!(class_scope.kind(), ScopeKind::Class);
+        assert_eq!(
+            class_scope
+                .defining_symbol()
+                .map(FileSymbolId::scoped_symbol_id),
+            root_table.symbol_id_by_name("C")
+        );
         assert_eq!(names(&index.symbol_table(func_scope_id)), vec!["x"]);
     }
 
@@ -654,6 +696,27 @@ class C[T]:
 
     #[test]
     fn scope_iterators() {
+        fn scope_names<'a>(
+            scopes: impl Iterator<Item = (FileScopeId, &'a Scope)>,
+            index: &'a SemanticIndex,
+        ) -> Vec<&'a str> {
+            let mut names = Vec::new();
+
+            for (_, scope) in scopes {
+                if let Some(defining_symbol) = scope.defining_symbol {
+                    let symbol_table = &index.symbol_tables[defining_symbol.scope()];
+                    let symbol = symbol_table.symbol(defining_symbol.scoped_symbol_id());
+                    names.push(symbol.name().as_str());
+                } else if scope.parent.is_none() {
+                    names.push("<module>");
+                } else {
+                    panic!("Unsupported");
+                }
+            }
+
+            names
+        }
+
         let TestCase { db, file } = test_case(
             r#"
 class Test:
@@ -669,35 +732,29 @@ def x():
 
         let index = semantic_index(&db, file);
 
-        let descendents: Vec<_> = index
-            .descendent_scopes(FileScopeId::root())
-            .map(|(_, scope)| scope.name().as_str())
-            .collect();
-        assert_eq!(descendents, vec!["Test", "foo", "bar", "baz", "x"]);
+        let descendents = index.descendent_scopes(FileScopeId::root());
+        assert_eq!(
+            scope_names(descendents, index),
+            vec!["Test", "foo", "bar", "baz", "x"]
+        );
 
-        let children: Vec<_> = index
-            .child_scopes(FileScopeId::root())
-            .map(|(_, scope)| scope.name.as_str())
-            .collect();
-        assert_eq!(children, vec!["Test", "x"]);
+        let children = index.child_scopes(FileScopeId::root());
+        assert_eq!(scope_names(children, index), vec!["Test", "x"]);
 
         let test_class = index.child_scopes(FileScopeId::root()).next().unwrap().0;
-        let test_child_scopes: Vec<_> = index
-            .child_scopes(test_class)
-            .map(|(_, scope)| scope.name.as_str())
-            .collect();
-        assert_eq!(test_child_scopes, vec!["foo", "baz"]);
+        let test_child_scopes = index.child_scopes(test_class);
+        assert_eq!(scope_names(test_child_scopes, index), vec!["foo", "baz"]);
 
         let bar_scope = index
             .descendent_scopes(FileScopeId::root())
             .nth(2)
             .unwrap()
             .0;
-        let ancestors: Vec<_> = index
-            .ancestor_scopes(bar_scope)
-            .map(|(_, scope)| scope.name())
-            .collect();
+        let ancestors = index.ancestor_scopes(bar_scope);
 
-        assert_eq!(ancestors, vec!["bar", "foo", "Test", "<module>"]);
+        assert_eq!(
+            scope_names(ancestors, index),
+            vec!["bar", "foo", "Test", "<module>"]
+        );
     }
 }

crates/red_knot_python_semantic/src/semantic_index/builder.rs

@@ -48,11 +48,7 @@ impl<'a> SemanticIndexBuilder<'a> {
         };
 
         builder.push_scope_with_parent(
-            NodeWithScope::new(
-                parsed.syntax(),
-                NodeWithScopeId::Module,
-                Name::new_static("<module>"),
-            ),
+            &NodeWithScope::new(parsed.syntax(), NodeWithScopeId::Module),
             None,
             None,
             None,
@@ -70,7 +66,7 @@ impl<'a> SemanticIndexBuilder<'a> {
 
     fn push_scope(
         &mut self,
-        node: NodeWithScope,
+        node: &NodeWithScope,
         defining_symbol: Option<FileSymbolId>,
         definition: Option<Definition>,
     ) {
@@ -80,7 +76,7 @@ impl<'a> SemanticIndexBuilder<'a> {
 
     fn push_scope_with_parent(
         &mut self,
-        node: NodeWithScope,
+        node: &NodeWithScope,
         defining_symbol: Option<FileSymbolId>,
         definition: Option<Definition>,
         parent: Option<FileScopeId>,
@@ -91,7 +87,6 @@ impl<'a> SemanticIndexBuilder<'a> {
         let scope_kind = node.scope_kind();
 
         let scope = Scope {
-            name: node.name,
             parent,
             defining_symbol,
             definition,
@@ -154,7 +149,6 @@ impl<'a> SemanticIndexBuilder<'a> {
 
     fn with_type_params(
         &mut self,
-        name: Name,
         with_params: &WithTypeParams,
         defining_symbol: FileSymbolId,
         nested: impl FnOnce(&mut Self) -> FileScopeId,
@@ -168,7 +162,7 @@ impl<'a> SemanticIndexBuilder<'a> {
             };
 
             self.push_scope(
-                NodeWithScope::new(type_params, type_params_id, name),
+                &NodeWithScope::new(type_params, type_params_id),
                 Some(defining_symbol),
                 Some(with_params.definition()),
             );
@@ -254,7 +248,6 @@ impl Visitor<'_> for SemanticIndexBuilder<'_> {
                 );
 
                 self.with_type_params(
-                    name.clone(),
                     &WithTypeParams::FunctionDef {
                         node: function_def,
                         id: AstId::new(scope, function_id),
@@ -267,10 +260,9 @@ impl Visitor<'_> for SemanticIndexBuilder<'_> {
                         }
 
                         builder.push_scope(
-                            NodeWithScope::new(
+                            &NodeWithScope::new(
                                 function_def,
                                 NodeWithScopeId::Function(AstId::new(scope, function_id)),
-                                name.clone(),
                             ),
                             Some(symbol),
                             Some(definition),
@@ -294,7 +286,6 @@ impl Visitor<'_> for SemanticIndexBuilder<'_> {
                     self.add_or_update_symbol_with_definition(name.clone(), definition),
                 );
                 self.with_type_params(
-                    name.clone(),
                     &WithTypeParams::ClassDef {
                         node: class,
                         id: AstId::new(scope, class_id),
@@ -306,10 +297,9 @@ impl Visitor<'_> for SemanticIndexBuilder<'_> {
                         }
 
                         builder.push_scope(
-                            NodeWithScope::new(
+                            &NodeWithScope::new(
                                 class,
                                 NodeWithScopeId::Class(AstId::new(scope, class_id)),
-                                name.clone(),
                             ),
                             Some(id),
                             Some(definition),
@@ -471,15 +461,13 @@ impl<'a> WithTypeParams<'a> {
 struct NodeWithScope {
     id: NodeWithScopeId,
     key: NodeWithScopeKey,
-    name: Name,
 }
 
 impl NodeWithScope {
-    fn new(node: impl Into<NodeWithScopeKey>, id: NodeWithScopeId, name: Name) -> Self {
+    fn new(node: impl Into<NodeWithScopeKey>, id: NodeWithScopeId) -> Self {
         Self {
             id,
             key: node.into(),
-            name,
         }
     }
 

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

@@ -214,7 +214,6 @@ impl FileScopeId {
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct Scope {
-    pub(super) name: Name,
     pub(super) parent: Option<FileScopeId>,
     pub(super) definition: Option<Definition>,
     pub(super) defining_symbol: Option<FileSymbolId>,
@@ -223,10 +222,6 @@ pub struct Scope {
 }
 
 impl Scope {
-    pub fn name(&self) -> &Name {
-        &self.name
-    }
-
     pub fn definition(&self) -> Option<Definition> {
         self.definition
     }