Show diagnostics for definitions files which failed to load

Ref: #29 (comment)

CHANGELOG.md

@@ -9,6 +9,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Added
 
 - Added `luau-lsp.sourcemap.enabled` option which dictates whether sourcemap-related features are enabled
+- Diagnostics will now be provided for definitions files which errored
+
+### Changed
+
+- Moved definitions file loading to post-initialization
 
 ## [1.5.2] - 2022-06-22
 

src/Client.cpp

@@ -149,6 +149,11 @@ void Client::applyEdit(const lsp::ApplyWorkspaceEditParams& params, std::optiona
     sendRequest(nextRequestId++, "workspace/applyEdit", params, handler);
 }
 
+void Client::publishDiagnostics(const lsp::PublishDiagnosticsParams& params)
+{
+    sendNotification("textDocument/publishDiagnostics", params);
+}
+
 void Client::refreshWorkspaceDiagnostics()
 {
     if (capabilities.workspace && capabilities.workspace->diagnostics && capabilities.workspace->diagnostics->refreshSupport)

src/LanguageServer.cpp

@@ -325,8 +325,7 @@ void LanguageServer::onInitialized(const lsp::InitializedParams& params)
                 {
                     if (report.kind == lsp::DocumentDiagnosticReportKind::Full)
                     {
-                        client->sendNotification(
-                            "textDocument/publishDiagnostics", lsp::PublishDiagnosticsParams{report.uri, report.version, report.items});
+                        client->publishDiagnostics(lsp::PublishDiagnosticsParams{report.uri, report.version, report.items});
                     }
                 }
             }
