diff --git a/verilog/tools/ls/BUILD b/verilog/tools/ls/BUILD index bda4e6dfe..520b70b24 100644 --- a/verilog/tools/ls/BUILD +++ b/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", ], ) diff --git a/verilog/tools/ls/document-symbol-filler.cc b/verilog/tools/ls/document-symbol-filler.cc index b725283a0..689935505 100644 --- a/verilog/tools/ls/document-symbol-filler.cc +++ b/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,7 +41,8 @@ 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), @@ -48,6 +50,7 @@ DocumentSymbolFiller::DocumentSymbolFiller( : 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) { diff --git a/verilog/tools/ls/document-symbol-filler.h b/verilog/tools/ls/document-symbol-filler.h index b7169a0fd..68e563392 100644 --- a/verilog/tools/ls/document-symbol-filler.h +++ b/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_; }; diff --git a/verilog/tools/ls/verible-lsp-adapter.cc b/verilog/tools/ls/verible-lsp-adapter.cc index d770586e2..46976ac64 100644 --- a/verilog/tools/ls/verible-lsp-adapter.cc +++ b/verilog/tools/ls/verible-lsp-adapter.cc @@ -213,7 +213,7 @@ std::vector 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(); @@ -221,8 +221,8 @@ nlohmann::json CreateDocumentSymbolOutline( 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: diff --git a/verilog/tools/ls/verible-lsp-adapter.h b/verilog/tools/ls/verible-lsp-adapter.h index a2e55e370..44a7d2fbd 100644 --- a/verilog/tools/ls/verible-lsp-adapter.h +++ b/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. diff --git a/verilog/tools/ls/verilog-language-server.cc b/verilog/tools/ls/verilog-language-server.cc index 0af2d0f08..f33de737a 100644 --- a/verilog/tools/ls/verilog-language-server.cc +++ b/verilog/tools/ls/verilog-language-server.cc @@ -17,6 +17,7 @@ #include #include +#include "absl/flags/flag.h" #include "absl/strings/string_view.h" #include "common/lsp/lsp-file-utils.h" #include "common/lsp/lsp-protocol.h" @@ -24,6 +25,9 @@ #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 diff --git a/verilog/tools/ls/verilog-language-server.h b/verilog/tools/ls/verilog-language-server.h index 303c5447d..83c807b99 100644 --- a/verilog/tools/ls/verilog-language-server.h +++ b/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(); diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc index 7f8b230f6..646714e44 100644 --- a/verilog/tools/ls/verilog-language-server_test.cc +++ b/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 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 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 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 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 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.