Make hassfest import detection better (#29932)

* Make hassfest import detection better

* Fix tests

homeassistant/components/filter/manifest.json

@@ -3,8 +3,6 @@
   "name": "Filter",
   "documentation": "https://www.home-assistant.io/integrations/filter",
   "requirements": [],
-  "dependencies": [],
+  "dependencies": ["history"],
-  "codeowners": [
+  "codeowners": ["@dgomes"]
-    "@dgomes"
-  ]
 }

homeassistant/components/history/__init__.py

@@ -8,7 +8,7 @@ import time
 from sqlalchemy import and_, func
 import voluptuous as vol
 
-from homeassistant.components import recorder, script
+from homeassistant.components import recorder
 from homeassistant.components.http import HomeAssistantView
 from homeassistant.components.recorder.models import States
 from homeassistant.components.recorder.util import execute, session_scope
@@ -430,4 +430,4 @@ def _is_significant(state):
     Will only test for things that are not filtered out in SQL.
     """
     # scripts that are not cancellable will never change state
-    return state.domain != "script" or state.attributes.get(script.ATTR_CAN_CANCEL)
+    return state.domain != "script" or state.attributes.get("can_cancel")

script/hassfest/dependencies.py

@@ -1,6 +1,5 @@
 """Validate dependencies."""
-import pathlib
+import ast
-import re
 from typing import Dict, Set
 
 from homeassistant.requirements import DISCOVERY_INTEGRATIONS
@@ -8,31 +7,80 @@ from homeassistant.requirements import DISCOVERY_INTEGRATIONS
 from .model import Integration
 
 
-def grep_dir(path: pathlib.Path, glob_pattern: str, search_pattern: str) -> Set[str]:
+class ImportCollector(ast.NodeVisitor):
-    """Recursively go through a dir and it's children and find the regex."""
+    """Collect all integrations referenced."""
-    pattern = re.compile(search_pattern)
-    found = set()
 
-    for fil in path.glob(glob_pattern):
+    def __init__(self, integration: Integration):
-        if not fil.is_file():
+        """Initialize the import collector."""
-            continue
+        self.integration = integration
+        self.referenced: Set[str] = set()
 
-        for match in pattern.finditer(fil.read_text()):
+    def maybe_add_reference(self, reference_domain: str):
-            integration = match.groups()[1]
+        """Add a reference."""
+        if (
+            # If it's importing something from itself
+            reference_domain == self.integration.path.name
+            # Platform file
+            or (self.integration.path / f"{reference_domain}.py").exists()
+            # Platform dir
+            or (self.integration.path / reference_domain).exists()
+        ):
+            return
 
-            if (
+        self.referenced.add(reference_domain)
-                # If it's importing something from itself
-                integration == path.name
-                # Platform file
-                or (path / f"{integration}.py").exists()
-                # Dir for platform
-                or (path / integration).exists()
-            ):
-                continue
 
-            found.add(match.groups()[1])
+    def visit_ImportFrom(self, node):
+        """Visit ImportFrom node."""
+        if node.module is None:
+            return
 
