fix: fix duplicate variable name and schema attr name in right value (#…

…1523) * fix: fix duplicate variable name in right value goto_def, and schema attr name complete in right value * fix ut Signed-off-by: he1pa <[email protected]> --------- Signed-off-by: he1pa <[email protected]>
kcl-lang · Jul 26, 2024 · 42844f3 · 42844f3
1 parent 3cbb93f
commit 42844f3
Show file tree

Hide file tree

Showing 14 changed files with 190 additions and 46 deletions.
diff --git a/kclvm/loader/src/lib.rs b/kclvm/loader/src/lib.rs
@@ -274,6 +274,18 @@ fn collect_scope_info(
         } else {
             kind
         };
+        let get_def_from_owner = match scope_ref.get_kind() {
+            kclvm_sema::core::scope::ScopeKind::Local => {
+                match scope_data.try_get_local_scope(&scope_ref) {
+                    Some(local) => match local.get_kind() {
+                        LocalSymbolScopeKind::SchemaConfig | LocalSymbolScopeKind::Check => true,
+                        _ => false,
+                    },
+                    None => false,
+                }
+            }
+            kclvm_sema::core::scope::ScopeKind::Root => false,
+        };
         scopes.insert(
             *scope_ref,
             ScopeInfo {
@@ -282,12 +294,13 @@ fn collect_scope_info(
                 owner: scope.get_owner(),
                 children: scope.get_children(),
                 defs: scope
-                    .get_all_defs(scope_data, symbol_data, None, false)
+                    .get_all_defs(scope_data, symbol_data, None, false, get_def_from_owner)
                     .values()
                     .copied()
                     .collect::<Vec<_>>(),
             },
         );
+
         for s in scope.get_children() {
             collect_scope_info(scopes, &s, scope_data, symbol_data, ScopeKind::Module);
         }

diff --git a/kclvm/sema/src/advanced_resolver/mod.rs b/kclvm/sema/src/advanced_resolver/mod.rs
@@ -81,6 +81,8 @@ pub struct Context<'ctx> {
     // which means advanced resolver will will create the corresponding
     // ValueSymbol instead of an UnresolvedSymbol
     maybe_def: bool,
+    // whether lookup def in scope owner, default true, only in schema attr value is false
+    look_up_in_owner: bool,
 }
 
 impl<'ctx> Context<'ctx> {
@@ -111,6 +113,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
                 end_pos: Position::dummy_pos(),
                 cur_node: AstIndex::default(),
                 maybe_def: false,
+                look_up_in_owner: true,
             },
         };
         // Scan all scehma symbol
