From 2d1dd1195256ffd0c186c4da517bcec5ae0f5ee1 Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Mon, 29 Jan 2024 23:39:10 -0800 Subject: [PATCH] Implement textDocument/hover request. Rebased from PR #1922 This will implement the method, but currently it is not offered yet in the initialization to the editor; to be enabled once tests finished. The implementation requires a global symbol table, which is potentially a bit heavy. We might need to introduce the concept of a per-file symbol table for faster local lookups. --- verilog/analysis/symbol_table.cc | 2 +- verilog/analysis/symbol_table.h | 2 + verilog/tools/ls/BUILD | 26 +++ verilog/tools/ls/hover.cc | 190 ++++++++++++++++++ verilog/tools/ls/hover.h | 30 +++ verilog/tools/ls/symbol-table-handler.cc | 34 ++-- verilog/tools/ls/symbol-table-handler.h | 18 +- verilog/tools/ls/verilog-language-server.cc | 12 +- .../tools/ls/verilog-language-server_test.cc | 88 ++++++++ 9 files changed, 379 insertions(+), 23 deletions(-) create mode 100644 verilog/tools/ls/hover.cc create mode 100644 verilog/tools/ls/hover.h diff --git a/verilog/analysis/symbol_table.cc b/verilog/analysis/symbol_table.cc index a24038b07..c95e7890b 100644 --- a/verilog/analysis/symbol_table.cc +++ b/verilog/analysis/symbol_table.cc @@ -103,7 +103,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 93f5304d7..26469d831 100644 --- a/verilog/analysis/symbol_table.h +++ b/verilog/analysis/symbol_table.h @@ -73,6 +73,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. diff --git a/verilog/tools/ls/BUILD b/verilog/tools/ls/BUILD index ed3e55dbd..a954427f6 100644 --- a/verilog/tools/ls/BUILD +++ b/verilog/tools/ls/BUILD @@ -131,6 +131,31 @@ cc_library( ], ) +cc_library( + name = "hover", + srcs = ["hover.cc"], + hdrs = ["hover.h"], + deps = [ + ":lsp-parse-buffer", + ":symbol-table-handler", + "//common/lsp:lsp-protocol", + "//common/text:concrete-syntax-leaf", + "//common/text:concrete-syntax-tree", + "//common/text:symbol", + "//common/text:token-info", + "//common/text:tree-context-visitor", + "//common/text:tree-utils", + "//common/util:casts", + "//common/util:range", + "//verilog/CST:seq-block", + "//verilog/CST:verilog-nonterminals", + "//verilog/analysis:symbol-table", + "//verilog/parser:verilog-token-enum", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:string_view", + ], +) + cc_library( name = "symbol-table-handler", srcs = ["symbol-table-handler.cc"], @@ -185,6 +210,7 @@ cc_library( srcs = ["verilog-language-server.cc"], hdrs = ["verilog-language-server.h"], deps = [ + ":hover", ":lsp-parse-buffer", ":symbol-table-handler", ":verible-lsp-adapter", diff --git a/verilog/tools/ls/hover.cc b/verilog/tools/ls/hover.cc new file mode 100644 index 000000000..fdebbd113 --- /dev/null +++ b/verilog/tools/ls/hover.cc @@ -0,0 +1,190 @@ +// 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 +#include + +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include "common/lsp/lsp-protocol.h" +#include "common/text/concrete_syntax_leaf.h" +#include "common/text/concrete_syntax_tree.h" +#include "common/text/symbol.h" +#include "common/text/token_info.h" +#include "common/text/tree_context_visitor.h" +#include "common/text/tree_utils.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" +#include "verilog/parser/verilog_token_enum.h" +#include "verilog/tools/ls/lsp-parse-buffer.h" +#include "verilog/tools/ls/symbol-table-handler.h" + +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: + // 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) { + substring_ = substring; + begintag_ = begintag; + endtag_ = endtag; + substring_found_ = false; + finished_ = false; + tree->Accept(this); + return label_; + } + + private: + void Visit(const SyntaxTreeLeaf &leaf) final { + if (verible::IsSubRange(leaf.get().text(), substring_)) { + substring_found_ = true; + } + } + + void Visit(const SyntaxTreeNode &node) final { + 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_; +}; + +// 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) {} + + verible::lsp::Hover Build() { + std::optional token = + symbol_table_handler_->GetTokenAtTextDocumentPosition( + params_, 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) { + 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(); + 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_; + const verible::lsp::HoverParams ¶ms_; +}; + +} // namespace + +verible::lsp::Hover CreateHoverInformation( + SymbolTableHandler *symbol_table_handler, + const BufferTrackerContainer &tracker, const verible::lsp::HoverParams &p) { + HoverBuilder builder(symbol_table_handler, tracker, p); + return builder.Build(); +} + +} // namespace verilog diff --git a/verilog/tools/ls/hover.h b/verilog/tools/ls/hover.h new file mode 100644 index 000000000..54221b565 --- /dev/null +++ b/verilog/tools/ls/hover.h @@ -0,0 +1,30 @@ +// 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 { +// Provides hover information for given location +verible::lsp::Hover CreateHoverInformation( + SymbolTableHandler *symbol_table_handler, + const BufferTrackerContainer &tracker, const verible::lsp::HoverParams &p); +} // namespace verilog + +#endif // hover_h_INCLUDED diff --git a/verilog/tools/ls/symbol-table-handler.cc b/verilog/tools/ls/symbol-table-handler.cc index 78f2c0a98..19c30084b 100644 --- a/verilog/tools/ls/symbol-table-handler.cc +++ b/verilog/tools/ls/symbol-table-handler.cc @@ -291,9 +291,10 @@ SymbolTableHandler::GetTokenInfoAtTextDocumentPosition( return cursor_token; } -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) { @@ -310,8 +311,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); } verible::LineColumnRange @@ -357,14 +357,17 @@ 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 std::string 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); @@ -390,11 +393,15 @@ std::vector SymbolTableHandler::FindDefinitionLocation( return locations; } -const verible::Symbol *SymbolTableHandler::FindDefinitionSymbol( +const SymbolTableNode *SymbolTableHandler::FindDefinitionNode( absl::string_view symbol) { Prepare(); - const SymbolTableNode *symbol_table_node = - ScanSymbolTreeForDefinition(&symbol_table_->Root(), symbol); + return ScanSymbolTreeForDefinition(&symbol_table_->Root(), symbol); +} + +const verible::Symbol *SymbolTableHandler::FindDefinitionSymbol( + absl::string_view symbol) { + const SymbolTableNode *symbol_table_node = FindDefinitionNode(symbol); if (symbol_table_node) return symbol_table_node->Value().syntax_origin; return nullptr; } @@ -403,8 +410,10 @@ std::vector SymbolTableHandler::FindReferencesLocations( const verible::lsp::ReferenceParams ¶ms, const verilog::BufferTrackerContainer &parsed_buffers) { Prepare(); - const absl::string_view symbol = + std::optional token = GetTokenAtTextDocumentPosition(params, parsed_buffers); + if (!token) return {}; + const absl::string_view symbol = token->text(); const SymbolTableNode &root = symbol_table_->Root(); const SymbolTableNode *node = ScanSymbolTreeForDefinition(&root, symbol); if (!node) { @@ -440,9 +449,10 @@ SymbolTableHandler::FindRenameLocationsAndCreateEdits( const verible::lsp::RenameParams ¶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) return {}; diff --git a/verilog/tools/ls/symbol-table-handler.h b/verilog/tools/ls/symbol-table-handler.h index b928bab24..6c25a554a 100644 --- a/verilog/tools/ls/symbol-table-handler.h +++ b/verilog/tools/ls/symbol-table-handler.h @@ -55,9 +55,12 @@ 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 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); @@ -76,6 +79,12 @@ class SymbolTableHandler { const verible::lsp::RenameParams ¶ms, const verilog::BufferTrackerContainer &parsed_buffers); + // Returns TokenInfo for token pointed by the LSP request based on + // TextDocumentPositionParams. If text is not found, nullopt is returned. + std::optional GetTokenAtTextDocumentPosition( + const verible::lsp::TextDocumentPositionParams ¶ms, + const verilog::BufferTrackerContainer &parsed_buffers) const; + // Creates a symbol table for entire project (public: needed in unit-test) std::vector BuildProjectSymbolTable(); @@ -96,13 +105,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 a range in which a token exists in the file by the LSP request // based on TextDocumentPositionParams. If text is not found, // empty-initialized LineColumnRange is returned. diff --git a/verilog/tools/ls/verilog-language-server.cc b/verilog/tools/ls/verilog-language-server.cc index db4a3b787..ad1f32586 100644 --- a/verilog/tools/ls/verilog-language-server.cc +++ b/verilog/tools/ls/verilog-language-server.cc @@ -29,6 +29,7 @@ #include "common/util/init_command_line.h" #include "common/util/logging.h" #include "nlohmann/json.hpp" +#include "verilog/tools/ls/hover.h" #include "verilog/tools/ls/verible-lsp-adapter.h" ABSL_FLAG(bool, variables_in_outline, true, @@ -80,8 +81,10 @@ verible::lsp::InitializeResult VerilogLanguageServer::GetCapabilities() { {"documentHighlightProvider", true}, // Highlight same symbol {"definitionProvider", true}, // Provide going to definition {"referencesProvider", true}, // Provide going to references - {"renameProvider", true}, // Provide symbol renaming - {"diagnosticProvider", // Pull model of diagnostics. + // Hover enabled, but not yet offered to client until tested. + {"hoverProvider", false}, // Hover info over cursor + {"renameProvider", true}, // Provide symbol renaming + {"diagnosticProvider", // Pull model of diagnostics. { {"interFileDependencies", false}, {"workspaceDiagnostics", false}, @@ -166,6 +169,11 @@ void VerilogLanguageServer::SetRequestHandlers() { return symbol_table_handler_.FindRenameLocationsAndCreateEdits( 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; diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc index 2c9f4a5bf..3c89715fc 100644 --- a/verilog/tools/ls/verilog-language-server_test.cc +++ b/verilog/tools/ls/verilog-language-server_test.cc @@ -734,6 +734,93 @@ void CheckDefinitionResponseSingleDefinition(const json &response, int id, CheckDefinitionEntry(response["result"][0], start, end, file_uri); } +// 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); +} + +// 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 // + 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; // hover over sum + 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, + /* line */ 6, /* column */ 12); + + ASSERT_OK(SendRequest(hover_request)); + json response = json::parse(GetResponse()); + verible::lsp::Hover hover = response["result"]; + ASSERT_EQ(hover.contents.kind, "markdown"); + ASSERT_TRUE( + 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 +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 // hover over 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, + /* line */ 7, /* column */ 3); + + ASSERT_OK(SendRequest(hover_request)); + json response = json::parse(GetResponse()); + verible::lsp::Hover hover = response["result"]; + + 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")); +} + // Performs simple textDocument/definition request with no VerilogProject set TEST_F(VerilogLanguageServerSymbolTableTest, DefinitionRequestNoProjectTest) { std::string definition_request = DefinitionRequest("file://b.sv", 2, 3, 18); @@ -2128,6 +2215,7 @@ TEST_F(VerilogLanguageServerSymbolTableTest, RenameTestPackageDistinction) { EXPECT_EQ(response["result"]["changes"][file_uri].size(), 2) << "Invalid result size for id: "; } + // Tests correctness of Language Server shutdown request TEST_F(VerilogLanguageServerTest, ShutdownTest) { const absl::string_view shutdown_request =