From 6bf352c023fe0f23cdc7caa099cc93459c7ba614 Mon Sep 17 00:00:00 2001 From: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> Date: Tue, 21 May 2024 20:47:07 +0530 Subject: [PATCH] Syntax highlight in hover (#1336) * LSP Hover shows highlight(Incomplete, not classified) Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Hover differentiates between LanguageString and String Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Updated tests for attributes and schema Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Changed "String" and "LanguageString" to enum types Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Added new formatting for hover Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Added hover for all types Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Fixed tests Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Fixed CI Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Fixed CI Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Put docs in bottom and fixed schema indent and tested it. Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Added 4 spaces instead of tabs Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Fixed CI(config_hover_test_main) Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> * Fixed dict_key_in_schema test Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> --------- Signed-off-by: Wck-iipi <110763795+Wck-iipi@users.noreply.github.com> --- kclvm/sema/src/ty/mod.rs | 31 ++++ kclvm/tools/src/LSP/src/hover.rs | 168 ++++++++++++------ ...r__hover__tests__dict_key_in_schema-2.snap | 2 +- ...r__hover__tests__dict_key_in_schema-3.snap | 2 +- ...ver__hover__tests__dict_key_in_schema.snap | 2 +- kclvm/tools/src/LSP/src/tests.rs | 49 +++-- 6 files changed, 183 insertions(+), 71 deletions(-) diff --git a/kclvm/sema/src/ty/mod.rs b/kclvm/sema/src/ty/mod.rs index ad47f5262..20255d151 100644 --- a/kclvm/sema/src/ty/mod.rs +++ b/kclvm/sema/src/ty/mod.rs @@ -324,6 +324,37 @@ impl SchemaType { format!("{}\n\nschema {}{}", self.pkgpath, self.name, params_str) } + + pub fn schema_ty_signature_no_pkg(&self) -> String { + let base: String = if let Some(base) = &self.base { + format!("({})", base.name) + } else { + "".to_string() + }; + let params: String = if self.func.params.is_empty() { + "".to_string() + } else { + format!( + "[{}]", + self.func + .params + .iter() + .map(|p| format!("{}: {}", p.name.clone(), p.ty.ty_str())) + .collect::>() + .join(", ") + ) + }; + let params_str = if !params.is_empty() && !base.is_empty() { + format!("\\{}{}", params, base) + } else if !params.is_empty() { + format!("{}", params) + } else if !base.is_empty() { + format!("{}", base) + } else { + "".to_string() + }; + format!("schema {}{}", self.name, params_str) + } } #[derive(Debug, Clone, PartialEq)] diff --git a/kclvm/tools/src/LSP/src/hover.rs b/kclvm/tools/src/LSP/src/hover.rs index c2d90c670..5b8fe7ef3 100644 --- a/kclvm/tools/src/LSP/src/hover.rs +++ b/kclvm/tools/src/LSP/src/hover.rs @@ -11,12 +11,18 @@ use crate::goto_def::find_def_with_gs; /// Returns a short text describing element at position. /// Specifically, the doc for schema and schema attr(todo) + +enum MarkedStringType { + String, + LanguageString, +} + pub(crate) fn hover( _program: &Program, kcl_pos: &KCLPos, gs: &GlobalState, ) -> Option { - let mut docs: Vec = vec![]; + let mut docs: Vec<(String, MarkedStringType)> = vec![]; let def = find_def_with_gs(kcl_pos, gs, true); match def { Some(def_ref) => match gs.get_symbols().get_symbol(def_ref) { @@ -28,24 +34,24 @@ pub(crate) fn hover( // ``` // pkg // schema Foo(Base)[param: type] + // attr1: type + // attr2? type + // ``` // ----------------- // doc // ----------------- - // Attributes: - // attr1: type - // attr2? type // ``` let schema_ty = ty.into_schema_type(); - docs.push(schema_ty.schema_ty_signature_str()); - if !schema_ty.doc.is_empty() { - docs.push(schema_ty.doc.clone()); - } + + let schema_ty_signature_no_pkg = schema_ty.schema_ty_signature_no_pkg(); + + docs.push((schema_ty.pkgpath, MarkedStringType::String)); // The attr of schema_ty does not contain the attrs from inherited base schema. // Use the api provided by GlobalState to get all attrs let module_info = gs.get_packages().get_module_info(&kcl_pos.filename); let schema_attrs = obj.get_all_attributes(gs.get_symbols(), module_info); - let mut attrs = vec!["Attributes:".to_string()]; + let mut attrs = vec![schema_ty_signature_no_pkg]; for schema_attr in schema_attrs { if let kclvm_sema::core::symbol::SymbolKind::Attribute = schema_attr.get_kind() @@ -59,14 +65,17 @@ pub(crate) fn hover( None => ANY_TYPE_STR.to_string(), }; attrs.push(format!( - "{}{}: {}", + " {}{}: {}", name, if attr_symbol.is_optional() { "?" } else { "" }, attr_ty_str, )); } } - docs.push(attrs.join("\n\n")); + docs.push((attrs.join("\n"), MarkedStringType::LanguageString)); + if !schema_ty.doc.is_empty() { + docs.push((schema_ty.doc.clone(), MarkedStringType::String)); + } } _ => {} }, @@ -74,10 +83,13 @@ pub(crate) fn hover( let sema_info = obj.get_sema_info(); match &sema_info.ty { Some(ty) => { - docs.push(format!("{}: {}", &obj.get_name(), ty.ty_str())); + docs.push(( + format!("{}: {}", &obj.get_name(), ty.ty_str()), + MarkedStringType::LanguageString, + )); if let Some(doc) = &sema_info.doc { if !doc.is_empty() { - docs.push(doc.clone()); + docs.push((doc.clone(), MarkedStringType::String)); } } } @@ -87,10 +99,16 @@ pub(crate) fn hover( kclvm_sema::core::symbol::SymbolKind::Value => match &obj.get_sema_info().ty { Some(ty) => match &ty.kind { kclvm_sema::ty::TypeKind::Function(func_ty) => { - docs.extend(build_func_hover_content(func_ty, obj.get_name().clone())); + docs.append(&mut build_func_hover_content( + func_ty.clone(), + obj.get_name().clone(), + )); } _ => { - docs.push(format!("{}: {}", &obj.get_name(), ty.ty_str())); + docs.push(( + format!("{}: {}", &obj.get_name(), ty.ty_str()), + MarkedStringType::LanguageString, + )); } }, _ => {} @@ -100,10 +118,12 @@ pub(crate) fn hover( kclvm_sema::core::symbol::SymbolKind::Decorator => { match BUILTIN_DECORATORS.get(&obj.get_name()) { Some(ty) => { - docs.extend(build_func_hover_content( - &ty.into_func_type(), + let mut hover_content = build_func_hover_content( + ty.into_func_type(), obj.get_name().clone(), - )); + ); + + docs.append(&mut hover_content); } None => todo!(), } @@ -113,7 +133,10 @@ pub(crate) fn hover( Some(ty) => ty.ty_str(), None => "".to_string(), }; - docs.push(format!("{}: {}", &obj.get_name(), ty_str)); + docs.push(( + format!("{}: {}", &obj.get_name(), ty_str), + MarkedStringType::LanguageString, + )); } }, None => {} @@ -123,19 +146,31 @@ pub(crate) fn hover( docs_to_hover(docs) } +fn convert_doc_to_marked_string(doc: &(String, MarkedStringType)) -> MarkedString { + match doc.1 { + MarkedStringType::String => MarkedString::String(doc.0.clone()), + MarkedStringType::LanguageString => { + MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: doc.0.clone(), + }) + } + } +} + // Convert docs to Hover. This function will convert to // None, Scalar or Array according to the number of positions -fn docs_to_hover(docs: Vec) -> Option { +fn docs_to_hover(docs: Vec<(String, MarkedStringType)>) -> Option { match docs.len() { 0 => None, 1 => Some(Hover { - contents: HoverContents::Scalar(MarkedString::String(docs[0].clone())), + contents: HoverContents::Scalar(convert_doc_to_marked_string(&docs[0])), range: None, }), _ => Some(Hover { contents: HoverContents::Array( docs.iter() - .map(|doc| MarkedString::String(doc.clone())) + .map(|doc| convert_doc_to_marked_string(doc)) .collect(), ), range: None, @@ -151,11 +186,14 @@ fn docs_to_hover(docs: Vec) -> Option { // ----------------- // doc // ``` -fn build_func_hover_content(func_ty: &FunctionType, name: String) -> Vec { - let mut docs = vec![]; +fn build_func_hover_content( + func_ty: FunctionType, + name: String, +) -> Vec<(String, MarkedStringType)> { + let mut docs: Vec<(String, MarkedStringType)> = vec![]; if let Some(ty) = &func_ty.self_ty { let self_ty = format!("{}\n\n", ty.ty_str()); - docs.push(self_ty); + docs.push((self_ty, MarkedStringType::String)); } let mut sig = format!("fn {}(", name); @@ -172,10 +210,13 @@ fn build_func_hover_content(func_ty: &FunctionType, name: String) -> Vec sig.push(')'); } sig.push_str(&format!(" -> {}", func_ty.return_ty.ty_str())); - docs.push(sig); + docs.push((sig, MarkedStringType::LanguageString)); if !func_ty.doc.is_empty() { - docs.push(func_ty.doc.clone().replace('\n', "\n\n")); + docs.push(( + func_ty.doc.clone().replace('\n', "\n\n"), + MarkedStringType::String, + )); } docs } @@ -183,6 +224,7 @@ fn build_func_hover_content(func_ty: &FunctionType, name: String) -> Vec #[cfg(test)] mod tests { use crate::hover::docs_to_hover; + use crate::hover::MarkedStringType; use std::path::PathBuf; use kclvm_error::Position as KCLPos; @@ -213,13 +255,20 @@ mod tests { match got.contents { lsp_types::HoverContents::Array(vec) => { if let MarkedString::String(s) = vec[0].clone() { - assert_eq!(s, "pkg\n\nschema Person"); + assert_eq!(s, "pkg"); } - if let MarkedString::String(s) = vec[1].clone() { - assert_eq!(s, "hover doc test"); + if let MarkedString::LanguageString(s) = vec[1].clone() { + assert_eq!( + s.value, + "schema Person\n name: str\n age: int".to_string() + ); + } else { + unreachable!("Wrong type"); } if let MarkedString::String(s) = vec[2].clone() { - assert_eq!(s, "Attributes:\n\nname: str\n\nage: int"); + assert_eq!(s, "hover doc test"); + } else { + unreachable!("Wrong type"); } } _ => unreachable!("test error"), @@ -232,8 +281,8 @@ mod tests { let got = hover(&program, &pos, &gs).unwrap(); match got.contents { lsp_types::HoverContents::Scalar(marked_string) => { - if let MarkedString::String(s) = marked_string { - assert_eq!(s, "name: str"); + if let MarkedString::LanguageString(s) = marked_string { + assert_eq!(s.value, "name: str"); } } _ => unreachable!("test error"), @@ -245,13 +294,22 @@ mod tests { fn test_docs_to_hover_multiple_docs() { // Given multiple documentation strings let docs = vec![ - "Documentation string 1".to_string(), - "Documentation string 2".to_string(), - "Documentation string 3".to_string(), + ( + "Documentation string 1".to_string(), + MarkedStringType::String, + ), + ( + "Documentation string 2".to_string(), + MarkedStringType::String, + ), + ( + "Documentation string 3".to_string(), + MarkedStringType::String, + ), ]; // When converting to hover content - let hover = docs_to_hover(docs.clone()); + let hover = docs_to_hover(docs); // Then the result should be a Hover object with an Array of MarkedString::String assert!(hover.is_some()); @@ -291,13 +349,17 @@ mod tests { match got.contents { lsp_types::HoverContents::Array(vec) => { if let MarkedString::String(s) = vec[0].clone() { - assert_eq!(s, "__main__\n\nschema Person"); + assert_eq!(s, "__main__"); } - if let MarkedString::String(s) = vec[1].clone() { - assert_eq!(s, "hover doc test"); + if let MarkedString::LanguageString(s) = vec[1].clone() { + assert_eq!(s.value, "schema Person\n name: str\n age?: int"); + } else { + unreachable!("Wrong type"); } if let MarkedString::String(s) = vec[2].clone() { - assert_eq!(s, "Attributes:\n\nname: str\n\nage?: int"); + assert_eq!(s, "hover doc test"); + } else { + unreachable!("Wrong type"); } } _ => unreachable!("test error"), @@ -526,10 +588,10 @@ mod tests { lsp_types::HoverContents::Array(vec) => { assert_eq!(vec.len(), 2); if let MarkedString::String(s) = vec[0].clone() { - assert_eq!(s, "fib\n\nschema Fib"); + assert_eq!(s, "fib"); } if let MarkedString::String(s) = vec[1].clone() { - assert_eq!(s, "Attributes:\n\nn: int\n\nvalue: int"); + assert_eq!(s, "schema Fib\n\n n: int\n\n value: int"); } } _ => unreachable!("test error"), @@ -586,11 +648,11 @@ mod tests { column: Some(1), }; let got = hover(&program, &pos, &gs).unwrap(); - let expect_content = vec![MarkedString::String( - "fn deprecated(version: str, reason: str, strict: bool) -> any".to_string(), - ), MarkedString::String( - "This decorator is used to get the deprecation message according to the wrapped key-value pair.".to_string(), - )]; + let expect_content = vec![MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: "fn deprecated(version: str, reason: str, strict: bool) -> any".to_string(), + }), + MarkedString::String("This decorator is used to get the deprecation message according to the wrapped key-value pair.".to_string())]; match got.contents { lsp_types::HoverContents::Array(vec) => { assert_eq!(vec, expect_content) @@ -623,9 +685,13 @@ mod tests { }; let got = hover(&program, &pos, &gs).unwrap(); - let expect_content = vec![ - MarkedString::String("__main__\n\nschema Data1\\[m: {str:str}](Data)".to_string()), - MarkedString::String("Attributes:\n\nname: str\n\nage: int".to_string()), + let expect_content: Vec = vec![ + MarkedString::String("__main__".to_string()), + MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: "schema Data1\\[m: {str:str}](Data)\n name: str\n age: int" + .to_string(), + }), ]; match got.contents { diff --git a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-2.snap b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-2.snap index ffbe0c87c..1f95f47a9 100644 --- a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-2.snap +++ b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-2.snap @@ -2,4 +2,4 @@ source: tools/src/LSP/src/hover.rs expression: "format!(\"{:?}\", got)" --- -Hover { contents: Scalar(String("name: int")), range: None } +Hover { contents: Scalar(LanguageString(LanguageString { language: "kcl", value: "name: int" })), range: None } diff --git a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-3.snap b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-3.snap index ffbe0c87c..1f95f47a9 100644 --- a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-3.snap +++ b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema-3.snap @@ -2,4 +2,4 @@ source: tools/src/LSP/src/hover.rs expression: "format!(\"{:?}\", got)" --- -Hover { contents: Scalar(String("name: int")), range: None } +Hover { contents: Scalar(LanguageString(LanguageString { language: "kcl", value: "name: int" })), range: None } diff --git a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema.snap b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema.snap index ffbe0c87c..1f95f47a9 100644 --- a/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema.snap +++ b/kclvm/tools/src/LSP/src/snapshots/kcl_language_server__hover__tests__dict_key_in_schema.snap @@ -2,4 +2,4 @@ source: tools/src/LSP/src/hover.rs expression: "format!(\"{:?}\", got)" --- -Hover { contents: Scalar(String("name: int")), range: None } +Hover { contents: Scalar(LanguageString(LanguageString { language: "kcl", value: "name: int" })), range: None } diff --git a/kclvm/tools/src/LSP/src/tests.rs b/kclvm/tools/src/LSP/src/tests.rs index 8e750d8bb..52539a12b 100644 --- a/kclvm/tools/src/LSP/src/tests.rs +++ b/kclvm/tools/src/LSP/src/tests.rs @@ -1169,9 +1169,12 @@ fn hover_test() { res.result.unwrap(), to_json(Hover { contents: HoverContents::Array(vec![ - MarkedString::String("__main__\n\nschema Person".to_string()), + MarkedString::String("__main__".to_string()), + MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: "schema Person\n name: str\n age?: int".to_string() + }), MarkedString::String("hover doc test".to_string()), - MarkedString::String("Attributes:\n\nname: str\n\nage?: int".to_string()), ]), range: None }) @@ -1225,7 +1228,12 @@ fn hover_assign_in_lambda_test() { assert_eq!( res.result.unwrap(), to_json(Hover { - contents: HoverContents::Scalar(MarkedString::String("images: [str]".to_string()),), + contents: HoverContents::Scalar(MarkedString::LanguageString( + lsp_types::LanguageString { + language: "kcl".to_string(), + value: "images: [str]".to_string() + } + )), range: None }) .unwrap() @@ -1663,12 +1671,14 @@ fn konfig_hover_test_main() { let got = hover(&program, &pos, &gs).unwrap(); match got.contents { HoverContents::Array(arr) => { - let expect: Vec = ["base.pkg.kusion_models.kube.frontend\n\nschema Server", - "Server is abstaction of Deployment and StatefulSet.", - "Attributes:\n\nname?: str\n\nworkloadType: str(Deployment) | str(StatefulSet)\n\nrenderType?: str(Server) | str(KubeVelaApplication)\n\nreplicas: int\n\nimage: str\n\nschedulingStrategy: SchedulingStrategy\n\nmainContainer: Main\n\nsidecarContainers?: [Sidecar]\n\ninitContainers?: [Sidecar]\n\nuseBuiltInLabels?: bool\n\nlabels?: {str:str}\n\nannotations?: {str:str}\n\nuseBuiltInSelector?: bool\n\nselector?: {str:str}\n\npodMetadata?: ObjectMeta\n\nvolumes?: [Volume]\n\nneedNamespace?: bool\n\nenableMonitoring?: bool\n\nconfigMaps?: [ConfigMap]\n\nsecrets?: [Secret]\n\nservices?: [Service]\n\ningresses?: [Ingress]\n\nserviceAccount?: ServiceAccount\n\nstorage?: ObjectStorage\n\ndatabase?: DataBase"] - .iter() - .map(|s| MarkedString::String(s.to_string())) - .collect(); + let expect: Vec = vec![ + MarkedString::String("base.pkg.kusion_models.kube.frontend".to_string()), + MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: "schema Server\n name?: str\n workloadType: str(Deployment) | str(StatefulSet)\n renderType?: str(Server) | str(KubeVelaApplication)\n replicas: int\n image: str\n schedulingStrategy: SchedulingStrategy\n mainContainer: Main\n sidecarContainers?: [Sidecar]\n initContainers?: [Sidecar]\n useBuiltInLabels?: bool\n labels?: {str:str}\n annotations?: {str:str}\n useBuiltInSelector?: bool\n selector?: {str:str}\n podMetadata?: ObjectMeta\n volumes?: [Volume]\n needNamespace?: bool\n enableMonitoring?: bool\n configMaps?: [ConfigMap]\n secrets?: [Secret]\n services?: [Service]\n ingresses?: [Ingress]\n serviceAccount?: ServiceAccount\n storage?: ObjectStorage\n database?: DataBase".to_string() + }), + MarkedString::String("Server is abstaction of Deployment and StatefulSet.".to_string()), + ]; assert_eq!(expect, arr); } _ => unreachable!("test error"), @@ -1683,13 +1693,15 @@ fn konfig_hover_test_main() { let got = hover(&program, &pos, &gs).unwrap(); match got.contents { HoverContents::Array(arr) => { - let expect: Vec = [ - "schedulingStrategy: SchedulingStrategy", - "SchedulingStrategy represents scheduling strategy.", - ] - .iter() - .map(|s| MarkedString::String(s.to_string())) - .collect(); + let expect: Vec = vec![ + MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: "schedulingStrategy: SchedulingStrategy".to_string(), + }), + MarkedString::String( + "SchedulingStrategy represents scheduling strategy.".to_string(), + ), + ]; assert_eq!(expect, arr); } _ => unreachable!("test error"), @@ -1706,7 +1718,10 @@ fn konfig_hover_test_main() { HoverContents::Scalar(s) => { assert_eq!( s, - MarkedString::String("appConfiguration: Server".to_string()) + MarkedString::LanguageString(lsp_types::LanguageString { + language: "kcl".to_string(), + value: "appConfiguration: Server".to_string() + }) ); } _ => unreachable!("test error"),