lsp: improve LSP IR2 hovers (#1891)

Some new hover features:

docstrings (`@param/@return` syntax is not yet supported, nor is a
syntax-highlighted preview, will do later)

![image](https://user-images.githubusercontent.com/13153231/190830490-d0ef774c-e7e5-4bb9-8007-b366be0f491e.png)

better MIPs instruction descriptions

![image](https://user-images.githubusercontent.com/13153231/190830507-0bb35c13-7e88-4b74-a63b-b7fb3587b82e.png)

numeric hovers convert to different bases / tell you the OpenGOAL method
id (i can't tell you the amount of times ive done -16 / 4 manually
myself...)

![image](https://user-images.githubusercontent.com/13153231/190830674-f66ed15a-f983-48ff-b251-259374dfbcac.png)

Closes https://github.com/open-goal/opengoal-vscode/issues/33
Closes https://github.com/open-goal/opengoal-vscode/issues/31
Related to https://github.com/open-goal/opengoal-vscode/issues/29

lsp/handlers/lsp_router.cpp

@@ -62,12 +62,11 @@ std::optional<std::vector<std::string>> LSPRouter::route_message(
     const MessageBuffer& message_buffer,
     AppState& appstate) {
   const json& body = message_buffer.body();
-  auto method = body["method"];
-  lg::info(method);
+  auto& method = body["method"];
 
   // If the workspace has not yet been initialized but the client sends a
   // message that doesn't have method "initialize" then we'll return an error
-  // as per LSP spec.
+  // as per the LSP spec.
   if (method != "initialize" && !appstate.workspace.is_initialized()) {
     auto error = {
         make_response(error_resp(ErrorCodes::ServerNotInitialized, "Server not yet initialized."))};
@@ -94,13 +93,14 @@ std::optional<std::vector<std::string>> LSPRouter::route_message(
         break;
       case LSPRouteType::REQUEST_RESPONSE:
         auto resp_body = route.m_request_handler(appstate.workspace, body["id"], body["params"]);
+        json resp;
+        resp["id"] = body["id"];
         if (resp_body) {
-          json resp;
-          // TODO - this should be the job of the handler, not here!
-          resp["id"] = body["id"];
           resp["result"] = resp_body.value();
-          resp_bodies.push_back(resp);
+        } else {
+          resp["result"] = nullptr;
         }
+        resp_bodies.push_back(resp);
         break;
     }
 

lsp/handlers/text_document/go_to.h

@@ -19,25 +19,25 @@ std::optional<json> go_to_definition_handler(Workspace& workspace, int id, json
 
   if (!symbol_name) {
     lg::debug("GOTODEF - no symbol");
-    return locations;
+    return {};
   }
 
   lg::debug("GOTODEF - symbol - {}", symbol_name.value());
 
-  auto symbol_info =
-      workspace.get_symbol_info_from_all_types(symbol_name.value(), tracked_file->m_all_types_uri);
+  auto symbol_info = workspace.get_definition_info_from_all_types(symbol_name.value(),
+                                                                  tracked_file->m_all_types_uri);
 
   if (!symbol_info) {
     lg::debug("GOTODEF - no symbol info");
-    return locations;
+    return {};
   }
 
   LSPSpec::Location location;
   location.m_uri = tracked_file->m_all_types_uri;
-  location.m_range.m_start = {(uint32_t)symbol_info->line_idx_to_display,
-                              (uint32_t)symbol_info->pos_in_line};
-  location.m_range.m_end = {(uint32_t)symbol_info->line_idx_to_display,
-                            (uint32_t)symbol_info->pos_in_line};
+  location.m_range.m_start = {(uint32_t)symbol_info.value().definition_info->line_idx_to_display,
+                              (uint32_t)symbol_info.value().definition_info->pos_in_line};
+  location.m_range.m_end = {(uint32_t)symbol_info.value().definition_info->line_idx_to_display,
+                            (uint32_t)symbol_info.value().definition_info->pos_in_line};
   locations.push_back(location);
 
   return locations;

lsp/handlers/text_document/hover.h

@@ -1,10 +1,19 @@
 #include <optional>
+#include <regex>
 
 #include "lsp/protocol/common_types.h"
 #include "lsp/protocol/hover.h"
 #include "lsp/state/data/mips_instructions.h"
 #include "lsp/state/workspace.h"
 
+bool is_number(const std::string& s) {
+  return !s.empty() && std::find_if(s.begin(), s.end(),
+                                    [](unsigned char c) { return !std::isdigit(c); }) == s.end();
+}
+
+std::vector<std::string> og_method_names = {"new",      "delete", "print",    "inspect",  "length",
+                                            "asize-of", "copy",   "relocate", "mem-usage"};
+
 std::optional<json> hover_handler(Workspace& workspace, int id, json params) {
   auto converted_params = params.get<LSPSpec::TextDocumentPositionParams>();
   auto tracked_file = workspace.get_tracked_ir_file(converted_params.m_textDocument.m_uri);
@@ -13,32 +22,143 @@ std::optional<json> hover_handler(Workspace& workspace, int id, json params) {
     return {};
   }
 
+  // See if it's an OpenGOAL symbol or a MIPS mnemonic
+  auto symbol_name = tracked_file->get_symbol_at_position(converted_params.m_position);
   auto token_at_pos = tracked_file->get_mips_instruction_at_position(converted_params.m_position);
+  if (!symbol_name && !token_at_pos) {
+    return {};
+  }
+
+  // TODO - try specifying the range so it highlights everything, ie. `c.lt.s`
 
   LSPSpec::MarkupContent markup;
   markup.m_kind = "markdown";
+
+  // Prefer symbols
+  if (symbol_name) {
+    lg::debug("hover - symbol match - {}", symbol_name.value());
+    auto symbol_info = workspace.get_definition_info_from_all_types(symbol_name.value(),
+                                                                    tracked_file->m_all_types_uri);
+    if (symbol_info && symbol_info.value().docstring.has_value()) {
+      std::string docstring = symbol_info.value().docstring.value();
+      lg::debug("hover - symbol has docstring - {}", docstring);
+      // A docstring exists, print it!
+      // By convention, docstrings are assumed to be markdown, they support code-blocks everything
+      // the only thing extra we do, is replace [[<symbol>]] with links if available
+      std::unordered_map<std::string, std::string> symbol_replacements = {};
+      std::smatch match;
+
+      std::string::const_iterator searchStart(docstring.cbegin());
+      while (
+          std::regex_search(searchStart, docstring.cend(), match, std::regex("\\[{2}(.*)\\]{2}"))) {
+        // Have we already accounted for this symbol?
+        const auto& name = match[1].str();
+        if (symbol_replacements.count(name) != 0) {
+          continue;
+        }
+        // Get this symbols info
+        auto symbol_info =
+            workspace.get_definition_info_from_all_types(name, tracked_file->m_all_types_uri);
+        if (!symbol_info) {
+          symbol_replacements[name] = fmt::format("_{}_", name);
+        } else {
+          // Construct path
+          auto symbol_uri =
+              fmt::format("{}#L{}%2C{}", tracked_file->m_all_types_uri,
+                          symbol_info.value().definition_info->line_idx_to_display + 1,
+                          symbol_info.value().definition_info->pos_in_line);
+          symbol_replacements[name] = fmt::format("[{}]({})", name, symbol_uri);
+        }
+        searchStart = match.suffix().first;
+      }
+      // Replace all symbol occurences
+      for (const auto& [key, val] : symbol_replacements) {
+        docstring = std::regex_replace(docstring, std::regex("\\[{2}" + key + "\\]{2}"), val);
+      }
+
+      markup.m_value = docstring;
+      LSPSpec::Hover hover_resp;
+      hover_resp.m_contents = markup;
+      return hover_resp;
+    } else if (!token_at_pos) {
+      // Check if it's a number, and if so we'll do some numeric conversions
+      if (!is_number(symbol_name.value())) {
+        return {};
+      }
+      lg::debug("hover - numeric match - {}", symbol_name.value());
+      // Construct the body
+      std::string body = "";
+      uint32_t num = std::atoi(symbol_name.value().data());
+      // Assuming it comes in as Decimal
+      body += "| Base    | Value |\n";
+      body += "|---------|-------|\n";
+      body += fmt::format("| Decimal        | `{:d}` |\n", num);
+      body += fmt::format("| Hex            | `{:X}` |\n", num);
+      // TODO - would be nice to format as groups of 4
+      body += fmt::format("| Binary         | `{:b}` |\n", num);
+      if (num >= 16 && (num - 16) % 4 == 0) {
+        uint32_t method_id = (num - 16) / 4;
+        std::string method_name = "not built-in";
+        if (method_id <= 8) {
+          method_name = og_method_names.at(method_id);
+        }
+        body += fmt::format("| Method ID   | `{}` - `{}` |\n", method_id, method_name);
+      }
+      body += fmt::format("| Octal          | `{:o}` |\n", num);
+
+      markup.m_value = body;
+      LSPSpec::Hover hover_resp;
+      hover_resp.m_contents = markup;
+      return hover_resp;
+    }
+  }
+
+  // Otherwise, maybe it's a MIPS instruction
   if (token_at_pos) {
-    auto token = token_at_pos.value();
+    lg::debug("hover - token match - {}", token_at_pos.value());
+    auto& token = token_at_pos.value();
     std::transform(token.begin(), token.end(), token.begin(),
                    [](unsigned char c) { return std::tolower(c); });
     // Find the instruction, there are some edge-cases here where they could be multiple
-    // TODO - handle those (print both is an easy fix?)
     // TODO - havn't addressed `bc` and such instructions!  Those need to be prefixed matched
+    std::vector<std::string> ee_instructions = {};
+    std::vector<std::string> vu_instructions = {};
     for (const auto& instr : LSPData::MIPS_INSTRUCTION_LIST) {
       auto mnemonic_lower = instr.mnemonic;
       std::transform(mnemonic_lower.begin(), mnemonic_lower.end(), mnemonic_lower.begin(),
                      [](unsigned char c) { return std::tolower(c); });
       if (mnemonic_lower == token) {
-        markup.m_value = fmt::format("### {}\n{}", instr.mnemonic, instr.description);
-        break;
+        if (instr.type == "ee") {
+          ee_instructions.push_back(fmt::format("- _{}_\n\n", instr.description));
+        } else {
+          vu_instructions.push_back(fmt::format("- _{}_\n\n", instr.description));
+        }
       }
     }
-  } else {
-    return {};
-  }
-  LSPSpec::Hover hover_resp;
-  hover_resp.m_contents = markup;
-  // TODO - try specifying the range so it highlights everything, ie. `c.lt.s`
 
-  return hover_resp;
+    // Construct the body
+    std::string body = "";
+    if (!ee_instructions.empty()) {
+      body += "**EE Instructions**\n\n";
+      for (const auto& instr : ee_instructions) {
+        body += instr;
+      }
+      body += "___\n\n";
+    }
+
+    if (!vu_instructions.empty()) {
+      body += "**VU Instructions**\n\n";
+      for (const auto& instr : vu_instructions) {
+        body += instr;
+      }
+      body += "___\n\n";
+    }
+
+    markup.m_value = body;
+    LSPSpec::Hover hover_resp;
+    hover_resp.m_contents = markup;
+    return hover_resp;
+  }
+
+  return {};
 }

lsp/main.cpp

@@ -92,7 +92,7 @@ int main(int argc, char** argv) {
         for (const auto& response : responses.value()) {
           std::cout << response.c_str() << std::flush;
           if (appstate.verbose) {
-            lg::trace("<<< Sending message: {}", response);
+            lg::debug("<<< Sending message: {}", response);
           } else {
             lg::info("<<< Sending message of method '{}'", method_name);
           }

lsp/state/workspace.cpp

@@ -61,7 +61,7 @@ std::optional<WorkspaceIRFile> Workspace::get_tracked_ir_file(const LSPSpec::URI
   return m_tracked_ir_files[file_uri];
 }
 
-std::optional<goos::TextDb::ShortInfo> Workspace::get_symbol_info_from_all_types(
+std::optional<DefinitionMetadata> Workspace::get_definition_info_from_all_types(
     const std::string& symbol_name,
     const LSPSpec::DocumentUri& all_types_uri) {
   if (m_tracked_all_types_files.count(all_types_uri) == 0) {
@@ -71,7 +71,7 @@ std::optional<goos::TextDb::ShortInfo> Workspace::get_symbol_info_from_all_types
   if (dts.symbol_metadata_map.count(symbol_name) == 0) {
     return {};
   }
-  return dts.symbol_metadata_map.at(symbol_name).definition_info;
+  return dts.symbol_metadata_map.at(symbol_name);
 }
 
 void Workspace::start_tracking_file(const LSPSpec::DocumentUri& file_uri,
@@ -163,12 +163,11 @@ void WorkspaceIRFile::find_all_types_path(const std::string& line) {
 
   if (std::regex_search(line, matches, regex)) {
     if (matches.size() == 3) {
-      auto game_version = matches[1];
-      auto all_types_path = matches[2];
+      const auto& game_version = matches[1];
+      const auto& all_types_path = matches[2];
       lg::debug("Found DTS Path - {} : {}", game_version.str(), all_types_path.str());
       auto all_types_uri = uri_from_path(fs::path(all_types_path.str()));
       lg::debug("DTS URI - {}", all_types_uri);
-
       if (valid_game_version(game_version.str())) {
         m_game_version = game_name_to_version(game_version.str());
         m_all_types_uri = all_types_uri;
@@ -188,7 +187,7 @@ void WorkspaceIRFile::find_function_symbol(const uint32_t line_num_zero_based,
   if (std::regex_search(line, matches, regex)) {
     // NOTE - assumes we can only find 1 function per line
     if (matches.size() == 2) {
-      auto match = matches[1];
+      const auto& match = matches[1];
       lg::info("Adding Symbol - {}", match.str());
       LSPSpec::DocumentSymbol new_symbol;
       new_symbol.m_name = match.str();
@@ -209,7 +208,6 @@ void WorkspaceIRFile::find_function_symbol(const uint32_t line_num_zero_based,
 
   std::regex end_function("^;; \\.endfunction\\s*$");
   if (std::regex_match(line, end_function)) {
-    lg::debug("Found end of previous function on line - {}", line);
     // Set the previous symbols end-line
     if (!m_symbols.empty()) {
       m_symbols[m_symbols.size() - 1].m_range.m_end.m_line = line_num_zero_based - 1;
@@ -234,8 +232,7 @@ void WorkspaceIRFile::identify_diagnostics(const uint32_t line_num_zero_based,
   if (std::regex_search(line, info_matches, info_regex)) {
     // NOTE - assumes we can only find 1 function per line
     if (info_matches.size() == 2) {
-      auto match = info_matches[1];
-      lg::debug("Found info-level diagnostic - {}", match.str());
+      const auto& match = info_matches[1];
       LSPSpec::Diagnostic new_diag;
       new_diag.m_severity = LSPSpec::DiagnosticSeverity::Information;
       new_diag.m_message = match.str();
@@ -249,8 +246,7 @@ void WorkspaceIRFile::identify_diagnostics(const uint32_t line_num_zero_based,
   if (std::regex_search(line, warn_matches, warn_regex)) {
     // NOTE - assumes we can only find 1 function per line
     if (warn_matches.size() == 2) {
-      auto match = warn_matches[1];
-      lg::debug("Found warn-level diagnostic - {}", match.str());
+      const auto& match = warn_matches[1];
       LSPSpec::Diagnostic new_diag;
       new_diag.m_severity = LSPSpec::DiagnosticSeverity::Warning;
       new_diag.m_message = match.str();
@@ -264,8 +260,7 @@ void WorkspaceIRFile::identify_diagnostics(const uint32_t line_num_zero_based,
   if (std::regex_search(line, error_matches, error_regex)) {
     // NOTE - assumes we can only find 1 function per line
     if (error_matches.size() == 2) {
-      auto match = error_matches[1];
-      lg::debug("Found error-level diagnostic - {}", match.str());
+      const auto& match = error_matches[1];
       LSPSpec::Diagnostic new_diag;
       new_diag.m_severity = LSPSpec::DiagnosticSeverity::Error;
       new_diag.m_message = match.str();
@@ -285,8 +280,7 @@ std::optional<std::string> WorkspaceIRFile::get_mips_instruction_at_position(
   std::regex regex("[\\w\\.]+");
 
   if (std::regex_search(line, matches, regex)) {
-    auto match = matches[0];
-    lg::info("hover first match - {}", match.str());
+    const auto& match = matches[0];
     auto match_start = matches.position(0);
     auto match_end = match_start + match.length();
     if (position.m_character >= match_start && position.m_character <= match_end) {
@@ -301,18 +295,15 @@ std::optional<std::string> WorkspaceIRFile::get_symbol_at_position(
     const LSPSpec::Position position) {
   // Split the line on typical word boundaries
   std::string line = m_lines.at(position.m_line);
-  lg::debug("symbol checking - '{}'", line);
   std::smatch matches;
   std::regex regex("[\\w\\.\\-_!<>*]+");
   std::regex_token_iterator<std::string::iterator> rend;
 
   std::regex_token_iterator<std::string::iterator> match(line.begin(), line.end(), regex);
   while (match != rend) {
-    lg::debug("match - '{}'", match->str());
     auto match_start = std::distance(line.begin(), match->first);
     auto match_end = match_start + match->length();
     if (position.m_character >= match_start && position.m_character <= match_end) {
-      lg::debug("returning");
       return match->str();
     }
     match++;

lsp/state/workspace.h

@@ -69,9 +69,9 @@ class Workspace {
   void update_tracked_file(const LSPSpec::DocumentUri& file_uri, const std::string& content);
   void stop_tracking_file(const LSPSpec::DocumentUri& file_uri);
   std::optional<WorkspaceIRFile> get_tracked_ir_file(const LSPSpec::URI& file_uri);
-  std::optional<goos::TextDb::ShortInfo> get_symbol_info_from_all_types(
+  std::optional<DefinitionMetadata> get_definition_info_from_all_types(
       const std::string& symbol_name,
-      const std::string& all_types_uri);
+      const LSPSpec::DocumentUri& all_types_uri);
 
  private:
   bool m_initialized = false;