lsp: Code-review fixups.

Author: christianparpart

libsolidity/lsp/LanguageServer.cpp

@@ -53,9 +53,24 @@ using namespace solidity::lsp;
 using namespace solidity::langutil;
 using namespace solidity::frontend;
 
+namespace fs = boost::filesystem;
+
 namespace
 {
 
+bool resolvesToRegularFile(boost::filesystem::path _path)
+{
+	fs::file_status fileStatus = fs::status(_path);
+
+	while (fileStatus.type() == fs::file_type::symlink_file)
+	{
+		_path = boost::filesystem::read_symlink(_path);
+		fileStatus = fs::status(_path);
+	}
+
+	return fileStatus.type() == fs::file_type::regular_file;
+}
+
 int toDiagnosticSeverity(Error::Type _errorType)
 {
 	// 1=Error, 2=Warning, 3=Info, 4=Hint
@@ -198,22 +213,18 @@ void LanguageServer::changeConfiguration(Json::Value const& _settings)
 
 vector<boost::filesystem::path> LanguageServer::allSolidityFilesFromProject() const
 {
-	namespace fs = boost::filesystem;
-
-	std::vector<fs::path> collectedPaths{};
+	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.status().type() == fs::file_type::regular_file &&
-			dirEntry.path().extension() == ".sol"
+			dirEntry.path().extension() == ".sol" &&
+			(dirEntry.status().type() == fs::file_type::regular_file || resolvesToRegularFile(dirEntry.path()))
 		)
-		collectedPaths.push_back(dirEntry.path());
-	}
+			collectedPaths.push_back(dirEntry.path());
 
 	return collectedPaths;
 }

test/lsp.py

@@ -539,16 +539,9 @@ def at_end(self):
         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
+    Undefined = None
+    ProjectDirectory = 'project-directory'
+    DirectlyOpenedAndOnImport =  'directly-opened-and-on-import'
 
 class FileTestRunner:
     """
@@ -932,7 +925,6 @@ def setup_lsp(
             # Enable traces to receive the amount of expected diagnostics before
             # actually receiving them.
             'trace': 'messages',
-            # 'initializationOptions': {},
             'capabilities': {
                 'textDocument': {
                     'publishDiagnostics': {'relatedInformation': True}
@@ -949,7 +941,7 @@ def setup_lsp(
 
         if file_load_strategy != FileLoadStrategy.Undefined:
             params['initializationOptions'] = {}
-            params['initializationOptions']['file-load-strategy'] = file_load_strategy.lsp_name()
+            params['initializationOptions']['file-load-strategy'] = file_load_strategy.value
 
         if custom_include_paths is not None and len(custom_include_paths) != 0:
             if params['initializationOptions'] is None:
@@ -1337,7 +1329,7 @@ def user_interaction_failed_autoupdate(self, test, sub_dir):
     # }}}
 
     # {{{ actual tests
-    def test_analyze_all_project_files1(self, solc: JsonRpcProcess) -> None:
+    def test_analyze_all_project_files_flat(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
@@ -1370,42 +1362,41 @@ def test_analyze_all_project_files1(self, solc: JsonRpcProcess) -> None:
         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_analyze_all_project_files2(self, solc: JsonRpcProcess) -> None:
+    def test_analyze_all_project_files_nested(self, solc: JsonRpcProcess) -> None:
         """
         Same as first test on that matter but with deeper nesting levels.
         """
         SUBDIR = 'include-paths-nested'
-        EXPECTED_FILES = [
+        EXPECTED_FILES = {
             "A/B/C/foo",
             "A/B/foo",
             "A/foo",
             "foo",
-        ]
-        EXPECTED_URIS = [self.get_test_file_uri(x, SUBDIR) for x in EXPECTED_FILES]
+        }
+        EXPECTED_URIS = {self.get_test_file_uri(x, SUBDIR) for x in EXPECTED_FILES}
         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), len(EXPECTED_FILES), "Test number of files analyzed.")
-        for report in published_diagnostics:
-            self.expect_true(report['uri'] in EXPECTED_URIS, "Correct file URI")
-            self.expect_equal(len(report['diagnostics']), 0, "no diagnostics")
+        self.expect_equal({report['uri'] for report in published_diagnostics}, EXPECTED_URIS)
+        self.expect_equal([len(report['diagnostics']) for report in published_diagnostics], [0] * len(EXPECTED_URIS))
 
-    def test_analyze_all_project_files3(self, solc: JsonRpcProcess) -> None:
+    def test_analyze_all_project_files_nested_with_include_paths(self, solc: JsonRpcProcess) -> None:
         """
         Same as first test on that matter but with deeper nesting levels.
         """
         SUBDIR = 'include-paths-nested-2'
-        EXPECTED_FILES = [
+        EXPECTED_FILES = {
             "A/B/C/foo",
             "A/B/foo",
             "A/foo",
             "foo",
-        ]
+        }
         IMPLICITLY_LOADED_FILE_COUNT = 1
-        EXPECTED_URIS = [self.get_test_file_uri(x, SUBDIR) for x in EXPECTED_FILES]
+        EXPECTED_URIS = {self.get_test_file_uri(x, SUBDIR) for x in EXPECTED_FILES}
         self.setup_lsp(
             solc,
             file_load_strategy=FileLoadStrategy.ProjectDirectory,
@@ -1426,9 +1417,9 @@ def test_analyze_all_project_files3(self, solc: JsonRpcProcess) -> None:
 
         # Check last report (should be the custom imported lib).
         # This file is analyzed because it was imported via "A/B/C/foo.sol".
-        report = published_diagnostics[len(EXPECTED_URIS)]
-        self.expect_equal(report['uri'], f"{self.project_root_uri}/other-include-dir/otherlib/second.sol", "Correct file URI")
-        self.expect_equal(len(report['diagnostics']), 0, "no diagnostics")
+        last_report = published_diagnostics[len(EXPECTED_URIS)]
+        self.expect_equal(last_report['uri'], self.get_test_file_uri('second', 'other-include-dir/otherlib'), "Correct file URI")
+        self.expect_equal(len(last_report['diagnostics']), 0, "no diagnostics")
 
 
     def test_publish_diagnostics_errors_multiline(self, solc: JsonRpcProcess) -> None:
@@ -1545,7 +1536,7 @@ def test_custom_includes(self, solc: JsonRpcProcess) -> None:
 
     def test_custom_includes_with_full_project(self, solc: JsonRpcProcess) -> None:
         """
-        Tests loading all all project files while having custom include directories configured.
+        Tests loading all project files while having custom include directories configured.
         In such a scenario, all project files should be analyzed and those being included via search path
         but not those include files that are not directly nor indirectly included.
         """
@@ -1577,7 +1568,7 @@ def test_custom_includes_with_full_project(self, solc: JsonRpcProcess) -> None:
         report = published_diagnostics[1]
         self.expect_equal(report['uri'], f"{self.project_root_uri}/other-include-dir/otherlib/otherlib.sol")
         diagnostics = report['diagnostics']
-        self.expect_equal(len(diagnostics), 1, "no diagnostics")
+        self.expect_equal(len(diagnostics), 1)
         self.expect_diagnostic(diagnostics[0], code=2018, lineNo=5, startEndColumns=(4, 62))
 
     def test_didChange_in_A_causing_error_in_B(self, solc: JsonRpcProcess) -> None: