Language server: extract documentation from ordinary comments (#2308)

* Language server: improved documentation formatting.

* Extend doc extractor to search in ordinary comments.
Reuse language server instance between tests.

* Fix test case.

* Fix comment.

* Fix crash.

* Fix enum case doc extraction.

* Doc extractor fixes.

* Fix.

* Fix.

Co-authored-by: Yong He <yhe@nvidia.com>

7 files changed, 144 insertions, 40 deletions

diff --git a/source/slang/slang-doc-ast.cpp b/source/slang/slang-doc-ast.cpp
index 386c8e6d0..eec0aa650 100644
--- a/source/slang/slang-doc-ast.cpp
+++ b/source/slang/slang-doc-ast.cpp
@@ -80,7 +80,7 @@ static void _addDeclRec(Decl* decl, List<Decl*>& outDecls)
     }
 }
 
-SlangResult ASTMarkupUtil::extract(ModuleDecl* moduleDecl, SourceManager* sourceManager, DiagnosticSink* sink, ASTMarkup* outDoc)
+SlangResult ASTMarkupUtil::extract(ModuleDecl* moduleDecl, SourceManager* sourceManager, DiagnosticSink* sink, ASTMarkup* outDoc, bool searchOrindaryComments)
 {
     List<Decl*> decls;
     findDecls(moduleDecl, decls);
@@ -106,6 +106,7 @@ SlangResult ASTMarkupUtil::extract(ModuleDecl* moduleDecl, SourceManager* source
         }
 
         DocMarkupExtractor extractor;
+        extractor.setSearchInOrdinaryComments(searchOrindaryComments);
 
         List<SourceView*> views;
         SLANG_RETURN_ON_FAIL(extractor.extract(inputItems.getBuffer(), declsCount, sourceManager, sink, views, outItems));
diff --git a/source/slang/slang-doc-ast.h b/source/slang/slang-doc-ast.h
index c5291a31a..1013e5518 100644
--- a/source/slang/slang-doc-ast.h
+++ b/source/slang/slang-doc-ast.h
@@ -74,7 +74,7 @@ struct ASTMarkupUtil
 
         /// Extracts documentation from the nodes held in the module using the source manager. Found documentation is placed
         /// in outMarkup
-    static SlangResult extract(ModuleDecl* moduleDecl, SourceManager* sourceManager, DiagnosticSink* sink, ASTMarkup* outMarkup);
+    static SlangResult extract(ModuleDecl* moduleDecl, SourceManager* sourceManager, DiagnosticSink* sink, ASTMarkup* outMarkup, bool searchOrindaryComments = false);
 };
 
 } // namespace Slang
diff --git a/source/slang/slang-language-server-auto-format.cpp b/source/slang/slang-language-server-auto-format.cpp
index 8d31c44c6..8d877eaca 100644
--- a/source/slang/slang-language-server-auto-format.cpp
+++ b/source/slang/slang-language-server-auto-format.cpp
@@ -26,13 +26,12 @@ String findClangFormatTool()
     auto localProcess = Path::combine(dirName, processName);
     if (File::exists(localProcess))
         return localProcess;