@@ -373,16 +372,15 @@ void LanguageServer::pushDiagnostics(WorkspaceFolderPtr& workspace, const lsp::D
     // Convert the diagnostics report into a series of diagnostics published for each relevant file
     lsp::DocumentDiagnosticParams params{lsp::TextDocumentIdentifier{uri}};
     auto diagnostics = workspace->documentDiagnostics(params);
-    client->sendNotification("textDocument/publishDiagnostics", lsp::PublishDiagnosticsParams{uri, version, diagnostics.items});
+    client->publishDiagnostics(lsp::PublishDiagnosticsParams{uri, version, diagnostics.items});
 
     if (!diagnostics.relatedDocuments.empty())
     {
         for (const auto& [uri, diagnostics] : diagnostics.relatedDocuments)
         {
             if (diagnostics.kind == lsp::DocumentDiagnosticReportKind::Full)
             {
-                client->sendNotification(
-                    "textDocument/publishDiagnostics", lsp::PublishDiagnosticsParams{Uri::parse(uri), std::nullopt, diagnostics.items});
+                client->publishDiagnostics(lsp::PublishDiagnosticsParams{Uri::parse(uri), std::nullopt, diagnostics.items});
             }
         }
     }
@@ -419,8 +417,7 @@ void LanguageServer::onDidChangeTextDocument(const lsp::DidChangeTextDocumentPar
     {
         // Convert the diagnostics report into a series of diagnostics published for each relevant file
         auto diagnostics = workspace->documentDiagnostics(lsp::DocumentDiagnosticParams{params.textDocument});
-        client->sendNotification("textDocument/publishDiagnostics",
-            lsp::PublishDiagnosticsParams{params.textDocument.uri, params.textDocument.version, diagnostics.items});
+        client->publishDiagnostics(lsp::PublishDiagnosticsParams{params.textDocument.uri, params.textDocument.version, diagnostics.items});
 
         // Compute diagnostics for reverse dependencies
         // TODO: should we put this inside documentDiagnostics so it works in the pull based model as well? (its a reverse BFS which is expensive)
@@ -453,8 +450,7 @@ void LanguageServer::onDidChangeTextDocument(const lsp::DidChangeTextDocumentPar
             {
                 if (diagnostics.kind == lsp::DocumentDiagnosticReportKind::Full)
                 {
-                    client->sendNotification(
-                        "textDocument/publishDiagnostics", lsp::PublishDiagnosticsParams{Uri::parse(uri), std::nullopt, diagnostics.items});
+                    client->publishDiagnostics(lsp::PublishDiagnosticsParams{Uri::parse(uri), std::nullopt, diagnostics.items});
                 }
             }
         }
@@ -470,7 +466,7 @@ void LanguageServer::onDidCloseTextDocument(const lsp::DidCloseTextDocumentParam
     // If this was an ignored file then lets clear the diagnostics for it
     if (workspace->isIgnoredFile(params.textDocument.uri.fsPath()))
     {
-        client->sendNotification("textDocument/publishDiagnostics", lsp::PublishDiagnosticsParams{params.textDocument.uri, std::nullopt, {}});
+        client->publishDiagnostics(lsp::PublishDiagnosticsParams{params.textDocument.uri, std::nullopt, {}});
     }
 }
 

src/LuauExt.cpp

@@ -700,6 +700,18 @@ lsp::Diagnostic createLintDiagnostic(const Luau::LintWarning& lint)
     return diagnostic;
 }
 
+lsp::Diagnostic createParseErrorDiagnostic(const Luau::ParseError& error)
+{
+    lsp::Diagnostic diagnostic;
+    diagnostic.source = "Luau";
+    diagnostic.code = "SyntaxError";
+    diagnostic.message = "SyntaxError: " + error.getMessage();
+    diagnostic.severity = lsp::DiagnosticSeverity::Error;
+    diagnostic.range = {convertPosition(error.getLocation().begin), convertPosition(error.getLocation().end)};
+    diagnostic.codeDescription = {Uri::parse("https://luau-lang.org/syntax")};
+    return diagnostic;
+}
+
 struct FindSymbolReferences : public Luau::AstVisitor
 {
     const Luau::Symbol symbol;

src/Workspace.cpp

@@ -844,10 +844,28 @@ void WorkspaceFolder::initialize()
         auto result = types::registerDefinitions(frontend.typeChecker, definitionsFile);
         types::registerDefinitions(frontend.typeCheckerForAutocomplete, definitionsFile);
 
-        if (!result.success)
+        auto uri = Uri::file(definitionsFile);
+
+        if (result.success)
+        {
+            // Clear any set diagnostics
+            client->publishDiagnostics({uri, std::nullopt, {}});
+        }
+        else
         {
-            client->sendWindowMessage(lsp::MessageType::Error, "Failed to read definitions file. Extended types will not be provided");
-            // TODO: Display diagnostics? We can't right now since this is currently called during initialisation, need to move
+            client->sendWindowMessage(lsp::MessageType::Error,
+                "Failed to read definitions file " + definitionsFile.generic_string() + ". Extended types will not be provided");
+
+            // Display relevant diagnostics
+            std::vector<lsp::Diagnostic> diagnostics;
+            for (auto& error : result.parseResult.errors)
+                diagnostics.emplace_back(createParseErrorDiagnostic(error));
+
+            if (result.module)
+                for (auto& error : result.module->errors)
+                    diagnostics.emplace_back(createTypeErrorDiagnostic(error));
+
+            client->publishDiagnostics({uri, std::nullopt, diagnostics});
         }
     }
     Luau::freeze(frontend.typeChecker.globalTypes);

src/include/LSP/Client.hpp

@@ -50,6 +50,7 @@ class Client
     // TODO: this function only supports getting requests for workspaces
     void requestConfiguration(const std::vector<lsp::DocumentUri>& uris);
     void applyEdit(const lsp::ApplyWorkspaceEditParams& params, std::optional<ResponseHandler> handler = std::nullopt);
+    void publishDiagnostics(const lsp::PublishDiagnosticsParams& params);
     void refreshWorkspaceDiagnostics();
 
     void setTrace(const lsp::SetTraceParams& params);

src/include/LSP/LuauExt.hpp

@@ -106,4 +106,5 @@ Luau::Position convertPosition(const lsp::Position& position);
 lsp::Position convertPosition(const Luau::Position& position);
 
 lsp::Diagnostic createTypeErrorDiagnostic(const Luau::TypeError& error);
-lsp::Diagnostic createLintDiagnostic(const Luau::LintWarning& lint);
+lsp::Diagnostic createLintDiagnostic(const Luau::LintWarning& lint);
+lsp::Diagnostic createParseErrorDiagnostic(const Luau::ParseError& error);