From 02e84ea547ffacf818ead5efc66f713e63f8e321 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Wed, 28 Sep 2022 15:48:49 +0200 Subject: [PATCH] Applying final review --- libsolidity/lsp/LanguageServer.cpp | 2 +- libsolidity/lsp/ReferenceCollector.cpp | 30 ++++++++---------- libsolidity/lsp/ReferenceCollector.h | 44 +++++++++++++++----------- 3 files changed, 41 insertions(+), 35 deletions(-) diff --git a/libsolidity/lsp/LanguageServer.cpp b/libsolidity/lsp/LanguageServer.cpp index 70f2429a4..aec2e8185 100644 --- a/libsolidity/lsp/LanguageServer.cpp +++ b/libsolidity/lsp/LanguageServer.cpp @@ -548,7 +548,7 @@ void LanguageServer::handleTextDocumentDidClose(Json::Value const& _args) } -tuple LanguageServer::astNodeAndOffsetAtSourceLocation(std::string const& _sourceUnitName, langutil::LineColumn const& _filePos) +tuple LanguageServer::astNodeAndOffsetAtSourceLocation(std::string const& _sourceUnitName, LineColumn const& _filePos) { if (m_compilerStack.state() < CompilerStack::AnalysisPerformed) return {nullptr, -1}; diff --git a/libsolidity/lsp/ReferenceCollector.cpp b/libsolidity/lsp/ReferenceCollector.cpp index 9fd0f9462..2c328cc94 100644 --- a/libsolidity/lsp/ReferenceCollector.cpp +++ b/libsolidity/lsp/ReferenceCollector.cpp @@ -35,23 +35,23 @@ namespace solidity::lsp ReferenceCollector::ReferenceCollector( Declaration const& _declaration, - string const& _sourceIdentifierName + string const& _identifierAtCursorLocation ): m_declaration{_declaration}, - m_sourceIdentifierName{_sourceIdentifierName.empty() ? _declaration.name() : _sourceIdentifierName} + m_identifierAtCursorLocation{_identifierAtCursorLocation.empty() ? _declaration.name() : _identifierAtCursorLocation} { } vector ReferenceCollector::collect( Declaration const* _declaration, ASTNode const& _astSearchRoot, - string const& _sourceIdentifierName + string const& _identifierAtCursorLocation ) { if (!_declaration) return {}; - ReferenceCollector collector(*_declaration, _sourceIdentifierName); + ReferenceCollector collector(*_declaration, _identifierAtCursorLocation); _astSearchRoot.accept(collector); return std::move(collector.m_collectedReferences); } @@ -65,10 +65,10 @@ vector ReferenceCollector::collect( if (!_sourceNode) return {}; - auto references = vector{}; + solAssert(_sourceNode->location().containsOffset(_sourceOffset), ""); if (auto const* identifier = dynamic_cast(_sourceNode)) - references += collect(identifier->annotation().referencedDeclaration, _sourceUnit, identifier->name()); + return collect(identifier->annotation().referencedDeclaration, _sourceUnit, identifier->name()); else if (auto const* identifierPath = dynamic_cast(_sourceNode)) { solAssert(identifierPath->path().size() >= 1, ""); @@ -79,19 +79,17 @@ vector ReferenceCollector::collect( { Declaration const* declaration = identifierPath->annotation().pathDeclarations.at(i); ASTString const& name = identifierPath->path().at(i); - references += collect(declaration, _sourceUnit, name); - break; + return collect(declaration, _sourceUnit, name); } } + return {}; } else if (auto const* memberAccess = dynamic_cast(_sourceNode)) - references += collect(memberAccess->annotation().referencedDeclaration, _sourceUnit, memberAccess->memberName()); + return collect(memberAccess->annotation().referencedDeclaration, _sourceUnit, memberAccess->memberName()); else if (auto const* declaration = dynamic_cast(_sourceNode)) - references += collect(declaration, _sourceUnit, declaration->name()); + return collect(declaration, _sourceUnit, declaration->name()); else lspRequire(false, ErrorCode::InternalError, "Unhandled AST node "s + typeid(*_sourceNode).name()); - - return references; } bool ReferenceCollector::visit(ImportDirective const& _import) @@ -99,7 +97,7 @@ bool ReferenceCollector::visit(ImportDirective const& _import) for (ImportDirective::SymbolAlias const& symbolAlias: _import.symbolAliases()) { if ( - symbolAlias.alias && *symbolAlias.alias == m_sourceIdentifierName && + symbolAlias.alias && *symbolAlias.alias == m_identifierAtCursorLocation && symbolAlias.symbol && symbolAlias.symbol->annotation().referencedDeclaration == &m_declaration ) m_collectedReferences.emplace_back(symbolAlias.location, DocumentHighlightKind::Read); @@ -122,7 +120,7 @@ void ReferenceCollector::endVisit(IdentifierPath const& _identifierPath) _identifierPath.annotation().pathDeclarations.at(i) : nullptr; - if (declaration == &m_declaration && name == m_sourceIdentifierName) + if (declaration == &m_declaration && name == m_identifierAtCursorLocation) m_collectedReferences.emplace_back(_identifierPath.pathLocations().at(i), m_kind); } } @@ -148,7 +146,7 @@ bool ReferenceCollector::visit(Assignment const& _assignment) bool ReferenceCollector::visit(VariableDeclaration const& _variableDeclaration) { - if (&_variableDeclaration == &m_declaration && _variableDeclaration.name() == m_sourceIdentifierName) + if (&_variableDeclaration == &m_declaration && _variableDeclaration.name() == m_identifierAtCursorLocation) m_collectedReferences.emplace_back(_variableDeclaration.nameLocation(), DocumentHighlightKind::Write); return true; } @@ -158,7 +156,7 @@ bool ReferenceCollector::visitNode(ASTNode const& _genericNode) if (&_genericNode == &m_declaration) { Declaration const* declaration = dynamic_cast(&_genericNode); - if (declaration->name() == m_sourceIdentifierName) + if (declaration->name() == m_identifierAtCursorLocation) m_collectedReferences.emplace_back(declaration->nameLocation(), m_kind); } diff --git a/libsolidity/lsp/ReferenceCollector.h b/libsolidity/lsp/ReferenceCollector.h index 9e7794389..701b534d7 100644 --- a/libsolidity/lsp/ReferenceCollector.h +++ b/libsolidity/lsp/ReferenceCollector.h @@ -24,6 +24,8 @@ namespace solidity::lsp { /** + * Describes the way a reference to be highlighted is used in the code. + * * See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#documentHighlightKind */ enum class DocumentHighlightKind @@ -41,31 +43,19 @@ using Reference = std::tuple; /** * ReferenceCollector can be used to collect all source locations and their usage - * of a given symbol in the AST tree. + * of a given symbol in the AST. */ class ReferenceCollector: public frontend::ASTConstVisitor { -private: - ReferenceCollector(frontend::Declaration const& _declaration, std::string const& _sourceIdentifierName = ""); - - /// Collects all occurrences of a given identifier matching the same declaration. - /// Just passing the AST node and infer the name from there is not enough as they might have been aliased. +public: + /// Collects all references (symbols with the same matching declaration as @p _sourceNode) in the given source unit. /// - /// @param _declaration the declaration to match against for the symbols to collect + /// @param _sourceNode AST node to start collecting recursively down from. /// @param _sourceOffset byte offset into the respective file reflecting the exact location of the cursor - /// @param _astSearchRoot the AST base root node to start the recursive traversal to collect the references - /// @param _sourceIdentifierName the symbolic name that must match, which is explicitly given as it might have been altered due to aliases. + /// @param _sourceUnit the related source unit the given AST node belongs to. /// /// @returns a vector of Reference objects that contain the source location of each match as well as their /// semantic use (e.g. read access or write access). - static std::vector collect( - frontend::Declaration const* _declaration, - frontend::ASTNode const& _astSearchRoot, - std::string const& _sourceIdentifierName - ); - -public: - /// Collects all references (symbols with the same matching declaration as @p _sourceNode) in the given source unit. static std::vector collect(frontend::ASTNode const* _sourceNode, int _sourceOffset, frontend::SourceUnit const& _sourceUnit); bool visit(frontend::ImportDirective const& _import) override; @@ -76,9 +66,27 @@ public: bool visit(frontend::VariableDeclaration const& _node) override; bool visitNode(frontend::ASTNode const& _node) override; +private: + ReferenceCollector(frontend::Declaration const& _declaration, std::string const& _identifierAtCursorLocation = ""); + + /// Collects all occurrences of a given identifier matching the same declaration. + /// Just passing the AST node and inferring the name from there is not enough as they might have been aliased. + /// + /// @param _declaration the declaration to match against for the symbols to collect + /// @param _astSearchRoot the AST base root node to start the recursive traversal to collect the references + /// @param _identifierAtCursorLocation the symbolic name that must match, which is explicitly given as it might have been altered due to aliases. + /// + /// @returns a vector of Reference objects that contain the source location of each match as well as their + /// semantic use (e.g. read access or write access). + static std::vector collect( + frontend::Declaration const* _declaration, + frontend::ASTNode const& _astSearchRoot, + std::string const& _identifierAtCursorLocation + ); + private: frontend::Declaration const& m_declaration; - std::string const& m_sourceIdentifierName; + std::string const& m_identifierAtCursorLocation; std::vector m_collectedReferences; DocumentHighlightKind m_kind = DocumentHighlightKind::Read; };