Check for forbidden files in dependencies with hassfest (home-assistant#150772)

Author: cdce8p

script/hassfest/requirements.py

@@ -11,7 +11,7 @@
 import re
 import subprocess
 import sys
-from typing import Any
+from typing import Any, TypedDict
 
 from awesomeversion import AwesomeVersion, AwesomeVersionStrategy
 from tqdm import tqdm
@@ -295,6 +295,9 @@
     },
 }
 
+FORBIDDEN_FILE_NAMES: set[str] = {
+    "py.typed",  # should be placed inside a package
+}
 FORBIDDEN_PACKAGE_NAMES: set[str] = {
     "doc",
     "docs",
@@ -364,7 +367,15 @@
     },
 }
 
-_packages_checked_files_cache: dict[str, set[str]] = {}
+
+class _PackageFilesCheckResult(TypedDict):
+    """Data structure to store results of package files check."""
+
+    top_level: set[str]
+    file_names: set[str]
+
+
+_packages_checked_files_cache: dict[str, _PackageFilesCheckResult] = {}
 
 
 def validate(integrations: dict[str, Integration], config: Config) -> None:
@@ -733,24 +744,33 @@ def check_dependency_files(
     pkg: str,
     package_exceptions: Collection[str],
 ) -> bool:
-    """Check dependency files for forbidden package names."""
+    """Check dependency files for forbidden files and forbidden package names."""
     if (results := _packages_checked_files_cache.get(pkg)) is None:
         top_level: set[str] = set()
+        file_names: set[str] = set()
         for file in files(pkg) or ():
-            top = file.parts[0].lower()
-            if top.endswith((".dist-info", ".py")):
-                continue
-            top_level.add(top)
-        results = FORBIDDEN_PACKAGE_NAMES & top_level
+            if not (top := file.parts[0].lower()).endswith((".dist-info", ".py")):
+                top_level.add(top)
+            if (name := str(file)).lower() in FORBIDDEN_FILE_NAMES:
+                file_names.add(name)
+        results = _PackageFilesCheckResult(
+            top_level=FORBIDDEN_PACKAGE_NAMES & top_level,
+            file_names=file_names,
+        )
         _packages_checked_files_cache[pkg] = results
-    if not results:
+    if not (results["top_level"] or results["file_names"]):
         return True
 
-    for dir_name in results:
+    for dir_name in results["top_level"]:
         integration.add_warning_or_error(
             pkg in package_exceptions,
             "requirements",
-            f"Package {pkg} has a forbidden top level directory {dir_name} in {package}",
+            f"Package {pkg} has a forbidden top level directory '{dir_name}' in {package}",
+        )
+    for file_name in results["file_names"]:
+        integration.add_error(
+            "requirements",
+            f"Package {pkg} has a forbidden file '{file_name}' in {package}",
         )
     return False
 

tests/hassfest/test_requirements.py

@@ -170,8 +170,8 @@ def test_dependency_version_range_prepare_update(
 
 
 @pytest.mark.usefixtures("mock_forbidden_package_names")
-def test_check_dependency_files(integration: Integration) -> None:
-    """Test dependency files check for forbidden package names is working correctly."""
+def test_check_dependency_package_names(integration: Integration) -> None:
+    """Test dependency package names check for forbidden package names is working correctly."""
     package = "homeassistant"
     pkg = "my_package"
 
@@ -190,17 +190,15 @@ def test_check_dependency_files(integration: Integration) -> None:
     ):
         assert not _packages_checked_files_cache
         assert check_dependency_files(integration, package, pkg, ()) is False
-        assert _packages_checked_files_cache[pkg] == {"tests", "test"}
+        assert _packages_checked_files_cache[pkg]["top_level"] == {"tests", "test"}
         assert len(integration.errors) == 2
         assert (
-            f"Package {pkg} has a forbidden top level directory tests in {package}"
-            in x.error
-            for x in integration.errors
+            f"Package {pkg} has a forbidden top level directory 'tests' in {package}"
+            in [x.error for x in integration.errors]
         )
         assert (
-            f"Package {pkg} has a forbidden top level directory test in {package}"
-            in x.error
-            for x in integration.errors
+            f"Package {pkg} has a forbidden top level directory 'test' in {package}"
+            in [x.error for x in integration.errors]
         )
         integration.errors.clear()
 
@@ -227,13 +225,12 @@ def test_check_dependency_files(integration: Integration) -> None:
             check_dependency_files(integration, package, pkg, package_exceptions={pkg})
             is False
         )
-        assert _packages_checked_files_cache[pkg] == {"tests"}
+        assert _packages_checked_files_cache[pkg]["top_level"] == {"tests"}
         assert len(integration.errors) == 0
         assert len(integration.warnings) == 1
         assert (
-            f"Package {pkg} has a forbidden top level directory tests in {package}"
-            in x.error
-            for x in integration.warnings
+            f"Package {pkg} has a forbidden top level directory 'tests' in {package}"
+            in [x.error for x in integration.warnings]
         )
         integration.warnings.clear()
 
@@ -260,7 +257,62 @@ def test_check_dependency_files(integration: Integration) -> None:
     ):
         assert not _packages_checked_files_cache
         assert check_dependency_files(integration, package, pkg, ()) is True
-        assert _packages_checked_files_cache[pkg] == set()
+        assert _packages_checked_files_cache[pkg]["top_level"] == set()
+        assert len(integration.errors) == 0
+
+        # Repeated call should use cache
+        assert check_dependency_files(integration, package, pkg, ()) is True
+        assert mock_files.call_count == 1
+        assert len(integration.errors) == 0
+
+
+def test_check_dependency_file_names(integration: Integration) -> None:
+    """Test dependency file name check for forbidden files is working correctly."""
+    package = "homeassistant"
+    pkg = "my_package"
+
+    # Forbidden file: 'py.typed' at top level
+    pkg_files = [
+        PackagePath("py.typed"),
+        PackagePath("my_package.py"),
+        PackagePath("my_package-1.0.0.dist-info/METADATA"),
+    ]
+    with (
+        patch(
+            "script.hassfest.requirements.files", return_value=pkg_files
+        ) as mock_files,
+        patch.dict(_packages_checked_files_cache, {}, clear=True),
+    ):
+        assert not _packages_checked_files_cache
+        assert check_dependency_files(integration, package, pkg, ()) is False
+        assert _packages_checked_files_cache[pkg]["file_names"] == {"py.typed"}
+        assert len(integration.errors) == 1
+        assert f"Package {pkg} has a forbidden file 'py.typed' in {package}" in [
+            x.error for x in integration.errors
+        ]
+        integration.errors.clear()
+
+        # Repeated call should use cache
+        assert check_dependency_files(integration, package, pkg, ()) is False
+        assert mock_files.call_count == 1
+        assert len(integration.errors) == 1
+        integration.errors.clear()
+
+    # All good
+    pkg_files = [
+        PackagePath("my_package/__init__.py"),
+        PackagePath("my_package/py.typed"),
+        PackagePath("my_package.dist-info/METADATA"),
+    ]
+    with (
+        patch(
+            "script.hassfest.requirements.files", return_value=pkg_files
+        ) as mock_files,
+        patch.dict(_packages_checked_files_cache, {}, clear=True),
+    ):
+        assert not _packages_checked_files_cache
+        assert check_dependency_files(integration, package, pkg, ()) is True
+        assert _packages_checked_files_cache[pkg]["file_names"] == set()
         assert len(integration.errors) == 0
 
         # Repeated call should use cache