Support more documentation comments (#262)

JohnnyMorganz · web-flow · commit 72973c67d6a4 · 2023-01-11T16:50:14.000Z
* Support doc comments attached to table function properties

* Add more tests

* Support comments attached to vars

* Improve comment extraction on hover

* Update changelog

* Allow variable number of equals signs
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,11 +10,37 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - Added syntax highlighting support for interpolated strings
 - Added color viewers for Color3.new/fromRGB/fromHSV/fromHex
+- Support documentation comments on variables:
+
+```lua
+--- documentation comment
+local x = "string"
+
+--- another doc comment
+local y = function()
+end
+```
+
+- Support documentation comments on table properties, such as the following:
+
+```lua
+local tbl = {
+    --- This is some special information
+    data = "hello",
+    --- This is a doc comment
+    values = function()
+    end,
+}
+
+local x = tbl.values -- Should give "This is a doc comment"
+local y = tbl.data -- Should give "This is some special information"
+```
 
 ### Changed
 
 - Sync to upstream Luau 0.558
 - All Luau FFlags are no longer enabled by default. This can be re-enabled by configuring `luau-lsp.fflags.enableByDefault`. It is recommended to keep `luau-lsp.fflags.sync` enabled so that FFlags sync with upstream Luau
+- Allow variable number of `=` sign for multiline doc comments, so `--[[` and `--[===[` etc. are valid openers
 
 ### Fixed
 
diff --git a/src/DocumentationParser.cpp b/src/DocumentationParser.cpp
@@ -279,21 +279,74 @@ struct AttachCommentsVisitor : public Luau::AstVisitor
         return result;
     }
 
+    bool visit(Luau::AstExprTable* tbl) override
+    {
+        if (tbl->location.begin >= pos)
+            return false;
+        if (tbl->location.begin > closestPreviousNode)
+            closestPreviousNode = tbl->location.begin;
+
+        for (Luau::AstExprTable::Item item : tbl->items)
+        {
+            if (item.value->location.begin >= pos)
+                continue;
+            if (item.value->location.begin > closestPreviousNode)
+                closestPreviousNode = item.value->location.begin;
+            item.value->visit(this);
+            if (item.value->location.end <= pos && item.value->location.end > closestPreviousNode)
+                closestPreviousNode = item.value->location.end;
+        }
+
+        return false;
+    }
+
+    bool visit(Luau::AstTypeTable* tbl) override
+    {
+        if (tbl->location.begin >= pos)
+            return false;
+        if (tbl->location.begin > closestPreviousNode)
+            closestPreviousNode = tbl->location.begin;
+
+        for (Luau::AstTableProp item : tbl->props)
+        {
+            if (item.type->location.begin >= pos)
+                continue;
+            if (item.type->location.begin > closestPreviousNode)
+                closestPreviousNode = item.type->location.begin;
+            item.type->visit(this);
+            if (item.type->location.end <= pos && item.type->location.end > closestPreviousNode)
+                closestPreviousNode = item.type->location.end;
+        }
+
+        return false;
+    }
+
     bool visit(Luau::AstStatBlock* block) override
     {
+        // If the position is after the block, then it can be ignored
+        // If the position is within the block, then we know we can cut anything before the block,
+        // so set the previous node location to the block entry
+        if (block->location.begin >= pos)
+            return false;
+        if (block->location.begin > closestPreviousNode)
+            closestPreviousNode = block->location.begin;
+
         for (Luau::AstStat* stat : block->body)
         {
             if (stat->location.begin >= pos)
                 continue;
-            if (stat->location.begin > closestPreviousNode)
-                closestPreviousNode = stat->location.begin;
             stat->visit(this);
             if (stat->location.end <= pos && stat->location.end > closestPreviousNode)
                 closestPreviousNode = stat->location.end;
         }
 
         return false;
     }
+
+    bool visit(Luau::AstType* ty) override
+    {
+        return true;
+    }
 };
 
 std::vector<Luau::Comment> getCommentLocations(const Luau::SourceModule* module, const Luau::Location& node)
@@ -355,8 +408,16 @@ std::vector<std::string> WorkspaceFolder::getComments(const Luau::ModuleName& mo
         }
         else if (comment.type == Luau::Lexeme::Type::BlockComment)
         {
-            if (Luau::startsWith(commentText, "--[=["))
+            std::regex comment_regex("^--\\[(=*)\\[");
+            std::smatch comment_match;
+
+            if (std::regex_search(commentText, comment_match, comment_regex))
             {
+                LUAU_ASSERT(comment_match.size() == 2);
+                // Construct "--[=[" and "--]=]" which will be ignored
+                std::string start_string = "--[" + std::string(comment_match[1].length(), '=') + "[";
+                std::string end_string = "]" + std::string(comment_match[1].length(), '=') + "]";
+
                 // Parse each line separately
                 for (auto& line : Luau::split(commentText, '\n'))
                 {
@@ -365,7 +426,7 @@ std::vector<std::string> WorkspaceFolder::getComments(const Luau::ModuleName& mo
 
                     auto trimmedLineText = std::string(line);
                     trim(trimmedLineText);
-                    if (trimmedLineText == "--[=[" || trimmedLineText == "]=]")
+                    if (trimmedLineText == start_string || trimmedLineText == end_string)
                         continue;
                     comments.emplace_back(lineText);
                 }
diff --git a/src/operations/Hover.cpp b/src/operations/Hover.cpp
@@ -6,6 +6,12 @@
 #include "LSP/LuauExt.hpp"
 #include "LSP/DocumentationParser.hpp"
 
+struct DocumentationLocation
+{
+    Luau::ModuleName moduleName;
+    Luau::Location location;
+};
+
 std::optional<lsp::Hover> WorkspaceFolder::hover(const lsp::HoverParams& params)
 {
     auto config = client->getConfiguration(rootUri);
@@ -40,6 +46,7 @@ std::optional<lsp::Hover> WorkspaceFolder::hover(const lsp::HoverParams& params)
     std::string typeName;
     std::optional<Luau::TypeId> type = std::nullopt;
     std::optional<std::string> documentationSymbol = getDocumentationSymbolAtPosition(*sourceModule, *module, position);
+    std::optional<DocumentationLocation> documentationLocation = std::nullopt;
 
     if (auto ref = node->as<Luau::AstTypeReference>())
     {
@@ -76,6 +83,7 @@ std::optional<lsp::Hover> WorkspaceFolder::hover(const lsp::HoverParams& params)
     else if (auto local = exprOrLocal.getLocal()) // TODO: can we just use node here instead of also calling exprOrLocal?
     {
         type = scope->lookup(local);
+        documentationLocation = {moduleName, local->location};
     }
     else if (auto expr = exprOrLocal.getExpr())
     {
@@ -100,6 +108,29 @@ std::optional<lsp::Hover> WorkspaceFolder::hover(const lsp::HoverParams& params)
             }
         }
 
+        // Handle table properties (so that we can get documentation info)
+        if (auto index = expr->as<Luau::AstExprIndexName>())
+        {
+            if (auto parentIt = module->astTypes.find(index->expr))
+            {
+                auto parentType = Luau::follow(*parentIt);
+                auto indexName = index->index.value;
+                auto prop = lookupProp(parentType, indexName);
+                if (prop)
+                {
+                    type = prop->type;
+                    if (auto definitionModuleName = Luau::getDefinitionModuleName(parentType); definitionModuleName && prop->location)
+                        documentationLocation = {definitionModuleName.value(), prop->location.value()};
+                }
+            }
+        }
+
+        // Handle local variables separately to retrieve documentation location info
+        if (auto local = expr->as<Luau::AstExprLocal>(); !documentationLocation.has_value() && local && local->local)
+        {
+            documentationLocation = {moduleName, local->local->location};
+        }
+
         if (!type)
         {
             if (auto it = module->astTypes.find(expr))
@@ -114,17 +145,6 @@ std::optional<lsp::Hover> WorkspaceFolder::hover(const lsp::HoverParams& params)
             {
                 type = scope->lookup(local->local);
             }
-            else if (auto index = expr->as<Luau::AstExprIndexName>())
-            {
-                if (auto parentIt = module->astTypes.find(index->expr))
-                {
-                    auto parentType = Luau::follow(*parentIt);
-                    auto indexName = index->index.value;
-                    auto prop = lookupProp(parentType, indexName);
-                    if (prop)
-                        type = prop->type;
-                }
-            }
         }
     }
 
@@ -191,13 +211,20 @@ std::optional<lsp::Hover> WorkspaceFolder::hover(const lsp::HoverParams& params)
         typeString += "\n----------\n";
         typeString += printDocumentation(client->documentation, *documentationSymbol);
     }
-    else if (auto ftv = Luau::get<Luau::FunctionType>(*type))
+    else if (auto ftv = Luau::get<Luau::FunctionType>(*type); ftv && ftv->definition && ftv->definition->definitionModuleName)
     {
-        if (ftv->definition && ftv->definition->definitionModuleName)
-        {
-            typeString += "\n----------\n";
-            typeString += printMoonwaveDocumentation(getComments(ftv->definition->definitionModuleName.value(), ftv->definition->definitionLocation));
-        }
+        typeString += "\n----------\n";
+        typeString += printMoonwaveDocumentation(getComments(ftv->definition->definitionModuleName.value(), ftv->definition->definitionLocation));
+    }
+    // else if (auto ttv = Luau::get<Luau::TableType>(*type); ttv && !ttv->definitionModuleName.empty())
+    // {
+    //     typeString += "\n----------\n";
+    //     typeString += printMoonwaveDocumentation(getComments(ttv->definitionModuleName, ttv->definitionLocation));
+    // }
+    else if (documentationLocation)
+    {
+        typeString += "\n----------\n";
+        typeString += printMoonwaveDocumentation(getComments(documentationLocation->moduleName, documentationLocation->location));
     }
 
     return lsp::Hover{{lsp::MarkupKind::Markdown, typeString}};
diff --git a/tests/Documentation.test.cpp b/tests/Documentation.test.cpp
