lsp: Always load all solidity files from project for analyzing.

Co-authored-by: Kamil Śliwak <kamil.sliwak@codepoets.it>

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:

libsolidity/lsp/FileRepository.cpp

@@ -17,6 +17,7 @@
 // SPDX-License-Identifier: GPL-3.0
 
 #include <libsolidity/lsp/FileRepository.h>
+#include <libsolidity/lsp/Transport.h>
 #include <libsolidity/lsp/Utils.h>
 
 #include <libsolutil/StringUtils.h>
@@ -25,11 +26,14 @@
 #include <range/v3/algorithm/none_of.hpp>
 #include <range/v3/range/conversion.hpp>
 #include <range/v3/view/transform.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 
 #include <regex>
 
 #include <boost/algorithm/string/predicate.hpp>
 
+#include <fmt/format.h>
+
 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);
 }

libsolidity/lsp/LanguageServer.cpp

@@ -32,6 +32,7 @@
 #include <liblangutil/SourceReferenceExtractor.h>
 #include <liblangutil/CharStream.h>
 
+#include <libsolutil/CommonIO.h>
 #include <libsolutil/Visitor.h>
 #include <libsolutil/JSON.h>
 
@@ -42,6 +43,8 @@
 #include <ostream>
 #include <string>
 
+#include <fmt/format.h>
+
 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<boost::filesystem::path> LanguageServer::allSolidityFilesFromProject() const
+{
+	namespace fs = boost::filesystem;
+
+	std::vector<fs::path> 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"];

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<boost::filesystem::path> allSolidityFilesFromProject() const;
+
 	using MessageHandler = std::function<void(MessageID, Json::Value const&)>;
 
 	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<std::string> m_nonemptyDiagnostics;
 	FileRepository m_fileRepository;
+	FileLoadStrategy m_fileLoadStrategy = FileLoadStrategy::ProjectDirectory;
 
 	frontend::CompilerStack m_compilerStack;
 

libsolidity/lsp/Transport.cpp

@@ -16,6 +16,7 @@
 */
 // SPDX-License-Identifier: GPL-3.0
 #include <libsolidity/lsp/Transport.h>
+#include <libsolidity/lsp/Utils.h>
 
 #include <libsolutil/JSON.h>
 #include <libsolutil/Visitor.h>
@@ -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());
 }

libsolutil/CommonIO.h

@@ -48,46 +48,6 @@ inline std::ostream& operator<<(std::ostream& os, bytes const& _bytes)
 namespace util
 {
 
-namespace detail
-{
-
-template <typename Predicate>
-struct RecursiveFileCollector
-{
-	Predicate predicate;
-	std::vector<boost::filesystem::path> 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 <typename Predicate>
-RecursiveFileCollector(Predicate) -> RecursiveFileCollector<Predicate>;
-}
-
-template <typename Predicate>
-std::vector<boost::filesystem::path> 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.

test/libsolidity/lsp/analyze-full-project/C.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.0;
+
+contract C
+{
+}

test/libsolidity/lsp/analyze-full-project/D.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.0;
+
+contract D
+{
+}

test/libsolidity/lsp/analyze-full-project/E.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.0;
+
+contract E
+{
+}

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)