From 9e51a18845bde3baa873dcd37c3e1b0a8476473e Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Mon, 16 Dec 2019 08:22:20 +0100 Subject: [PATCH] Make hassfest import detection better (#29932) * Make hassfest import detection better * Fix tests --- homeassistant/components/filter/manifest.json | 6 +- homeassistant/components/history/__init__.py | 4 +- script/hassfest/dependencies.py | 110 ++++++++++++---- tests/components/filter/test_sensor.py | 1 + tests/hassfest/__init__.py | 1 + tests/hassfest/test_dependencies.py | 120 ++++++++++++++++++ 6 files changed, 209 insertions(+), 33 deletions(-) create mode 100644 tests/hassfest/__init__.py create mode 100644 tests/hassfest/test_dependencies.py diff --git a/homeassistant/components/filter/manifest.json b/homeassistant/components/filter/manifest.json index f28007ba552..f84a8b5192f 100644 --- a/homeassistant/components/filter/manifest.json +++ b/homeassistant/components/filter/manifest.json @@ -3,8 +3,6 @@ "name": "Filter", "documentation": "https://www.home-assistant.io/integrations/filter", "requirements": [], - "dependencies": [], - "codeowners": [ - "@dgomes" - ] + "dependencies": ["history"], + "codeowners": ["@dgomes"] } diff --git a/homeassistant/components/history/__init__.py b/homeassistant/components/history/__init__.py index 133151c7f73..7fcbf519bf3 100644 --- a/homeassistant/components/history/__init__.py +++ b/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") diff --git a/script/hassfest/dependencies.py b/script/hassfest/dependencies.py index b6f277438b4..6ba228b5bc7 100644 --- a/script/hassfest/dependencies.py +++ b/script/hassfest/dependencies.py @@ -1,6 +1,5 @@ """Validate dependencies.""" -import pathlib -import re +import ast 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]: - """Recursively go through a dir and it's children and find the regex.""" - pattern = re.compile(search_pattern) - found = set() +class ImportCollector(ast.NodeVisitor): + """Collect all integrations referenced.""" - for fil in path.glob(glob_pattern): - if not fil.is_file(): - continue + def __init__(self, integration: Integration): + """Initialize the import collector.""" + self.integration = integration + self.referenced: Set[str] = set() - for match in pattern.finditer(fil.read_text()): - integration = match.groups()[1] + def maybe_add_reference(self, reference_domain: str): + """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 ( - # 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 + self.referenced.add(reference_domain) - 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( - integration.path, "**/*.py", r"(hass|homeassistant)\.components\.(\w+)" + collector = ImportCollector(integration) + + 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(): diff --git a/tests/components/filter/test_sensor.py b/tests/components/filter/test_sensor.py index 17e5bd8fd5d..a5f23b464dd 100644 --- a/tests/components/filter/test_sensor.py +++ b/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 = [] diff --git a/tests/hassfest/__init__.py b/tests/hassfest/__init__.py new file mode 100644 index 00000000000..1ec5a22a567 --- /dev/null +++ b/tests/hassfest/__init__.py @@ -0,0 +1 @@ +"""Tests for hassfest.""" diff --git a/tests/hassfest/test_dependencies.py b/tests/hassfest/test_dependencies.py new file mode 100644 index 00000000000..0f07c45317e --- /dev/null +++ b/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", + }