From e924346e1b89b584d8cf5470b224d9574e6f908d Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Tue, 4 Oct 2022 16:42:43 +0200 Subject: [PATCH] Applying more review. --- libsolidity/CMakeLists.txt | 8 ++++---- libsolidity/lsp/LanguageServer.cpp | 12 ++++++------ libsolidity/lsp/ReferenceCollector.cpp | 12 +++++++++--- libsolidity/lsp/ReferenceCollector.h | 1 - .../lsp/{References.cpp => ReferencesHandler.cpp} | 4 ++-- .../lsp/{References.h => ReferencesHandler.h} | 4 ++-- ...SemanticHighlight.cpp => SemanticHighlighter.cpp} | 11 +++++------ .../{SemanticHighlight.h => SemanticHighlighter.h} | 6 +++--- libsolidity/lsp/Transport.h | 5 +++++ test/libsolidity/lsp/references/enum.sol | 7 ++++--- 10 files changed, 40 insertions(+), 30 deletions(-) rename libsolidity/lsp/{References.cpp => ReferencesHandler.cpp} (92%) rename libsolidity/lsp/{References.h => ReferencesHandler.h} (89%) rename libsolidity/lsp/{SemanticHighlight.cpp => SemanticHighlighter.cpp} (77%) rename libsolidity/lsp/{SemanticHighlight.h => SemanticHighlighter.h} (84%) diff --git a/libsolidity/CMakeLists.txt b/libsolidity/CMakeLists.txt index 432cbe0f1..7a1c038fb 100644 --- a/libsolidity/CMakeLists.txt +++ b/libsolidity/CMakeLists.txt @@ -165,14 +165,14 @@ set(sources lsp/HandlerBase.h lsp/LanguageServer.cpp lsp/LanguageServer.h - lsp/SemanticHighlight.cpp - lsp/SemanticHighlight.h + lsp/SemanticHighlighter.cpp + lsp/SemanticHighlighter.h lsp/SemanticTokensBuilder.cpp lsp/SemanticTokensBuilder.h lsp/ReferenceCollector.cpp lsp/ReferenceCollector.h - lsp/References.cpp - lsp/References.h + lsp/ReferencesHandler.cpp + lsp/ReferencesHandler.h lsp/Transport.cpp lsp/Transport.h lsp/Utils.cpp diff --git a/libsolidity/lsp/LanguageServer.cpp b/libsolidity/lsp/LanguageServer.cpp index aec2e8185..47fb7cf2b 100644 --- a/libsolidity/lsp/LanguageServer.cpp +++ b/libsolidity/lsp/LanguageServer.cpp @@ -1,4 +1,4 @@ -/* +/*LanguageServer.cpp This file is part of solidity. solidity is free software: you can redistribute it and/or modify @@ -26,9 +26,9 @@ // LSP feature implementations #include -#include +#include #include -#include +#include #include #include @@ -147,10 +147,10 @@ LanguageServer::LanguageServer(Transport& _transport): {"textDocument/didChange", bind(&LanguageServer::handleTextDocumentDidChange, this, _2)}, {"textDocument/didClose", bind(&LanguageServer::handleTextDocumentDidClose, this, _2)}, {"textDocument/rename", RenameSymbol(*this) }, - {"textDocument/documentHighlight", SemanticHighlight(*this) }, + {"textDocument/documentHighlight", SemanticHighlighter(*this) }, {"textDocument/implementation", GotoDefinition(*this) }, {"textDocument/semanticTokens/full", bind(&LanguageServer::semanticTokensFull, this, _1, _2)}, - {"textDocument/references", References(*this) }, + {"textDocument/references", ReferencesHandler(*this) }, {"workspace/didChangeConfiguration", bind(&LanguageServer::handleWorkspaceDidChangeConfiguration, this, _2)}, }, m_fileRepository("/" /* basePath */, {} /* no search paths */), @@ -187,7 +187,7 @@ void LanguageServer::changeConfiguration(Json::Value const& _settings) else if (text == "directly-opened-and-on-import") m_fileLoadStrategy = FileLoadStrategy::DirectlyOpenedAndOnImported; else - lspRequire(false, ErrorCode::InvalidParams, "Invalid file load strategy: " + text); + lspAssert(false, "Invalid file load strategy: " + text); } m_settingsObject = _settings; diff --git a/libsolidity/lsp/ReferenceCollector.cpp b/libsolidity/lsp/ReferenceCollector.cpp index 2c328cc94..b9a2263fd 100644 --- a/libsolidity/lsp/ReferenceCollector.cpp +++ b/libsolidity/lsp/ReferenceCollector.cpp @@ -107,7 +107,7 @@ bool ReferenceCollector::visit(ImportDirective const& _import) void ReferenceCollector::endVisit(Identifier const& _identifier) { - if (Declaration const* declaration = _identifier.annotation().referencedDeclaration; declaration == &m_declaration) + if (_identifier.annotation().referencedDeclaration == &m_declaration) m_collectedReferences.emplace_back(_identifier.location(), m_kind); } @@ -133,12 +133,18 @@ void ReferenceCollector::endVisit(MemberAccess const& _memberAccess) bool ReferenceCollector::visit(Assignment const& _assignment) { + auto const getDocumentHighlightKind = [](Expression const& _expression) noexcept -> DocumentHighlightKind { + if (_expression.annotation().willBeWrittenTo) + return DocumentHighlightKind::Write; + return DocumentHighlightKind::Read; + }; + auto const restoreKind = ScopeGuard{[this, savedKind=m_kind]() { m_kind = savedKind; }}; - m_kind = DocumentHighlightKind::Write; + m_kind = getDocumentHighlightKind(_assignment.leftHandSide()); _assignment.leftHandSide().accept(*this); - m_kind = DocumentHighlightKind::Read; + m_kind = getDocumentHighlightKind(_assignment.rightHandSide()); _assignment.rightHandSide().accept(*this); return false; diff --git a/libsolidity/lsp/ReferenceCollector.h b/libsolidity/lsp/ReferenceCollector.h index 701b534d7..041044618 100644 --- a/libsolidity/lsp/ReferenceCollector.h +++ b/libsolidity/lsp/ReferenceCollector.h @@ -30,7 +30,6 @@ namespace solidity::lsp */ enum class DocumentHighlightKind { - Unspecified = 0, //!< could be for example a highlight found in a comment Text = 1, //!< a textual occurrence Read = 2, //!< read access to a variable Write = 3, //!< write access to a variable diff --git a/libsolidity/lsp/References.cpp b/libsolidity/lsp/ReferencesHandler.cpp similarity index 92% rename from libsolidity/lsp/References.cpp rename to libsolidity/lsp/ReferencesHandler.cpp index 8355ee4d0..96ee07722 100644 --- a/libsolidity/lsp/References.cpp +++ b/libsolidity/lsp/ReferencesHandler.cpp @@ -15,7 +15,7 @@ along with solidity. If not, see . */ // SPDX-License-Identifier: GPL-3.0 -#include +#include #include #include #include @@ -29,7 +29,7 @@ using namespace solidity::frontend; namespace solidity::lsp { -void References::operator()(MessageID _id, Json::Value const& _args) +void ReferencesHandler::operator()(MessageID _id, Json::Value const& _args) { auto const [sourceUnitName, lineColumn] = extractSourceUnitNameAndLineColumn(_args); diff --git a/libsolidity/lsp/References.h b/libsolidity/lsp/ReferencesHandler.h similarity index 89% rename from libsolidity/lsp/References.h rename to libsolidity/lsp/ReferencesHandler.h index a2a2b7fc6..c6011fb84 100644 --- a/libsolidity/lsp/References.h +++ b/libsolidity/lsp/ReferencesHandler.h @@ -23,10 +23,10 @@ namespace solidity::lsp /** * Implements JSON RPC for `textDocument/references`. */ -class References: public HandlerBase +class ReferencesHandler: public HandlerBase { public: - explicit References(LanguageServer& _server): HandlerBase(_server) {} + using HandlerBase::HandlerBase; void operator()(MessageID _id, Json::Value const& _args); }; diff --git a/libsolidity/lsp/SemanticHighlight.cpp b/libsolidity/lsp/SemanticHighlighter.cpp similarity index 77% rename from libsolidity/lsp/SemanticHighlight.cpp rename to libsolidity/lsp/SemanticHighlighter.cpp index 11010a473..867a9746d 100644 --- a/libsolidity/lsp/SemanticHighlight.cpp +++ b/libsolidity/lsp/SemanticHighlighter.cpp @@ -15,7 +15,7 @@ along with solidity. If not, see . */ // SPDX-License-Identifier: GPL-3.0 -#include +#include #include using namespace std; @@ -23,24 +23,23 @@ using namespace solidity; using namespace solidity::lsp; using namespace solidity::frontend; -void SemanticHighlight::operator()(MessageID _id, Json::Value const& _args) +void SemanticHighlighter::operator()(MessageID _id, Json::Value const& _args) { auto const [sourceUnitName, lineColumn] = extractSourceUnitNameAndLineColumn(_args); auto const [sourceNode, sourceOffset] = m_server.astNodeAndOffsetAtSourceLocation(sourceUnitName, lineColumn); Json::Value jsonReply = Json::arrayValue; - for (auto const& [location, kind]: semanticHighlight(sourceNode, sourceOffset, sourceUnitName)) + for (auto const& [location, kind]: findAllReferences(sourceNode, sourceOffset, sourceUnitName)) { Json::Value item = Json::objectValue; item["range"] = toRange(location); - if (kind != DocumentHighlightKind::Unspecified) - item["kind"] = static_cast(kind); + item["kind"] = static_cast(kind); jsonReply.append(item); } client().reply(_id, jsonReply); } -vector SemanticHighlight::semanticHighlight(ASTNode const* _sourceNode, int _sourceOffset, string const& _sourceUnitName) +vector SemanticHighlighter::findAllReferences(ASTNode const* _sourceNode, int _sourceOffset, string const& _sourceUnitName) { if (!_sourceNode) return {}; diff --git a/libsolidity/lsp/SemanticHighlight.h b/libsolidity/lsp/SemanticHighlighter.h similarity index 84% rename from libsolidity/lsp/SemanticHighlight.h rename to libsolidity/lsp/SemanticHighlighter.h index 491284905..34fa29a19 100644 --- a/libsolidity/lsp/SemanticHighlight.h +++ b/libsolidity/lsp/SemanticHighlighter.h @@ -26,15 +26,15 @@ namespace solidity::lsp /** * Implements JSON RPC for `textDocument/documentHighlight`. */ -class SemanticHighlight: public HandlerBase +class SemanticHighlighter: public HandlerBase { public: - explicit SemanticHighlight(LanguageServer& _server): HandlerBase(_server) {} + using HandlerBase::HandlerBase; void operator()(MessageID _id, Json::Value const& _args); private: - std::vector semanticHighlight(frontend::ASTNode const* _sourceNode, int _sourceOffset, std::string const& _sourceUnitName); + std::vector findAllReferences(frontend::ASTNode const* _sourceNode, int _sourceOffset, std::string const& _sourceUnitName); }; } diff --git a/libsolidity/lsp/Transport.h b/libsolidity/lsp/Transport.h index 3e94e0398..f9fefaede 100644 --- a/libsolidity/lsp/Transport.h +++ b/libsolidity/lsp/Transport.h @@ -87,6 +87,11 @@ private: ); \ } +/** + * Fatally halts the process if the condition fails. + */ +#define lspAssert(condition, errorMessage) solAssert((condition), (errorMessage)) + /** * Transport layer API * diff --git a/test/libsolidity/lsp/references/enum.sol b/test/libsolidity/lsp/references/enum.sol index 38cc1e5c5..305a0662f 100644 --- a/test/libsolidity/lsp/references/enum.sol +++ b/test/libsolidity/lsp/references/enum.sol @@ -23,14 +23,14 @@ contract MyContract // ^^^^^ @ColorType3 // ^^^^^ @ColorType4 { - Color result = Color(a); + Color Color = Color(a); // ^^^^^ @ColorType5 // ^^^^^ @ColorType6 if (a != lastColor) // ^^^^^^^^^ @lastCursor2 - result = Color.Green; + Color = Color.Green; // ^^^^^ @ColorType7 - return result; + return Color; } function f() public pure returns (uint) @@ -41,6 +41,7 @@ contract MyContract } // ---- +// lib: @diagnostics 2519 // -> textDocument/documentHighlight { // "position": @EnumDef // }