Validate component usage (#23037)

* Update manifest validator

* Update circle

* Update text

* Typo

* fix link to codeowners

* Merge CODEOWNERS into hassfest

* Annotate errors with fixable

* Convert error to warning

* Lint

* Make abs path

* Python 3.5...

* Typo

* Fix tests

.circleci/config.yml

@@ -91,12 +91,6 @@ jobs:
             . venv/bin/activate
             flake8
 
-      - run:
-          name: validate CODEOWNERS
-          command: |
-            . venv/bin/activate
-            python script/manifest/codeowners.py validate
-
       - run:
           name: run static type check
           command: |
@@ -110,7 +104,7 @@ jobs:
           name: validate manifests
           command: |
             . venv/bin/activate
-            python script/manifest/validate.py
+            python -m script.hassfest validate
 
       - run:
           name: run gen_requirements_all

homeassistant/components/cloud/manifest.json

@@ -6,7 +6,8 @@
     "hass-nabucasa==0.11"
   ],
   "dependencies": [
-    "http"
+    "http",
+    "webhook"
   ],
   "codeowners": [
     "@home-assistant/core"

homeassistant/components/demo/manifest.json

@@ -5,7 +5,9 @@
   "requirements": [],
   "dependencies": [
     "conversation",
-    "zone"
+    "zone",
+    "group",
+    "configurator"
   ],
   "codeowners": [
     "@home-assistant/core"

homeassistant/components/hassio/manifest.json

@@ -4,7 +4,8 @@
   "documentation": "https://www.home-assistant.io/hassio",
   "requirements": [],
   "dependencies": [
-    "http"
+    "http",
+    "panel_custom"
   ],
   "codeowners": [
     "@home-assistant/hass-io"

homeassistant/components/map/manifest.json

@@ -3,6 +3,8 @@
   "name": "Map",
   "documentation": "https://www.home-assistant.io/components/map",
   "requirements": [],
-  "dependencies": [],
+  "dependencies": [
+    "frontend"
+  ],
   "codeowners": []
 }

homeassistant/components/panel_custom/__init__.py

@@ -124,9 +124,12 @@ async def async_register_panel(
 
 async def async_setup(hass, config):
     """Initialize custom panel."""
+    if DOMAIN not in config:
+        return True
+
     success = False
 
-    for panel in config.get(DOMAIN):
+    for panel in config[DOMAIN]:
         name = panel[CONF_COMPONENT_NAME]
 
         kwargs = {

homeassistant/components/websocket_api/__init__.py

@@ -43,5 +43,5 @@ def async_register_command(hass, command_or_handler, handler=None,
 async def async_setup(hass, config):
     """Initialize the websocket API."""
     hass.http.register_view(http.WebsocketAPIView)
-    commands.async_register_commands(hass)
+    commands.async_register_commands(hass, async_register_command)
     return True

homeassistant/components/websocket_api/commands.py

@@ -14,16 +14,15 @@ from . import const, decorators, messages
 
 
 @callback
-def async_register_commands(hass):
+def async_register_commands(hass, async_reg):
     """Register commands."""
-    async_reg = hass.components.websocket_api.async_register_command
+    async_reg(hass, handle_subscribe_events)
-    async_reg(handle_subscribe_events)
+    async_reg(hass, handle_unsubscribe_events)
-    async_reg(handle_unsubscribe_events)
+    async_reg(hass, handle_call_service)
-    async_reg(handle_call_service)
+    async_reg(hass, handle_get_states)
-    async_reg(handle_get_states)
+    async_reg(hass, handle_get_services)
-    async_reg(handle_get_services)
+    async_reg(hass, handle_get_config)
-    async_reg(handle_get_config)
+    async_reg(hass, handle_ping)
-    async_reg(handle_ping)
 
 
 def pong_message(iden):

script/gen_requirements_all.py

@@ -3,11 +3,12 @@
 import fnmatch
 import importlib
 import os
+import pathlib
 import pkgutil
 import re
 import sys
 
-from script.manifest.requirements import gather_requirements_from_manifests
+from script.hassfest.model import Integration
 
 COMMENT_REQUIREMENTS = (
     'Adafruit-DHT',
@@ -219,7 +220,7 @@ def gather_modules():
 
     errors = []
 
-    gather_requirements_from_manifests(process_requirements, errors, reqs)
+    gather_requirements_from_manifests(errors, reqs)
     gather_requirements_from_modules(errors, reqs)
 
     for key in reqs:
@@ -235,6 +236,28 @@ def gather_modules():
     return reqs
 
 
+def gather_requirements_from_manifests(errors, reqs):
+    """Gather all of the requirements from manifests."""
+    integrations = Integration.load_dir(pathlib.Path(
+        'homeassistant/components'
+    ))
+    for domain in sorted(integrations):
+        integration = integrations[domain]
+
+        if not integration.manifest:
+            errors.append(
+                'The manifest for component {} is invalid.'.format(domain)
+            )
+            continue
+
+        process_requirements(
+            errors,
+            integration.manifest['requirements'],
+            'homeassistant.components.{}'.format(domain),
+            reqs
+        )
+
+
 def gather_requirements_from_modules(errors, reqs):
     """Collect the requirements from the modules directly."""
     for package in sorted(

script/hassfest/__init__.py

@@ -0,0 +1 @@
+"""Manifest validator."""

script/hassfest/__main__.py

@@ -0,0 +1,84 @@
+"""Validate manifests."""
+import pathlib
+import sys
+
+from .model import Integration, Config
+from . import dependencies, manifest, codeowners
+
+PLUGINS = [
+    manifest,
+    dependencies,
+    codeowners,
+]
+
+
+def get_config() -> Config:
+    """Return config."""
+    if not pathlib.Path('requirements_all.txt').is_file():
+        raise RuntimeError("Run from project root")
+
+    return Config(
+        root=pathlib.Path('.').absolute(),
+        action='validate' if sys.argv[-1] == 'validate' else 'generate',
+    )
+
+
+def main():
+    """Validate manifests."""
+    try:
+        config = get_config()
+    except RuntimeError as err:
+        print(err)
+        return 1
+
+    integrations = Integration.load_dir(
+        pathlib.Path('homeassistant/components')
+    )
+    manifest.validate(integrations, config)
+    dependencies.validate(integrations, config)
+    codeowners.validate(integrations, config)
+
+    # When we generate, all errors that are fixable will be ignored,
+    # as generating them will be fixed.
+    if config.action == 'generate':
+        general_errors = [err for err in config.errors if not err.fixable]
+        invalid_itg = [
+            itg for itg in integrations.values()
+            if any(
+                not error.fixable for error in itg.errors
+            )
+        ]
+    else:
+        # action == validate
+        general_errors = config.errors
+        invalid_itg = [itg for itg in integrations.values() if itg.errors]
+
+    print("Integrations:", len(integrations))
+    print("Invalid integrations:", len(invalid_itg))
+
+    if not invalid_itg and not general_errors:
+        codeowners.generate(integrations, config)
+        return 0
+
+    print()
+    if config.action == 'generate':
+        print("Found errors. Generating files canceled.")
+        print()
+
+    if general_errors:
+        print("General errors:")
+        for error in general_errors:
+            print("*", error)
+        print()
+
+    for integration in sorted(invalid_itg, key=lambda itg: itg.domain):
+        print("Integration {}:".format(integration.domain))
+        for error in integration.errors:
+            print("*", error)
+        print()
+
+    return 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())

script/hassfest/codeowners.py

@@ -0,0 +1,85 @@
+"""Generate CODEOWNERS."""
+from typing import Dict
+
+from .model import Integration, Config
+
+BASE = """
+# This file is generated by script/manifest/codeowners.py
+# People marked here will be automatically requested for a review
+# when the code that they own is touched.
+# https://github.com/blog/2392-introducing-code-owners
+
+# Home Assistant Core
+setup.py @home-assistant/core
+homeassistant/*.py @home-assistant/core
+homeassistant/helpers/* @home-assistant/core
+homeassistant/util/* @home-assistant/core
+
+# Virtualization
+Dockerfile @home-assistant/docker
+virtualization/Docker/* @home-assistant/docker
+
+# Other code
+homeassistant/scripts/check_config.py @kellerza
+
+# Integrations
+""".strip()
+
+INDIVIDUAL_FILES = """
+# Individual files
+homeassistant/components/group/cover @cdce8p
+homeassistant/components/demo/weather @fabaff
+"""
+
+
+def generate_and_validate(integrations: Dict[str, Integration]):
+    """Generate CODEOWNERS."""
+    parts = [BASE]
+
+    for domain in sorted(integrations):
+        integration = integrations[domain]
+
+        if not integration.manifest:
+            continue
+
+        codeowners = integration.manifest['codeowners']
+
+        if not codeowners:
+            continue
+
+        for owner in codeowners:
+            if not owner.startswith('@'):
+                integration.add_error(
+                    'codeowners',
+                    'Code owners need to be valid GitHub handles.',
+                )
+
+        parts.append("homeassistant/components/{}/* {}".format(
+            domain, ' '.join(codeowners)))
+
+    parts.append('\n' + INDIVIDUAL_FILES.strip())
+
+    return '\n'.join(parts)
+
+
+def validate(integrations: Dict[str, Integration], config: Config):
+    """Validate CODEOWNERS."""
+    codeowners_path = config.root / 'CODEOWNERS'
+    config.cache['codeowners'] = content = generate_and_validate(integrations)
+
+    with open(str(codeowners_path), 'r') as fp:
+        if fp.read().strip() != content:
+            config.add_error(
+                "codeowners",
+                "File CODEOWNERS is not up to date. "
+                "Run python3 -m script.hassfest",
+                fixable=True
+            )
+        return
+
+
+def generate(integrations: Dict[str, Integration], config: Config):
+    """Generate CODEOWNERS."""
+    codeowners_path = config.root / 'CODEOWNERS'
+    with open(str(codeowners_path), 'w') as fp:
+        fp.write(config.cache['codeowners'] + '\n')

script/hassfest/dependencies.py

@@ -0,0 +1,65 @@
+"""Validate dependencies."""
+import pathlib
+import re
+from typing import Set, Dict
+
+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()
+
+    for fil in path.glob(glob_pattern):
+        if not fil.is_file():
+            continue
+
+        for match in pattern.finditer(fil.read_text()):
+            found.add(match.groups()[0])
+
+    return found
+
+
+# These components will always be set up
+ALLOWED_USED_COMPONENTS = {
+    'persistent_notification',
+}
+
+
+def validate_dependencies(integration: Integration):
+    """Validate all dependencies."""
+    # Find usage of hass.components
+    referenced = grep_dir(integration.path, "**/*.py",
+                          r"hass\.components\.(\w+)")
+    referenced -= ALLOWED_USED_COMPONENTS
+    referenced -= set(integration.manifest['dependencies'])
+
+    if referenced:
+        for domain in sorted(referenced):
+            print("Warning: {} references integration {} but it's not a "
+                  "dependency".format(integration.domain, domain))
+            # Not enforced yet.
+            # integration.add_error(
+            #     'dependencies',
+            #     "Using component {} but it's not a dependency".format(domain)
+            # )
+
+
+def validate(integrations: Dict[str, Integration], config):
+    """Handle dependencies for integrations."""
+    # check for non-existing dependencies
+    for integration in integrations.values():
+        if not integration.manifest:
+            continue
+
+        validate_dependencies(integration)
+
+        # check that all referenced dependencies exist
+        for dep in integration.manifest['dependencies']:
+            if dep not in integrations:
+                integration.add_error(
+                    'dependencies',
+                    "Dependency {} does not exist"
+                )

script/hassfest/manifest.py

@@ -0,0 +1,40 @@
+"""Manifest validation."""
+from typing import Dict
+
+import voluptuous as vol
+from voluptuous.humanize import humanize_error
+
+from .model import Integration
+
+
+MANIFEST_SCHEMA = vol.Schema({
+    vol.Required('domain'): str,
+    vol.Required('name'): str,
+    vol.Required('documentation'): str,
+    vol.Required('requirements'): [str],
+    vol.Required('dependencies'): [str],
+    vol.Required('codeowners'): [str],
+})
+
+
+def validate_manifest(integration: Integration):
+    """Validate manifest."""
+    try:
+        MANIFEST_SCHEMA(integration.manifest)
+    except vol.Invalid as err:
+        integration.add_error(
+            'manifest',
+            "Invalid manifest: {}".format(
+                humanize_error(integration.manifest, err)))
+        integration.manifest = None
+        return
+
+    if integration.manifest['domain'] != integration.path.name:
+        integration.add_error('manifest', 'Domain does not match dir name')
+
+
+def validate(integrations: Dict[str, Integration], config):
+    """Handle all integrations manifests."""
+    for integration in integrations.values():
+        if integration.manifest:
+            validate_manifest(integration)

script/hassfest/model.py

@@ -0,0 +1,91 @@
+"""Models for manifest validator."""
+import json
+from typing import List, Dict, Any
+import pathlib
+
+import attr
+
+
+@attr.s
+class Error:
+    """Error validating an integration."""
+
+    plugin = attr.ib(type=str)
+    error = attr.ib(type=str)
+    fixable = attr.ib(type=bool, default=False)
+
+    def __str__(self) -> str:
+        """Represent error as string."""
+        return "[{}] {}".format(self.plugin.upper(), self.error)
+
+
+@attr.s
+class Config:
+    """Config for the run."""
+
+    root = attr.ib(type=pathlib.Path)
+    action = attr.ib(type=str)
+    errors = attr.ib(type=List[Error], factory=list)
+    cache = attr.ib(type=Dict[str, Any], factory=dict)
+
+    def add_error(self, *args, **kwargs):
+        """Add an error."""
+        self.errors.append(Error(*args, **kwargs))
+
+
+@attr.s
+class Integration:
+    """Represent an integration in our validator."""
+
+    @classmethod
+    def load_dir(cls, path: pathlib.Path):
+        """Load all integrations in a directory."""
+        assert path.is_dir()
+        integrations = {}
+        for fil in path.iterdir():
+            if fil.is_file() or fil.name == '__pycache__':
+                continue
+
+            integration = cls(fil)
+            integration.load_manifest()
+            integrations[integration.domain] = integration
+
+        return integrations
+
+    path = attr.ib(type=pathlib.Path)
+    manifest = attr.ib(type=dict, default=None)
+    errors = attr.ib(type=List[Error], factory=list)
+
+    @property
+    def domain(self) -> str:
+        """Integration domain."""
+        return self.path.name
+
+    @property
+    def manifest_path(self) -> pathlib.Path:
+        """Integration manifest path."""
+        return self.path / 'manifest.json'
+
+    def add_error(self, *args, **kwargs):
+        """Add an error."""
+        self.errors.append(Error(*args, **kwargs))
+
+    def load_manifest(self) -> None:
+        """Load manifest."""
+        if not self.manifest_path.is_file():
+            self.add_error(
+                'model',
+                "Manifest file {} not found".format(self.manifest_path)
+            )
+            return
+
+        try:
+            manifest = json.loads(self.manifest_path.read_text())
+        except ValueError as err:
+            self.add_error(
+                'model',
+                "Manifest contains invalid JSON: {}".format(err)
+            )
+            return
+
+        self.manifest = manifest

script/manifest/codeowners.py

@@ -1,74 +0,0 @@
-#!/usr/bin/env python3
-"""Generate CODEOWNERS."""
-import os
-import sys
-
-from manifest_helper import iter_manifests
-
-BASE = """
-# This file is generated by script/manifest/codeowners.py
-# People marked here will be automatically requested for a review
-# when the code that they own is touched.
-# https://github.com/blog/2392-introducing-code-owners
-
-# Home Assistant Core
-setup.py @home-assistant/core
-homeassistant/*.py @home-assistant/core
-homeassistant/helpers/* @home-assistant/core
-homeassistant/util/* @home-assistant/core
-
-# Virtualization
-Dockerfile @home-assistant/docker
-virtualization/Docker/* @home-assistant/docker
-
-# Other code
-homeassistant/scripts/check_config.py @kellerza
-
-# Integrations
-"""
-
-INDIVIDUAL_FILES = """
-# Individual files
-homeassistant/components/group/cover @cdce8p
-homeassistant/components/demo/weather @fabaff
-"""
-
-
-def generate():
-    """Generate CODEOWNERS."""
-    parts = [BASE.strip()]
-
-    for manifest in iter_manifests():
-        if not manifest['codeowners']:
-            continue
-
-        parts.append("homeassistant/components/{}/* {}".format(
-            manifest['domain'], ' '.join(manifest['codeowners'])))
-
-    parts.append('\n' + INDIVIDUAL_FILES.strip())
-
-    return '\n'.join(parts)
-
-
-def main(validate):
-    """Runner for CODEOWNERS gen."""
-    if not os.path.isfile('requirements_all.txt'):
-        print('Run this from HA root dir')
-        return 1
-
-    content = generate()
-
-    if validate:
-        with open('CODEOWNERS', 'r') as fp:
-            if fp.read().strip() != content:
-                print("CODEOWNERS is not up to date. "
-                      "Run python script/manifest/codeowners.py")
-                return 1
-            return 0
-
-    with open('CODEOWNERS', 'w') as fp:
-        fp.write(content + '\n')
-
-
-if __name__ == '__main__':
-    sys.exit(main(sys.argv[-1] == 'validate'))

script/manifest/requirements.py

@@ -1,22 +0,0 @@
-"""Helpers to gather requirements from manifests."""
-from .manifest_helper import iter_manifests
-
-
-def gather_requirements_from_manifests(process_requirements, errors, reqs):
-    """Gather all of the requirements from manifests."""
-    for manifest in iter_manifests():
-        assert manifest['domain']
-
-        if manifest.get('requirements') is None:
-            errors.append(
-                'The manifest for component {} is invalid. Please run'
-                'script/manifest/validate.py'.format(manifest['domain'])
-            )
-            continue
-
-        process_requirements(
-            errors,
-            manifest['requirements'],
-            'homeassistant.components.{}'.format(manifest['domain']),
-            reqs
-        )

script/manifest/validate.py

@@ -1,100 +0,0 @@
-#!/usr/bin/env python3
-"""Validate all integrations have manifests and that they are valid."""
-import json
-import pathlib
-import sys
-
-import voluptuous as vol
-from voluptuous.humanize import humanize_error
-
-
-MANIFEST_SCHEMA = vol.Schema({
-    vol.Required('domain'): str,
-    vol.Required('name'): str,
-    vol.Required('documentation'): str,
-    vol.Required('requirements'): [str],
-    vol.Required('dependencies'): [str],
-    vol.Required('codeowners'): [str],
-})
-
-
-COMPONENTS_PATH = pathlib.Path('homeassistant/components')
-
-
-def validate_dependency(path, dependency, loaded, loading):
-    """Validate dependency is exist and no circular dependency."""
-    dep_path = path.parent / dependency
-    return validate_integration(dep_path, loaded, loading)
-
-
-def validate_integration(path, loaded, loading):
-    """Validate that an integrations has a valid manifest."""
-    errors = []
-    path = pathlib.Path(path)
-
-    manifest_path = path / 'manifest.json'
-
-    if not manifest_path.is_file():
-        errors.append('Manifest file {} not found'.format(manifest_path))
-        return errors  # Fatal error
-
-    try:
-        manifest = json.loads(manifest_path.read_text())
-    except ValueError as err:
-        errors.append("Manifest contains invalid JSON: {}".format(err))
-        return errors  # Fatal error
-
-    try:
-        MANIFEST_SCHEMA(manifest)
-    except vol.Invalid as err:
-        errors.append(humanize_error(manifest, err))
-
-    if manifest['domain'] != path.name:
-        errors.append('Domain does not match dir name')
-
-    for dep in manifest['dependencies']:
-        if dep in loaded:
-            continue
-        if dep in loading:
-            errors.append("Found circular dependency {} in {}".format(
-                dep, path
-            ))
-            continue
-        loading.add(dep)
-
-        errors.extend(validate_dependency(path, dep, loaded, loading))
-
-    loaded.add(path.name)
-    return errors
-
-
-def validate_all():
-    """Validate all integrations."""
-    invalid = []
-
-    for fil in COMPONENTS_PATH.iterdir():
-        if fil.is_file() or fil.name == '__pycache__':
-            continue
-
-        errors = validate_integration(fil, set(), set())
-
-        if errors:
-            invalid.append((fil, errors))
-
-    if not invalid:
-        return 0
-
-    print("Found invalid manifests")
-    print()
-
-    for integration, errors in invalid:
-        print(integration)
-        for error in errors:
-            print("*", error)
-        print()
-
-    return 1
-
-
-if __name__ == '__main__':
-    sys.exit(validate_all())