-    return found
+        if node.module.startswith("homeassistant.components."):
+            # from homeassistant.components.alexa.smart_home import EVENT_ALEXA_SMART_HOME
+            # from homeassistant.components.logbook import bla
+            self.maybe_add_reference(node.module.split(".")[2])
+
+        elif node.module == "homeassistant.components":
+            # from homeassistant.components import sun
+            for name_node in node.names:
+                self.maybe_add_reference(name_node.name)
+
+    def visit_Import(self, node):
+        """Visit Import node."""
+        # import homeassistant.components.hue as hue
+        for name_node in node.names:
+            if name_node.name.startswith("homeassistant.components."):
+                self.maybe_add_reference(name_node.name.split(".")[2])
+
+    def visit_Attribute(self, node):
+        """Visit Attribute node."""
+        # hass.components.hue.async_create()
+        # Name(id=hass)
+        #   .Attribute(attr=hue)
+        #   .Attribute(attr=async_create)
+
+        # self.hass.components.hue.async_create()
+        # Name(id=self)
+        #   .Attribute(attr=hass)
+        #   .Attribute(attr=hue)
+        #   .Attribute(attr=async_create)
+        if (
+            isinstance(node.value, ast.Attribute)
+            and node.value.attr == "components"
+            and (
+                (
+                    isinstance(node.value.value, ast.Name)
+                    and node.value.value.id == "hass"
+                )
+                or (
+                    isinstance(node.value.value, ast.Attribute)
+                    and node.value.value.attr == "hass"
+                )
+            )
+        ):
+            self.maybe_add_reference(node.attr)
+        else:
+            # Have it visit other kids
+            self.generic_visit(node)
 
 
 ALLOWED_USED_COMPONENTS = {
@@ -87,12 +135,20 @@ IGNORE_VIOLATIONS = [
 def validate_dependencies(integration: Integration):
     """Validate all dependencies."""
     # Find usage of hass.components
-    referenced = grep_dir(
+    collector = ImportCollector(integration)
-        integration.path, "**/*.py", r"(hass|homeassistant)\.components\.(\w+)"
+
+    for fil in integration.path.glob("**/*.py"):
+        if not fil.is_file():
+            continue
+
+        collector.visit(ast.parse(fil.read_text()))
+
+    referenced = (
+        collector.referenced
+        - ALLOWED_USED_COMPONENTS
+        - set(integration.manifest["dependencies"])
+        - set(integration.manifest.get("after_dependencies", []))
     )
-    referenced -= ALLOWED_USED_COMPONENTS
-    referenced -= set(integration.manifest["dependencies"])
-    referenced -= set(integration.manifest.get("after_dependencies", []))
 
     # Discovery requirements are ok if referenced in manifest
     for check_domain, to_check in DISCOVERY_INTEGRATIONS.items():

tests/components/filter/test_sensor.py

@@ -28,6 +28,7 @@ class TestFilterSensor(unittest.TestCase):
     def setup_method(self, method):
         """Set up things to be run when tests are started."""
         self.hass = get_test_home_assistant()
+        self.hass.config.components.add("history")
         raw_values = [20, 19, 18, 21, 22, 0]
         self.values = []
 

tests/hassfest/__init__.py

@@ -0,0 +1 @@
+"""Tests for hassfest."""

tests/hassfest/test_dependencies.py

@@ -0,0 +1,120 @@
+"""Tests for hassfest dependency finder."""
+import ast
+
+import pytest
+from script.hassfest.dependencies import ImportCollector
+
+
+@pytest.fixture
+def mock_collector():
+    """Fixture with import collector that adds all referenced nodes."""
+    collector = ImportCollector(None)
+    collector.maybe_add_reference = collector.referenced.add
+    return collector
+
+
+def test_child_import(mock_collector):
+    """Test detecting a child_import reference."""
+    mock_collector.visit(
+        ast.parse(
+            """
+from homeassistant.components import child_import
+"""
+        )
+    )
+    assert mock_collector.referenced == {"child_import"}
+
+
+def test_subimport(mock_collector):
+    """Test detecting a subimport reference."""
+    mock_collector.visit(
+        ast.parse(
+            """
+from homeassistant.components.subimport.smart_home import EVENT_ALEXA_SMART_HOME
+"""
+        )
+    )
+    assert mock_collector.referenced == {"subimport"}
+
+
+def test_child_import_field(mock_collector):
+    """Test detecting a child_import_field reference."""
+    mock_collector.visit(
+        ast.parse(
+            """
+from homeassistant.components.child_import_field import bla
+"""
+        )
+    )
+    assert mock_collector.referenced == {"child_import_field"}
+
+
+def test_renamed_absolute(mock_collector):
+    """Test detecting a renamed_absolute reference."""
+    mock_collector.visit(
+        ast.parse(
+            """
+import homeassistant.components.renamed_absolute as hue
+"""
+        )
+    )
+    assert mock_collector.referenced == {"renamed_absolute"}
+
+
+def test_hass_components_var(mock_collector):
+    """Test detecting a hass_components_var reference."""
+    mock_collector.visit(
+        ast.parse(
+            """
+def bla(hass):
+    hass.components.hass_components_var.async_do_something()
+"""
+        )
+    )
+    assert mock_collector.referenced == {"hass_components_var"}
+
+
+def test_hass_components_class(mock_collector):
+    """Test detecting a hass_components_class reference."""
+    mock_collector.visit(
+        ast.parse(
+            """
+class Hello:
+    def something(self):
+        self.hass.components.hass_components_class.async_yo()
+"""
+        )
+    )
+    assert mock_collector.referenced == {"hass_components_class"}
+
+
+def test_all_imports(mock_collector):
+    """Test all imports together."""
+    mock_collector.visit(
+        ast.parse(
+            """
+from homeassistant.components import child_import
+
+from homeassistant.components.subimport.smart_home import EVENT_ALEXA_SMART_HOME
+
+from homeassistant.components.child_import_field import bla
+
+import homeassistant.components.renamed_absolute as hue
+
+def bla(hass):
+    hass.components.hass_components_var.async_do_something()
+
+class Hello:
+    def something(self):
+        self.hass.components.hass_components_class.async_yo()
+"""
+        )
+    )
+    assert mock_collector.referenced == {
+        "child_import",
+        "subimport",
+        "child_import_field",
+        "renamed_absolute",
+        "hass_components_var",
+        "hass_components_class",
+    }