fix: contract type collision issues from deleted files and empty file…

… issues (#1818)

src/ape/managers/compilers.py

@@ -115,14 +115,18 @@ def compile(
         cached_manifest = self.project_manager.local_project.cached_manifest
 
         # Load past compiled contracts for verifying type-collision and other things.
-        already_compiled_contracts = (
-            (cached_manifest.contract_types or {}) if cached_manifest else {}
-        )
-        already_compiled_paths = [
-            contracts_folder / x.source_id
-            for x in already_compiled_contracts.values()
-            if x.source_id
-        ]
+        already_compiled_contracts: Dict[str, ContractType] = {}
+        already_compiled_paths: List[Path] = []
+        for name, ct in ((cached_manifest.contract_types or {}) if cached_manifest else {}).items():
+            if not ct.source_id:
+                continue
+
+            _file = contracts_folder / ct.source_id
+            if not _file.is_file():
+                continue
+
+            already_compiled_contracts[name] = ct
+            already_compiled_paths.append(_file)
 
         exclusions = self.config_manager.get_config("compile").exclude
         for extension in extensions:

src/ape/managers/project/types.py

@@ -86,7 +86,9 @@ def _check_needs_compiling(self, source_path: Path) -> bool:
         # ethpm_types strips trailing white space and ensures
         # a newline at the end so content so `splitlines()` works.
         # We need to do the same here for to prevent the endless recompiling bug.
-        content = f"{source_file.read_text('utf8').rstrip()}\n"
+        text = source_file.read_text("utf8").rstrip()
+        content = f"{text}\n" if text else ""
+
         checksum = compute_checksum(content.encode("utf8"), algorithm=cached_checksum.algorithm)
         return checksum != cached_checksum.hash  # Contents changed
 
@@ -177,7 +179,11 @@ def create_manifest(
             )
         )
 
-        manifest = self.manifest if use_cache else PackageManifest()
+        if use_cache:
+            manifest = self.manifest
+        else:
+            self._contracts = None
+            manifest = PackageManifest()
 
         # Generate sources and contract types.
         project_sources = _ProjectSources(
@@ -211,7 +217,9 @@ def create_manifest(
         # Is cached.
         return self.manifest
 
-    def _compile(self, project_sources: _ProjectSources) -> Dict[str, ContractType]:
+    def _compile(
+        self, project_sources: _ProjectSources, use_cache: bool = True
+    ) -> Dict[str, ContractType]:
         def _compile_sources(proj_srcs: _ProjectSources) -> Dict[str, ContractType]:
             contracts_folder = self.contracts_folder
             srcs_to_compile = proj_srcs.sources_needing_compilation

tests/conftest.py

@@ -402,9 +402,9 @@ def zero_address():
 @pytest.fixture
 def ape_caplog(caplog):
     class ApeCaplog:
-        def __init__(self):
+        def __init__(self, caplog_level: LogLevel = LogLevel.WARNING):
             self.messages_at_start = list(caplog.messages)
-            self.set_levels()
+            self.set_levels(caplog_level=caplog_level)
 
         def __getattr__(self, name: str) -> Any:
             return getattr(caplog, name)
@@ -434,9 +434,9 @@ def head(self) -> str:
             return caplog.messages[-1] if len(caplog.messages) else ""
 
         @classmethod
-        def set_levels(cls):
+        def set_levels(cls, caplog_level: LogLevel = LogLevel.WARNING):
             logger.set_level(LogLevel.INFO)
-            caplog.set_level(LogLevel.WARNING)
+            caplog.set_level(caplog_level)
 
         def assert_last_log(self, message: str):
             assert message in self.head, self.fail_message

tests/functional/conftest.py

@@ -676,9 +676,19 @@ def mock_compile(paths, base_path=None):
             if path.suffix == mock.ext:
                 name = path.stem
                 code = HexBytes(123).hex()
-                contract_type = ContractType(
-                    contractName=name, abi=[], deploymentBytecode=code, sourceId=path.name
-                )
+                data = {
+                    "contractName": name,
+                    "abi": [],
+                    "deploymentBytecode": code,
+                    "sourceId": path.name,
+                }
+
+                # Check for mocked overrides
+                overrides = mock.overrides
+                if isinstance(overrides, dict):
+                    data = {**data, **overrides}
+
+                contract_type = ContractType.model_validate(data)
                 result.append(contract_type)
 
         return result

tests/functional/test_project.py

@@ -1,5 +1,8 @@
 import os
+import random
 import shutil
+import string
+import tempfile
 from pathlib import Path
 
 import pytest
@@ -11,6 +14,7 @@
 
 from ape import Contract
 from ape.exceptions import ProjectError
+from ape.logging import LogLevel
 from ape.managers.project import BrownieProject
 
 WITH_DEPS_PROJECT = (
@@ -119,7 +123,7 @@ def test_extract_manifest(project_with_dependency_config):
 
 
 def test_cached_manifest_when_sources_missing(
-    ape_project, manifest_with_non_existent_sources, existing_source_path, caplog
+    ape_project, manifest_with_non_existent_sources, existing_source_path, ape_caplog
 ):
     """
     Show that if a source is missing, it is OK. This happens when changing branches
@@ -143,7 +147,7 @@ def test_cached_manifest_when_sources_missing(
 
     # Show the contract type does not get added and we don't get the corrupted manifest.
     assert not any(ct.name == name for ct in manifest.contract_types.values())
-    assert not any("corrupted. Re-building" in msg for msg in caplog.messages)
+    assert not any("corrupted. Re-building" in msg for msg in ape_caplog.messages)
 
 
 def test_create_manifest_when_file_changed_with_cached_references_that_no_longer_exist(
@@ -172,6 +176,44 @@ def test_create_manifest_when_file_changed_with_cached_references_that_no_longer
     assert manifest
 
 
+def test_create_manifest_empty_files(compilers, mock_compiler, config, ape_caplog):
+    """
+    Tests again a bug where empty contracts would infinitely compile.
+    """
+
+    # Using a random name to prevent async conflicts.
+    letters = string.ascii_letters
+    name = "".join(random.choice(letters) for _ in range(10))
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        base_dir = Path(temp_dir)
+        contracts = base_dir / "contracts"
+        contracts.mkdir()
+        file_1 = contracts / f"{name}.__mock__"
+        file_1.write_text("")
+
+        with config.using_project(base_dir) as proj:
+            compilers.registered_compilers[".__mock__"] = mock_compiler
+
+            # NOTE: Set levels as close to the operation as possible
+            #  to lessen chance of caplog race conditions.
+            ape_caplog.set_levels(caplog_level=LogLevel.INFO)
+
+            # Run twice to show use_cache=False works.
+            proj.local_project.create_manifest()
+            manifest = proj.local_project.create_manifest(use_cache=False)
+
+            assert name in manifest.contract_types
+            assert f"{name}.__mock__" in manifest.sources
+
+            ape_caplog.assert_last_log(f"Compiling '{name}.__mock__'.")
+            ape_caplog.clear()
+
+            # Ensure is not double compiled!
+            proj.local_project.create_manifest()
+            assert f"Compiling '{name}.__mock__'." not in ape_caplog.head
+
+
 def test_meta(temp_config, project):
     meta_config = {
         "meta": {
@@ -470,6 +512,41 @@ def test_load_contracts(project_with_contract):
     assert contracts == project_with_contract.contracts
 
 
+def test_load_contracts_after_deleting_same_named_contract(config, compilers, mock_compiler):
+    """
+    Tests against a scenario where you:
+
+    1. Add and compile a contract
+    2. Delete that contract
+    3. Add a new contract with same name somewhere else
+
+    Test such that we are able to compile successfully and not get a misleading
+    collision error from deleted files.
+    """
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        path = Path(temp_dir)
+        contracts = path / "contracts"
+        contracts.mkdir()
+        init_contract = contracts / "foo.__mock__"
+        init_contract.write_text("LALA")
+        with config.using_project(path) as proj:
+            compilers.registered_compilers[".__mock__"] = mock_compiler
+            result = proj.load_contracts()
+            assert "foo" in result
+
+            # Delete file
+            init_contract.unlink()
+
+            # Create new contract that yields same name as deleted one.
+            new_contract = contracts / "bar.__mock__"
+            new_contract.write_text("BAZ")
+            mock_compiler.overrides = {"contractName": "foo"}
+
+            result = proj.load_contracts()
+            assert "foo" in result
+
+
 def test_add_compiler_data(project_with_dependency_config):
     # NOTE: Using different project than default to lessen
     #   chance of race-conditions from multi-process test runners.