From 367521e369839e6504989603b1282c2ba31dad49 Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Tue, 25 Jan 2022 18:21:59 +0100 Subject: [PATCH] Adjust pylint plugin to enforce device_tracker type hints (#64903) * Adjust pylint plugin to enforce device_tracker type hints * Use a constant for the type hint matchers * Add tests * Add x_of_y match * Adjust bluetooth_tracker * Adjust mysensors * Adjust tile Co-authored-by: epenet --- .../bluetooth_tracker/device_tracker.py | 6 +- .../components/mysensors/device_tracker.py | 13 +-- .../components/tile/device_tracker.py | 5 +- pylint/plugins/hass_enforce_type_hints.py | 96 +++++++++++++++++-- tests/pylint/__init__.py | 1 + tests/pylint/test_enforce_type_hints.py | 41 ++++++++ 6 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 tests/pylint/__init__.py create mode 100644 tests/pylint/test_enforce_type_hints.py diff --git a/homeassistant/components/bluetooth_tracker/device_tracker.py b/homeassistant/components/bluetooth_tracker/device_tracker.py index 484a684b4d3..b62333c0489 100644 --- a/homeassistant/components/bluetooth_tracker/device_tracker.py +++ b/homeassistant/components/bluetooth_tracker/device_tracker.py @@ -5,7 +5,7 @@ import asyncio from collections.abc import Awaitable, Callable from datetime import datetime, timedelta import logging -from typing import Any, Final +from typing import Final import bluetooth # pylint: disable=import-error from bt_proximity import BluetoothRSSI @@ -30,7 +30,7 @@ from homeassistant.const import CONF_DEVICE_ID from homeassistant.core import HomeAssistant, ServiceCall import homeassistant.helpers.config_validation as cv from homeassistant.helpers.event import async_track_time_interval -from homeassistant.helpers.typing import ConfigType +from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from .const import ( BT_PREFIX, @@ -131,7 +131,7 @@ async def async_setup_scanner( hass: HomeAssistant, config: ConfigType, async_see: Callable[..., Awaitable[None]], - discovery_info: dict[str, Any] | None = None, + discovery_info: DiscoveryInfoType | None = None, ) -> bool: """Set up the Bluetooth Scanner.""" device_id: int = config[CONF_DEVICE_ID] diff --git a/homeassistant/components/mysensors/device_tracker.py b/homeassistant/components/mysensors/device_tracker.py index a6d976f753c..d5332ab70bf 100644 --- a/homeassistant/components/mysensors/device_tracker.py +++ b/homeassistant/components/mysensors/device_tracker.py @@ -1,13 +1,14 @@ """Support for tracking MySensors devices.""" from __future__ import annotations -from collections.abc import Callable -from typing import Any +from collections.abc import Awaitable, Callable +from typing import Any, cast from homeassistant.components import mysensors from homeassistant.const import Platform from homeassistant.core import HomeAssistant from homeassistant.helpers.dispatcher import async_dispatcher_connect +from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from homeassistant.util import slugify from .const import ATTR_GATEWAY_ID, DevId, DiscoveryInfo, GatewayId @@ -16,9 +17,9 @@ from .helpers import on_unload async def async_setup_scanner( hass: HomeAssistant, - config: dict[str, Any], - async_see: Callable, - discovery_info: DiscoveryInfo | None = None, + config: ConfigType, + async_see: Callable[..., Awaitable[None]], + discovery_info: DiscoveryInfoType | None = None, ) -> bool: """Set up the MySensors device scanner.""" if not discovery_info: @@ -27,7 +28,7 @@ async def async_setup_scanner( new_devices = mysensors.setup_mysensors_platform( hass, Platform.DEVICE_TRACKER, - discovery_info, + cast(DiscoveryInfo, discovery_info), MySensorsDeviceScanner, device_args=(hass, async_see), ) diff --git a/homeassistant/components/tile/device_tracker.py b/homeassistant/components/tile/device_tracker.py index 5416c63ccee..df5f035a849 100644 --- a/homeassistant/components/tile/device_tracker.py +++ b/homeassistant/components/tile/device_tracker.py @@ -3,7 +3,6 @@ from __future__ import annotations from collections.abc import Awaitable, Callable import logging -from typing import Any from pytile.tile import Tile @@ -13,7 +12,7 @@ from homeassistant.config_entries import SOURCE_IMPORT, ConfigEntry from homeassistant.const import CONF_PASSWORD, CONF_USERNAME from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.entity_platform import AddEntitiesCallback -from homeassistant.helpers.typing import ConfigType +from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from homeassistant.helpers.update_coordinator import ( CoordinatorEntity, DataUpdateCoordinator, @@ -55,7 +54,7 @@ async def async_setup_scanner( hass: HomeAssistant, config: ConfigType, async_see: Callable[..., Awaitable[None]], - discovery_info: dict[str, Any] | None = None, + discovery_info: DiscoveryInfoType | None = None, ) -> bool: """Detect a legacy configuration and import it.""" hass.async_create_task( diff --git a/pylint/plugins/hass_enforce_type_hints.py b/pylint/plugins/hass_enforce_type_hints.py index c9707b833b5..288fcc560c3 100644 --- a/pylint/plugins/hass_enforce_type_hints.py +++ b/pylint/plugins/hass_enforce_type_hints.py @@ -19,15 +19,29 @@ class TypeHintMatch: module_filter: re.Pattern function_name: str arg_types: dict[int, str] - return_type: str | None + return_type: list[str] | str | None +_TYPE_HINT_MATCHERS: dict[str, re.Pattern] = { + # a_or_b matches items such as "DiscoveryInfoType | None" + "a_or_b": re.compile(r"^(\w+) \| (\w+)$"), + # x_of_y matches items such as "Awaitable[None]" + "x_of_y": re.compile(r"^(\w+)\[(.*?]*)\]$"), + # x_of_y_comma_z matches items such as "Callable[..., Awaitable[None]]" + "x_of_y_comma_z": re.compile(r"^(\w+)\[(.*?]*), (.*?]*)\]$"), +} + _MODULE_FILTERS: dict[str, re.Pattern] = { # init matches only in the package root (__init__.py) "init": re.compile(r"^homeassistant\.components\.\w+$"), + # any_platform matches any platform in the package root ({platform}.py) "any_platform": re.compile( f"^homeassistant\\.components\\.\\w+\\.({'|'.join([platform.value for platform in Platform])})$" ), + # device_tracker matches only in the package root (device_tracker.py) + "device_tracker": re.compile( + f"^homeassistant\\.components\\.\\w+\\.({Platform.DEVICE_TRACKER.value})$" + ), } _METHOD_MATCH: list[TypeHintMatch] = [ @@ -117,21 +131,89 @@ _METHOD_MATCH: list[TypeHintMatch] = [ }, return_type=None, ), + TypeHintMatch( + module_filter=_MODULE_FILTERS["device_tracker"], + function_name="setup_scanner", + arg_types={ + 0: "HomeAssistant", + 1: "ConfigType", + 2: "Callable[..., None]", + 3: "DiscoveryInfoType | None", + }, + return_type="bool", + ), + TypeHintMatch( + module_filter=_MODULE_FILTERS["device_tracker"], + function_name="async_setup_scanner", + arg_types={ + 0: "HomeAssistant", + 1: "ConfigType", + 2: "Callable[..., Awaitable[None]]", + 3: "DiscoveryInfoType | None", + }, + return_type="bool", + ), + TypeHintMatch( + module_filter=_MODULE_FILTERS["device_tracker"], + function_name="get_scanner", + arg_types={ + 0: "HomeAssistant", + 1: "ConfigType", + }, + return_type=["DeviceScanner", "DeviceScanner | None"], + ), + TypeHintMatch( + module_filter=_MODULE_FILTERS["device_tracker"], + function_name="async_get_scanner", + arg_types={ + 0: "HomeAssistant", + 1: "ConfigType", + }, + return_type=["DeviceScanner", "DeviceScanner | None"], + ), ] -def _is_valid_type(expected_type: str | None, node: astroid.NodeNG) -> bool: +def _is_valid_type(expected_type: list[str] | str | None, node: astroid.NodeNG) -> bool: """Check the argument node against the expected type.""" + if isinstance(expected_type, list): + for expected_type_item in expected_type: + if _is_valid_type(expected_type_item, node): + return True + return False + # Const occurs when the type is None - if expected_type is None: + if expected_type is None or expected_type == "None": return isinstance(node, astroid.Const) and node.value is None - # Special case for DiscoveryInfoType | None" - if expected_type == "DiscoveryInfoType | None": + # Const occurs when the type is an Ellipsis + if expected_type == "...": + return isinstance(node, astroid.Const) and node.value == Ellipsis + + # Special case for `xxx | yyy` + if match := _TYPE_HINT_MATCHERS["a_or_b"].match(expected_type): return ( isinstance(node, astroid.BinOp) - and _is_valid_type("DiscoveryInfoType", node.left) - and _is_valid_type(None, node.right) + and _is_valid_type(match.group(1), node.left) + and _is_valid_type(match.group(2), node.right) + ) + + # Special case for xxx[yyy, zzz]` + if match := _TYPE_HINT_MATCHERS["x_of_y_comma_z"].match(expected_type): + return ( + isinstance(node, astroid.Subscript) + and _is_valid_type(match.group(1), node.value) + and isinstance(node.slice, astroid.Tuple) + and _is_valid_type(match.group(2), node.slice.elts[0]) + and _is_valid_type(match.group(3), node.slice.elts[1]) + ) + + # Special case for xxx[yyy]` + if match := _TYPE_HINT_MATCHERS["x_of_y"].match(expected_type): + return ( + isinstance(node, astroid.Subscript) + and _is_valid_type(match.group(1), node.value) + and _is_valid_type(match.group(2), node.slice) ) # Name occurs when a namespace is not used, eg. "HomeAssistant" diff --git a/tests/pylint/__init__.py b/tests/pylint/__init__.py new file mode 100644 index 00000000000..d6bdd6675f0 --- /dev/null +++ b/tests/pylint/__init__.py @@ -0,0 +1 @@ +"""Tests for pylint.""" diff --git a/tests/pylint/test_enforce_type_hints.py b/tests/pylint/test_enforce_type_hints.py new file mode 100644 index 00000000000..fe60ed022f4 --- /dev/null +++ b/tests/pylint/test_enforce_type_hints.py @@ -0,0 +1,41 @@ +"""Tests for pylint hass_enforce_type_hints plugin.""" +# pylint:disable=protected-access + +from importlib.machinery import SourceFileLoader +import re + +import pytest + +loader = SourceFileLoader( + "hass_enforce_type_hints", "pylint/plugins/hass_enforce_type_hints.py" +) +hass_enforce_type_hints = loader.load_module(None) +_TYPE_HINT_MATCHERS: dict[str, re.Pattern] = hass_enforce_type_hints._TYPE_HINT_MATCHERS + + +@pytest.mark.parametrize( + ("string", "expected_x", "expected_y", "expected_z"), + [ + ("Callable[..., None]", "Callable", "...", "None"), + ("Callable[..., Awaitable[None]]", "Callable", "...", "Awaitable[None]"), + ], +) +def test_regex_x_of_y_comma_z(string, expected_x, expected_y, expected_z): + """Test x_of_y_comma_z regexes.""" + assert (match := _TYPE_HINT_MATCHERS["x_of_y_comma_z"].match(string)) + assert match.group(0) == string + assert match.group(1) == expected_x + assert match.group(2) == expected_y + assert match.group(3) == expected_z + + +@pytest.mark.parametrize( + ("string", "expected_a", "expected_b"), + [("DiscoveryInfoType | None", "DiscoveryInfoType", "None")], +) +def test_regex_a_or_b(string, expected_a, expected_b): + """Test a_or_b regexes.""" + assert (match := _TYPE_HINT_MATCHERS["a_or_b"].match(string)) + assert match.group(0) == string + assert match.group(1) == expected_a + assert match.group(2) == expected_b