From 38ca74b547d21f87c16ffe021adce2f489aca5bf Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Fri, 26 Aug 2022 14:27:13 +0200 Subject: [PATCH] Adjust pylint plugin for absolute/relative imports (#77219) * Adjust pylint plugin for absolute/relative imports * Adjust components * One more * Adjust mqtt * Adjust mqtt.DOMAIN import * Adjust internal import * Add tests for valid local component imports * Adjust relative path check * Fixes * Fixes --- homeassistant/components/plaato/sensor.py | 3 +- homeassistant/components/pushover/notify.py | 2 +- homeassistant/components/sabnzbd/sensor.py | 16 ++- homeassistant/components/wallbox/__init__.py | 2 +- pylint/plugins/hass_imports.py | 33 +++++- tests/pylint/conftest.py | 18 +++ tests/pylint/test_imports.py | 117 +++++++++++++++++++ 7 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 tests/pylint/test_imports.py diff --git a/homeassistant/components/plaato/sensor.py b/homeassistant/components/plaato/sensor.py index dc3039ea355..b43e18e52f6 100644 --- a/homeassistant/components/plaato/sensor.py +++ b/homeassistant/components/plaato/sensor.py @@ -6,7 +6,7 @@ from pyplaato.plaato import PlaatoKeg from homeassistant.components.sensor import SensorDeviceClass, SensorEntity from homeassistant.config_entries import ConfigEntry -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.dispatcher import ( async_dispatcher_connect, async_dispatcher_send, @@ -15,7 +15,6 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from . import ATTR_TEMP, SENSOR_UPDATE -from ...core import callback from .const import ( CONF_USE_WEBHOOK, COORDINATOR, diff --git a/homeassistant/components/pushover/notify.py b/homeassistant/components/pushover/notify.py index 16ad452fd9a..dd711c80aaf 100644 --- a/homeassistant/components/pushover/notify.py +++ b/homeassistant/components/pushover/notify.py @@ -18,11 +18,11 @@ from homeassistant.components.notify import ( from homeassistant.config_entries import SOURCE_IMPORT from homeassistant.const import CONF_API_KEY from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError import homeassistant.helpers.config_validation as cv from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType -from ...exceptions import HomeAssistantError from .const import ( ATTR_ATTACHMENT, ATTR_CALLBACK_URL, diff --git a/homeassistant/components/sabnzbd/sensor.py b/homeassistant/components/sabnzbd/sensor.py index 043a344ec7b..b2828a30969 100644 --- a/homeassistant/components/sabnzbd/sensor.py +++ b/homeassistant/components/sabnzbd/sensor.py @@ -8,15 +8,19 @@ from homeassistant.components.sensor import ( SensorEntityDescription, SensorStateClass, ) +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import ( + DATA_GIGABYTES, + DATA_MEGABYTES, + DATA_RATE_MEGABYTES_PER_SECOND, +) +from homeassistant.core import HomeAssistant +from homeassistant.helpers.device_registry import DeviceEntryType from homeassistant.helpers.dispatcher import async_dispatcher_connect +from homeassistant.helpers.entity import DeviceInfo +from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import DOMAIN, SIGNAL_SABNZBD_UPDATED -from ...config_entries import ConfigEntry -from ...const import DATA_GIGABYTES, DATA_MEGABYTES, DATA_RATE_MEGABYTES_PER_SECOND -from ...core import HomeAssistant -from ...helpers.device_registry import DeviceEntryType -from ...helpers.entity import DeviceInfo -from ...helpers.entity_platform import AddEntitiesCallback from .const import DEFAULT_NAME, KEY_API_DATA, KEY_NAME diff --git a/homeassistant/components/wallbox/__init__.py b/homeassistant/components/wallbox/__init__.py index f0392f808ae..9175f42827d 100644 --- a/homeassistant/components/wallbox/__init__.py +++ b/homeassistant/components/wallbox/__init__.py @@ -13,12 +13,12 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_PASSWORD, CONF_USERNAME, Platform from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryAuthFailed, HomeAssistantError +from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.update_coordinator import ( CoordinatorEntity, DataUpdateCoordinator, ) -from ...helpers.entity import DeviceInfo from .const import ( CHARGER_CURRENT_VERSION_KEY, CHARGER_DATA_KEY, diff --git a/pylint/plugins/hass_imports.py b/pylint/plugins/hass_imports.py index c7abe0ad6ac..62fd262eacc 100644 --- a/pylint/plugins/hass_imports.py +++ b/pylint/plugins/hass_imports.py @@ -41,15 +41,15 @@ _OBSOLETE_IMPORT: dict[str, list[ObsoleteImportMatch]] = { "homeassistant.components.automation": [ ObsoleteImportMatch( reason="replaced by TriggerActionType from helpers.trigger", - constant=re.compile(r"^AutomationActionType$") + constant=re.compile(r"^AutomationActionType$"), ), ObsoleteImportMatch( reason="replaced by TriggerData from helpers.trigger", - constant=re.compile(r"^AutomationTriggerData$") + constant=re.compile(r"^AutomationTriggerData$"), ), ObsoleteImportMatch( reason="replaced by TriggerInfo from helpers.trigger", - constant=re.compile(r"^AutomationTriggerInfo$") + constant=re.compile(r"^AutomationTriggerInfo$"), ), ], "homeassistant.components.binary_sensor": [ @@ -111,13 +111,13 @@ _OBSOLETE_IMPORT: dict[str, list[ObsoleteImportMatch]] = { "homeassistant.components.device_tracker": [ ObsoleteImportMatch( reason="replaced by SourceType enum", - constant=re.compile(r"^SOURCE_TYPE_\w+$") + constant=re.compile(r"^SOURCE_TYPE_\w+$"), ), ], "homeassistant.components.device_tracker.const": [ ObsoleteImportMatch( reason="replaced by SourceType enum", - constant=re.compile(r"^SOURCE_TYPE_\w+$") + constant=re.compile(r"^SOURCE_TYPE_\w+$"), ), ], "homeassistant.components.fan": [ @@ -277,6 +277,11 @@ class HassImportsFormatChecker(BaseChecker): # type: ignore[misc] "hass-deprecated-import", "Used when import is deprecated", ), + "W7423": ( + "Absolute import should be used", + "hass-absolute-import", + "Used when relative import should be replaced with absolute import", + ), } options = () @@ -298,9 +303,27 @@ class HassImportsFormatChecker(BaseChecker): # type: ignore[misc] if module.startswith(f"{self.current_package}."): self.add_message("hass-relative-import", node=node) + def _visit_importfrom_relative(self, current_package: str, node: nodes.ImportFrom) -> None: + """Called when a ImportFrom node is visited.""" + if node.level <= 1 or not current_package.startswith("homeassistant.components"): + return + split_package = current_package.split(".") + if not node.modname and len(split_package) == node.level + 1: + for name in node.names: + # Allow relative import to component root + if name[0] != split_package[2]: + self.add_message("hass-absolute-import", node=node) + return + return + if len(split_package) < node.level + 2: + self.add_message("hass-absolute-import", node=node) + def visit_importfrom(self, node: nodes.ImportFrom) -> None: """Called when a ImportFrom node is visited.""" + if not self.current_package: + return if node.level is not None: + self._visit_importfrom_relative(self.current_package, node) return if node.modname == self.current_package or node.modname.startswith( f"{self.current_package}." diff --git a/tests/pylint/conftest.py b/tests/pylint/conftest.py index edbcec27375..e8748434350 100644 --- a/tests/pylint/conftest.py +++ b/tests/pylint/conftest.py @@ -32,3 +32,21 @@ def type_hint_checker_fixture(hass_enforce_type_hints, linter) -> BaseChecker: type_hint_checker = hass_enforce_type_hints.HassTypeHintChecker(linter) type_hint_checker.module = "homeassistant.components.pylint_test" return type_hint_checker + + +@pytest.fixture(name="hass_imports", scope="session") +def hass_imports_fixture() -> ModuleType: + """Fixture to provide a requests mocker.""" + loader = SourceFileLoader( + "hass_imports", + str(BASE_PATH.joinpath("pylint/plugins/hass_imports.py")), + ) + return loader.load_module(None) + + +@pytest.fixture(name="imports_checker") +def imports_checker_fixture(hass_imports, linter) -> BaseChecker: + """Fixture to provide a requests mocker.""" + type_hint_checker = hass_imports.HassImportsFormatChecker(linter) + type_hint_checker.module = "homeassistant.components.pylint_test" + return type_hint_checker diff --git a/tests/pylint/test_imports.py b/tests/pylint/test_imports.py new file mode 100644 index 00000000000..6367427eea7 --- /dev/null +++ b/tests/pylint/test_imports.py @@ -0,0 +1,117 @@ +"""Tests for pylint hass_imports plugin.""" +# pylint:disable=protected-access +from __future__ import annotations + +import astroid +from pylint.checkers import BaseChecker +import pylint.testutils +from pylint.testutils.unittest_linter import UnittestLinter +import pytest + +from . import assert_adds_messages, assert_no_messages + + +@pytest.mark.parametrize( + ("module_name", "import_from", "import_what"), + [ + ( + "homeassistant.components.pylint_test.sensor", + "homeassistant.const", + "CONSTANT", + ), + ("homeassistant.components.pylint_test.sensor", ".const", "CONSTANT"), + ("homeassistant.components.pylint_test.sensor", ".", "CONSTANT"), + ("homeassistant.components.pylint_test.sensor", "..", "pylint_test"), + ( + "homeassistant.components.pylint_test.api.hub", + "homeassistant.const", + "CONSTANT", + ), + ("homeassistant.components.pylint_test.api.hub", "..const", "CONSTANT"), + ("homeassistant.components.pylint_test.api.hub", "..", "CONSTANT"), + ("homeassistant.components.pylint_test.api.hub", "...", "pylint_test"), + ], +) +def test_good_import( + linter: UnittestLinter, + imports_checker: BaseChecker, + module_name: str, + import_from: str, + import_what: str, +) -> None: + """Ensure good imports pass through ok.""" + + import_node = astroid.extract_node( + f"from {import_from} import {import_what} #@", + module_name, + ) + imports_checker.visit_module(import_node.parent) + + with assert_no_messages(linter): + imports_checker.visit_importfrom(import_node) + + +@pytest.mark.parametrize( + ("module_name", "import_from", "import_what", "error_code"), + [ + ( + "homeassistant.components.pylint_test.sensor", + "homeassistant.components.pylint_test.const", + "CONSTANT", + "hass-relative-import", + ), + ( + "homeassistant.components.pylint_test.sensor", + "..const", + "CONSTANT", + "hass-absolute-import", + ), + ( + "homeassistant.components.pylint_test.sensor", + "...const", + "CONSTANT", + "hass-absolute-import", + ), + ( + "homeassistant.components.pylint_test.api.hub", + "homeassistant.components.pylint_test.api.const", + "CONSTANT", + "hass-relative-import", + ), + ( + "homeassistant.components.pylint_test.api.hub", + "...const", + "CONSTANT", + "hass-absolute-import", + ), + ], +) +def test_bad_import( + linter: UnittestLinter, + imports_checker: BaseChecker, + module_name: str, + import_from: str, + import_what: str, + error_code: str, +) -> None: + """Ensure bad imports are rejected.""" + + import_node = astroid.extract_node( + f"from {import_from} import {import_what} #@", + module_name, + ) + imports_checker.visit_module(import_node.parent) + + with assert_adds_messages( + linter, + pylint.testutils.MessageTest( + msg_id=error_code, + node=import_node, + args=None, + line=1, + col_offset=0, + end_line=1, + end_col_offset=len(import_from) + 21, + ), + ): + imports_checker.visit_importfrom(import_node)