From 7faf195a16fe28ce7abe64ce6bd1582344eac280 Mon Sep 17 00:00:00 2001 From: JohnnyMorganz Date: Sat, 10 Aug 2024 13:30:40 +0200 Subject: [PATCH] Fix documentation not showing for properties of intersected type tables Fixes #715 --- CHANGELOG.md | 1 + src/LuauExt.cpp | 27 ++++++++++++---------- src/include/LSP/LuauExt.hpp | 2 +- src/include/LSP/Workspace.hpp | 4 ++-- src/operations/CallHierarchy.cpp | 7 +++--- src/operations/Completion.cpp | 12 +++++++--- src/operations/GotoDefinition.cpp | 11 +++++---- src/operations/Hover.cpp | 18 +++++++-------- src/operations/References.cpp | 12 ++++++---- src/operations/SemanticTokens.cpp | 7 +++--- tests/Autocomplete.test.cpp | 38 +++++++++++++++++++++++++++++++ tests/Documentation.test.cpp | 4 ++-- tests/Hover.test.cpp | 28 +++++++++++++++++++++++ 13 files changed, 126 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79262ebb..4f4ed127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Fixed a regression in 1.32.0 causing `luau-lsp.ignoreGlobs` and `luau-lsp.completion.imports.ignoreGlobs` to not work ([#719](https://github.com/JohnnyMorganz/luau-lsp/issues/719)) - Fixed auto-imports injecting a require in the middle of a multi-line require when introducing a require with lower lexicographical ordering ([#725](https://github.com/JohnnyMorganz/luau-lsp/issues/725)) +- Fixed documentation not showing for properties of an intersected type table in Hover and Autocomplete ([#715](https://github.com/JohnnyMorganz/luau-lsp/issues/715)) ## [1.32.1] - 2024-07-23 diff --git a/src/LuauExt.cpp b/src/LuauExt.cpp index 723605d0..1be4eb38 100644 --- a/src/LuauExt.cpp +++ b/src/LuauExt.cpp @@ -283,18 +283,19 @@ std::optional lookupTypeLocation(const Luau::Scope& deepScope, c } } -std::optional lookupProp(const Luau::TypeId& parentType, const Luau::Name& name) +// Returns [base, property] - base is important during intersections +std::optional> lookupProp(const Luau::TypeId& parentType, const Luau::Name& name) { if (auto ctv = Luau::get(parentType)) { if (auto prop = Luau::lookupClassProp(ctv, name)) - return *prop; + return std::make_pair(parentType, *prop); } else if (auto tbl = Luau::get(parentType)) { if (tbl->props.find(name) != tbl->props.end()) { - return tbl->props.at(name); + return std::make_pair(parentType, tbl->props.at(name)); } } else if (auto mt = Luau::get(parentType)) @@ -317,21 +318,23 @@ std::optional lookupProp(const Luau::TypeId& parentType, const L } } - if (auto mtBaseTable = Luau::get(Luau::follow(mt->table))) + auto baseTableTy = Luau::follow(mt->table); + if (auto mtBaseTable = Luau::get(baseTableTy)) { if (mtBaseTable->props.find(name) != mtBaseTable->props.end()) { - return mtBaseTable->props.at(name); + return std::make_pair(baseTableTy, mtBaseTable->props.at(name)); } } } - // else if (auto i = get(parentType)) - // { - // for (Luau::TypeId ty : i->parts) - // { - // // TODO: find the corresponding ty - // } - // } + else if (auto i = Luau::get(parentType)) + { + for (Luau::TypeId ty : i->parts) + { + if (auto prop = lookupProp(Luau::follow(ty), name)) + return prop; + } + } // else if (auto u = get(parentType)) // { // // Find the corresponding ty diff --git a/src/include/LSP/LuauExt.hpp b/src/include/LSP/LuauExt.hpp index f42f3db9..156b9133 100644 --- a/src/include/LSP/LuauExt.hpp +++ b/src/include/LSP/LuauExt.hpp @@ -52,7 +52,7 @@ std::vector findTypeReferences(const Luau::SourceModule& source, std::optional getLocation(Luau::TypeId type); std::optional lookupTypeLocation(const Luau::Scope& deepScope, const Luau::Name& name); -std::optional lookupProp(const Luau::TypeId& parentType, const Luau::Name& name); +std::optional> lookupProp(const Luau::TypeId& parentType, const Luau::Name& name); std::optional lookupImportedModule(const Luau::Scope& deepScope, const Luau::Name& name); // Converts a UTF-8 position to a UTF-16 position, using the provided text document if available diff --git a/src/include/LSP/Workspace.hpp b/src/include/LSP/Workspace.hpp index 7a1d50ed..de4a9291 100644 --- a/src/include/LSP/Workspace.hpp +++ b/src/include/LSP/Workspace.hpp @@ -96,8 +96,8 @@ class WorkspaceFolder public: std::vector getComments(const Luau::ModuleName& moduleName, const Luau::Location& node); std::optional getDocumentationForType(const Luau::TypeId ty); - std::optional getDocumentationForAutocompleteEntry( - const Luau::AutocompleteEntry& entry, const std::vector& ancestry, const Luau::ModuleName& moduleName); + std::optional getDocumentationForAutocompleteEntry(const std::string& name, const Luau::AutocompleteEntry& entry, + const std::vector& ancestry, const Luau::ModuleName& moduleName); std::vector findAllReferences(const Luau::TypeId ty, std::optional property = std::nullopt); std::vector findAllTypeReferences(const Luau::ModuleName& moduleName, const Luau::Name& typeName); diff --git a/src/operations/CallHierarchy.cpp b/src/operations/CallHierarchy.cpp index 9926a740..b5af007e 100644 --- a/src/operations/CallHierarchy.cpp +++ b/src/operations/CallHierarchy.cpp @@ -51,9 +51,8 @@ static Luau::TypeId lookupFunctionCallType(Luau::ModulePtr module, const Luau::A if (auto parentIt = module->astTypes.find(index->expr)) { auto parentType = Luau::follow(*parentIt); - auto prop = lookupProp(parentType, index->index.value); - if (prop) - return Luau::follow(prop->type()); + if (auto prop = lookupProp(parentType, index->index.value)) + return Luau::follow(prop->second.type()); } } @@ -163,7 +162,7 @@ std::vector WorkspaceFolder::prepareCallHierarchy(const auto prop = lookupProp(ty, *it); if (!prop) return {}; - ty = Luau::follow(prop->type()); + ty = Luau::follow(prop->second.type()); } } diff --git a/src/operations/Completion.cpp b/src/operations/Completion.cpp index 339ba497..fc769c7b 100644 --- a/src/operations/Completion.cpp +++ b/src/operations/Completion.cpp @@ -331,7 +331,7 @@ static std::pair computeLabelDetailsForFunction(const } std::optional WorkspaceFolder::getDocumentationForAutocompleteEntry( - const Luau::AutocompleteEntry& entry, const std::vector& ancestry, const Luau::ModuleName& moduleName) + const std::string& name, const Luau::AutocompleteEntry& entry, const std::vector& ancestry, const Luau::ModuleName& moduleName) { if (entry.documentationSymbol) if (auto docs = printDocumentation(client->documentation, *entry.documentationSymbol)) @@ -366,7 +366,13 @@ std::optional WorkspaceFolder::getDocumentationForAutocompleteEntry } if (parentTy) - definitionModuleName = Luau::getDefinitionModuleName(*parentTy); + { + // parentTy might be an intersected type, find the actual base ttv + if (auto propInformation = lookupProp(*parentTy, name)) + definitionModuleName = Luau::getDefinitionModuleName(propInformation->first); + else + definitionModuleName = Luau::getDefinitionModuleName(*parentTy); + } } } @@ -432,7 +438,7 @@ std::vector WorkspaceFolder::completion(const lsp::Completi lsp::CompletionItem item; item.label = name; - if (auto documentationString = getDocumentationForAutocompleteEntry(entry, result.ancestry, moduleName)) + if (auto documentationString = getDocumentationForAutocompleteEntry(name, entry, result.ancestry, moduleName)) item.documentation = {lsp::MarkupKind::Markdown, documentationString.value()}; item.deprecated = deprecated(entry, item.documentation); diff --git a/src/operations/GotoDefinition.cpp b/src/operations/GotoDefinition.cpp index 1677e8fa..51a33fd9 100644 --- a/src/operations/GotoDefinition.cpp +++ b/src/operations/GotoDefinition.cpp @@ -73,13 +73,14 @@ lsp::DefinitionResult WorkspaceFolder::gotoDefinition(const lsp::DefinitionParam for (auto it = keys.rbegin(); it != keys.rend(); ++it) { auto base = properties.empty() ? *baseType : Luau::follow(properties.back().type()); - auto prop = lookupProp(base, *it); - if (!prop) + auto propInformation = lookupProp(base, *it); + if (!propInformation) return result; - definitionModuleName = Luau::getDefinitionModuleName(base); - location = prop->location; - properties.push_back(*prop); + auto [baseTy, prop] = propInformation.value(); + definitionModuleName = Luau::getDefinitionModuleName(baseTy); + location = prop.location; + properties.push_back(prop); } } diff --git a/src/operations/Hover.cpp b/src/operations/Hover.cpp index 831ed84e..35f0e0a2 100644 --- a/src/operations/Hover.cpp +++ b/src/operations/Hover.cpp @@ -181,7 +181,7 @@ std::optional WorkspaceFolder::hover(const lsp::HoverParams& params) documentationLocation = {definitionModuleName.value(), prop.location}; auto resolvedProperty = lookupProp(parentType, prop.name.value); if (resolvedProperty) - type = resolvedProperty->type(); + type = resolvedProperty->second.type(); break; } } @@ -229,16 +229,16 @@ std::optional WorkspaceFolder::hover(const lsp::HoverParams& params) { auto parentType = Luau::follow(*parentIt); auto indexName = index->index.value; - auto prop = lookupProp(parentType, indexName); - if (prop) + if (auto propInformation = lookupProp(parentType, indexName)) { - type = prop->type(); - if (auto definitionModuleName = Luau::getDefinitionModuleName(parentType)) + auto [baseTy, prop] = propInformation.value(); + type = prop.type(); + if (auto definitionModuleName = Luau::getDefinitionModuleName(baseTy)) { - if (prop->location) - documentationLocation = {definitionModuleName.value(), prop->location.value()}; - else if (prop->typeLocation) - documentationLocation = {definitionModuleName.value(), prop->typeLocation.value()}; + if (prop.location) + documentationLocation = {definitionModuleName.value(), prop.location.value()}; + else if (prop.typeLocation) + documentationLocation = {definitionModuleName.value(), prop.typeLocation.value()}; } } } diff --git a/src/operations/References.cpp b/src/operations/References.cpp index c2479334..3b428f8c 100644 --- a/src/operations/References.cpp +++ b/src/operations/References.cpp @@ -115,11 +115,15 @@ std::vector WorkspaceFolder::findAllReferences(Luau::TypeId ty, std:: // If its a property, include its original declaration location if not yet found if (property) { - if (auto prop = lookupProp(ty, *property); prop && prop->location) + if (auto propInformation = lookupProp(ty, *property)) { - auto reference = Reference{ttv->definitionModuleName, prop->location.value()}; - if (!contains(references, reference)) - references.push_back(reference); + auto [baseTy, prop] = propInformation.value(); + if (prop.location) + { + auto reference = Reference{Luau::getDefinitionModuleName(baseTy).value_or(ttv->definitionModuleName), prop.location.value()}; + if (!contains(references, reference)) + references.push_back(reference); + } } } diff --git a/src/operations/SemanticTokens.cpp b/src/operations/SemanticTokens.cpp index 1dce0a04..6f4c226b 100644 --- a/src/operations/SemanticTokens.cpp +++ b/src/operations/SemanticTokens.cpp @@ -289,15 +289,16 @@ struct SemanticTokensVisitor : public Luau::AstVisitor } auto ty = Luau::follow(*parentTy); - if (auto prop = lookupProp(ty, std::string(index->index.value))) + if (auto propInformation = lookupProp(ty, std::string(index->index.value))) { + auto prop = propInformation->second; auto defaultType = lsp::SemanticTokenTypes::Property; if (parentIsEnum) defaultType = lsp::SemanticTokenTypes::Enum; - else if (Luau::hasTag(prop->tags, "EnumItem")) + else if (Luau::hasTag(prop.tags, "EnumItem")) defaultType = lsp::SemanticTokenTypes::EnumMember; - auto type = inferTokenType(prop->type(), defaultType); + auto type = inferTokenType(prop.type(), defaultType); auto modifiers = lsp::SemanticTokenModifiers::None; if (parentIsBuiltin) { diff --git a/tests/Autocomplete.test.cpp b/tests/Autocomplete.test.cpp index f9dbae47..16132c51 100644 --- a/tests/Autocomplete.test.cpp +++ b/tests/Autocomplete.test.cpp @@ -76,6 +76,44 @@ TEST_CASE_FIXTURE(Fixture, "function_autocomplete_has_documentation") CHECK_EQ(item.documentation->value, "This is a function documentation comment"); } +TEST_CASE_FIXTURE(Fixture, "interesected_type_table_property_has_documentation") +{ + auto [source, marker] = sourceWithMarker(R"( + type A = { + --- Example sick number + Hello: number + } + + type B = { + --- Example sick string + Heya: string + } & A + + local item: B = nil + item.| + )"); + + auto uri = newDocument("foo.luau", source); + + lsp::CompletionParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = marker; + + auto result = workspace.completion(params); + + auto item = requireItem(result, "Hello"); + REQUIRE(item.documentation); + CHECK_EQ(item.documentation->kind, lsp::MarkupKind::Markdown); + trim(item.documentation->value); + CHECK_EQ(item.documentation->value, "Example sick number"); + + auto item2 = requireItem(result, "Heya"); + REQUIRE(item2.documentation); + CHECK_EQ(item2.documentation->kind, lsp::MarkupKind::Markdown); + trim(item2.documentation->value); + CHECK_EQ(item2.documentation->value, "Example sick string"); +} + TEST_CASE_FIXTURE(Fixture, "deprecated_marker_in_documentation_comment_applies_to_autocomplete_entry") { auto [source, marker] = sourceWithMarker(R"( diff --git a/tests/Documentation.test.cpp b/tests/Documentation.test.cpp index aa0c0710..6858edf0 100644 --- a/tests/Documentation.test.cpp +++ b/tests/Documentation.test.cpp @@ -126,9 +126,9 @@ TEST_CASE_FIXTURE(Fixture, "attach_comments_to_props_1") auto indexName = index->index.value; auto prop = lookupProp(parentTy, indexName); REQUIRE(prop); - REQUIRE(prop->location); + REQUIRE(prop->second.location); - auto comments = getCommentLocations(getMainSourceModule(), prop->location.value()); + auto comments = getCommentLocations(getMainSourceModule(), prop->second.location.value()); CHECK_EQ(1, comments.size()); } diff --git a/tests/Hover.test.cpp b/tests/Hover.test.cpp index 191b4d1c..e41f849a 100644 --- a/tests/Hover.test.cpp +++ b/tests/Hover.test.cpp @@ -240,6 +240,34 @@ TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_member_of_a_type_table_ CHECK_EQ(result->contents.value, codeBlock("luau", "string") + kDocumentationBreaker + "This is a member bar\n"); } +TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_member_of_an_intersected_type_table_when_hovering_over_property") +{ + auto source = R"( + type A = { + --- Example sick number + Hello: number + } + + type B = { + --- Example sick string + Heya: string + } & A + + local item: B = nil + print(item.Heya) + )"; + + auto uri = newDocument("foo.luau", source); + + lsp::HoverParams params; + params.textDocument = lsp::TextDocumentIdentifier{uri}; + params.position = lsp::Position{12, 21}; + + auto result = workspace.hover(params); + REQUIRE(result); + CHECK_EQ(result->contents.value, codeBlock("luau", "string") + kDocumentationBreaker + "Example sick string\n"); +} + TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_function") { auto source = R"(