Merge pull request #1949 from antmicro/lsp-fix-symbol-gathering

[LSP] Add variable symbol gathering to symbol filler

verilog/tools/ls/BUILD

@@ -101,6 +101,7 @@ cc_library(
         "//verilog/CST:module",
         "//verilog/CST:package",
         "//verilog/CST:seq-block",
+        "@com_google_absl//absl/flags:flag",
     ],
 )
 

verilog/tools/ls/document-symbol-filler.cc

@@ -14,6 +14,7 @@
 
 #include "verilog/tools/ls/document-symbol-filler.h"
 
+#include "absl/flags/flag.h"
 #include "common/lsp/lsp-protocol-enums.h"
 #include "common/lsp/lsp-protocol.h"
 #include "common/util/value_saver.h"
@@ -40,14 +41,16 @@ static constexpr verible::lsp::SymbolKind kVSCodeModule =
 
 namespace verilog {
 DocumentSymbolFiller::DocumentSymbolFiller(
-    bool kate_workaround, const verible::TextStructureView &text,
+    bool kate_workaround, bool include_variables,
+    const verible::TextStructureView &text,
     verible::lsp::DocumentSymbol *toplevel)
     : kModuleSymbolKind(kate_workaround ? verible::lsp::SymbolKind::kMethod
                                         : kVSCodeModule),
       kBlockSymbolKind(kate_workaround ? verible::lsp::SymbolKind::kClass
                                        : verible::lsp::SymbolKind::kNamespace),
       text_view_(text),
       current_symbol_(toplevel) {
+  this->include_variables = include_variables;
   toplevel->range.start = {.line = 0, .character = 0};
 }
 
@@ -106,7 +109,28 @@ void DocumentSymbolFiller::Visit(const verible::SyntaxTreeNode &node) {
       }
       break;
     }
