lsp: Code-review fixups.

This commit is contained in:
Christian Parpart 2022-08-24 10:31:16 +02:00
parent b22d149e3c
commit 3fc7debbef
2 changed files with 39 additions and 37 deletions

View File

@ -53,9 +53,24 @@ using namespace solidity::lsp;
using namespace solidity::langutil; using namespace solidity::langutil;
using namespace solidity::frontend; using namespace solidity::frontend;
namespace fs = boost::filesystem;
namespace 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) int toDiagnosticSeverity(Error::Type _errorType)
{ {
// 1=Error, 2=Warning, 3=Info, 4=Hint // 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 vector<boost::filesystem::path> LanguageServer::allSolidityFilesFromProject() const
{ {
namespace fs = boost::filesystem; vector<fs::path> collectedPaths{};
std::vector<fs::path> collectedPaths{};
// We explicitly decided against including all files from include paths but leave the possibility // 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). // 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); auto directoryIterator = fs::recursive_directory_iterator(m_fileRepository.basePath(), fs::symlink_option::recurse);
for (fs::directory_entry const& dirEntry: directoryIterator) for (fs::directory_entry const& dirEntry: directoryIterator)
{
if ( 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; return collectedPaths;
} }

View File

@ -539,16 +539,9 @@ class TestParser:
return self.current_line_tuple is None return self.current_line_tuple is None
class FileLoadStrategy(Enum): class FileLoadStrategy(Enum):
Undefined = auto() Undefined = None
ProjectDirectory = auto() ProjectDirectory = 'project-directory'
DirectlyOpenedAndOnImport = auto() DirectlyOpenedAndOnImport = 'directly-opened-and-on-import'
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: class FileTestRunner:
""" """
@ -932,7 +925,6 @@ class SolidityLSPTestSuite: # {{{
# Enable traces to receive the amount of expected diagnostics before # Enable traces to receive the amount of expected diagnostics before
# actually receiving them. # actually receiving them.
'trace': 'messages', 'trace': 'messages',
# 'initializationOptions': {},
'capabilities': { 'capabilities': {
'textDocument': { 'textDocument': {
'publishDiagnostics': {'relatedInformation': True} 'publishDiagnostics': {'relatedInformation': True}
@ -949,7 +941,7 @@ class SolidityLSPTestSuite: # {{{
if file_load_strategy != FileLoadStrategy.Undefined: if file_load_strategy != FileLoadStrategy.Undefined:
params['initializationOptions'] = {} 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 custom_include_paths is not None and len(custom_include_paths) != 0:
if params['initializationOptions'] is None: if params['initializationOptions'] is None:
@ -1337,7 +1329,7 @@ class SolidityLSPTestSuite: # {{{
# }}} # }}}
# {{{ actual tests # {{{ 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 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 opened yet. This is how other LSPs (at least for C++) work too and it makes cross-unit tasks
@ -1370,18 +1362,18 @@ class SolidityLSPTestSuite: # {{{
self.expect_equal(report['uri'], self.get_test_file_uri('E', SUBDIR), "Correct file URI") self.expect_equal(report['uri'], self.get_test_file_uri('E', SUBDIR), "Correct file URI")
self.expect_equal(len(report['diagnostics']), 0, "no diagnostics") 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. Same as first test on that matter but with deeper nesting levels.
""" """
SUBDIR = 'include-paths-nested' SUBDIR = 'include-paths-nested'
EXPECTED_FILES = [ EXPECTED_FILES = {
"A/B/C/foo", "A/B/C/foo",
"A/B/foo", "A/B/foo",
"A/foo", "A/foo",
"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( self.setup_lsp(
solc, solc,
file_load_strategy=FileLoadStrategy.ProjectDirectory, file_load_strategy=FileLoadStrategy.ProjectDirectory,
@ -1389,23 +1381,22 @@ class SolidityLSPTestSuite: # {{{
) )
published_diagnostics = self.wait_for_diagnostics(solc) published_diagnostics = self.wait_for_diagnostics(solc)
self.expect_equal(len(published_diagnostics), len(EXPECTED_FILES), "Test number of files analyzed.") self.expect_equal(len(published_diagnostics), len(EXPECTED_FILES), "Test number of files analyzed.")
for report in published_diagnostics: self.expect_equal({report['uri'] for report in published_diagnostics}, EXPECTED_URIS)
self.expect_true(report['uri'] in EXPECTED_URIS, "Correct file URI") self.expect_equal([len(report['diagnostics']) for report in published_diagnostics], [0] * len(EXPECTED_URIS))
self.expect_equal(len(report['diagnostics']), 0, "no diagnostics")
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. Same as first test on that matter but with deeper nesting levels.
""" """
SUBDIR = 'include-paths-nested-2' SUBDIR = 'include-paths-nested-2'
EXPECTED_FILES = [ EXPECTED_FILES = {
"A/B/C/foo", "A/B/C/foo",
"A/B/foo", "A/B/foo",
"A/foo", "A/foo",
"foo", "foo",
] }
IMPLICITLY_LOADED_FILE_COUNT = 1 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( self.setup_lsp(
solc, solc,
file_load_strategy=FileLoadStrategy.ProjectDirectory, file_load_strategy=FileLoadStrategy.ProjectDirectory,
@ -1426,9 +1417,9 @@ class SolidityLSPTestSuite: # {{{
# Check last report (should be the custom imported lib). # Check last report (should be the custom imported lib).
# This file is analyzed because it was imported via "A/B/C/foo.sol". # This file is analyzed because it was imported via "A/B/C/foo.sol".
report = published_diagnostics[len(EXPECTED_URIS)] last_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(last_report['uri'], self.get_test_file_uri('second', 'other-include-dir/otherlib'), "Correct file URI")
self.expect_equal(len(report['diagnostics']), 0, "no diagnostics") self.expect_equal(len(last_report['diagnostics']), 0, "no diagnostics")
def test_publish_diagnostics_errors_multiline(self, solc: JsonRpcProcess) -> None: def test_publish_diagnostics_errors_multiline(self, solc: JsonRpcProcess) -> None:
@ -1545,7 +1536,7 @@ class SolidityLSPTestSuite: # {{{
def test_custom_includes_with_full_project(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 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. but not those include files that are not directly nor indirectly included.
""" """
@ -1577,7 +1568,7 @@ class SolidityLSPTestSuite: # {{{
report = published_diagnostics[1] report = published_diagnostics[1]
self.expect_equal(report['uri'], f"{self.project_root_uri}/other-include-dir/otherlib/otherlib.sol") self.expect_equal(report['uri'], f"{self.project_root_uri}/other-include-dir/otherlib/otherlib.sol")
diagnostics = report['diagnostics'] 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)) 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: def test_didChange_in_A_causing_error_in_B(self, solc: JsonRpcProcess) -> None: