simplify using reviewer suggestions

Makefile

@@ -1,6 +1,7 @@
 SHELL := /bin/bash
 
 PY_MODULE := slither
+TEST_MODULE := tests
 
 ALL_PY_SRCS := $(shell find $(PY_MODULE) -name '*.py') \
 	$(shell find test -name '*.py')
@@ -33,7 +34,7 @@ ifneq ($(TESTS),)
 	COV_ARGS :=
 else
 	TEST_ARGS := -n auto
-	COV_ARGS := --cov-append # --fail-under 100
+	COV_ARGS := # --fail-under 100
 endif
 
 .PHONY: all
@@ -56,15 +57,15 @@ $(VENV)/pyvenv.cfg: pyproject.toml
 .PHONY: lint
 lint: $(VENV)/pyvenv.cfg
 	. $(VENV_BIN)/activate && \
-		black --check $(ALL_PY_SRCS) && \
-		pylint $(ALL_PY_SRCS)
+		black --check . && \
+		pylint $(PY_MODULE) $(TEST_MODULE) 
 		# ruff $(ALL_PY_SRCS) && \
 		# mypy $(PY_MODULE) && 
 
 .PHONY: reformat
 reformat:
 	. $(VENV_BIN)/activate && \
-		black $(PY_MODULE) 
+		black .
 
 .PHONY: test tests
 test tests: $(VENV)/pyvenv.cfg

tests/conftest.py

@@ -2,26 +2,15 @@
 from filelock import FileLock
 from solc_select import solc_select
 
-@pytest.fixture(scope="session")
-def solc_versions_installed():
-    """List of solc versions available in the test environment."""
-    return []
-
-@pytest.fixture(scope="session", autouse=True)
-def register_solc_versions_installed(solc_versions_installed):
-    solc_versions_installed.extend(solc_select.installed_versions())
 
 @pytest.fixture(scope="session")
-def use_solc_version(request, solc_versions_installed):
-    def _use_solc_version(version):
-        print(version)
-        if version not in solc_versions_installed:
-            print("Installing solc version", version)
-            solc_select.install_artifacts([version])
-        artifact_path = solc_select.artifact_path(version)
-        lock = FileLock(artifact_path)
-        try:
-            yield artifact_path
-        finally:
-            lock.release()
-    return _use_solc_version
+def solc_binary_path():
+    def inner(version):
+        lock = FileLock(f"{version}.lock", timeout=60)
+        with lock:
+            if not solc_select.artifact_path(version).exists():
+                print("Installing solc version", version)
+                solc_select.install_artifacts([version])
+        return solc_select.artifact_path(version)
+
+    return inner

tests/e2e/compilation/test_resolution.py

@@ -3,7 +3,6 @@
 
 from crytic_compile import CryticCompile
 from crytic_compile.platform.solc_standard_json import SolcStandardJson
-from solc_select import solc_select
 
 from slither import Slither
 
@@ -24,8 +23,8 @@ def test_node_modules() -> None:
     _run_all_detectors(slither)
 
 
