Merge pull request #2332 from crytic/fix/inheritance-resolving-duplicate-names

fix: support inheritance resolution when contract name is reused

slither/solc_parsing/slither_compilation_unit_solc.py

@@ -434,17 +434,28 @@ Please rename it, this name is reserved for Slither's internals"""
 
         def resolve_remapping_and_renaming(contract_parser: ContractSolc, want: str) -> Contract:
             contract_name = contract_parser.remapping[want]
-            if contract_name in contract_parser.underlying_contract.file_scope.renaming:
-                contract_name = contract_parser.underlying_contract.file_scope.renaming[
-                    contract_name
-                ]
+            target = None
+            # For contracts that are imported and aliased e.g. 'import {A as B} from "./C.sol"',
+            # we look through the imports's (`Import`) renaming to find the original contract name
+            # and then look up the original contract in the import path's scope (`FileScope`).
+            for import_ in contract_parser.underlying_contract.file_scope.imports:
+                if contract_name in import_.renaming:
+                    target = self.compilation_unit.get_scope(
+                        import_.filename
+                    ).get_contract_from_name(import_.renaming[contract_name])
+
+            # Fallback to the current file scope if the contract is not found in the import path's scope.
+            # It is assumed that it isn't possible to defined a contract with the same name as "aliased" names.
+            if target is None:
                 target = contract_parser.underlying_contract.file_scope.get_contract_from_name(
                     contract_name
                 )
+
             if target == contract_parser.underlying_contract:
                 raise InheritanceResolutionError(
                     "Could not resolve contract inheritance. This is likely caused by an import renaming that collides with existing names (see https://github.com/crytic/slither/issues/1758)."
-                    f"\n Try changing `contract {target}` ({target.source_mapping}) to a unique name."
+                    f"\n Try changing `contract {target}` ({target.source_mapping}) to a unique name as a workaround."
+                    "\n Please share the source code that caused this error here: https://github.com/crytic/slither/issues/"
                 )
             assert target, f"Contract {contract_name} not found"
             return target

tests/unit/core/test_error_messages.py

@@ -1,18 +0,0 @@
-from pathlib import Path
-import pytest
-
-
-from slither import Slither
-from slither.solc_parsing.slither_compilation_unit_solc import InheritanceResolutionError
-
-TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data"
-INHERITANCE_ERROR_ROOT = Path(TEST_DATA_DIR, "inheritance_resolution_error")
-
-
-def test_inheritance_resolution_error(solc_binary_path) -> None:
-    with pytest.raises(InheritanceResolutionError):
-        solc_path = solc_binary_path("0.8.0")
-        Slither(
-            Path(INHERITANCE_ERROR_ROOT, "contract_with_duplicate_names.sol").as_posix(),
-            solc=solc_path,
-        )

tests/unit/core/test_inheritance.py

@@ -4,14 +4,13 @@ from crytic_compile import CryticCompile
 from crytic_compile.platform.solc_standard_json import SolcStandardJson
 from slither import Slither
 
-TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "inheritance_with_renaming"
+TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "inheritance_resolution"
 
 # https://github.com/crytic/slither/issues/2304
 def test_inheritance_with_renaming(solc_binary_path) -> None:
-    solc_path = solc_binary_path("0.8.15")
+    solc_path = solc_binary_path("0.8.0")
     standard_json = SolcStandardJson()
-    for source_file in Path(TEST_DATA_DIR).rglob("*.sol"):
-        print(source_file)
+    for source_file in Path(TEST_DATA_DIR / "renaming").rglob("*.sol"):
         standard_json.add_source_file(Path(source_file).as_posix())
     compilation = CryticCompile(standard_json, solc=solc_path)
     slither = Slither(compilation)
@@ -34,3 +33,11 @@ def test_inheritance_with_renaming(solc_binary_path) -> None:
     assert len(b.immediate_inheritance) == 1
     assert b.immediate_inheritance[0] == c
     assert len(b.explicit_base_constructor_calls) == 0
+
+
+def test_inheritance_with_duplicate_names(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.0")
+    Slither(
+        Path(TEST_DATA_DIR / "duplicate_names", "contract_with_duplicate_names.sol").as_posix(),
+        solc=solc_path,
+    )