-
+    case verilog::NodeEnum::kRegisterVariable: {
+      const auto *variable_name =
+          GetSubtreeAsLeaf(node, NodeEnum::kRegisterVariable, 0);
+      if (variable_name && this->include_variables) {
+        is_visible_node = true;
+        node_symbol.kind = verible::lsp::SymbolKind::kVariable;
+        node_symbol.selectionRange = RangeFromToken(variable_name->get());
+        node_symbol.name = std::string(variable_name->get().text());
+      }
+      break;
+    }
+    case verilog::NodeEnum::kGateInstance: {
+      const auto *variable_name =
+          GetSubtreeAsLeaf(node, NodeEnum::kGateInstance, 0);
+      if (variable_name && this->include_variables) {
+        is_visible_node = true;
+        node_symbol.kind = verible::lsp::SymbolKind::kVariable;
+        node_symbol.selectionRange = RangeFromToken(variable_name->get());
+        node_symbol.name = std::string(variable_name->get().text());
+      }
+      break;
+    }
     case verilog::NodeEnum::kPackageDeclaration: {
       const auto *package_name = verilog::GetPackageNameToken(node);
       if (package_name) {

verilog/tools/ls/document-symbol-filler.h

@@ -26,7 +26,7 @@ class DocumentSymbolFiller : public verible::SymbolVisitor {
   // DocumentSymbol structure.
   // The "kate_workaround" changes some emitted node-types as Kate can't
   // deal with all of them (todo: fix in kate).
-  DocumentSymbolFiller(bool kate_workaround,
+  DocumentSymbolFiller(bool kate_workaround, bool include_variables,
                        const verible::TextStructureView &text,
                        verible::lsp::DocumentSymbol *toplevel);
 
@@ -40,7 +40,7 @@ class DocumentSymbolFiller : public verible::SymbolVisitor {
 
   const int kModuleSymbolKind;  // Might be different if kate-workaround.
   const int kBlockSymbolKind;
-
+  bool include_variables;
   const verible::TextStructureView &text_view_;
   verible::lsp::DocumentSymbol *current_symbol_;
 };

verilog/tools/ls/verible-lsp-adapter.cc

@@ -213,16 +213,16 @@ std::vector<verible::lsp::CodeAction> GenerateCodeActions(
 
 nlohmann::json CreateDocumentSymbolOutline(
     const BufferTracker *tracker, const verible::lsp::DocumentSymbolParams &p,
-    bool kate_compatible_tags) {
+    bool kate_compatible_tags, bool include_variables) {
   if (!tracker) return nlohmann::json::array();
   // Only if the tree has been fully parsed, it makes sense to create an outline
   const auto last_good = tracker->last_good();
   if (!last_good) return nlohmann::json::array();
 
   verible::lsp::DocumentSymbol toplevel;
   const auto &text_structure = last_good->parser().Data();
-  verilog::DocumentSymbolFiller filler(kate_compatible_tags, text_structure,
-                                       &toplevel);
+  verilog::DocumentSymbolFiller filler(kate_compatible_tags, include_variables,
+                                       text_structure, &toplevel);
   const auto &syntax_tree = text_structure.SyntaxTree();
   syntax_tree->Accept(&filler);
   // We cut down one level, not interested in toplevel file:

verilog/tools/ls/verible-lsp-adapter.h

@@ -52,7 +52,7 @@ verible::lsp::FullDocumentDiagnosticReport GenerateDiagnosticReport(
 // boolean to make it visible what is needed.
 nlohmann::json CreateDocumentSymbolOutline(
     const BufferTracker *tracker, const verible::lsp::DocumentSymbolParams &p,
-    bool kate_compatible_tags = false);
+    bool kate_compatible_tags = false, bool include_variables = true);
 
 // Given a position in a document, return ranges in the buffer that should
 // be highlighted.

verilog/tools/ls/verilog-language-server.cc

@@ -17,13 +17,17 @@
 #include <functional>
 #include <memory>
 
+#include "absl/flags/flag.h"
 #include "absl/strings/string_view.h"
 #include "common/lsp/lsp-file-utils.h"
 #include "common/lsp/lsp-protocol.h"
 #include "common/util/file_util.h"
 #include "common/util/init_command_line.h"
 #include "verilog/tools/ls/verible-lsp-adapter.h"
 
+ABSL_FLAG(bool, variables_in_outline, true,
+          "Variables should be included into the symbol outline");
+
 namespace verilog {
 
 VerilogLanguageServer::VerilogLanguageServer(const WriteFun &write_fun)
@@ -50,6 +54,7 @@ VerilogLanguageServer::VerilogLanguageServer(const WriteFun &write_fun)
 verible::lsp::InitializeResult VerilogLanguageServer::GetCapabilities() {
   // send response with information what we do.
   verible::lsp::InitializeResult result;
+  this->include_variables = absl::GetFlag(FLAGS_variables_in_outline);
   result.serverInfo = {
       .name = "Verible Verilog language server.",
       .version = verible::GetRepositoryVersion(),
@@ -104,8 +109,11 @@ void VerilogLanguageServer::SetRequestHandlers() {
   dispatcher_.AddRequestHandler(  // Provide document outline/index
       "textDocument/documentSymbol",
       [this](const verible::lsp::DocumentSymbolParams &p) {
+        // The `false` sets the kate workaround to the set default, as it was
+        // not set here
         return verilog::CreateDocumentSymbolOutline(
-            parsed_buffers_.FindBufferTrackerOrNull(p.textDocument.uri), p);
+            parsed_buffers_.FindBufferTrackerOrNull(p.textDocument.uri), p,
+            false, this->include_variables);
       });
 
   dispatcher_.AddRequestHandler(  // Highlight related symbols under cursor

verilog/tools/ls/verilog-language-server.h

@@ -46,6 +46,9 @@ class VerilogLanguageServer {
   // Prints statistics of the current Language Server session.
   void PrintStatistics() const;
 
+  // A flag that sets inclusion of variables into documentSymbol requests
+  bool include_variables = true;
+
  private:
   // Creates callbacks for requests from Language Server Client
   void SetRequestHandlers();

verilog/tools/ls/verilog-language-server_test.cc

@@ -288,6 +288,11 @@ endpackage
 module mini(input clk);
   always@(posedge clk) begin : labelled_block
   end
+
+  reg foo;
+  net bar;
+  some_class baz();
+
 endmodule
 )");
 
@@ -335,11 +340,94 @@ endmodule
 
   // Descent tree into module and find labelled block.
   std::vector<verible::lsp::DocumentSymbol> module = toplevel[1].children;
-  EXPECT_EQ(module.size(), 1);
+  EXPECT_EQ(module.size(), 4);
   EXPECT_EQ(module[0].kind, verible::lsp::SymbolKind::kNamespace);
   EXPECT_EQ(module[0].name, "labelled_block");
+
+  EXPECT_EQ(module[1].kind, verible::lsp::SymbolKind::kVariable);
+  EXPECT_EQ(module[1].name, "foo");
+
+  EXPECT_EQ(module[2].kind, verible::lsp::SymbolKind::kVariable);
+  EXPECT_EQ(module[2].name, "bar");
+
+  EXPECT_EQ(module[3].kind, verible::lsp::SymbolKind::kVariable);
+  EXPECT_EQ(module[3].name, "baz");
 }
 
+TEST_F(VerilogLanguageServerTest, DocumentSymbolRequestWithoutVariablesTest) {
+  server_->include_variables = false;
+  // Create file, absorb diagnostics
+  const std::string mini_module = DidOpenRequest("file://mini_pkg.sv", R"(
+package mini;
+
+function static void fun_foo();
+endfunction
+
+class some_class;
+   function void member();
+   endfunction
+endclass
+endpackage
+
+module mini(input clk);
+  always@(posedge clk) begin : labelled_block
+  end
+
+  reg foo;
+  net bar;
+  some_class baz();
+
+endmodule
+)");
+
+  ASSERT_OK(SendRequest(mini_module));
+
+  // Expect to receive diagnostics right away. Ignore.
+  const json diagnostics = json::parse(GetResponse());
+  EXPECT_EQ(diagnostics["method"], "textDocument/publishDiagnostics")
+      << "textDocument/publishDiagnostics not received";
+
+  // Request a document symbol
+  const absl::string_view document_symbol_request =
+      R"({"jsonrpc":"2.0", "id":11, "method":"textDocument/documentSymbol","params":{"textDocument":{"uri":"file://mini_pkg.sv"}}})";
+  ASSERT_OK(SendRequest(document_symbol_request));
+
+  // TODO: by default, the Kate workarounds are active, so
+  // Module -> Method and Namespace -> Class. Remove by default.
+  const json document_symbol = json::parse(GetResponse());
+  EXPECT_EQ(document_symbol["id"], 11);
+
+  std::vector<verible::lsp::DocumentSymbol> toplevel =
+      document_symbol["result"];
+  EXPECT_EQ(toplevel.size(), 2);
+
+  EXPECT_EQ(toplevel[0].kind, verible::lsp::SymbolKind::kPackage);
+  EXPECT_EQ(toplevel[0].name, "mini");
+
+  EXPECT_EQ(toplevel[1].kind, verible::lsp::SymbolKind::kMethod);  // module.
+  EXPECT_EQ(toplevel[1].name, "mini");
+
+  // Descend tree into package and look at expected nested symbols there.
+  std::vector<verible::lsp::DocumentSymbol> package = toplevel[0].children;
+  EXPECT_EQ(package.size(), 2);
+  EXPECT_EQ(package[0].kind, verible::lsp::SymbolKind::kFunction);
+  EXPECT_EQ(package[0].name, "fun_foo");
+
+  EXPECT_EQ(package[1].kind, verible::lsp::SymbolKind::kClass);
+  EXPECT_EQ(package[1].name, "some_class");
+
+  // Descend tree into class and find nested function.
+  std::vector<verible::lsp::DocumentSymbol> class_block = package[1].children;
+  EXPECT_EQ(class_block.size(), 1);
+  EXPECT_EQ(class_block[0].kind, verible::lsp::SymbolKind::kFunction);
+  EXPECT_EQ(class_block[0].name, "member");
+
+  // Descent tree into module and find labelled block.
+  std::vector<verible::lsp::DocumentSymbol> module = toplevel[1].children;
+  EXPECT_EQ(module.size(), 1);
+  EXPECT_EQ(module[0].kind, verible::lsp::SymbolKind::kNamespace);
+  EXPECT_EQ(module[0].name, "labelled_block");
+}
 // Tests closing of the file in the LS context and checks if the LS
 // responds gracefully to textDocument/documentSymbol request for
 // closed file.