-
-    Index vsCodeLoc = dirName.indexOf(".vscode");
+    auto extensionsStr = UnownedStringSlice("extensions");
+    Index vsCodeLoc = dirName.indexOf(extensionsStr);
     if (vsCodeLoc != -1)
     {
         // If we still cannot find clang-format, try to use the clang-format bundled with VSCode's C++ extension.
-        String vsCodeExtDir = dirName.subString(0, vsCodeLoc + 7);
-        vsCodeExtDir = Path::combine(vsCodeExtDir, "extensions");
+        String vsCodeExtDir = dirName.subString(0, vsCodeLoc + extensionsStr.getLength());
         struct CallbackContext
         {
             String foundPath;
diff --git a/source/slang/slang-language-server-semantic-tokens.cpp b/source/slang/slang-language-server-semantic-tokens.cpp
index 6b3725828..3754c46aa 100644
--- a/source/slang/slang-language-server-semantic-tokens.cpp
+++ b/source/slang/slang-language-server-semantic-tokens.cpp
@@ -175,6 +175,16 @@ List<SemanticToken> getSemanticTokens(Linkage* linkage, Module* module, UnownedS
                     maybeInsertToken(token);
                 }
             }
+            else if (auto paramDecl = as<ParamDecl>(node))
+            {
+                if (paramDecl->getName())
+                {
+                    SemanticToken token = _createSemanticToken(
+                        manager, paramDecl->getNameLoc(), paramDecl->getName());
+                    token.type = SemanticTokenType::Parameter;
+                    maybeInsertToken(token);
+                }
+            }
             else if (auto varDecl = as<VarDeclBase>(node))
             {
                 if (varDecl->getName())
diff --git a/source/slang/slang-language-server.cpp b/source/slang/slang-language-server.cpp
index 0818aea07..7dc2f6dcc 100644
--- a/source/slang/slang-language-server.cpp
+++ b/source/slang/slang-language-server.cpp
@@ -67,7 +67,7 @@ slang::IGlobalSession* LanguageServer::getOrCreateGlobalSession()
     return m_session;
 }
 
-void LanguageServer::resetDiagnosticUpdateTime() { time(&m_lastDiagnosticUpdateTime); }
+void LanguageServer::resetDiagnosticUpdateTime() { m_lastDiagnosticUpdateTime = std::chrono::system_clock::now(); }
 
 String uriToCanonicalPath(const String& uri)
 {
@@ -159,7 +159,7 @@ SlangResult LanguageServer::parseNextMessage()
                 if (response.result.getKind() == JSONValue::Kind::Array)
                 {
                     auto arr = m_connection->getContainer()->getArray(response.result);
-                    if (arr.getCount() == 8)
+                    if (arr.getCount() == 9)
                     {
                         updatePredefinedMacros(arr[0]);
                         updateSearchPaths(arr[1]);
@@ -167,6 +167,7 @@ SlangResult LanguageServer::parseNextMessage()
                         updateCommitCharacters(arr[3]);
                         updateFormattingOptions(arr[4], arr[5]);
                         updateInlayHintOptions(arr[6], arr[7]);
+                        updateTraceOptions(arr[8]);
                     }
                 }
                 break;
@@ -222,43 +223,90 @@ String getDeclSignatureString(DeclRef<Decl> declRef, ASTBuilder* astBuilder)
 
 static String _formatDocumentation(String doc)
 {
+    bool hasDoxygen = false;
     // TODO: may want to use DocMarkdownWriter in the future to format the text.
     // For now just insert line breaks before `\param` and `\returns` markups.
     List<UnownedStringSlice> lines;
     StringUtil::split(doc.getUnownedSlice(), '\n', lines);
     StringBuilder result;
-    
+    StringBuilder returnDocSB;
+    StringBuilder parameterDocSB;
+    StringBuilder* currentSection = &result;
+    bool isFirstParam = true;
     for (Index i = 0; i < lines.getCount(); i++)
     {
         auto trimedLine = lines[i].trimStart();
-        if (i > 0)
-        {
-            if ((trimedLine.startsWith("\\") || trimedLine.startsWith("@")) && lines[i - 1].trim().getLength() != 0)
-            {
-                result << "  \n";
-            }
-            else
-            {
-                result << "\n";
-            }
-        }
         if (trimedLine.startsWith("\\") || trimedLine.startsWith("@"))
         {
+            hasDoxygen = true;
             trimedLine = trimedLine.tail(1);
             if (trimedLine.startsWith("returns "))
             {
-                trimedLine = trimedLine.subString(8, trimedLine.getLength());
-                result << "**returns** ";
+                trimedLine = trimedLine.tail(8);
+                currentSection = &returnDocSB;
+                (*currentSection) << trimedLine << "  \n";
             }
             else if (trimedLine.startsWith("return "))
             {
-                trimedLine = trimedLine.subString(7, trimedLine.getLength());
-                result << "**Returns** ";
+                trimedLine = trimedLine.tail(7);
+                currentSection = &returnDocSB;
+                (*currentSection) << trimedLine << "  \n";
             }
+            else if (trimedLine.startsWith("param"))
+            {
+                trimedLine = trimedLine.tail(5).trimStart();
+                Index endOfParamName = 0;
+                if (trimedLine.getLength() > 0 && trimedLine[0] == '[')
+                {
+                    endOfParamName = trimedLine.indexOf(']') + 1;
+                    while (endOfParamName < trimedLine.getLength() && CharUtil::isWhitespace(trimedLine[endOfParamName]))
+                        endOfParamName++;
+                }
+                while (endOfParamName < trimedLine.getLength() && !CharUtil::isWhitespace(trimedLine[endOfParamName]))
+                    endOfParamName++;
+                if (isFirstParam)
+                {
+                    isFirstParam = false;
+                }
+                else
+                {
+                    (*currentSection) << "  \n";
+                }
+                if (endOfParamName < trimedLine.getLength())
+                {
+                    parameterDocSB << "`" << trimedLine.head(endOfParamName) << "`";
+                    trimedLine = trimedLine.tail(endOfParamName);
+                }
+                currentSection = &parameterDocSB;
+                (*currentSection) << trimedLine;
+            }
+        }
+        else
+        {
+            (*currentSection) << trimedLine << "\n";
         }
-        result << trimedLine;
     }
     result << "\n";
+    if (parameterDocSB.getLength())
+    {
+        result << "**Parameters**  \n";
+        result << parameterDocSB.ProduceString() << "\n\n";
+    }
+    if (returnDocSB.getLength())
+    {
+        result << "**Returns**  \n";
+        result << returnDocSB.ProduceString();
+    }
+
+    if (!hasDoxygen)
+    {
+        // For ordinary comments, we want to preserve line breaks in the original comment.
+        result.Clear();
+        for (Index i = 0; i < lines.getCount(); i++)
+        {
+            result << lines[i] << "  \n";
+        }
+    }
     return result.ProduceString();
 }
 
@@ -278,6 +326,27 @@ static void _tryGetDocumentation(StringBuilder& sb, WorkspaceVersion* workspace,
     }
 }
 
+void appendDefinitionLocation(StringBuilder& sb, Workspace* workspace, const HumaneSourceLoc& loc)
+{
+    auto path = loc.pathInfo.foundPath;
+    Path::getCanonical(path, path);
+    UnownedStringSlice pathSlice = path.getUnownedSlice();
+    if (workspace)
+    {
+        for (auto& root : workspace->rootDirectories)
+        {
+            if (pathSlice.startsWith(root.getUnownedSlice()))
+            {
+                pathSlice = pathSlice.tail(root.getLength());
+                if (pathSlice.startsWith("\\") || pathSlice.startsWith("/"))
+                    pathSlice = pathSlice.tail(1);
+                break;
+            }
+        }
+    }
+    sb << "Defined in " << pathSlice << "(" << loc.line << ")\n";
+}
+
 SlangResult LanguageServer::hover(
     const LanguageServerProtocol::HoverParams& args, const JSONValue& responseId)
 {
@@ -329,8 +398,7 @@ SlangResult LanguageServer::hover(
 
             auto humaneLoc = version->linkage->getSourceManager()->getHumaneLoc(
                 declRef.getLoc(), SourceLocType::Actual);
-            sb << "Defined in " << humaneLoc.pathInfo.foundPath << "(" << humaneLoc.line
-                << ")\n";
+            appendDefinitionLocation(sb, m_workspace, humaneLoc);
 
             auto nodeHumaneLoc =
                 version->linkage->getSourceManager()->getHumaneLoc(leafNode->loc);
@@ -786,8 +854,7 @@ SlangResult LanguageServer::signatureHelp(
         StringBuilder docSB;
         auto humaneLoc = version->linkage->getSourceManager()->getHumaneLoc(declRef.getLoc(), SourceLocType::Actual);
         _tryGetDocumentation(docSB, version, declRef.getDecl());
-
-        docSB << "Defined in " << humaneLoc.pathInfo.foundPath << "(" << humaneLoc.line << ")\n";
+        appendDefinitionLocation(docSB, m_workspace, humaneLoc);
         sigInfo.documentation.value = docSB.ProduceString();
         sigInfo.documentation.kind = "markdown";
 
@@ -1013,14 +1080,12 @@ SlangResult LanguageServer::onTypeFormatting(const LanguageServerProtocol::Docum
 
 void LanguageServer::publishDiagnostics()
 {
-    time_t timeNow = 0;
-    time(&timeNow);
 
-    if (timeNow - m_lastDiagnosticUpdateTime < 3)
+    if (std::chrono::system_clock::now() - m_lastDiagnosticUpdateTime < std::chrono::milliseconds(1000))
     {
         return;
     }
-    m_lastDiagnosticUpdateTime = timeNow;
+    m_lastDiagnosticUpdateTime = std::chrono::system_clock::now();
 
     auto version = m_workspace->getCurrentVersion();
     // Send updates to clear diagnostics for files that no longer have any messages.
@@ -1166,6 +1231,25 @@ void LanguageServer::updateInlayHintOptions(const JSONValue& deducedTypes, const
     m_inlayHintOptions.showParameterNames = showParameterNames;
 }
 
+void LanguageServer::updateTraceOptions(const JSONValue& value)
+{
+    if (value.isValid())
+    {
+        auto container = m_connection->getContainer();
+        JSONToNativeConverter converter(container, m_connection->getSink());
+        String str;
+        if (SLANG_SUCCEEDED(converter.convert(value, &str)))
+        {
+            if (str == "messages")
+                m_traceOptions = TraceOptions::Messages;
+            else if (str == "verbose")
+                m_traceOptions = TraceOptions::Verbose;
+            else
+                m_traceOptions = TraceOptions::Off;
+        }
+    }
+}
+
 void LanguageServer::sendConfigRequest()
 {
     ConfigurationParams args;
@@ -1186,6 +1270,8 @@ void LanguageServer::sendConfigRequest()
     args.items.add(item);
     item.section = "slang.inlayHints.parameterNames";
     args.items.add(item);
+    item.section = "slangLanguageServer.trace.server";
+    args.items.add(item);
     m_connection->sendCall(
         ConfigurationParams::methodName,
         &args,
@@ -1255,7 +1341,7 @@ SlangResult LanguageServer::tryGetMacroHoverInfo(
     sb << "\n```\n\n";
     auto humaneLoc =
         version->linkage->getSourceManager()->getHumaneLoc(def->loc, SourceLocType::Actual);
-    sb << "Defined in " << humaneLoc.pathInfo.foundPath << "(" << humaneLoc.line << ")\n";
+    appendDefinitionLocation(sb, m_workspace, humaneLoc);
     hover.contents.kind = "markdown";
     hover.contents.value = sb.ProduceString();
     m_connection->sendResult(&hover, responseId);
@@ -1576,7 +1662,7 @@ SlangResult LanguageServer::execute()
 
         auto workTime = platform::PerformanceCounter::getElapsedTimeInSeconds(parseEnd);
 
-        if (commands.getCount() > 0 && m_initialized)
+        if (commands.getCount() > 0 && m_initialized && m_traceOptions != TraceOptions::Off)
         {
             StringBuilder msgBuilder;
             msgBuilder << "Server processed " << commands.getCount() << " commands, executed in "
diff --git a/source/slang/slang-language-server.h b/source/slang/slang-language-server.h
index 963df65f6..7f297d18b 100644
--- a/source/slang/slang-language-server.h
+++ b/source/slang/slang-language-server.h
@@ -1,9 +1,8 @@
 #pragma once
-
+#include <chrono>
 #include "../../slang.h"
 #include "../compiler-core/slang-json-rpc.h"
 #include "../compiler-core/slang-json-rpc-connection.h"
-
 #include "slang-workspace-version.h"
 #include "slang-language-server-completion.h"
 #include "slang-language-server-auto-format.h"
@@ -79,15 +78,22 @@ class LanguageServer
 {
 private:
     static const int kConfigResponseId = 0x1213;
-
+    
 public:
+    enum class TraceOptions
+    {
+        Off,
+        Messages,
+        Verbose
+    };
     bool m_initialized = false;
+    TraceOptions m_traceOptions = TraceOptions::Messages;
     CommitCharacterBehavior m_commitCharacterBehavior = CommitCharacterBehavior::MembersOnly;
     RefPtr<JSONRPCConnection> m_connection;
     ComPtr<slang::IGlobalSession> m_session;
     RefPtr<Workspace> m_workspace;
     Dictionary<String, String> m_lastPublishedDiagnostics;
-    time_t m_lastDiagnosticUpdateTime = 0;
+    std::chrono::time_point<std::chrono::system_clock> m_lastDiagnosticUpdateTime;
     FormatOptions m_formatOptions;
     Slang::InlayHintOptions m_inlayHintOptions;
     bool m_quit = false;
@@ -136,6 +142,7 @@ private:
     void updateCommitCharacters(const JSONValue& value);
     void updateFormattingOptions(const JSONValue& clangFormatLoc, const JSONValue& clangFormatStyle);
     void updateInlayHintOptions(const JSONValue& deducedTypes, const JSONValue& parameterNames);
+    void updateTraceOptions(const JSONValue& value);
 
     void sendConfigRequest();
     void registerCapability(const char* methodName);
diff --git a/source/slang/slang-workspace-version.cpp b/source/slang/slang-workspace-version.cpp
index 4caa711c5..f09b8666d 100644
--- a/source/slang/slang-workspace-version.cpp
+++ b/source/slang/slang-workspace-version.cpp
@@ -150,6 +150,7 @@ void Workspace::init(List<URI> rootDirURI, slang::IGlobalSession* globalSession)
     for (auto uri : rootDirURI)
     {
         auto path = uri.getPath();
+        Path::getCanonical(path, path);
         rootDirectories.add(path);
         DirEnumerationContext context;
         context.workList.add(path);
@@ -459,7 +460,7 @@ ASTMarkup* WorkspaceVersion::getOrCreateMarkupAST(ModuleDecl* module)
     DiagnosticSink sink;
     astMarkup = new ASTMarkup();
     sink.setSourceManager(linkage->getSourceManager());
-    ASTMarkupUtil::extract(module, linkage->getSourceManager(), &sink, astMarkup.Ptr());
+    ASTMarkupUtil::extract(module, linkage->getSourceManager(), &sink, astMarkup.Ptr(), true);
     markupASTs[module] = astMarkup;
     return astMarkup.Ptr();
 }