From bca2b7c47b37072362d6b6e93c410d7b8ff01215 Mon Sep 17 00:00:00 2001 From: Tsuf Cohen <39455181+tsuf239@users.noreply.github.com> Date: Fri, 27 Sep 2024 05:47:36 +0300 Subject: [PATCH] fix(compiler): inherit parent docstring in case no docstring was provided in child member (#7172) fixes: #6488 ## Checklist - [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted) - [ ] Description explains motivation and solution - [ ] Tests added (always) - [ ] Docs updated (only required for features) - [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing *By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*. --- packages/@winglang/wingc/src/docs.rs | 2 +- packages/@winglang/wingc/src/lib.rs | 5 + packages/@winglang/wingc/src/lsp/hover.rs | 91 +++++++++++++++++++ .../completions/hide_abstract_members.snap | 14 +-- ...rent_class_docs_are_inherited_on_call.snap | 14 +++ ...ass_docs_are_inherited_on_declaration.snap | 14 +++ ...ass_docs_are_inherited_on_method_call.snap | 14 +++ ...s_are_inherited_on_method_declaration.snap | 14 +++ .../parent_class_docs_are_overridden.snap | 14 +++ .../lsp/snapshots/hovers/static_method.snap | 2 +- .../hovers/user_defined_type_annotation.snap | 2 +- .../user_defined_type_reference_property.snap | 2 +- .../user_defined_type_reference_type.snap | 2 +- .../snapshots/hovers/user_defined_types.snap | 2 +- .../lsp/snapshots/hovers/variadic_args.snap | 2 +- packages/@winglang/wingc/src/type_check.rs | 45 +++++++-- 16 files changed, 219 insertions(+), 20 deletions(-) create mode 100644 packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_call.snap create mode 100644 packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_declaration.snap create mode 100644 packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_call.snap create mode 100644 packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_declaration.snap create mode 100644 packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_overridden.snap diff --git a/packages/@winglang/wingc/src/docs.rs b/packages/@winglang/wingc/src/docs.rs index 1900389cb25..960b0c34187 100644 --- a/packages/@winglang/wingc/src/docs.rs +++ b/packages/@winglang/wingc/src/docs.rs @@ -349,7 +349,7 @@ fn render_docs(markdown: &mut CodeMaker, docs: &Docs) { // Psuedo-abstract marker, mostly useful internally | "abstract" // Marker type use, not for users - | "skipDocs" | "wingType" + | "skipDocs" | "wingType" | "noinflight" ) { return; } diff --git a/packages/@winglang/wingc/src/lib.rs b/packages/@winglang/wingc/src/lib.rs index 193091d3ba5..4c51a967665 100644 --- a/packages/@winglang/wingc/src/lib.rs +++ b/packages/@winglang/wingc/src/lib.rs @@ -130,6 +130,11 @@ const WINGSDK_SIM_IRESOURCE_FQN: &'static str = formatcp!( assembly = WINGSDK_ASSEMBLY_NAME, iface = WINGSDK_SIM_IRESOURCE ); +const WINGSDK_RESOURCE_FQN: &'static str = formatcp!( + "{assembly}.{class}", + assembly = WINGSDK_ASSEMBLY_NAME, + class = WINGSDK_RESOURCE +); const CONSTRUCT_BASE_CLASS: &'static str = "constructs.Construct"; const CONSTRUCT_BASE_INTERFACE: &'static str = "constructs.IConstruct"; diff --git a/packages/@winglang/wingc/src/lsp/hover.rs b/packages/@winglang/wingc/src/lsp/hover.rs index dcb30179c7a..dedbcfa4a11 100644 --- a/packages/@winglang/wingc/src/lsp/hover.rs +++ b/packages/@winglang/wingc/src/lsp/hover.rs @@ -560,6 +560,97 @@ Json.stringify({}); "#, ); + test_hover_list!( + parent_class_docs_are_inherited_on_declaration, + r#" + /// Some parent docs +class Parent { + /// Some method docs + pub method() {} +} + +class Child extends Parent { + //^ + pub method() {} +} + +new Child().method(); + "#, + ); + + test_hover_list!( + parent_class_docs_are_inherited_on_method_declaration, + r#" + /// Some parent docs +class Parent { + /// Some method docs + pub method() {} +} + +class Child extends Parent { + pub method() {} + //^ +} + +new Child().method(); + "#, + ); + + test_hover_list!( + parent_class_docs_are_inherited_on_call, + r#" + /// Some parent docs +class Parent { + /// Some method docs + pub method() {} +} + +class Child extends Parent { + pub method() {} +} + +new Child().method(); + //^ + "#, + ); + + test_hover_list!( + parent_class_docs_are_inherited_on_method_call, + r#" + /// Some parent docs +class Parent { + /// Some method docs + pub method() {} +} + +class Child extends Parent { + pub method() {} + //^ +} + +new Child().method(); + //^ + "#, + ); + + test_hover_list!( + parent_class_docs_are_overridden, + r#" + /// Some parent docs +class Parent { + /// Some method docs + pub method() {} +} + + /// Some child docs +class Child extends Parent { +} + +new Child().method(); + //^ + "#, + ); + test_hover_list!( ignoe_empty_lines_in_doc, r#" diff --git a/packages/@winglang/wingc/src/lsp/snapshots/completions/hide_abstract_members.snap b/packages/@winglang/wingc/src/lsp/snapshots/completions/hide_abstract_members.snap index 811810d696f..bedb4208c6d 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/completions/hide_abstract_members.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/completions/hide_abstract_members.snap @@ -5,7 +5,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass Button extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA button can be used to perform an action.\n\n*@noinflight*" + value: "```wing\nclass Button extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA button can be used to perform an action." sortText: gg|Button insertText: Button($1) insertTextFormat: 2 @@ -16,7 +16,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass Field extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA field can be used to display a value.\n\n*@noinflight*" + value: "```wing\nclass Field extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA field can be used to display a value." sortText: gg|Field insertText: Field($1) insertTextFormat: 2 @@ -27,7 +27,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass FileBrowser extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA file browser can be used to browse files.\n\n*@noinflight*" + value: "```wing\nclass FileBrowser extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA file browser can be used to browse files." sortText: gg|FileBrowser insertText: FileBrowser($1) insertTextFormat: 2 @@ -38,7 +38,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass HttpClient extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nAn HttpClient can be used to make HTTP requests.\n\n*@noinflight*" + value: "```wing\nclass HttpClient extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nAn HttpClient can be used to make HTTP requests." sortText: gg|HttpClient insertText: HttpClient($1) insertTextFormat: 2 @@ -49,7 +49,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass Section extends VisualComponent {\n add(...): void;\n addButton(...): void;\n addField(...): void;\n static isVisualComponent(...): bool;\n}\n```\n---\nA section can be used to group other visual components.\n\n*@noinflight*" + value: "```wing\nclass Section extends VisualComponent {\n add(...): void;\n addButton(...): void;\n addField(...): void;\n static isVisualComponent(...): bool;\n}\n```\n---\nA section can be used to group other visual components." sortText: gg|Section insertText: Section($1) insertTextFormat: 2 @@ -60,7 +60,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass Table extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA table can be used to browse files.\n\n*@noinflight*" + value: "```wing\nclass Table extends VisualComponent {\n static isVisualComponent(...): bool;\n}\n```\n---\nA table can be used to browse files." sortText: gg|Table insertText: Table($1) insertTextFormat: 2 @@ -71,7 +71,7 @@ source: packages/@winglang/wingc/src/lsp/completions.rs kind: 7 documentation: kind: markdown - value: "```wing\nclass ValueField extends Field {\n static isVisualComponent(...): bool;\n}\n```\n---\nA value field can be used to display a string value.\n\n*@noinflight*" + value: "```wing\nclass ValueField extends Field {\n static isVisualComponent(...): bool;\n}\n```\n---\nA value field can be used to display a string value." sortText: gg|ValueField insertText: ValueField($1) insertTextFormat: 2 diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_call.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_call.snap new file mode 100644 index 00000000000..d6a30791c62 --- /dev/null +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_call.snap @@ -0,0 +1,14 @@ +--- +source: packages/@winglang/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\nnew(): Child\n```\n---\nSome parent docs" +range: + start: + line: 11 + character: 4 + end: + line: 11 + character: 9 + diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_declaration.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_declaration.snap new file mode 100644 index 00000000000..02ae5e13529 --- /dev/null +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_declaration.snap @@ -0,0 +1,14 @@ +--- +source: packages/@winglang/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\nclass Child extends Parent {\n method(): void;\n}\n```\n---\nSome parent docs" +range: + start: + line: 7 + character: 6 + end: + line: 7 + character: 11 + diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_call.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_call.snap new file mode 100644 index 00000000000..4a54721be47 --- /dev/null +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_call.snap @@ -0,0 +1,14 @@ +--- +source: packages/@winglang/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\npreflight method(): void\n```\n---\nSome method docs" +range: + start: + line: 8 + character: 6 + end: + line: 8 + character: 12 + diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_declaration.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_declaration.snap new file mode 100644 index 00000000000..4a54721be47 --- /dev/null +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_inherited_on_method_declaration.snap @@ -0,0 +1,14 @@ +--- +source: packages/@winglang/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\npreflight method(): void\n```\n---\nSome method docs" +range: + start: + line: 8 + character: 6 + end: + line: 8 + character: 12 + diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_overridden.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_overridden.snap new file mode 100644 index 00000000000..e57ce9dca9a --- /dev/null +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/parent_class_docs_are_overridden.snap @@ -0,0 +1,14 @@ +--- +source: packages/@winglang/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\nnew(): Child\n```\n---\nSome child docs" +range: + start: + line: 11 + character: 4 + end: + line: 11 + character: 9 + diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/static_method.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/static_method.snap index 9b2d87b76b2..d3c41fe20f2 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/hovers/static_method.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/static_method.snap @@ -1,5 +1,5 @@ --- -source: libs/wingc/src/lsp/hover.rs +source: packages/@winglang/wingc/src/lsp/hover.rs --- contents: kind: markdown diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap index 6b13f2f31fc..0f7fec75880 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap @@ -1,5 +1,5 @@ --- -source: libs/wingc/src/lsp/hover.rs +source: packages/@winglang/wingc/src/lsp/hover.rs --- contents: kind: markdown diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap index c27d7bf34b3..4db37987892 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap @@ -1,5 +1,5 @@ --- -source: libs/wingc/src/lsp/hover.rs +source: packages/@winglang/wingc/src/lsp/hover.rs --- contents: kind: markdown diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap index 12427da76ad..6b8331e43b4 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap @@ -1,5 +1,5 @@ --- -source: libs/wingc/src/lsp/hover.rs +source: packages/@winglang/wingc/src/lsp/hover.rs --- contents: kind: markdown diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_types.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_types.snap index dcdd73534f3..a0b6e6a9d20 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_types.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/user_defined_types.snap @@ -1,5 +1,5 @@ --- -source: libs/wingc/src/lsp/hover.rs +source: packages/@winglang/wingc/src/lsp/hover.rs --- contents: kind: markdown diff --git a/packages/@winglang/wingc/src/lsp/snapshots/hovers/variadic_args.snap b/packages/@winglang/wingc/src/lsp/snapshots/hovers/variadic_args.snap index b5b91c403d4..d3c17ce441b 100644 --- a/packages/@winglang/wingc/src/lsp/snapshots/hovers/variadic_args.snap +++ b/packages/@winglang/wingc/src/lsp/snapshots/hovers/variadic_args.snap @@ -1,5 +1,5 @@ --- -source: libs/wingc/src/lsp/hover.rs +source: packages/@winglang/wingc/src/lsp/hover.rs --- contents: kind: markdown diff --git a/packages/@winglang/wingc/src/type_check.rs b/packages/@winglang/wingc/src/type_check.rs index 66ba7fa0bdd..82730b7f243 100644 --- a/packages/@winglang/wingc/src/type_check.rs +++ b/packages/@winglang/wingc/src/type_check.rs @@ -18,7 +18,7 @@ use crate::ast::{ }; use crate::comp_ctx::{CompilationContext, CompilationPhase}; use crate::diagnostic::{report_diagnostic, Diagnostic, DiagnosticAnnotation, DiagnosticSeverity, TypeError, WingSpan}; -use crate::docs::Docs; +use crate::docs::{Docs, Documented}; use crate::file_graph::{File, FileGraph}; use crate::parser::normalize_path; use crate::type_check::has_type_stmt::HasStatementVisitor; @@ -31,8 +31,8 @@ use crate::{ debug, CONSTRUCT_BASE_CLASS, CONSTRUCT_BASE_INTERFACE, CONSTRUCT_NODE_PROPERTY, DEFAULT_PACKAGE_NAME, UTIL_CLASS_NAME, WINGSDK_APP, WINGSDK_ARRAY, WINGSDK_ASSEMBLY_NAME, WINGSDK_BRINGABLE_MODULES, WINGSDK_BYTES, WINGSDK_DATETIME, WINGSDK_DURATION, WINGSDK_GENERIC, WINGSDK_IRESOURCE, WINGSDK_JSON, WINGSDK_MAP, WINGSDK_MUT_ARRAY, - WINGSDK_MUT_JSON, WINGSDK_MUT_MAP, WINGSDK_MUT_SET, WINGSDK_NODE, WINGSDK_REGEX, WINGSDK_RESOURCE, WINGSDK_SET, - WINGSDK_SIM_IRESOURCE_FQN, WINGSDK_STD_MODULE, WINGSDK_STRING, WINGSDK_STRUCT, + WINGSDK_MUT_JSON, WINGSDK_MUT_MAP, WINGSDK_MUT_SET, WINGSDK_NODE, WINGSDK_REGEX, WINGSDK_RESOURCE, + WINGSDK_RESOURCE_FQN, WINGSDK_SET, WINGSDK_SIM_IRESOURCE_FQN, WINGSDK_STD_MODULE, WINGSDK_STRING, WINGSDK_STRUCT, }; use camino::{Utf8Path, Utf8PathBuf}; use derivative::Derivative; @@ -4157,7 +4157,7 @@ This value is set by the CLI at compile time and can be used to conditionally co let interface_spec = Interface { name: iface.name.clone(), fqn: format!("{}.{}", self.base_fqn_for_current_file(), iface.name), - docs: doc.as_ref().map_or(Docs::default(), |s| Docs::with_summary(s)), + docs: doc.as_ref().map_or(Docs::default(), |s: &String| Docs::with_summary(s)), env: dummy_env, extends: extend_interfaces.clone(), phase: iface.phase, @@ -4748,6 +4748,21 @@ This value is set by the CLI at compile time and can be used to conditionally co } } + let mut default_docs = Docs::default(); + // if parent docs exist we use them as the defualt + if let Some(parent_class) = parent_class { + // New classes defined in Wing shouldn't inherit docs from std.Resource + let is_parent_resource = parent_class + .as_class() + .map(|c| c.fqn.as_deref() == Some(WINGSDK_RESOURCE_FQN)) + .unwrap_or(false); + if !is_parent_resource { + if let Some(parent_docs) = parent_class.docs() { + default_docs = parent_docs.clone(); + } + } + } + // Create the resource/class type and add it to the current environment (so class implementation can reference itself) let class_spec = Class { name: ast_class.name.clone(), @@ -4758,7 +4773,7 @@ This value is set by the CLI at compile time and can be used to conditionally co is_abstract: false, phase: ast_class.phase, defined_in_phase: env.phase, - docs: stmt.doc.as_ref().map_or(Docs::default(), |s| Docs::with_summary(s)), + docs: stmt.doc.as_ref().map_or(default_docs, |s| Docs::with_summary(s)), std_construct_args: ast_class.phase == Phase::Preflight, lifts: None, uid: self.types.class_counter, @@ -4821,6 +4836,7 @@ This value is set by the CLI at compile time and can be used to conditionally co method_def.access, &mut class_env, method_name, + parent_class, ); method_types.insert(&method_name, method_type); } @@ -4839,6 +4855,7 @@ This value is set by the CLI at compile time and can be used to conditionally co ast_class.initializer.access, &mut class_env, &init_symb, + parent_class, ); method_types.insert(&init_symb, init_func_type); @@ -4857,6 +4874,7 @@ This value is set by the CLI at compile time and can be used to conditionally co ast_class.inflight_initializer.access, &mut class_env, &inflight_init_symb, + parent_class, ); method_types.insert(&inflight_init_symb, inflight_init_func_type); @@ -5759,6 +5777,7 @@ This value is set by the CLI at compile time and can be used to conditionally co access: AccessModifier, class_env: &mut SymbolEnv, method_name: &Symbol, + parent_class: Option>, ) { // Modify the method's type based on the fact we know it's a method and not just a function let method_sig = method_type @@ -5829,6 +5848,16 @@ This value is set by the CLI at compile time and can be used to conditionally co let method_phase = method_type.as_function_sig().unwrap().phase; + // use the parent's method docs as default, if exist. + let mut default_method_docs = None; + if let Some(parent_class) = parent_class { + if let Some(c) = parent_class.as_class() { + if let Some(parent_method) = c.methods(true).find(|m| m.1.name.eq(&method_name)) { + default_method_docs = parent_method.1.docs; + } + } + }; + match class_env.define( method_name, SymbolKind::make_member_variable( @@ -5838,7 +5867,11 @@ This value is set by the CLI at compile time and can be used to conditionally co instance_type.is_none(), method_phase, access, - method_def.doc.as_ref().map(|s| Docs::with_summary(s)), + method_def + .doc + .as_ref() + .map(|s| Docs::with_summary(s)) + .or(default_method_docs), ), access, StatementIdx::Top,