@@ -1404,7 +1407,7 @@ mod tests {
                     .replace("/", &std::path::MAIN_SEPARATOR.to_string()),
                 17_u64,
                 26_u64,
-                10_usize,
+                5_usize,
             ),
             // __main__.Main schema expr scope
             (
@@ -1422,7 +1425,7 @@ mod tests {
                     .replace("/", &std::path::MAIN_SEPARATOR.to_string()),
                 30,
                 20,
-                10,
+                5,
             ),
             // pkg.Person schema expr scope
             (
@@ -1440,7 +1443,7 @@ mod tests {
                     .replace("/", &std::path::MAIN_SEPARATOR.to_string()),
                 34,
                 17,
-                6,
+                5,
             ),
             // __main__ package scope
             (
@@ -1458,7 +1461,7 @@ mod tests {
                     .replace("/", &std::path::MAIN_SEPARATOR.to_string()),
                 15,
                 11,
-                6,
+                4,
             ),
             // import_test.a.Name expr scope
             (
@@ -1476,7 +1479,7 @@ mod tests {
                     .replace("/", &std::path::MAIN_SEPARATOR.to_string()),
                 12,
                 21,
-                8,
+                4,
             ),
         ];
 
@@ -1489,7 +1492,6 @@ mod tests {
                     column: Some(*col),
                 })
                 .unwrap();
-
             let all_defs = gs.get_all_defs_in_scope(scope_ref).unwrap();
             assert_eq!(all_defs.len(), *def_num)
         }

diff --git a/kclvm/sema/src/advanced_resolver/node.rs b/kclvm/sema/src/advanced_resolver/node.rs
@@ -1043,6 +1043,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
             cur_scope,
             self.get_current_module_info(),
             maybe_def,
+            self.ctx.look_up_in_owner,
         );
         if first_symbol.is_none() {
             // Maybe import package symbol
@@ -1250,6 +1251,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
             cur_scope,
             self.get_current_module_info(),
             true,
+            self.ctx.look_up_in_owner,
         );
         match first_symbol {
             Some(symbol_ref) => {
@@ -1785,7 +1787,9 @@ impl<'ctx> AdvancedResolver<'ctx> {
         for entry in entries.iter() {
             if let Some(key) = &entry.node.key {
                 self.ctx.maybe_def = true;
+                self.ctx.look_up_in_owner = true;
                 self.expr(key)?;
+                self.ctx.look_up_in_owner = false;
                 self.ctx.maybe_def = false;
             }
 
@@ -1797,8 +1801,9 @@ impl<'ctx> AdvancedResolver<'ctx> {
                 end,
                 LocalSymbolScopeKind::Value,
             );
-
+            self.ctx.look_up_in_owner = false;
             self.expr(&entry.node.value)?;
+            self.ctx.look_up_in_owner = true;
             self.leave_scope();
         }
         self.leave_scope();

diff --git a/kclvm/sema/src/core/global_state.rs b/kclvm/sema/src/core/global_state.rs
@@ -86,13 +86,15 @@ impl GlobalState {
         scope_ref: ScopeRef,
         module_info: Option<&ModuleInfo>,
         local: bool,
+        get_def_from_owner: bool,
     ) -> Option<SymbolRef> {
         match self.scopes.get_scope(&scope_ref)?.look_up_def(
             name,
             &self.scopes,
             &self.symbols,
             module_info,
             local,
+            get_def_from_owner,
         ) {
             None => self
                 .symbols
@@ -179,15 +181,28 @@ impl GlobalState {
     ///
     /// result: [Option<Vec<SymbolRef>>]
     ///      all definition symbols in the scope
-    pub fn get_all_defs_in_scope(&self, scope: ScopeRef) -> Option<Vec<SymbolRef>> {
+    pub fn get_all_defs_in_scope(&self, scope_ref: ScopeRef) -> Option<Vec<SymbolRef>> {
         let scopes = &self.scopes;
-        let scope = scopes.get_scope(&scope)?;
+        let scope = scopes.get_scope(&scope_ref)?;
+        let get_def_from_owner = match scope_ref.kind {
+            ScopeKind::Local => match scopes.try_get_local_scope(&scope_ref) {
+                Some(local) => match local.kind {
+                    super::scope::LocalSymbolScopeKind::SchemaConfig
+                    | super::scope::LocalSymbolScopeKind::Check => true,
+                    _ => false,
+                },
+                None => true,
+            },
+            ScopeKind::Root => true,
+        };
+
         let all_defs: Vec<SymbolRef> = scope
             .get_all_defs(
                 scopes,
                 &self.symbols,
                 self.packages.get_module_info(scope.get_filename()),
                 false,
+                get_def_from_owner,
             )
             .values()
             .into_iter()
@@ -208,15 +223,28 @@ impl GlobalState {
     ///
     /// result: [Option<Vec<SymbolRef>>]
     ///      all definition symbols in the scope
-    pub fn get_defs_within_scope(&self, scope: ScopeRef) -> Option<Vec<SymbolRef>> {
+    pub fn get_defs_within_scope(&self, scope_ref: ScopeRef) -> Option<Vec<SymbolRef>> {
         let scopes = &self.scopes;
-        let scope = scopes.get_scope(&scope)?;
+        let get_def_from_owner = match scope_ref.kind {
+            ScopeKind::Local => match scopes.try_get_local_scope(&scope_ref) {
+                Some(local) => match local.kind {
+                    super::scope::LocalSymbolScopeKind::SchemaConfig
+                    | super::scope::LocalSymbolScopeKind::Check => true,
+                    _ => false,
+                },
+                None => false,
+            },
+            ScopeKind::Root => false,
+        };
+
+        let scope = scopes.get_scope(&scope_ref)?;
         let all_defs: Vec<SymbolRef> = scope
             .get_defs_within_scope(
                 scopes,
                 &self.symbols,
                 self.packages.get_module_info(scope.get_filename()),
                 false,
+                get_def_from_owner,
             )
             .values()
             .into_iter()