Support inline documentation in definition files (#518)

Support inline documentation in definitions files

Author: JohnnyMorganz

CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - The server now accepts DataModel information from the Studio Plugin even when there is no sourcemap present ([#1216](https://github.com/JohnnyMorganz/luau-lsp/pull/1216))
 - Added `luau-lsp.completion.imports.includedServices` and `luau-lsp.completion.imports.excludedServices` to configure what services show up during auto-importing. If non-empty, only the services listed in `includedServices` will show up. None of the services listed in `excludedServices` will ever show up.
+- Added support for inline documentation comments in global type definition files (relies on name being provided in settings, see below) ([#271](https://github.com/JohnnyMorganz/luau-lsp/issues/271))
 
 ### Changed
 

src/DocumentationParser.cpp

@@ -306,6 +306,26 @@ struct AttachCommentsVisitor : public Luau::AstVisitor
         return false;
     }
 
+    bool visit(Luau::AstStatDeclareExternType* klass) override
+    {
+        if (klass->location.begin >= pos)
+            return false;
+        if (klass->location.begin > closestPreviousNode)
+            closestPreviousNode = klass->location.begin;
+
+        for (const auto& item : klass->props)
+        {
+            if (item.ty->location.begin >= pos)
+                continue;
+            closestPreviousNode = std::max(closestPreviousNode, item.ty->location.begin);
+            item.ty->visit(this);
+            if (item.ty->location.end <= pos)
+                closestPreviousNode = std::max(closestPreviousNode, item.ty->location.end);
+        }
+
+        return false;
+    }
+
     bool visit(Luau::AstStatBlock* block) override
     {
         // If the position is after the block, then it can be ignored
@@ -353,19 +373,29 @@ std::vector<Luau::Comment> getCommentLocations(const Luau::SourceModule* module,
 /// Performs transformations so that the comments are normalised to lines inside of it (i.e., trimming whitespace, removing comment start/end)
 std::vector<std::string> WorkspaceFolder::getComments(const Luau::ModuleName& moduleName, const Luau::Location& node)
 {
-    auto sourceModule = frontend.getSourceModule(moduleName);
-    if (!sourceModule)
-        return {};
+    Luau::SourceModule* sourceModule;
+    TextDocumentPtr textDocument{nullptr};
+
+    if (auto it = definitionsSourceModules.find(moduleName); it != definitionsSourceModules.end())
+    {
+        sourceModule = &it->second.second;
+        textDocument = TextDocumentPtr(&it->second.first);
+    }
+    else
+    {
+        sourceModule = frontend.getSourceModule(moduleName);
+        if (!sourceModule)
+            return {};
+
+        textDocument = fileResolver.getOrCreateTextDocumentFromModuleName(moduleName);
+        if (!textDocument)
+            return {};
+    }
 
     auto commentLocations = getCommentLocations(sourceModule, node);
     if (commentLocations.empty())
         return {};
 
-    // Get relevant text document
-    auto textDocument = fileResolver.getOrCreateTextDocumentFromModuleName(moduleName);
-    if (!textDocument)
-        return {};
-
     std::vector<std::string> comments{};
     for (auto& comment : commentLocations)
     {
@@ -447,6 +477,10 @@ std::optional<std::string> WorkspaceFolder::getDocumentationForType(const Luau::
     {
         return printMoonwaveDocumentation(getComments(ttv->definitionModuleName, ttv->definitionLocation));
     }
+    else if (auto etv = Luau::get<Luau::ExternType>(followedTy); etv && !etv->definitionModuleName.empty() && etv->definitionLocation)
+    {
+        return printMoonwaveDocumentation(getComments(etv->definitionModuleName, *etv->definitionLocation));
+    }
     return std::nullopt;
 }
 

src/LuauExt.cpp

@@ -56,7 +56,7 @@ std::optional<nlohmann::json> parseDefinitionsFileMetadata(const std::string& de
 Luau::LoadDefinitionFileResult registerDefinitions(
     Luau::Frontend& frontend, Luau::GlobalTypes& globals, const std::string& packageName, const std::string& definitions)
 {
-    return frontend.loadDefinitionFile(globals, globals.globalScope, definitions, packageName, /* captureComments = */ false);
+    return frontend.loadDefinitionFile(globals, globals.globalScope, definitions, packageName, /* captureComments = */ true);
 }
 
 using NameOrExpr = std::variant<std::string, Luau::AstExpr*>;

src/Workspace.cpp

@@ -521,6 +521,8 @@ void WorkspaceFolder::registerTypes(const std::vector<std::string>& disabledGlob
         {
             // Clear any set diagnostics
             client->publishDiagnostics({uri, std::nullopt, {}});
+            TextDocument textDocument(Uri::file(resolvedFilePath), "luau", 0, *definitionsContents);
+            definitionsSourceModules.emplace(packageName, std::make_pair(textDocument, result.sourceModule));
         }
         else
         {

src/include/LSP/Workspace.hpp

@@ -50,6 +50,11 @@ class WorkspaceFolder
     /// When a new request comes in and the workspace is not ready, we will prepare it then.
     bool isReady = false;
 
+private:
+    /// Mapping between a definitions package name to the TextDocument / SourceModule that contains this definitions.
+    /// Used for documentation comment lookup within definition files.
+    std::unordered_map<std::string, std::pair<TextDocument, Luau::SourceModule>> definitionsSourceModules{};
+
 public:
     WorkspaceFolder(Client* client, std::string name, const lsp::DocumentUri& uri, std::optional<Luau::Config> defaultConfig)
         : client(client)

tests/Hover.test.cpp

@@ -323,13 +323,9 @@ TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_type_alias_declarations")
 
     auto result = workspace.hover(params, nullptr);
     REQUIRE(result);
-    CHECK_EQ(
-        result->contents.value, 
-        codeBlock("luau", "type Meters = number") + 
-        kDocumentationBreaker + 
-        "The metre (or meter in [US spelling]; symbol: m) is the [base unit] of [length]\n" + 
-        "in the [International System of Units] (SI)\n"
-    );
+    CHECK_EQ(result->contents.value, codeBlock("luau", "type Meters = number") + kDocumentationBreaker +
+                                         "The metre (or meter in [US spelling]; symbol: m) is the [base unit] of [length]\n" +
+                                         "in the [International System of Units] (SI)\n");
 }
 
 TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_type_alias_declarations_of_intersected_tables")
@@ -356,13 +352,9 @@ TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_type_alias_declarations_o
 
     auto result = workspace.hover(params, nullptr);
     REQUIRE(result);
-    CHECK_EQ(
-        result->contents.value, 
-        codeBlock("luau", "type Foobar = {\n    bar: \"Bar\"\n} & {\n    foo: \"Foo\"\n}") + 
-        kDocumentationBreaker + 
-        "The terms foobar (/ˈfuːbɑːr/), foo, bar, baz, qux, quux, and others are used as\n" +
-        "metasyntactic variables and placeholder names in computer programming or computer-related documentation\n"
-    );
+    CHECK_EQ(result->contents.value, codeBlock("luau", "type Foobar = {\n    bar: \"Bar\"\n} & {\n    foo: \"Foo\"\n}") + kDocumentationBreaker +
+                                         "The terms foobar (/ˈfuːbɑːr/), foo, bar, baz, qux, quux, and others are used as\n" +
+                                         "metasyntactic variables and placeholder names in computer programming or computer-related documentation\n");
 }
 
 TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_type_references")
@@ -417,6 +409,189 @@ TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_external_type_references"
     CHECK_EQ(result->contents.value, codeBlock("luau", "type Types.Value = string") + kDocumentationBreaker + "This is a type\n");
 }
 
+TEST_CASE_FIXTURE(Fixture, "show_type_of_global_variable")
+{
+    auto source = R"(
+        print(DocumentedGlobalVariable)
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{1, 23};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value, codeBlock("luau", "type DocumentedGlobalVariable = number"));
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_global_type_table_from_definitions_file")
+{
+    auto source = R"(
+        local x: DocumentedTable = nil
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{1, 24};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value, codeBlock("luau", "type DocumentedTable = {\n"
+                                                       "    member1: string\n"
+                                                       "}") +
+                                         kDocumentationBreaker + "This is a documented table\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_global_type_table_from_definitions_file_when_hovering_over_variable_with_type")
+{
+    auto source = R"(
+        local x: DocumentedTable = nil
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{1, 14};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value, codeBlock("luau", "local x: {\n"
+                                                       "    member1: string\n"
+                                                       "}") +
+                                         kDocumentationBreaker + "This is a documented table\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_global_type_table_from_definitions_file_when_hovering_over_property")
+{
+    auto source = R"(
+        local x: DocumentedTable = nil
+        local y = x.member1
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{2, 23};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value, codeBlock("luau", "string") + kDocumentationBreaker + "This is documented member1 of the table\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_for_a_global_function_call_from_definitions_file")
+{
+    auto source = R"(
+        DocumentedGlobalFunction()
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{1, 20};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value,
+        codeBlock("luau", "function DocumentedGlobalFunction(): number") + kDocumentationBreaker + "This is a documented global function\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_when_hovering_over_class_type_from_definitions_file")
+{
+    auto source = R"(
+        local x: DocumentedClass
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{1, 23};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(
+        result->contents.value, codeBlock("luau", "type DocumentedClass = DocumentedClass") + kDocumentationBreaker + "This is a documented class\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_when_hovering_over_variable_with_class_type")
+{
+    auto source = R"(
+        local x: DocumentedClass
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{1, 14};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value, codeBlock("luau", "local x: DocumentedClass") + kDocumentationBreaker + "This is a documented class\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_when_hovering_over_class_type_property")
+{
+    auto source = R"(
+        local x: DocumentedClass
+        local y = x.member1
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{2, 23};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value, codeBlock("luau", "string") + kDocumentationBreaker + "This is a documented member1 of the class\n");
+}
+
+TEST_CASE_FIXTURE(Fixture, "includes_documentation_when_hovering_over_class_type_method_call")
+{
+    auto source = R"(
+        local x: DocumentedClass
+        local y = x:function1()
+    )";
+
+    auto uri = newDocument("foo.luau", source);
+
+    lsp::HoverParams params;
+    params.textDocument = lsp::TextDocumentIdentifier{uri};
+    params.position = lsp::Position{2, 23};
+
+    auto result = workspace.hover(params, nullptr);
+    REQUIRE(result);
+    CHECK_EQ(result->contents.value,
+        codeBlock("luau", "function DocumentedClass:function1(): number") + kDocumentationBreaker + "This is a documented function1 of the class\n");
+}
+
+// TEST_CASE_FIXTURE(Fixture, "includes_documentation_when_hovering_over_global_variable_from_definitions_file")
+//{
+//     auto source = R"(
+//          print(DocumentedGlobalVariable)
+//      )";
+//
+//     auto uri = newDocument("foo.luau", source);
+//
+//     lsp::HoverParams params;
+//     params.textDocument = lsp::TextDocumentIdentifier{uri};
+//     params.position = lsp::Position{1, 23};
+//
+//     auto result = workspace.hover(params, nullptr);
+//     REQUIRE(result);
+//     CHECK_EQ(result->contents.value,
+//         codeBlock("luau", "type DocumentedGlobalVariable = number") + kDocumentationBreaker + "This is a documented global variable\n");
+// }
+
 TEST_CASE_FIXTURE(Fixture, "handles_type_references_without_types_graph")
 {
     auto source = newDocument("types.luau", R"(

tests/testdata/standard_definitions.d.luau

@@ -101,3 +101,23 @@ declare Instance: {
 }
 
 declare game: DataModel
+
+--- This is a documented table
+export type DocumentedTable = {
+    --- This is documented member1 of the table
+    member1: string,
+}
+
+--- This is a documented class
+declare class DocumentedClass
+    --- This is a documented member1 of the class
+    member1: string
+    --- This is a documented function1 of the class
+    function function1(self): number
+end
+
+--- This is a documented global function
+declare function DocumentedGlobalFunction(): number
+
+--- This is a documented global variable
+declare DocumentedGlobalVariable: number