Fix documentation not showing for properties of intersected type tables

Fixes #715
JohnnyMorganz · Aug 10, 2024 · 7faf195 · 7faf195
1 parent 095e371
commit 7faf195
Show file tree

Hide file tree

Showing 13 changed files with 126 additions and 45 deletions.
diff --git 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
@@ -283,18 +283,19 @@ std::optional<Luau::Location> lookupTypeLocation(const Luau::Scope& deepScope, c
     }
 }
 
-std::optional<Luau::Property> lookupProp(const Luau::TypeId& parentType, const Luau::Name& name)
+// Returns [base, property] - base is important during intersections
+std::optional<std::pair<Luau::TypeId, Luau::Property>> lookupProp(const Luau::TypeId& parentType, const Luau::Name& name)
 {
     if (auto ctv = Luau::get<Luau::ClassType>(parentType))
     {
         if (auto prop = Luau::lookupClassProp(ctv, name))
-            return *prop;
+            return std::make_pair(parentType, *prop);
     }
     else if (auto tbl = Luau::get<Luau::TableType>(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<Luau::MetatableType>(parentType))
@@ -317,21 +318,23 @@ std::optional<Luau::Property> lookupProp(const Luau::TypeId& parentType, const L
             }
         }
 
-        if (auto mtBaseTable = Luau::get<Luau::TableType>(Luau::follow(mt->table)))
+        auto baseTableTy = Luau::follow(mt->table);
+        if (auto mtBaseTable = Luau::get<Luau::TableType>(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<Luau::IntersectionType>(parentType))
-    // {
-    //     for (Luau::TypeId ty : i->parts)
-    //     {
-    //         // TODO: find the corresponding ty
-    //     }
-    // }
+    else if (auto i = Luau::get<Luau::IntersectionType>(parentType))
+    {
+        for (Luau::TypeId ty : i->parts)
+        {
+            if (auto prop = lookupProp(Luau::follow(ty), name))
+                return prop;
+        }
+    }
     // else if (auto u = get<Luau::UnionType>(parentType))
     // {
     //     // Find the corresponding ty

diff --git a/src/include/LSP/LuauExt.hpp b/src/include/LSP/LuauExt.hpp
@@ -52,7 +52,7 @@ std::vector<Luau::Location> findTypeReferences(const Luau::SourceModule& source,
 std::optional<Luau::Location> getLocation(Luau::TypeId type);
 
 std::optional<Luau::Location> lookupTypeLocation(const Luau::Scope& deepScope, const Luau::Name& name);
-std::optional<Luau::Property> lookupProp(const Luau::TypeId& parentType, const Luau::Name& name);
+std::optional<std::pair<Luau::TypeId, Luau::Property>> lookupProp(const Luau::TypeId& parentType, const Luau::Name& name);
 std::optional<Luau::ModuleName> 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
@@ -96,8 +96,8 @@ class WorkspaceFolder
 public:
     std::vector<std::string> getComments(const Luau::ModuleName& moduleName, const Luau::Location& node);
     std::optional<std::string> getDocumentationForType(const Luau::TypeId ty);
-    std::optional<std::string> getDocumentationForAutocompleteEntry(
-        const Luau::AutocompleteEntry& entry, const std::vector<Luau::AstNode*>& ancestry, const Luau::ModuleName& moduleName);
+    std::optional<std::string> getDocumentationForAutocompleteEntry(const std::string& name, const Luau::AutocompleteEntry& entry,
+        const std::vector<Luau::AstNode*>& ancestry, const Luau::ModuleName& moduleName);
     std::vector<Reference> findAllReferences(const Luau::TypeId ty, std::optional<Luau::Name> property = std::nullopt);
     std::vector<Reference> findAllTypeReferences(const Luau::ModuleName& moduleName, const Luau::Name& typeName);
 

diff --git 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<lsp::CallHierarchyItem> 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
@@ -331,7 +331,7 @@ static std::pair<std::string, std::string> computeLabelDetailsForFunction(const
 }
 
 std::optional<std::string> WorkspaceFolder::getDocumentationForAutocompleteEntry(
-    const Luau::AutocompleteEntry& entry, const std::vector<Luau::AstNode*>& ancestry, const Luau::ModuleName& moduleName)
+    const std::string& name, const Luau::AutocompleteEntry& entry, const std::vector<Luau::AstNode*>& ancestry, const Luau::ModuleName& moduleName)
 {
     if (entry.documentationSymbol)
         if (auto docs = printDocumentation(client->documentation, *entry.documentationSymbol))
@@ -366,7 +366,13 @@ std::optional<std::string> 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<lsp::CompletionItem> 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
@@ -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
@@ -181,7 +181,7 @@ std::optional<lsp::Hover> 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<lsp::Hover> 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
@@ -115,11 +115,15 @@ std::vector<Reference> 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
@@ -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
@@ -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
@@ -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
@@ -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"(