From b5c7d027a8930ad87dc545c24e2b619fde2a119a Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Tue, 9 May 2023 18:38:20 +0200 Subject: [PATCH 01/13] verilog: analysis: symbol_table: Added SymbolMetaTypeAsString to symbol_table header Method SymbolMetaTypeAsString is now provided by header to allow printing symbol meta types, e.g. for hover purposes Signed-off-by: Grzegorz Latosinski --- verilog/analysis/symbol_table.cc | 2 +- verilog/analysis/symbol_table.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/verilog/analysis/symbol_table.cc b/verilog/analysis/symbol_table.cc index 8385bd4ba..4bf6dbbe9 100644 --- a/verilog/analysis/symbol_table.cc +++ b/verilog/analysis/symbol_table.cc @@ -96,7 +96,7 @@ std::ostream& operator<<(std::ostream& stream, SymbolMetaType symbol_type) { return SymbolMetaTypeNames().Unparse(symbol_type, stream); } -static absl::string_view SymbolMetaTypeAsString(SymbolMetaType type) { +absl::string_view SymbolMetaTypeAsString(SymbolMetaType type) { return SymbolMetaTypeNames().EnumName(type); } diff --git a/verilog/analysis/symbol_table.h b/verilog/analysis/symbol_table.h index ac6a71a37..3ff395580 100644 --- a/verilog/analysis/symbol_table.h +++ b/verilog/analysis/symbol_table.h @@ -67,6 +67,8 @@ enum class SymbolMetaType { std::ostream& operator<<(std::ostream&, SymbolMetaType); +absl::string_view SymbolMetaTypeAsString(SymbolMetaType type); + // This classifies the type of reference that a single identifier is. enum class ReferenceType { // The base identifier in any chain. From 282c9b726bb676630334f170fe8ec28b5b835bee Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Tue, 9 May 2023 18:44:00 +0200 Subject: [PATCH 02/13] verilog: tools: ls: symbol-table-handler: Generalized methods for finding definitions and tokens To be able to use FindDefinitionLocation in other use cases than goto-definition, the parameters are now of type TextDocumentPositionParams. Also, GetTokenAtTextDocumentPosition now returns optional TokenInfo instead of absl::string_view, to provide more useful information regarding the symbol. Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/symbol-table-handler.cc | 18 +++++++++++------- verilog/tools/ls/symbol-table-handler.h | 16 ++++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/verilog/tools/ls/symbol-table-handler.cc b/verilog/tools/ls/symbol-table-handler.cc index 39d66b00c..a6f992fc1 100644 --- a/verilog/tools/ls/symbol-table-handler.cc +++ b/verilog/tools/ls/symbol-table-handler.cc @@ -221,9 +221,10 @@ void SymbolTableHandler::Prepare() { } } -absl::string_view SymbolTableHandler::GetTokenAtTextDocumentPosition( +std::optional +SymbolTableHandler::GetTokenAtTextDocumentPosition( const verible::lsp::TextDocumentPositionParams ¶ms, - const verilog::BufferTrackerContainer &parsed_buffers) { + const verilog::BufferTrackerContainer &parsed_buffers) const { const verilog::BufferTracker *tracker = parsed_buffers.FindBufferTrackerOrNull(params.textDocument.uri); if (!tracker) { @@ -240,8 +241,7 @@ absl::string_view SymbolTableHandler::GetTokenAtTextDocumentPosition( params.position.character}; const verible::TextStructureView &text = parsedbuffer->parser().Data(); - const verible::TokenInfo cursor_token = text.FindTokenAt(cursor); - return cursor_token.text(); + return text.FindTokenAt(cursor); } std::optional @@ -263,14 +263,16 @@ SymbolTableHandler::GetLocationFromSymbolName( } std::vector SymbolTableHandler::FindDefinitionLocation( - const verible::lsp::DefinitionParams ¶ms, + const verible::lsp::TextDocumentPositionParams ¶ms, const verilog::BufferTrackerContainer &parsed_buffers) { // TODO add iterating over multiple definitions Prepare(); const absl::string_view filepath = LSPUriToPath(params.textDocument.uri); std::string relativepath = curr_project_->GetRelativePathToSource(filepath); - absl::string_view symbol = + std::optional token = GetTokenAtTextDocumentPosition(params, parsed_buffers); + if (!token) return {}; + absl::string_view symbol = token->text(); VLOG(1) << "Looking for symbol: " << symbol; VerilogSourceFile *reffile = curr_project_->LookupRegisteredFile(relativepath); @@ -305,8 +307,10 @@ std::vector SymbolTableHandler::FindReferencesLocations( const verible::lsp::ReferenceParams ¶ms, const verilog::BufferTrackerContainer &parsed_buffers) { Prepare(); - absl::string_view symbol = + std::optional token = GetTokenAtTextDocumentPosition(params, parsed_buffers); + if (!token) return {}; + absl::string_view symbol = token->text(); const SymbolTableNode &root = symbol_table_->Root(); const SymbolTableNode *node = ScanSymbolTreeForDefinition(&root, symbol); if (!node) { diff --git a/verilog/tools/ls/symbol-table-handler.h b/verilog/tools/ls/symbol-table-handler.h index b80889a2b..54ce923fc 100644 --- a/verilog/tools/ls/symbol-table-handler.h +++ b/verilog/tools/ls/symbol-table-handler.h @@ -52,7 +52,7 @@ class SymbolTableHandler { // message delivered i.e. in textDocument/definition message. // Provides a list of locations with symbol's definitions. std::vector FindDefinitionLocation( - const verible::lsp::DefinitionParams ¶ms, + const verible::lsp::TextDocumentPositionParams ¶ms, const verilog::BufferTrackerContainer &parsed_buffers); // Finds the symbol of the definition for the given identifier. @@ -73,6 +73,13 @@ class SymbolTableHandler { // Creates a symbol table for entire project (public: needed in unit-test) std::vector BuildProjectSymbolTable(); + // Returns TokenInfo for token pointed by the LSP request based on + // TextDocumentPositionParams. If text is not found, empty-initialized + // string_view is returned. + std::optional GetTokenAtTextDocumentPosition( + const verible::lsp::TextDocumentPositionParams ¶ms, + const verilog::BufferTrackerContainer &parsed_buffers) const; + private: // prepares structures for symbol-based requests void Prepare(); @@ -81,13 +88,6 @@ class SymbolTableHandler { // method. void ResetSymbolTable(); - // Returns text pointed by the LSP request based on - // TextDocumentPositionParams. If text is not found, empty-initialized - // string_view is returned. - absl::string_view GetTokenAtTextDocumentPosition( - const verible::lsp::TextDocumentPositionParams ¶ms, - const verilog::BufferTrackerContainer &parsed_buffers); - // Returns the Location of the symbol name in source file // pointed by the file_origin. // If given symbol name is not found, std::nullopt is returned. From f02b3e48fd27f3c1506f8000df733caa9a1abe92 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Tue, 9 May 2023 18:47:33 +0200 Subject: [PATCH 03/13] verilog: tools: ls: symbol-table-handler: Extracted method for finding SymbolTableNode with definition Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/symbol-table-handler.cc | 12 +++++++----- verilog/tools/ls/symbol-table-handler.h | 3 +++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/verilog/tools/ls/symbol-table-handler.cc b/verilog/tools/ls/symbol-table-handler.cc index a6f992fc1..a41d557b5 100644 --- a/verilog/tools/ls/symbol-table-handler.cc +++ b/verilog/tools/ls/symbol-table-handler.cc @@ -292,13 +292,15 @@ std::vector SymbolTableHandler::FindDefinitionLocation( return {*location}; } +const SymbolTableNode *SymbolTableHandler::FindDefinitionNode( + absl::string_view symbol) { + Prepare(); + return ScanSymbolTreeForDefinition(&symbol_table_->Root(), symbol); +} + const verible::Symbol *SymbolTableHandler::FindDefinitionSymbol( absl::string_view symbol) { - if (files_dirty_) { - BuildProjectSymbolTable(); - } - const SymbolTableNode *symbol_table_node = - ScanSymbolTreeForDefinition(&symbol_table_->Root(), symbol); + const SymbolTableNode *symbol_table_node = FindDefinitionNode(symbol); if (symbol_table_node) return symbol_table_node->Value().syntax_origin; return nullptr; } diff --git a/verilog/tools/ls/symbol-table-handler.h b/verilog/tools/ls/symbol-table-handler.h index 54ce923fc..defc0ca35 100644 --- a/verilog/tools/ls/symbol-table-handler.h +++ b/verilog/tools/ls/symbol-table-handler.h @@ -55,6 +55,9 @@ class SymbolTableHandler { const verible::lsp::TextDocumentPositionParams ¶ms, const verilog::BufferTrackerContainer &parsed_buffers); + // Finds the node of the symbol table with definition for a given symbol. + const SymbolTableNode *FindDefinitionNode(absl::string_view symbol); + // Finds the symbol of the definition for the given identifier. const verible::Symbol *FindDefinitionSymbol(absl::string_view symbol); From 264f611c4f68995a9b28eb22eaf8ac4c8c3b86e2 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Tue, 9 May 2023 18:50:19 +0200 Subject: [PATCH 04/13] verilog: tools: ls: hover: Added initial implementation of the Hover content builder Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/BUILD | 12 ++++++ verilog/tools/ls/hover.cc | 86 +++++++++++++++++++++++++++++++++++++++ verilog/tools/ls/hover.h | 29 +++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 verilog/tools/ls/hover.cc create mode 100644 verilog/tools/ls/hover.h diff --git a/verilog/tools/ls/BUILD b/verilog/tools/ls/BUILD index 32140f813..110d52bc4 100644 --- a/verilog/tools/ls/BUILD +++ b/verilog/tools/ls/BUILD @@ -104,6 +104,18 @@ cc_library( ], ) +cc_library( + name = "hover", + srcs = ["hover.cc"], + hdrs = ["hover.h"], + deps = [ + ":lsp-parse-buffer", + ":symbol-table-handler", + "//common/lsp:lsp-protocol", + "//verilog/parser:verilog_token_enum", + ], +) + cc_library( name = "symbol-table-handler", srcs = ["symbol-table-handler.cc"], diff --git a/verilog/tools/ls/hover.cc b/verilog/tools/ls/hover.cc new file mode 100644 index 000000000..b3d82f913 --- /dev/null +++ b/verilog/tools/ls/hover.cc @@ -0,0 +1,86 @@ +// Copyright 2023 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#include "verilog/tools/ls/hover.h" + +#include "verilog/analysis/symbol_table.h" +#include "verilog/parser/verilog_token_enum.h" + +namespace verilog { + +namespace { + +class HoverBuilder { + public: + HoverBuilder(SymbolTableHandler *symbol_table_handler, + const BufferTrackerContainer &tracker_container) + : symbol_table_handler(symbol_table_handler), + tracker_container(tracker_container) {} + + verible::lsp::Hover Build(const verible::lsp::HoverParams &p) { + std::optional token = + symbol_table_handler->GetTokenAtTextDocumentPosition(p, + tracker_container); + if (!token) return {}; + verible::lsp::Hover response; + switch (token->token_enum()) { + case verilog_tokentype::TK_end: + HoverInfoEndToken(&response, *token); + break; + default: + HoverInfoIdentifier(&response, *token); + } + return response; + } + + private: + void HoverInfoEndToken(verible::lsp::Hover *response, + const verible::TokenInfo &token) { + // TODO implement walking up to begin + } + void HoverInfoIdentifier(verible::lsp::Hover *response, + const verible::TokenInfo &token) { + absl::string_view symbol = token.text(); + const SymbolTableNode *node = + symbol_table_handler->FindDefinitionNode(symbol); + if (!node) return; + const SymbolInfo &info = node->Value(); + response->contents.value = absl::StrCat( + "### ", SymbolMetaTypeAsString(info.metatype), " ", symbol, "\n\n"); + if (!info.declared_type.syntax_origin && info.declared_type.implicit) { + absl::StrAppend(&response->contents.value, + "---\n\nType: (implicit)\n\n---"); + } else if (info.declared_type.syntax_origin) { + absl::StrAppend( + &response->contents.value, "---\n\n", "Type: ", + verible::StringSpanOfSymbol(*info.declared_type.syntax_origin), + "\n\n---"); + } + } + SymbolTableHandler *symbol_table_handler; + const BufferTrackerContainer &tracker_container; +}; + +} // namespace + +verible::lsp::Hover CreateHoverInformation( + SymbolTableHandler *symbol_table_handler, + const BufferTrackerContainer &tracker, const verible::lsp::HoverParams &p) { + verible::lsp::Hover response; + HoverBuilder builder(symbol_table_handler, tracker); + return builder.Build(p); +} + +} // namespace verilog diff --git a/verilog/tools/ls/hover.h b/verilog/tools/ls/hover.h new file mode 100644 index 000000000..5089a0027 --- /dev/null +++ b/verilog/tools/ls/hover.h @@ -0,0 +1,29 @@ +// Copyright 2023 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef VERILOG_TOOLS_LS_HOVER_H_INCLUDED +#define VERILOG_TOOLS_LS_HOVER_H_INCLUDED + +#include "common/lsp/lsp-protocol.h" +#include "verilog/tools/ls/lsp-parse-buffer.h" +#include "verilog/tools/ls/symbol-table-handler.h" + +namespace verilog { +verible::lsp::Hover CreateHoverInformation( + SymbolTableHandler *symbol_table_handler, + const BufferTrackerContainer &tracker, const verible::lsp::HoverParams &p); +} // namespace verilog + +#endif // hover_h_INCLUDED From ae156ccee8248157b5432bac9392800f8d18077f Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Tue, 9 May 2023 18:51:39 +0200 Subject: [PATCH 05/13] verilog: tools: ls: verilog-language-server: Connected hover functions to Language Server Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/BUILD | 1 + verilog/tools/ls/verilog-language-server.cc | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/verilog/tools/ls/BUILD b/verilog/tools/ls/BUILD index 110d52bc4..782c92609 100644 --- a/verilog/tools/ls/BUILD +++ b/verilog/tools/ls/BUILD @@ -160,6 +160,7 @@ cc_library( ":lsp-parse-buffer", ":symbol-table-handler", ":verible-lsp-adapter", + ":hover", "//common/lsp:json-rpc-dispatcher", "//common/lsp:lsp-file-utils", "//common/lsp:lsp-protocol", diff --git a/verilog/tools/ls/verilog-language-server.cc b/verilog/tools/ls/verilog-language-server.cc index f8aff85cd..a7ee03136 100644 --- a/verilog/tools/ls/verilog-language-server.cc +++ b/verilog/tools/ls/verilog-language-server.cc @@ -22,6 +22,7 @@ #include "common/lsp/lsp-protocol.h" #include "common/util/file_util.h" #include "common/util/init_command_line.h" +#include "verilog/tools/ls/hover.h" #include "verilog/tools/ls/verible-lsp-adapter.h" namespace verilog { @@ -69,7 +70,8 @@ verible::lsp::InitializeResult VerilogLanguageServer::GetCapabilities() { {"documentHighlightProvider", true}, // Highlight same symbol {"definitionProvider", true}, // Provide going to definition {"referencesProvider", true}, // Provide going to references - {"diagnosticProvider", // Pull model of diagnostics. + {"hoverProvider", true}, // Provide hover info on cursor + {"diagnosticProvider", // Pull model of diagnostics. { {"interFileDependencies", false}, {"workspaceDiagnostics", false}, @@ -138,6 +140,11 @@ void VerilogLanguageServer::SetRequestHandlers() { return symbol_table_handler_.FindReferencesLocations(p, parsed_buffers_); }); + dispatcher_.AddRequestHandler( + "textDocument/hover", [this](const verible::lsp::HoverParams &p) { + return CreateHoverInformation(&symbol_table_handler_, parsed_buffers_, + p); + }); // The client sends a request to shut down. Use that to exit our loop. dispatcher_.AddRequestHandler("shutdown", [this](const nlohmann::json &) { shutdown_requested_ = true; From ceb045003e406f188b62e0142de6375f2fcabe35 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Tue, 9 May 2023 18:53:00 +0200 Subject: [PATCH 06/13] verilog: tools: ls: verilog-language-server: test: Added initial test for hovering over symbol Signed-off-by: Grzegorz Latosinski --- .../tools/ls/verilog-language-server_test.cc | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc index af8bbaeb4..ab211a844 100644 --- a/verilog/tools/ls/verilog-language-server_test.cc +++ b/verilog/tools/ls/verilog-language-server_test.cc @@ -612,6 +612,13 @@ std::string ReferencesRequest(absl::string_view file, int id, int line, line, character); } +// Creates a textDocument/hover request +std::string HoverRequest(absl::string_view file, int id, int line, + int character) { + return TextDocumentPositionBasedRequest("textDocument/hover", file, id, line, + character); +} + // Performs simple textDocument/definition request with no VerilogProject set TEST_F(VerilogLanguageServerSymbolTableTest, DefinitionRequestNoProjectTest) { std::string definition_request = DefinitionRequest("file://b.sv", 2, 3, 18); @@ -1442,6 +1449,47 @@ TEST_F(VerilogLanguageServerSymbolTableTest, CheckReferenceUnknownSymbol) { ASSERT_EQ(response_b["result"].size(), 0); } +TEST_F(VerilogLanguageServerSymbolTableTest, HoverOverSymbol) { + absl::string_view filelist_content = "mod.v\n"; + static constexpr absl::string_view // + module_content( + R"(module mod( + input clk, + input reg [31:0] a, + input reg [31:0] b, + output reg [31:0] sum); + always @(posedge clk) begin : addition + assign sum = a + b; + end +endmodule +)"); + + const verible::file::testing::ScopedTestFile filelist( + root_dir, filelist_content, "verible.filelist"); + const verible::file::testing::ScopedTestFile module(root_dir, module_content, + "mod.v"); + + const std::string module_open_request = + DidOpenRequest("file://" + module.filename(), module_content); + ASSERT_OK(SendRequest(module_open_request)); + + GetResponse(); + + std::string hover_request = + HoverRequest("file://" + module.filename(), 2, 6, 12); + + ASSERT_OK(SendRequest(hover_request)); + json response = json::parse(GetResponse()); + + ASSERT_EQ(response["id"], 2); + ASSERT_EQ(response["result"].size(), 1); + ASSERT_EQ(response["result"]["contents"]["kind"], "markdown"); + ASSERT_TRUE(absl::StrContains(response["result"]["contents"]["value"], + "data/net/var/instance sum")); + ASSERT_TRUE( + absl::StrContains(response["result"]["contents"]["value"], "reg [31:0]")); +} + // Tests correctness of Language Server shutdown request TEST_F(VerilogLanguageServerTest, ShutdownTest) { const absl::string_view shutdown_request = From dd1b586429373443d4377292d95a4e90493431ae Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 16:33:18 +0200 Subject: [PATCH 07/13] verilog: tools: ls: BUILD: Updated dependencies for hover Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/BUILD | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/verilog/tools/ls/BUILD b/verilog/tools/ls/BUILD index 782c92609..21d4eb843 100644 --- a/verilog/tools/ls/BUILD +++ b/verilog/tools/ls/BUILD @@ -105,15 +105,19 @@ cc_library( ) cc_library( - name = "hover", - srcs = ["hover.cc"], - hdrs = ["hover.h"], - deps = [ - ":lsp-parse-buffer", - ":symbol-table-handler", - "//common/lsp:lsp-protocol", - "//verilog/parser:verilog_token_enum", - ], + name = "hover", + srcs = ["hover.cc"], + hdrs = ["hover.h"], + deps = [ + ":lsp-parse-buffer", + ":symbol-table-handler", + "//common/util:casts", + "//verilog/CST:seq_block", + "//common/lsp:lsp-protocol", + "//verilog/CST:verilog_nonterminals", + "//common/text:tree_context_visitor", + "//verilog/parser:verilog_token_enum", + ], ) cc_library( From 13cb900c3652ae257e6465733e124b94880c48bf Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 16:33:45 +0200 Subject: [PATCH 08/13] verilog: tools: ls: symbol-table-handler: Made IsStringViewContained publicly available Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/symbol-table-handler.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/verilog/tools/ls/symbol-table-handler.h b/verilog/tools/ls/symbol-table-handler.h index defc0ca35..4f534ddda 100644 --- a/verilog/tools/ls/symbol-table-handler.h +++ b/verilog/tools/ls/symbol-table-handler.h @@ -33,6 +33,9 @@ namespace verilog { // Looks for FileList file for SymbolTableHandler std::string FindFileList(absl::string_view current_dir); +// Checks if one string_view is contained in another +bool IsStringViewContained(absl::string_view origin, absl::string_view substr); + // A class interfacing the SymbolTable with the LSP messages. // It manages the SymbolTable and its necessary components, // and provides such information as symbol definitions From 2204a6365f9cf51a6c5429a873eff921b36f3e40 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 16:54:23 +0200 Subject: [PATCH 09/13] verilog: tools: ls: hover: Added providing block name when available Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/hover.cc | 102 +++++++++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 8 deletions(-) diff --git a/verilog/tools/ls/hover.cc b/verilog/tools/ls/hover.cc index b3d82f913..fbc0fbf90 100644 --- a/verilog/tools/ls/hover.cc +++ b/verilog/tools/ls/hover.cc @@ -15,6 +15,10 @@ #include "verilog/tools/ls/hover.h" +#include "common/text/tree_context_visitor.h" +#include "common/util/casts.h" +#include "verilog/CST/seq_block.h" +#include "verilog/CST/verilog_nonterminals.h" #include "verilog/analysis/symbol_table.h" #include "verilog/parser/verilog_token_enum.h" @@ -22,16 +26,83 @@ namespace verilog { namespace { +using verible::Symbol; +using verible::SyntaxTreeLeaf; +using verible::SyntaxTreeNode; +using verible::TreeContextVisitor; + +// Finds names/labels of named blocks +class FindBeginLabel : public TreeContextVisitor { + public: + explicit FindBeginLabel() = default; + + absl::string_view LabelSearch(const verible::ConcreteSyntaxTree &tree, + absl::string_view substring, NodeEnum endtag, + NodeEnum begintag) { + this->substring = substring; + this->begintag = begintag; + this->endtag = endtag; + substring_found = false; + finished = false; + tree->Accept(this); + return label; + } + + private: + void Visit(const SyntaxTreeLeaf &leaf) override { + if (IsStringViewContained(leaf.get().text(), substring)) { + substring_found = true; + } + } + void Visit(const SyntaxTreeNode &node) override { + if (finished) return; + const std::unique_ptr *lastbegin = nullptr; + for (const std::unique_ptr &child : node.children()) { + if (!child) continue; + if (child->Kind() == verible::SymbolKind::kLeaf && + node.Tag().tag == static_cast(endtag)) { + Visit(verible::down_cast(*child)); + if (substring_found) return; + } else if (child->Tag().tag == static_cast(begintag)) { + lastbegin = &child; + } else if (child->Kind() == verible::SymbolKind::kNode) { + Visit(verible::down_cast(*child)); + if (!label.empty()) return; + if (substring_found) { + if (!lastbegin) { + finished = true; + return; + } + const verible::TokenInfo *info = GetBeginLabelTokenInfo(**lastbegin); + finished = true; + if (!info) return; + label = info->text(); + return; + } + } + if (finished) return; + } + } + absl::string_view substring; + NodeEnum endtag; + NodeEnum begintag; + absl::string_view label; + bool substring_found; + bool finished; +}; + class HoverBuilder { public: HoverBuilder(SymbolTableHandler *symbol_table_handler, - const BufferTrackerContainer &tracker_container) + const BufferTrackerContainer &tracker_container, + const verible::lsp::HoverParams ¶ms) : symbol_table_handler(symbol_table_handler), - tracker_container(tracker_container) {} + tracker_container(tracker_container), + params(params) {} - verible::lsp::Hover Build(const verible::lsp::HoverParams &p) { + verible::lsp::Hover Build() { std::optional token = - symbol_table_handler->GetTokenAtTextDocumentPosition(p, + symbol_table_handler->GetTokenAtTextDocumentPosition(params, tracker_container); if (!token) return {}; verible::lsp::Hover response; @@ -48,8 +119,23 @@ class HoverBuilder { private: void HoverInfoEndToken(verible::lsp::Hover *response, const verible::TokenInfo &token) { - // TODO implement walking up to begin + const verilog::BufferTracker *tracker = + tracker_container.FindBufferTrackerOrNull(params.textDocument.uri); + if (!tracker) return; + std::shared_ptr parsedbuffer = tracker->current(); + if (!parsedbuffer) return; + const verible::ConcreteSyntaxTree &tree = + parsedbuffer->parser().SyntaxTree(); + if (!tree) return; + FindBeginLabel search; + absl::string_view label = search.LabelSearch( + tree, token.text(), NodeEnum::kEnd, NodeEnum::kBegin); + if (label.empty()) return; + response->contents.value = "### End of block\n\n"; + absl::StrAppend(&response->contents.value, "---\n\nName: ", label, + "\n\n---"); } + void HoverInfoIdentifier(verible::lsp::Hover *response, const verible::TokenInfo &token) { absl::string_view symbol = token.text(); @@ -71,6 +157,7 @@ class HoverBuilder { } SymbolTableHandler *symbol_table_handler; const BufferTrackerContainer &tracker_container; + const verible::lsp::HoverParams ¶ms; }; } // namespace @@ -78,9 +165,8 @@ class HoverBuilder { verible::lsp::Hover CreateHoverInformation( SymbolTableHandler *symbol_table_handler, const BufferTrackerContainer &tracker, const verible::lsp::HoverParams &p) { - verible::lsp::Hover response; - HoverBuilder builder(symbol_table_handler, tracker); - return builder.Build(p); + HoverBuilder builder(symbol_table_handler, tracker, p); + return builder.Build(); } } // namespace verilog From 1ebd71b4b65b391a73fab9b43b65451a7271c525 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 16:57:04 +0200 Subject: [PATCH 10/13] verilog: tools: ls: verilog-language-server: test: Added test for hover on "end" Signed-off-by: Grzegorz Latosinski --- .../tools/ls/verilog-language-server_test.cc | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc index ab211a844..114e1229c 100644 --- a/verilog/tools/ls/verilog-language-server_test.cc +++ b/verilog/tools/ls/verilog-language-server_test.cc @@ -1490,6 +1490,48 @@ endmodule absl::StrContains(response["result"]["contents"]["value"], "reg [31:0]")); } +// Checks if the hover appears on "end" token when block name is available +TEST_F(VerilogLanguageServerSymbolTableTest, HoverOverEnd) { + absl::string_view filelist_content = "mod.v\n"; + static constexpr absl::string_view // + module_content( + R"(module mod( + input clk, + input reg [31:0] a, + input reg [31:0] b, + output reg [31:0] sum); + always @(posedge clk) begin : addition + assign sum = a + b; + end +endmodule +)"); + + const verible::file::testing::ScopedTestFile filelist( + root_dir, filelist_content, "verible.filelist"); + const verible::file::testing::ScopedTestFile module(root_dir, module_content, + "mod.v"); + + const std::string module_open_request = + DidOpenRequest("file://" + module.filename(), module_content); + ASSERT_OK(SendRequest(module_open_request)); + + GetResponse(); + + std::string hover_request = + HoverRequest("file://" + module.filename(), 2, 7, 3); + + ASSERT_OK(SendRequest(hover_request)); + json response = json::parse(GetResponse()); + + ASSERT_EQ(response["id"], 2); + ASSERT_EQ(response["result"].size(), 1); + ASSERT_EQ(response["result"]["contents"]["kind"], "markdown"); + ASSERT_TRUE(absl::StrContains(response["result"]["contents"]["value"], + "End of block")); + ASSERT_TRUE(absl::StrContains(response["result"]["contents"]["value"], + "Name: addition")); +} + // Tests correctness of Language Server shutdown request TEST_F(VerilogLanguageServerTest, ShutdownTest) { const absl::string_view shutdown_request = From 0265a8b226aa7050906d9326d99cb0b33deb6812 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 17:13:13 +0200 Subject: [PATCH 11/13] verilog: tools: ls: hover: Refactored classes, added basic docstrings Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/hover.cc | 71 ++++++++++++++++++++------------------- verilog/tools/ls/hover.h | 1 + 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/verilog/tools/ls/hover.cc b/verilog/tools/ls/hover.cc index fbc0fbf90..d3e1bafbf 100644 --- a/verilog/tools/ls/hover.cc +++ b/verilog/tools/ls/hover.cc @@ -36,74 +36,77 @@ class FindBeginLabel : public TreeContextVisitor { public: explicit FindBeginLabel() = default; + // Performs search of the label for end entry, based on its location in + // string and tags absl::string_view LabelSearch(const verible::ConcreteSyntaxTree &tree, absl::string_view substring, NodeEnum endtag, NodeEnum begintag) { - this->substring = substring; - this->begintag = begintag; - this->endtag = endtag; - substring_found = false; - finished = false; + substring_ = substring; + begintag_ = begintag; + endtag_ = endtag; + substring_found_ = false; + finished_ = false; tree->Accept(this); - return label; + return label_; } private: void Visit(const SyntaxTreeLeaf &leaf) override { - if (IsStringViewContained(leaf.get().text(), substring)) { - substring_found = true; + if (IsStringViewContained(leaf.get().text(), substring_)) { + substring_found_ = true; } } void Visit(const SyntaxTreeNode &node) override { - if (finished) return; + if (finished_) return; const std::unique_ptr *lastbegin = nullptr; for (const std::unique_ptr &child : node.children()) { if (!child) continue; if (child->Kind() == verible::SymbolKind::kLeaf && - node.Tag().tag == static_cast(endtag)) { + node.Tag().tag == static_cast(endtag_)) { Visit(verible::down_cast(*child)); - if (substring_found) return; - } else if (child->Tag().tag == static_cast(begintag)) { + if (substring_found_) return; + } else if (child->Tag().tag == static_cast(begintag_)) { lastbegin = &child; } else if (child->Kind() == verible::SymbolKind::kNode) { Visit(verible::down_cast(*child)); - if (!label.empty()) return; - if (substring_found) { + if (!label_.empty()) return; + if (substring_found_) { if (!lastbegin) { - finished = true; + finished_ = true; return; } const verible::TokenInfo *info = GetBeginLabelTokenInfo(**lastbegin); - finished = true; + finished_ = true; if (!info) return; - label = info->text(); + label_ = info->text(); return; } } - if (finished) return; + if (finished_) return; } } - absl::string_view substring; - NodeEnum endtag; - NodeEnum begintag; - absl::string_view label; - bool substring_found; - bool finished; + absl::string_view substring_; + NodeEnum endtag_; + NodeEnum begintag_; + absl::string_view label_; + bool substring_found_; + bool finished_; }; +// Constructs a Hover message for the given location class HoverBuilder { public: HoverBuilder(SymbolTableHandler *symbol_table_handler, const BufferTrackerContainer &tracker_container, const verible::lsp::HoverParams ¶ms) - : symbol_table_handler(symbol_table_handler), - tracker_container(tracker_container), - params(params) {} + : symbol_table_handler_(symbol_table_handler), + tracker_container_(tracker_container), + params_(params) {} verible::lsp::Hover Build() { std::optional token = - symbol_table_handler->GetTokenAtTextDocumentPosition(params, - tracker_container); + symbol_table_handler_->GetTokenAtTextDocumentPosition( + params_, tracker_container_); if (!token) return {}; verible::lsp::Hover response; switch (token->token_enum()) { @@ -120,7 +123,7 @@ class HoverBuilder { void HoverInfoEndToken(verible::lsp::Hover *response, const verible::TokenInfo &token) { const verilog::BufferTracker *tracker = - tracker_container.FindBufferTrackerOrNull(params.textDocument.uri); + tracker_container_.FindBufferTrackerOrNull(params_.textDocument.uri); if (!tracker) return; std::shared_ptr parsedbuffer = tracker->current(); if (!parsedbuffer) return; @@ -140,7 +143,7 @@ class HoverBuilder { const verible::TokenInfo &token) { absl::string_view symbol = token.text(); const SymbolTableNode *node = - symbol_table_handler->FindDefinitionNode(symbol); + symbol_table_handler_->FindDefinitionNode(symbol); if (!node) return; const SymbolInfo &info = node->Value(); response->contents.value = absl::StrCat( @@ -155,9 +158,9 @@ class HoverBuilder { "\n\n---"); } } - SymbolTableHandler *symbol_table_handler; - const BufferTrackerContainer &tracker_container; - const verible::lsp::HoverParams ¶ms; + SymbolTableHandler *symbol_table_handler_; + const BufferTrackerContainer &tracker_container_; + const verible::lsp::HoverParams ¶ms_; }; } // namespace diff --git a/verilog/tools/ls/hover.h b/verilog/tools/ls/hover.h index 5089a0027..54221b565 100644 --- a/verilog/tools/ls/hover.h +++ b/verilog/tools/ls/hover.h @@ -21,6 +21,7 @@ #include "verilog/tools/ls/symbol-table-handler.h" namespace verilog { +// Provides hover information for given location verible::lsp::Hover CreateHoverInformation( SymbolTableHandler *symbol_table_handler, const BufferTrackerContainer &tracker, const verible::lsp::HoverParams &p); From 09ab5f1441335b52e7955f21b0a12bed4c413795 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 17:20:23 +0200 Subject: [PATCH 12/13] verilog: tools: ls: verilog-language-server: test: Marked places where hover is applied, added using dedicated class Signed-off-by: Grzegorz Latosinski --- .../tools/ls/verilog-language-server_test.cc | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc index 114e1229c..6a8adc264 100644 --- a/verilog/tools/ls/verilog-language-server_test.cc +++ b/verilog/tools/ls/verilog-language-server_test.cc @@ -1449,6 +1449,9 @@ TEST_F(VerilogLanguageServerSymbolTableTest, CheckReferenceUnknownSymbol) { ASSERT_EQ(response_b["result"].size(), 0); } +// Checks if the hover appears on port symbols +// In this test the hover for "sum" symbol in assign +// is checked TEST_F(VerilogLanguageServerSymbolTableTest, HoverOverSymbol) { absl::string_view filelist_content = "mod.v\n"; static constexpr absl::string_view // @@ -1459,7 +1462,7 @@ TEST_F(VerilogLanguageServerSymbolTableTest, HoverOverSymbol) { input reg [31:0] b, output reg [31:0] sum); always @(posedge clk) begin : addition - assign sum = a + b; + assign sum = a + b; // hover over sum end endmodule )"); @@ -1475,19 +1478,17 @@ endmodule GetResponse(); - std::string hover_request = - HoverRequest("file://" + module.filename(), 2, 6, 12); + std::string hover_request = HoverRequest("file://" + module.filename(), 2, + /* line */ 6, /* column */ 12); ASSERT_OK(SendRequest(hover_request)); json response = json::parse(GetResponse()); + verible::lsp::Hover hover = response["result"]; - ASSERT_EQ(response["id"], 2); - ASSERT_EQ(response["result"].size(), 1); - ASSERT_EQ(response["result"]["contents"]["kind"], "markdown"); - ASSERT_TRUE(absl::StrContains(response["result"]["contents"]["value"], - "data/net/var/instance sum")); + ASSERT_EQ(hover.contents.kind, "markdown"); ASSERT_TRUE( - absl::StrContains(response["result"]["contents"]["value"], "reg [31:0]")); + absl::StrContains(hover.contents.value, "data/net/var/instance sum")); + ASSERT_TRUE(absl::StrContains(hover.contents.value, "reg [31:0]")); } // Checks if the hover appears on "end" token when block name is available @@ -1502,7 +1503,7 @@ TEST_F(VerilogLanguageServerSymbolTableTest, HoverOverEnd) { output reg [31:0] sum); always @(posedge clk) begin : addition assign sum = a + b; - end + end // hover over end endmodule )"); @@ -1517,19 +1518,16 @@ endmodule GetResponse(); - std::string hover_request = - HoverRequest("file://" + module.filename(), 2, 7, 3); + std::string hover_request = HoverRequest("file://" + module.filename(), 2, + /* line */ 7, /* column */ 3); ASSERT_OK(SendRequest(hover_request)); json response = json::parse(GetResponse()); + verible::lsp::Hover hover = response["result"]; - ASSERT_EQ(response["id"], 2); - ASSERT_EQ(response["result"].size(), 1); - ASSERT_EQ(response["result"]["contents"]["kind"], "markdown"); - ASSERT_TRUE(absl::StrContains(response["result"]["contents"]["value"], - "End of block")); - ASSERT_TRUE(absl::StrContains(response["result"]["contents"]["value"], - "Name: addition")); + ASSERT_EQ(hover.contents.kind, "markdown"); + ASSERT_TRUE(absl::StrContains(hover.contents.value, "End of block")); + ASSERT_TRUE(absl::StrContains(hover.contents.value, "Name: addition")); } // Tests correctness of Language Server shutdown request From 79ce3d076bffa4d337eecd5fa4ff1d35e4fc9e5f Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 18 May 2023 18:07:32 +0200 Subject: [PATCH 13/13] verilog: tools: ls: hover: Changed IsStringViewContained to IsSubRange Signed-off-by: Grzegorz Latosinski --- verilog/tools/ls/BUILD | 11 ++++++----- verilog/tools/ls/hover.cc | 3 ++- verilog/tools/ls/symbol-table-handler.h | 3 --- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/verilog/tools/ls/BUILD b/verilog/tools/ls/BUILD index 21d4eb843..28bb31bc1 100644 --- a/verilog/tools/ls/BUILD +++ b/verilog/tools/ls/BUILD @@ -111,12 +111,13 @@ cc_library( deps = [ ":lsp-parse-buffer", ":symbol-table-handler", - "//common/util:casts", - "//verilog/CST:seq_block", "//common/lsp:lsp-protocol", - "//verilog/CST:verilog_nonterminals", - "//common/text:tree_context_visitor", - "//verilog/parser:verilog_token_enum", + "//common/text:tree-context-visitor", + "//common/util:casts", + "//common/util:range", + "//verilog/CST:seq-block", + "//verilog/CST:verilog-nonterminals", + "//verilog/parser:verilog-token-enum", ], ) diff --git a/verilog/tools/ls/hover.cc b/verilog/tools/ls/hover.cc index d3e1bafbf..622575301 100644 --- a/verilog/tools/ls/hover.cc +++ b/verilog/tools/ls/hover.cc @@ -17,6 +17,7 @@ #include "common/text/tree_context_visitor.h" #include "common/util/casts.h" +#include "common/util/range.h" #include "verilog/CST/seq_block.h" #include "verilog/CST/verilog_nonterminals.h" #include "verilog/analysis/symbol_table.h" @@ -52,7 +53,7 @@ class FindBeginLabel : public TreeContextVisitor { private: void Visit(const SyntaxTreeLeaf &leaf) override { - if (IsStringViewContained(leaf.get().text(), substring_)) { + if (verible::IsSubRange(leaf.get().text(), substring_)) { substring_found_ = true; } } diff --git a/verilog/tools/ls/symbol-table-handler.h b/verilog/tools/ls/symbol-table-handler.h index 4f534ddda..defc0ca35 100644 --- a/verilog/tools/ls/symbol-table-handler.h +++ b/verilog/tools/ls/symbol-table-handler.h @@ -33,9 +33,6 @@ namespace verilog { // Looks for FileList file for SymbolTableHandler std::string FindFileList(absl::string_view current_dir); -// Checks if one string_view is contained in another -bool IsStringViewContained(absl::string_view origin, absl::string_view substr); - // A class interfacing the SymbolTable with the LSP messages. // It manages the SymbolTable and its necessary components, // and provides such information as symbol definitions