-def test_contract_name_collision(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.8.0"))
+def test_contract_name_collision(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.0")
     standard_json = SolcStandardJson()
     standard_json.add_source_file(
         Path(TEST_DATA_DIR, "test_contract_name_collisions", "a.sol").as_posix()
@@ -40,7 +39,7 @@ def test_contract_name_collision(use_solc_version) -> None:
     _run_all_detectors(slither)
 
 
-def test_cycle(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.8.0"))
+def test_cycle(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.0")
     slither = Slither(Path(TEST_DATA_DIR, "test_cyclic_import", "a.sol").as_posix(), solc=solc_path)
     _run_all_detectors(slither)

tests/tools/read-storage/test_read_storage.py

@@ -90,8 +90,8 @@ def deploy_contract(w3, ganache, contract_bin, contract_abi) -> Contract:
 
 # pylint: disable=too-many-locals
 @pytest.mark.usefixtures("web3", "ganache")
-def test_read_storage(web3, ganache, use_solc_version) -> None:
-    solc_path = next(use_solc_version(version="0.8.10"))
+def test_read_storage(web3, ganache, solc_binary_path) -> None:
+    solc_path = solc_binary_path(version="0.8.10")
 
     assert web3.is_connected()
     bin_path = Path(TEST_DATA_DIR, "StorageLayout.bin").as_posix()

tests/unit/core/test_arithmetic.py

@@ -1,5 +1,4 @@
 from pathlib import Path
-from solc_select import solc_select
 
 from slither import Slither
 from slither.utils.arithmetic import unchecked_arithemtic_usage
@@ -8,8 +7,8 @@
 TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "arithmetic_usage"
 
 
-def test_arithmetic_usage(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.8.15"))
+def test_arithmetic_usage(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.15")
     slither = Slither(Path(TEST_DATA_DIR, "test.sol").as_posix(), solc=solc_path)
 
     assert {

tests/unit/core/test_code_comments.py

@@ -8,8 +8,8 @@
 CUSTOM_COMMENTS_TEST_DATA_DIR = Path(TEST_DATA_DIR, "custom_comments")
 
 
-def test_upgradeable_comments(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.8.10"))
+def test_upgradeable_comments(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.10")
     slither = Slither(Path(CUSTOM_COMMENTS_TEST_DATA_DIR, "upgrade.sol").as_posix(), solc=solc_path)
     compilation_unit = slither.compilation_units[0]
     proxy = compilation_unit.get_contract_from_name("Proxy")[0]
@@ -27,11 +27,13 @@ def test_upgradeable_comments(use_solc_version) -> None:
     assert v1.upgradeable_version == "version_1"
 
 
-def test_contract_comments(use_solc_version) -> None:
+def test_contract_comments(solc_binary_path) -> None:
     comments = " @title Test Contract\n @dev Test comment"
 
-    solc_path = next(use_solc_version("0.8.10"))
-    slither = Slither(Path(CUSTOM_COMMENTS_TEST_DATA_DIR, "contract_comment.sol").as_posix(), solc=solc_path)
+    solc_path = solc_binary_path("0.8.10")
+    slither = Slither(
+        Path(CUSTOM_COMMENTS_TEST_DATA_DIR, "contract_comment.sol").as_posix(), solc=solc_path
+    )
     compilation_unit = slither.compilation_units[0]
     contract = compilation_unit.get_contract_from_name("A")[0]
 
@@ -40,8 +42,10 @@ def test_contract_comments(use_solc_version) -> None:
     # Old solc versions have a different parsing of comments
     # the initial space (after *) is also not kept on every line
     comments = "@title Test Contract\n@dev Test comment"
-    solc_path = next(use_solc_version("0.5.16"))
-    slither = Slither(Path(CUSTOM_COMMENTS_TEST_DATA_DIR, "contract_comment.sol").as_posix(), solc=solc_path)
+    solc_path = solc_binary_path("0.5.16")
+    slither = Slither(
+        Path(CUSTOM_COMMENTS_TEST_DATA_DIR, "contract_comment.sol").as_posix(), solc=solc_path
+    )
     compilation_unit = slither.compilation_units[0]
     contract = compilation_unit.get_contract_from_name("A")[0]
 

tests/unit/core/test_constant_folding.py

@@ -6,15 +6,17 @@
 CONSTANT_FOLDING_TEST_ROOT = Path(TEST_DATA_DIR, "constant_folding")
 
 
-def test_constant_folding_unary(use_solc_version):
-    solc_path = next(use_solc_version("0.8.0"))
+def test_constant_folding_unary(solc_binary_path):
+    solc_path = solc_binary_path("0.8.0")
     file = Path(CONSTANT_FOLDING_TEST_ROOT, "constant_folding_unary.sol").as_posix()
     Slither(file, solc=solc_path)
 
 
-def test_constant_folding_rational(use_solc_version):
-    solc_path = next(use_solc_version("0.8.0"))
-    s = Slither(Path(CONSTANT_FOLDING_TEST_ROOT, "constant_folding_rational.sol").as_posix(), solc=solc_path)
+def test_constant_folding_rational(solc_binary_path):
+    solc_path = solc_binary_path("0.8.0")
+    s = Slither(
+        Path(CONSTANT_FOLDING_TEST_ROOT, "constant_folding_rational.sol").as_posix(), solc=solc_path
+    )
     contract = s.get_contract_from_name("C")[0]
 
     variable_a = contract.get_state_variable_from_name("a")
@@ -52,9 +54,11 @@ def test_constant_folding_rational(use_solc_version):
     assert str(ConstantFolding(variable_g.expression, "int64").result()) == "-7"
 
 
-def test_constant_folding_binary_expressions(use_solc_version):
-    solc_path = next(use_solc_version("0.8.0"))
-    sl = Slither(Path(CONSTANT_FOLDING_TEST_ROOT, "constant_folding_binop.sol").as_posix(), solc=solc_path)
+def test_constant_folding_binary_expressions(solc_binary_path):
+    solc_path = solc_binary_path("0.8.0")
+    sl = Slither(
+        Path(CONSTANT_FOLDING_TEST_ROOT, "constant_folding_binop.sol").as_posix(), solc=solc_path
+    )
     contract = sl.get_contract_from_name("BinOp")[0]
 
     variable_a = contract.get_state_variable_from_name("a")

tests/unit/core/test_contract_declaration.py

@@ -1,6 +1,5 @@
 from pathlib import Path
 
-from solc_select import solc_select
 
 from slither import Slither
 from slither.core.variables.state_variable import StateVariable
@@ -9,26 +8,30 @@
 CONTRACT_DECL_TEST_ROOT = Path(TEST_DATA_DIR, "contract_declaration")
 
 
-def test_abstract_contract(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.8.0"))
+def test_abstract_contract(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.0")
     slither = Slither(Path(CONTRACT_DECL_TEST_ROOT, "abstract.sol").as_posix(), solc=solc_path)
     assert not slither.contracts[0].is_fully_implemented
 
-    solc_path = next(use_solc_version("0.5.0"))
-    slither = Slither(Path(CONTRACT_DECL_TEST_ROOT, "implicit_abstract.sol").as_posix(), solc=solc_path)
+    solc_path = solc_binary_path("0.5.0")
+    slither = Slither(
+        Path(CONTRACT_DECL_TEST_ROOT, "implicit_abstract.sol").as_posix(), solc=solc_path
+    )
     assert not slither.contracts[0].is_fully_implemented
 
     slither = Slither(
         Path(CONTRACT_DECL_TEST_ROOT, "implicit_abstract.sol").as_posix(),
         solc_force_legacy_json=True,
-        solc=solc_path
+        solc=solc_path,
     )
     assert not slither.contracts[0].is_fully_implemented
 
 
-def test_private_variable(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.8.15"))
-    slither = Slither(Path(CONTRACT_DECL_TEST_ROOT, "private_variable.sol").as_posix(), solc=solc_path)
+def test_private_variable(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.15")
+    slither = Slither(
+        Path(CONTRACT_DECL_TEST_ROOT, "private_variable.sol").as_posix(), solc=solc_path
+    )
     contract_c = slither.get_contract_from_name("C")[0]
     f = contract_c.functions[0]
     var_read = f.variables_read[0]

tests/unit/core/test_function_declaration.py

@@ -5,7 +5,6 @@
 and that these objects behave correctly.
 """
 from pathlib import Path
-from solc_select import solc_select
 
 from slither import Slither
 from slither.core.declarations.function import FunctionType
@@ -15,9 +14,9 @@
 FUNC_DELC_TEST_ROOT = Path(TEST_DATA_DIR, "function_declaration")
 
 
-def test_functions(use_solc_version):
+def test_functions(solc_binary_path):
     # pylint: disable=too-many-statements
-    solc_path = next(use_solc_version("0.6.12"))
+    solc_path = solc_binary_path("0.6.12")
     file = Path(FUNC_DELC_TEST_ROOT, "test_function.sol").as_posix()
     slither = Slither(file, solc=solc_path)
     functions = slither.get_contract_from_name("TestFunction")[0].available_functions_as_dict()
@@ -248,8 +247,8 @@ def test_functions(use_solc_version):
     assert f.return_type[0] == ElementaryType("bool")
 
 
-def test_function_can_send_eth(use_solc_version):
-    solc_path = next(use_solc_version("0.6.12"))
+def test_function_can_send_eth(solc_binary_path):
+    solc_path = solc_binary_path("0.6.12")
     file = Path(FUNC_DELC_TEST_ROOT, "test_function.sol").as_posix()
     slither = Slither(file, solc=solc_path)
     compilation_unit = slither.compilation_units[0]
@@ -273,8 +272,8 @@ def test_function_can_send_eth(use_solc_version):
     assert functions["highlevel_call_via_external()"].can_send_eth() is False
 
 
-def test_reentrant(use_solc_version):
-    solc_path = next(use_solc_version("0.8.10"))
+def test_reentrant(solc_binary_path):
+    solc_path = solc_binary_path("0.8.10")
     file = Path(FUNC_DELC_TEST_ROOT, "test_function_reentrant.sol").as_posix()
     slither = Slither(file, solc=solc_path)
     compilation_unit = slither.compilation_units[0]
@@ -290,8 +289,8 @@ def test_reentrant(use_solc_version):
     assert functions["internal_and_reentrant()"].is_reentrant
 
 
-def test_public_variable(use_solc_version) -> None:
-    solc_path = next(use_solc_version("0.6.12"))
+def test_public_variable(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.6.12")
     file = Path(FUNC_DELC_TEST_ROOT, "test_function.sol").as_posix()
     slither = Slither(file, solc=solc_path)
     contracts = slither.get_contract_from_name("TestFunction")

tests/unit/core/test_source_mapping.py

@@ -8,8 +8,8 @@
 SRC_MAPPING_TEST_ROOT = Path(TEST_DATA_DIR, "src_mapping")
 
 
-def test_source_mapping(use_solc_version):
-    solc_path = next(use_solc_version("0.6.12"))
+def test_source_mapping(solc_binary_path):
+    solc_path = solc_binary_path("0.6.12")
     file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix()
     slither = Slither(file, solc=solc_path)
 
@@ -78,11 +78,11 @@ def _sort_references_lines(refs: list) -> list:
     return sorted([ref.lines[0] for ref in refs])
 
 
-def test_references_user_defined_aliases(use_solc_version):
+def test_references_user_defined_aliases(solc_binary_path):
     """
     Tests if references are filled correctly for user defined aliases (declared using "type [...] is [...]" statement).
     """
-    solc_path = next(use_solc_version("0.8.16"))
+    solc_path = solc_binary_path("0.8.16")
     file = Path(SRC_MAPPING_TEST_ROOT, "ReferencesUserDefinedAliases.sol").as_posix()
     slither = Slither(file, solc=solc_path)
 
@@ -101,11 +101,11 @@ def test_references_user_defined_aliases(use_solc_version):
     assert lines == [13, 16]
 
 
-def test_references_user_defined_types_when_casting(use_solc_version):
+def test_references_user_defined_types_when_casting(solc_binary_path):
     """
     Tests if references are filled correctly for user defined types in case of casting.
     """
-    solc_path = next(use_solc_version("0.8.16"))
+    solc_path = solc_binary_path("0.8.16")
     file = Path(SRC_MAPPING_TEST_ROOT, "ReferencesUserDefinedTypesCasting.sol").as_posix()
     slither = Slither(file, solc=solc_path)
 

tests/unit/core/test_storage_layout.py

@@ -1,16 +1,15 @@
 import json
 from pathlib import Path
 from subprocess import PIPE, Popen
-from solc_select import solc_select
 from slither import Slither
 
 TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data"
 STORAGE_TEST_ROOT = Path(TEST_DATA_DIR, "storage_layout")
 
 
-def test_storage_layout(use_solc_version):
+def test_storage_layout(solc_binary_path):
     # the storage layout has not yet changed between solidity versions so we will test with one version of the compiler
-    solc_path = next(use_solc_version("0.8.10"))
+    solc_path = solc_binary_path("0.8.10")
     test_item = Path(STORAGE_TEST_ROOT, "storage_layout-0.8.10.sol").as_posix()
 
     sl = Slither(test_item, disallow_partial=True, solc=solc_path)
@@ -35,4 +34,4 @@ def test_storage_layout(use_solc_version):
                     except KeyError as e:
                         print(f"not found {e} ")
         process.communicate()
-        assert process.returncode == 0
+        assert process.returncode == 0