From b6ba43234e497ced4e14356207d8c621d3803c04 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Mon, 13 Jun 2022 15:56:55 +0200 Subject: [PATCH] lsp: Always load all solidity files from project for analyzing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Śliwak --- Changelog.md | 1 + libsolidity/lsp/FileRepository.cpp | 6 ++ libsolidity/lsp/LanguageServer.cpp | 65 ++++++++++++++++- libsolidity/lsp/LanguageServer.h | 22 ++++++ libsolidity/lsp/Transport.cpp | 3 + libsolutil/CommonIO.h | 40 ----------- .../lsp/analyze-full-project/C.sol | 6 ++ .../lsp/analyze-full-project/D.sol | 6 ++ .../lsp/analyze-full-project/E.sol | 6 ++ test/lsp.py | 70 ++++++++++++++++++- 10 files changed, 181 insertions(+), 44 deletions(-) create mode 100644 test/libsolidity/lsp/analyze-full-project/C.sol create mode 100644 test/libsolidity/lsp/analyze-full-project/D.sol create mode 100644 test/libsolidity/lsp/analyze-full-project/E.sol diff --git a/Changelog.md b/Changelog.md index dfa237e2d..f0f8b8485 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,6 +9,7 @@ Language Features: Compiler Features: * Code Generator: More efficient overflow checks for multiplication. * Yul Optimizer: Simplify the starting offset of zero-length operations to zero. + * Language Server: Analyze all files in a project by default (can be customized by setting ``'file-load-strategy'`` to ``'directly-opened-and-on-import'`` in LSP settings object). Bugfixes: diff --git a/libsolidity/lsp/FileRepository.cpp b/libsolidity/lsp/FileRepository.cpp index 6ba583c3a..58bc0bf8d 100644 --- a/libsolidity/lsp/FileRepository.cpp +++ b/libsolidity/lsp/FileRepository.cpp @@ -17,6 +17,7 @@ // SPDX-License-Identifier: GPL-3.0 #include +#include #include #include @@ -25,11 +26,14 @@ #include #include #include +#include #include #include +#include + using namespace std; using namespace solidity; using namespace solidity::lsp; @@ -84,6 +88,7 @@ string FileRepository::sourceUnitNameToUri(string const& _sourceUnitName) const string FileRepository::uriToSourceUnitName(string const& _path) const { + lspAssert(boost::algorithm::starts_with(_path, "file://"), ErrorCode::InternalError, "URI must start with file://"); return stripFileUriSchemePrefix(_path); } @@ -92,6 +97,7 @@ void FileRepository::setSourceByUri(string const& _uri, string _source) // This is needed for uris outside the base path. It can lead to collisions, // but we need to mostly rewrite this in a future version anyway. auto sourceUnitName = uriToSourceUnitName(_uri); + lspDebug(fmt::format("FileRepository.setSourceByUri({}): {}", _uri, _source)); m_sourceUnitNamesToUri.emplace(sourceUnitName, _uri); m_sourceCodes[sourceUnitName] = std::move(_source); } diff --git a/libsolidity/lsp/LanguageServer.cpp b/libsolidity/lsp/LanguageServer.cpp index 5465406c0..3020900c0 100644 --- a/libsolidity/lsp/LanguageServer.cpp +++ b/libsolidity/lsp/LanguageServer.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include @@ -42,6 +43,8 @@ #include #include +#include + using namespace std; using namespace std::string_literals; using namespace std::placeholders; @@ -118,7 +121,7 @@ LanguageServer::LanguageServer(Transport& _transport): {"cancelRequest", [](auto, auto) {/*nothing for now as we are synchronous */}}, {"exit", [this](auto, auto) { m_state = (m_state == State::ShutdownRequested ? State::ExitRequested : State::ExitWithoutShutdown); }}, {"initialize", bind(&LanguageServer::handleInitialize, this, _1, _2)}, - {"initialized", [](auto, auto) {}}, + {"initialized", bind(&LanguageServer::handleInitialized, this, _1, _2)}, {"$/setTrace", [this](auto, Json::Value const& args) { setTrace(args["value"]); }}, {"shutdown", [this](auto, auto) { m_state = State::ShutdownRequested; }}, {"textDocument/definition", GotoDefinition(*this) }, @@ -147,6 +150,26 @@ Json::Value LanguageServer::toJson(SourceLocation const& _location) void LanguageServer::changeConfiguration(Json::Value const& _settings) { + // The settings item: "file-load-strategy" (enum) defaults to "project-directory" if not (or not correctly) set. + // It can be overridden during client's handshake or at runtime, as usual. + // + // If this value is set to "project-directory" (default), all .sol files located inside the project directory or reachable through symbolic links will be subject to operations. + // + // Operations include compiler analysis, but also finding all symbolic references or symbolic renaming. + // + // If this value is set to "directly-opened-and-on-import", then only currently directly opened files and + // those files being imported directly or indirectly will be included in operations. + if (_settings["file-load-strategy"]) + { + auto const text = _settings["file-load-strategy"].asString(); + if (text == "project-directory") + m_fileLoadStrategy = FileLoadStrategy::ProjectDirectory; + else if (text == "directly-opened-and-on-import") + m_fileLoadStrategy = FileLoadStrategy::DirectlyOpenedAndOnImported; + else + lspAssert(false, ErrorCode::InvalidParams, "Invalid file load strategy: " + text); + } + m_settingsObject = _settings; Json::Value jsonIncludePaths = _settings["include-paths"]; @@ -173,6 +196,23 @@ void LanguageServer::changeConfiguration(Json::Value const& _settings) } } +vector LanguageServer::allSolidityFilesFromProject() const +{ + namespace fs = boost::filesystem; + + std::vector collectedPaths{}; + + // We explicitly decided against including all files from include paths but leave the possibility + // open for a future PR to enable such a feature to be optionally enabled (default disabled). + + auto directoryIterator = fs::recursive_directory_iterator(m_fileRepository.basePath(), fs::symlink_option::recurse); + for (fs::directory_entry const& dirEntry: directoryIterator) + if (dirEntry.path().extension() == ".sol") + collectedPaths.push_back(dirEntry.path()); + + return collectedPaths; +} + void LanguageServer::compile() { // For files that are not open, we have to take changes on disk into account, @@ -181,6 +221,18 @@ void LanguageServer::compile() FileRepository oldRepository(m_fileRepository.basePath(), m_fileRepository.includePaths()); swap(oldRepository, m_fileRepository); + // Load all solidity files from project. + if (m_fileLoadStrategy == FileLoadStrategy::ProjectDirectory) + for (auto const& projectFile: allSolidityFilesFromProject()) + { + lspDebug(fmt::format("adding project file: {}", projectFile.generic_string())); + m_fileRepository.setSourceByUri( + m_fileRepository.sourceUnitNameToUri(projectFile.generic_string()), + util::readFileAsString(projectFile) + ); + } + + // Overwrite all files as opened by the client, including the ones which might potentially have changes. for (string const& fileName: m_openFiles) m_fileRepository.setSourceByUri( fileName, @@ -269,6 +321,7 @@ bool LanguageServer::run() { string const methodName = (*jsonMessage)["method"].asString(); id = (*jsonMessage)["id"]; + lspDebug(fmt::format("received method call: {}", methodName)); if (auto handler = util::valueOrDefault(m_handlers, methodName)) handler(id, (*jsonMessage)["params"]); @@ -278,6 +331,10 @@ bool LanguageServer::run() else m_client.error({}, ErrorCode::ParseError, "\"method\" has to be a string."); } + catch (Json::Exception const&) + { + m_client.error(id, ErrorCode::InvalidParams, "JSON object access error. Most likely due to a badly formatted JSON request message."s); + } catch (RequestError const& error) { m_client.error(id, error.code(), error.comment() ? *error.comment() : ""s); @@ -347,6 +404,12 @@ void LanguageServer::handleInitialize(MessageID _id, Json::Value const& _args) m_client.reply(_id, move(replyArgs)); } +void LanguageServer::handleInitialized(MessageID, Json::Value const&) +{ + if (m_fileLoadStrategy == FileLoadStrategy::ProjectDirectory) + compileAndUpdateDiagnostics(); +} + void LanguageServer::semanticTokensFull(MessageID _id, Json::Value const& _args) { auto uri = _args["textDocument"]["uri"]; diff --git a/libsolidity/lsp/LanguageServer.h b/libsolidity/lsp/LanguageServer.h index ee4f06957..a05bec497 100644 --- a/libsolidity/lsp/LanguageServer.h +++ b/libsolidity/lsp/LanguageServer.h @@ -36,6 +36,23 @@ namespace solidity::lsp class RenameSymbol; enum class ErrorCode; +/** + * Enum to mandate what files to take into consideration for source code analysis. + */ +enum class FileLoadStrategy +{ + /// Takes only those files into consideration that are explicitly opened and those + /// that have been directly or indirectly imported. + DirectlyOpenedAndOnImported = 0, + + /// Takes all Solidity (.sol) files within the project root into account. + /// Symbolic links will be followed, even if they lead outside of the project directory + /// (`--allowed-paths` is currently ignored by the LSP). + /// + /// This resembles the closest what other LSPs should be doing already. + ProjectDirectory = 1, +}; + /** * Solidity Language Server, managing one LSP client. * This implements a subset of LSP version 3.16 that can be found at: @@ -68,6 +85,7 @@ private: /// Reports an error and returns false if not. void requireServerInitialized(); void handleInitialize(MessageID _id, Json::Value const& _args); + void handleInitialized(MessageID _id, Json::Value const& _args); void handleWorkspaceDidChangeConfiguration(Json::Value const& _args); void setTrace(Json::Value const& _args); void handleTextDocumentDidOpen(Json::Value const& _args); @@ -82,6 +100,9 @@ private: /// Compile everything until after analysis phase. void compile(); + + std::vector allSolidityFilesFromProject() const; + using MessageHandler = std::function; Json::Value toRange(langutil::SourceLocation const& _location); @@ -100,6 +121,7 @@ private: /// Set of source unit names for which we sent diagnostics to the client in the last iteration. std::set m_nonemptyDiagnostics; FileRepository m_fileRepository; + FileLoadStrategy m_fileLoadStrategy = FileLoadStrategy::ProjectDirectory; frontend::CompilerStack m_compilerStack; diff --git a/libsolidity/lsp/Transport.cpp b/libsolidity/lsp/Transport.cpp index aa85fd6b1..90c0b20a7 100644 --- a/libsolidity/lsp/Transport.cpp +++ b/libsolidity/lsp/Transport.cpp @@ -16,6 +16,7 @@ */ // SPDX-License-Identifier: GPL-3.0 #include +#include #include #include @@ -205,11 +206,13 @@ std::string StdioTransport::getline() { std::string line; std::getline(std::cin, line); + lspDebug(fmt::format("Received: {}", line)); return line; } void StdioTransport::writeBytes(std::string_view _data) { + lspDebug(fmt::format("Sending: {}", _data)); auto const bytesWritten = fwrite(_data.data(), 1, _data.size(), stdout); solAssert(bytesWritten == _data.size()); } diff --git a/libsolutil/CommonIO.h b/libsolutil/CommonIO.h index 56f62f0ac..e6cd0c161 100644 --- a/libsolutil/CommonIO.h +++ b/libsolutil/CommonIO.h @@ -48,46 +48,6 @@ inline std::ostream& operator<<(std::ostream& os, bytes const& _bytes) namespace util { -namespace detail -{ - -template -struct RecursiveFileCollector -{ - Predicate predicate; - std::vector result {}; - - RecursiveFileCollector& operator()(boost::filesystem::path const& _directory) - { - if (!boost::filesystem::is_directory(_directory)) - return *this; - auto iterator = boost::filesystem::directory_iterator(_directory); - auto const iteratorEnd = boost::filesystem::directory_iterator(); - - while (iterator != iteratorEnd) - { - if (boost::filesystem::is_directory(iterator->status())) - (*this)(iterator->path()); - - if (predicate(iterator->path())) - result.push_back(iterator->path()); - - ++iterator; - } - return *this; - } -}; - -template -RecursiveFileCollector(Predicate) -> RecursiveFileCollector; -} - -template -std::vector findFilesRecursively(boost::filesystem::path const& _rootDirectory, Predicate _predicate) -{ - return detail::RecursiveFileCollector{_predicate}(_rootDirectory).result; -} - /// Retrieves and returns the contents of the given file as a std::string. /// If the file doesn't exist, it will throw a FileNotFound exception. /// If the file exists but is not a regular file, it will throw NotAFile exception. diff --git a/test/libsolidity/lsp/analyze-full-project/C.sol b/test/libsolidity/lsp/analyze-full-project/C.sol new file mode 100644 index 000000000..d08ba140e --- /dev/null +++ b/test/libsolidity/lsp/analyze-full-project/C.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.0; + +contract C +{ +} diff --git a/test/libsolidity/lsp/analyze-full-project/D.sol b/test/libsolidity/lsp/analyze-full-project/D.sol new file mode 100644 index 000000000..93c8e92fc --- /dev/null +++ b/test/libsolidity/lsp/analyze-full-project/D.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.0; + +contract D +{ +} diff --git a/test/libsolidity/lsp/analyze-full-project/E.sol b/test/libsolidity/lsp/analyze-full-project/E.sol new file mode 100644 index 000000000..e0e5d1988 --- /dev/null +++ b/test/libsolidity/lsp/analyze-full-project/E.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.0; + +contract E +{ +} diff --git a/test/lsp.py b/test/lsp.py index 543a7ca43..98c16d3f5 100755 --- a/test/lsp.py +++ b/test/lsp.py @@ -537,6 +537,18 @@ class TestParser: """ return self.current_line_tuple is None +class FileLoadStrategy(Enum): + Undefined = auto() + ProjectDirectory = auto() + DirectlyOpenedAndOnImport = auto() + + def lsp_name(self): + if self == FileLoadStrategy.ProjectDirectory: + return 'project-directory' + elif self == FileLoadStrategy.DirectlyOpenedAndOnImport: + return 'directly-opened-and-on-import' + return None + class FileTestRunner: """ Runs all tests in a given file. @@ -898,18 +910,27 @@ class SolidityLSPTestSuite: # {{{ return min(max(self.test_counter.failed, self.assertion_counter.failed), 127) - def setup_lsp(self, lsp: JsonRpcProcess, expose_project_root=True): + def setup_lsp( + self, + lsp: JsonRpcProcess, + expose_project_root=True, + file_load_strategy: FileLoadStrategy=FileLoadStrategy.DirectlyOpenedAndOnImport, + project_root_subdir=None + ): """ Prepares the solc LSP server by calling `initialize`, and `initialized` methods. """ + project_root_uri_with_maybe_subdir = self.project_root_uri + if project_root_subdir is not None: + project_root_uri_with_maybe_subdir = self.project_root_uri + '/' + project_root_subdir params = { 'processId': None, - 'rootUri': self.project_root_uri, + 'rootUri': project_root_uri_with_maybe_subdir, # Enable traces to receive the amount of expected diagnostics before # actually receiving them. 'trace': 'messages', - 'initializationOptions': {}, + # 'initializationOptions': {}, 'capabilities': { 'textDocument': { 'publishDiagnostics': {'relatedInformation': True} @@ -923,6 +944,9 @@ class SolidityLSPTestSuite: # {{{ } } } + if file_load_strategy != FileLoadStrategy.Undefined: + params['initializationOptions'] = {} + params['initializationOptions']['file-load-strategy'] = file_load_strategy.lsp_name() if not expose_project_root: params['rootUri'] = None lsp.call_method('initialize', params) @@ -1059,6 +1083,14 @@ class SolidityLSPTestSuite: # {{{ ) return self.wait_for_diagnostics(solc_process) + def expect_true( + self, + actual, + description="Expected True value", + part=ExpectationFailed.Part.Diagnostics + ) -> None: + self.expect_equal(actual, True, description, part) + def expect_equal( self, actual, @@ -1295,6 +1327,38 @@ class SolidityLSPTestSuite: # {{{ # }}} # {{{ actual tests + def test_analyze_all_project_files1(self, solc: JsonRpcProcess) -> None: + """ + Tests the option (default) to analyze all .sol project files even when they have not been actively + opened yet. This is how other LSPs (at least for C++) work too and it makes cross-unit tasks + actually correct (e.g. symbolic rename, find all references, ...). + + In this test, we simply open up a custom project and ensure we're receiving the diagnostics + for all existing files in that project (while having none of these files opened). + """ + SUBDIR = 'analyze-full-project' + self.setup_lsp( + solc, + file_load_strategy=FileLoadStrategy.ProjectDirectory, + project_root_subdir=SUBDIR + ) + published_diagnostics = self.wait_for_diagnostics(solc) + self.expect_equal(len(published_diagnostics), 3, "Diagnostic reports for 3 files") + + # C.sol + report = published_diagnostics[0] + self.expect_equal(report['uri'], self.get_test_file_uri('C', SUBDIR), "Correct file URI") + self.expect_equal(len(report['diagnostics']), 0, "no diagnostics") + + # D.sol + report = published_diagnostics[1] + self.expect_equal(report['uri'], self.get_test_file_uri('D', SUBDIR), "Correct file URI") + self.expect_equal(len(report['diagnostics']), 0, "no diagnostics") + + # E.sol + report = published_diagnostics[2] + self.expect_equal(report['uri'], self.get_test_file_uri('E', SUBDIR), "Correct file URI") + self.expect_equal(len(report['diagnostics']), 0, "no diagnostics") def test_publish_diagnostics_errors_multiline(self, solc: JsonRpcProcess) -> None: self.setup_lsp(solc)