bugfix: pkg completion without dot (#705)

* bugfix: pkg completion without dot. Sometimes lsp handle completion request before didChange notification and without trigger character dot in vfs. fix pkg completion in this case * fix kcl lint err
kcl-lang · Sep 13, 2023 · 6c72ecb · 6c72ecb
1 parent 039a57b
commit 6c72ecb
Show file tree

Hide file tree

Showing 8 changed files with 234 additions and 20 deletions.
diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs
@@ -147,6 +147,28 @@ impl<'ctx> Resolver<'ctx> {
                                             kind.clone(),
                                         );
                                         let (start, end) = stmt.get_span_pos();
+
+                                        let name = match &import_stmt.asname {
+                                            Some(name) => name.clone(),
+                                            None => import_stmt.name.clone(),
+                                        };
+                                        scope.elems.insert(
+                                            name.clone(),
+                                            Rc::new(RefCell::new(ScopeObject {
+                                                name,
+                                                start: start.clone(),
+                                                end: end.clone(),
+                                                ty: Rc::new(ty.clone()),
+                                                kind: ScopeObjectKind::Module(Module {
+                                                    path: import_stmt.path.clone(),
+                                                    rawpath: import_stmt.rawpath.clone(),
+                                                    name: import_stmt.name.clone(),
+                                                    asname: import_stmt.asname.clone(),
+                                                }),
+                                                used: true,
+                                                doc: None,
+                                            })),
+                                        );
                                         scope.elems.insert(
                                             import_stmt.path.to_string(),
                                             Rc::new(RefCell::new(ScopeObject {

diff --git a/kclvm/tools/src/LSP/src/completion.rs b/kclvm/tools/src/LSP/src/completion.rs
@@ -14,12 +14,11 @@ use std::{fs, path::Path};
 use indexmap::IndexSet;
 use kclvm_ast::ast::{Expr, ImportStmt, Node, Program, Stmt};
 use kclvm_ast::pos::GetPos;
+use kclvm_compiler::pkgpath_without_prefix;
 use kclvm_config::modfile::KCL_FILE_EXTENSION;
 
 use kclvm_error::Position as KCLPos;
-use kclvm_sema::builtin::{
-    get_system_module_members, STANDARD_SYSTEM_MODULES, STRING_MEMBER_FUNCTIONS,
-};
+use kclvm_sema::builtin::{get_system_module_members, STRING_MEMBER_FUNCTIONS};
 use kclvm_sema::resolver::scope::{ProgramScope, ScopeObjectKind};
 use lsp_types::CompletionItem;
 
@@ -169,19 +168,7 @@ pub(crate) fn get_completion(
             match node.node {
                 Expr::Identifier(id) => {
                     let name = get_identifier_last_name(&id);
-                    if !id.pkgpath.is_empty() && STANDARD_SYSTEM_MODULES.contains(&name.as_str()) {
-                        items.extend(
-                            get_system_module_members(name.as_str())
-                                .iter()
-                                .map(|s| KCLCompletionItem {
-                                    label: s.to_string(),
-                                })
-                                .collect::<IndexSet<KCLCompletionItem>>(),
-                        )
-                    }
-
                     let def = find_def(stmt, pos, prog_scope);
-
                     if let Some(def) = def {
                         match def {
                             crate::goto_def::Definition::Object(obj) => {
@@ -204,6 +191,31 @@ pub(crate) fn get_completion(
                                             }
                                         }
                                     }
+
+                                    kclvm_sema::ty::TypeKind::Module(module) => match module.kind {
+                                        kclvm_sema::ty::ModuleKind::User => {
+                                            match prog_scope
+                                                .scope_map
+                                                .get(&pkgpath_without_prefix!(module.pkgpath))
+                                            {
+                                                Some(scope) => {
+                                                    items.extend(scope.borrow().elems.keys().map(
+                                                        |k| KCLCompletionItem { label: k.clone() },
+                                                    ))
+                                                }
+                                                None => {}
+                                            }
+                                        }
+                                        kclvm_sema::ty::ModuleKind::System => items.extend(
+                                            get_system_module_members(name.as_str())
+                                                .iter()
+                                                .map(|s| KCLCompletionItem {
+                                                    label: s.to_string(),
+                                                })
+                                                .collect::<IndexSet<KCLCompletionItem>>(),
+                                        ),
+                                        kclvm_sema::ty::ModuleKind::Plugin => {}
+                                    },
                                     _ => {}
                                 }
                             }
@@ -479,4 +491,139 @@ mod tests {
         let expect: CompletionResponse = into_completion_items(&items).into();
         assert_eq!(got, expect);
     }
+
+    #[test]
+    #[bench_test]
+    fn dot_completion_test_without_dot() {
+        // sometime lsp handle completion request before didChange notificaion, there is no dot in vfs
+        let (file, program, prog_scope, _) =
+            compile_test_file("src/test_data/completion_test/without_dot/completion.k");
+        let mut items: IndexSet<KCLCompletionItem> = IndexSet::new();
+
+        // test completion for schema attr
+        let pos = KCLPos {
+            filename: file.to_owned(),
+            line: 12,
+            column: Some(7),
+        };
+
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+
+        items.insert(KCLCompletionItem {
+            label: "name".to_string(),
+        });
+        items.insert(KCLCompletionItem {
+            label: "age".to_string(),
+        });
+
+        let expect: CompletionResponse = into_completion_items(&items).into();
+
+        assert_eq!(got, expect);
+        items.clear();
+
+        let pos = KCLPos {
+            filename: file.to_owned(),
+            line: 14,
+            column: Some(12),
+        };
+
+        // test completion for str builtin function
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+        let binding = STRING_MEMBER_FUNCTIONS;
+        for k in binding.keys() {
+            items.insert(KCLCompletionItem {
+                label: format!("{}{}", k, "()"),
+            });
+        }
+        let expect: CompletionResponse = into_completion_items(&items).into();
+
+        assert_eq!(got, expect);
+        items.clear();
+
+        // test completion for import pkg path
+        let pos = KCLPos {
+            filename: file.to_owned(),
+            line: 1,
+            column: Some(12),
+        };
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+        items.insert(KCLCompletionItem {
+            label: "file1".to_string(),
+        });
+        items.insert(KCLCompletionItem {
+            label: "file2".to_string(),
+        });
+        items.insert(KCLCompletionItem {
+            label: "subpkg".to_string(),
+        });
+
+        let expect: CompletionResponse = into_completion_items(&items).into();
+        assert_eq!(got, expect);
+        items.clear();
+
+        // test completion for import pkg' schema
+        let pos = KCLPos {
+            filename: file.to_owned(),
+            line: 16,
+            column: Some(12),
+        };
+
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+        items.insert(KCLCompletionItem {
+            label: "Person1".to_string(),
+        });
+
+        let expect: CompletionResponse = into_completion_items(&items).into();
+        assert_eq!(got, expect);
+        items.clear();
+
+        let pos = KCLPos {
+            filename: file.to_owned(),
+            line: 19,
+            column: Some(5),
+        };
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+
+        items.extend(MATH_FUNCTION_NAMES.iter().map(|s| KCLCompletionItem {
+            label: s.to_string(),
+        }));
+        let expect: CompletionResponse = into_completion_items(&items).into();
+        assert_eq!(got, expect);
+        items.clear();
+
+        // test completion for literal str builtin function
+        let pos = KCLPos {
+            filename: file.clone(),
+            line: 21,
+            column: Some(4),
+        };
+
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+        let binding = STRING_MEMBER_FUNCTIONS;
+        for k in binding.keys() {
+            items.insert(KCLCompletionItem {
+                label: format!("{}{}", k, "()"),
+            });
+        }
+        let expect: CompletionResponse = into_completion_items(&items).into();
+        items.clear();
+
+        assert_eq!(got, expect);
+
+        let pos = KCLPos {
+            filename: file,
+            line: 30,
+            column: Some(11),
+        };
+
+        let got = completion(Some('.'), &program, &pos, &prog_scope).unwrap();
+        items.insert(KCLCompletionItem {
+            label: "__settings__".to_string(),
+        });
+        items.insert(KCLCompletionItem {
+            label: "a".to_string(),
+        });
+        let expect: CompletionResponse = into_completion_items(&items).into();
+        assert_eq!(got, expect);
+    }
 }
diff --git a/kclvm/tools/src/LSP/src/goto_def.rs b/kclvm/tools/src/LSP/src/goto_def.rs
@@ -185,11 +185,23 @@ pub(crate) fn resolve_var(
         0 => None,
         1 => {
             let name = names[0].clone();
+
             match current_scope.lookup(&name) {
-                Some(obj) => match obj.borrow().kind {
-                    kclvm_sema::resolver::scope::ScopeObjectKind::Module(_) => scope_map
-                        .get(&name)
-                        .map(|scope| Definition::Scope(scope.borrow().clone())),
+                Some(obj) => match &obj.borrow().kind {
+                    kclvm_sema::resolver::scope::ScopeObjectKind::Module(_) => {
+                        match &obj.borrow().ty.kind {
+                            kclvm_sema::ty::TypeKind::Module(module_ty) => match module_ty.kind {
+                                kclvm_sema::ty::ModuleKind::User => scope_map
+                                    .get(&pkgpath_without_prefix!(module_ty.pkgpath))
+                                    .map(|scope| Definition::Scope(scope.borrow().clone())),
+                                kclvm_sema::ty::ModuleKind::System => {
+                                    Some(Definition::Object(obj.borrow().clone()))
+                                }
+                                kclvm_sema::ty::ModuleKind::Plugin => None,
+                            },
+                            _ => None,
+                        }
+                    }
                     _ => Some(Definition::Object(obj.borrow().clone())),
                 },
                 None => None,

diff --git a/kclvm/tools/src/LSP/src/test_data/completion_test/dot/completion.k b/kclvm/tools/src/LSP/src/test_data/completion_test/dot/completion.k
@@ -16,7 +16,7 @@ p2 = p.name.  # complete builtin (str) function
 p3 = subpkg.  # complete user module definition
 
 import math
-math.         # complete user module definition
+math.         # complete system module definition
 
 "a".
 

diff --git a/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/completion.k b/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/completion.k
@@ -0,0 +1,30 @@
+import .pkg  # complete import path
+import .pkg.subpkg
+schema Person:
+    name: str
+    age: int
+
+p = Person {
+    name: "alice"
+    age: 1
+}
+
+p1 = p       # complete schema attr
+
+p2 = p.name  # complete builtin (str) function
+
+p3 = subpkg  # complete user module definition
+
+import math
+math         # complete system module definition
+
+"a"
+
+p4 = Person{
+
+}
+
+schema P:
+    a: int = 1
+
+aaaa = P{}
diff --git a/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/pkg/file1.k b/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/pkg/file1.k
diff --git a/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/pkg/file2.k b/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/pkg/file2.k
diff --git a/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/pkg/subpkg/file1.k b/kclvm/tools/src/LSP/src/test_data/completion_test/without_dot/pkg/subpkg/file1.k
@@ -0,0 +1,3 @@
+schema Person1:
+    name: str
+    age: int