Add pylint checker for invalid MDI icon references (#171824)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Franck Nijhof
@@ -15,6 +15,7 @@ Dockerfile.dev linguist-language=Dockerfile
|
||||
# Generated files
|
||||
CODEOWNERS linguist-generated=true
|
||||
homeassistant/generated/*.py linguist-generated=true
|
||||
pylint/plugins/pylint_home_assistant/generated/*.py linguist-generated=true
|
||||
machine/* linguist-generated=true
|
||||
mypy.ini linguist-generated=true
|
||||
requirements.txt linguist-generated=true
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
"""Checker for invalid MDI icon references.
|
||||
|
||||
Validates that ``mdi:`` icon references in integration code and
|
||||
``icons.json`` files refer to icons that actually exist in the
|
||||
Material Design Icons set.
|
||||
|
||||
- ``E7409``: MDI icon reference not found in Python code
|
||||
- ``E7410``: MDI icon reference not found in icons.json
|
||||
"""
|
||||
|
||||
import re
|
||||
|
||||
from astroid import nodes
|
||||
from pylint.checkers import BaseChecker
|
||||
from pylint.lint import PyLinter
|
||||
|
||||
from pylint_home_assistant.generated.mdi_icons import MDI_ICONS
|
||||
from pylint_home_assistant.helpers.icons import collect_mdi_icons, load_icons
|
||||
from pylint_home_assistant.helpers.module_info import parse_module
|
||||
|
||||
# Matches strings that look like intentional icon name attempts
|
||||
# (letters, digits, hyphens, underscores). Rejects format templates
|
||||
# (%s, {}, {name}), empty names, and other dynamic patterns.
|
||||
_LOOKS_LIKE_ICON_NAME = re.compile(r"^[a-zA-Z][a-zA-Z0-9_-]*[a-zA-Z0-9]$")
|
||||
|
||||
|
||||
class MdiIconsChecker(BaseChecker):
|
||||
"""Checker for invalid MDI icon references."""
|
||||
|
||||
name = "home_assistant_mdi_icons"
|
||||
priority = -1
|
||||
msgs = {
|
||||
"E7409": (
|
||||
"MDI icon '%s' does not exist in the Material Design Icons set",
|
||||
"home-assistant-mdi-icon-not-found",
|
||||
"Used when an integration references an MDI icon in Python "
|
||||
"code that does not exist. Check the icon name at "
|
||||
"https://pictogrammers.com/library/mdi/",
|
||||
),
|
||||
"E7410": (
|
||||
"MDI icon '%s' in icons.json does not exist in the "
|
||||
"Material Design Icons set",
|
||||
"home-assistant-mdi-icon-json-not-found",
|
||||
"Used when an integration's icons.json references an MDI "
|
||||
"icon that does not exist. Check the icon name at "
|
||||
"https://pictogrammers.com/library/mdi/",
|
||||
),
|
||||
}
|
||||
options = ()
|
||||
|
||||
_in_integration: bool
|
||||
_checked_icons_json: set[str]
|
||||
|
||||
def open(self) -> None:
|
||||
"""Initialize per-run state."""
|
||||
self._checked_icons_json = set()
|
||||
|
||||
def visit_module(self, node: nodes.Module) -> None:
|
||||
"""Check icons.json and track integration context."""
|
||||
parsed = parse_module(node.name)
|
||||
self._in_integration = parsed is not None
|
||||
if parsed is None:
|
||||
return
|
||||
|
||||
# Only check icons.json once per integration
|
||||
if parsed.domain in self._checked_icons_json:
|
||||
return
|
||||
self._checked_icons_json.add(parsed.domain)
|
||||
|
||||
icons_data = load_icons(node)
|
||||
if icons_data is None:
|
||||
return
|
||||
|
||||
mdi_refs = collect_mdi_icons(icons_data)
|
||||
for icon_ref in sorted(mdi_refs):
|
||||
icon_name = icon_ref[4:] # Strip "mdi:" prefix
|
||||
if icon_name not in MDI_ICONS:
|
||||
self.add_message(
|
||||
"home-assistant-mdi-icon-json-not-found",
|
||||
node=node,
|
||||
args=(icon_ref,),
|
||||
)
|
||||
|
||||
def visit_const(self, node: nodes.Const) -> None:
|
||||
"""Check string constants for invalid MDI icon references."""
|
||||
if not self._in_integration:
|
||||
return
|
||||
|
||||
if not isinstance(node.value, str):
|
||||
return
|
||||
|
||||
if not node.value.startswith("mdi:"):
|
||||
return
|
||||
|
||||
icon_name = node.value[4:] # Strip "mdi:" prefix
|
||||
|
||||
# Only check names that look like intentional icon name attempts.
|
||||
# This skips f-string fragments, format templates (%s, {}),
|
||||
# partial names, and other dynamic patterns.
|
||||
if not _LOOKS_LIKE_ICON_NAME.match(icon_name):
|
||||
return
|
||||
|
||||
if icon_name not in MDI_ICONS:
|
||||
self.add_message(
|
||||
"home-assistant-mdi-icon-not-found",
|
||||
node=node,
|
||||
args=(node.value,),
|
||||
)
|
||||
|
||||
|
||||
def register(linter: PyLinter) -> None:
|
||||
"""Register the checker."""
|
||||
linter.register_checker(MdiIconsChecker(linter))
|
||||
@@ -0,0 +1 @@
|
||||
"""Generated files for the pylint Home Assistant plugin."""
|
||||
+7458
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,60 @@
|
||||
"""Helpers for reading integration icon files."""
|
||||
|
||||
import contextlib
|
||||
|
||||
from astroid import nodes
|
||||
import orjson
|
||||
|
||||
from .integration import get_integration_dir
|
||||
|
||||
_icons_cache: dict[str, dict | None] = {}
|
||||
|
||||
|
||||
def clear_icons_cache() -> None:
|
||||
"""Clear the icons cache (used by tests)."""
|
||||
_icons_cache.clear()
|
||||
|
||||
|
||||
def load_icons(module: nodes.Module) -> dict | None:
|
||||
"""Load and cache the icons.json for the current integration.
|
||||
|
||||
Returns the parsed JSON as a dict, or ``None`` if not found.
|
||||
"""
|
||||
integration_dir = get_integration_dir(module)
|
||||
if integration_dir is None:
|
||||
return None
|
||||
|
||||
cache_key = str(integration_dir)
|
||||
if cache_key in _icons_cache:
|
||||
return _icons_cache[cache_key]
|
||||
|
||||
icons_path = integration_dir / "icons.json"
|
||||
result: dict | None = None
|
||||
if icons_path.exists():
|
||||
with contextlib.suppress(orjson.JSONDecodeError, OSError):
|
||||
parsed = orjson.loads(icons_path.read_bytes())
|
||||
if isinstance(parsed, dict):
|
||||
result = parsed
|
||||
|
||||
_icons_cache[cache_key] = result
|
||||
return result
|
||||
|
||||
|
||||
def collect_mdi_icons(
|
||||
data: dict | list | str, icons: set[str] | None = None
|
||||
) -> set[str]:
|
||||
"""Recursively collect all mdi: icon references from a data structure."""
|
||||
if icons is None:
|
||||
icons = set()
|
||||
|
||||
if isinstance(data, str):
|
||||
if data.startswith("mdi:"):
|
||||
icons.add(data)
|
||||
elif isinstance(data, dict):
|
||||
for value in data.values():
|
||||
collect_mdi_icons(value, icons)
|
||||
elif isinstance(data, list):
|
||||
for item in data:
|
||||
collect_mdi_icons(item, icons)
|
||||
|
||||
return icons
|
||||
@@ -23,6 +23,7 @@ from . import (
|
||||
json,
|
||||
labs,
|
||||
manifest,
|
||||
mdi_icons,
|
||||
metadata,
|
||||
mqtt,
|
||||
mypy_config,
|
||||
@@ -65,6 +66,7 @@ INTEGRATION_PLUGINS = [
|
||||
HASS_PLUGINS = [
|
||||
core_files,
|
||||
docker,
|
||||
mdi_icons,
|
||||
mypy_config,
|
||||
metadata,
|
||||
]
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
"""Generate MDI icons file for the pylint plugin."""
|
||||
|
||||
from importlib.metadata import PackageNotFoundError, version
|
||||
from importlib.resources import files
|
||||
import json
|
||||
|
||||
from .model import Config, Integration
|
||||
from .serializer import format_python_namespace
|
||||
|
||||
_TARGET = "pylint/plugins/pylint_home_assistant/generated/mdi_icons.py"
|
||||
|
||||
|
||||
def _get_frontend_version() -> str | None:
|
||||
"""Get the installed home-assistant-frontend version."""
|
||||
try:
|
||||
return version("home-assistant-frontend")
|
||||
except PackageNotFoundError:
|
||||
return None
|
||||
|
||||
|
||||
def _load_mdi_icons() -> set[str]:
|
||||
"""Load the MDI icon names from the frontend package."""
|
||||
try:
|
||||
mdi_dir = files("hass_frontend") / "static" / "mdi"
|
||||
icon_list_path = mdi_dir / "iconList.json"
|
||||
data = json.loads(icon_list_path.read_text(encoding="utf-8"))
|
||||
return {icon["name"] for icon in data}
|
||||
except ImportError, FileNotFoundError, json.JSONDecodeError, KeyError:
|
||||
return set()
|
||||
|
||||
|
||||
def validate(integrations: dict[str, Integration], config: Config) -> None:
|
||||
"""Validate the generated MDI icons file is up to date."""
|
||||
frontend_version = _get_frontend_version()
|
||||
if frontend_version is None:
|
||||
return
|
||||
|
||||
icons = _load_mdi_icons()
|
||||
if not icons:
|
||||
config.add_error(
|
||||
"mdi_icons",
|
||||
"Could not load MDI icons from home-assistant-frontend",
|
||||
)
|
||||
return
|
||||
|
||||
content = format_python_namespace(
|
||||
{
|
||||
"FRONTEND_VERSION": frontend_version,
|
||||
"MDI_ICONS": icons,
|
||||
},
|
||||
annotations={
|
||||
"FRONTEND_VERSION": "Final[str]",
|
||||
"MDI_ICONS": "Final[set[str]]",
|
||||
},
|
||||
)
|
||||
|
||||
config.cache["mdi_icons_content"] = content
|
||||
|
||||
if config.specific_integrations:
|
||||
return
|
||||
|
||||
target_path = config.root / _TARGET
|
||||
if not target_path.exists() or target_path.read_text() != content:
|
||||
config.add_error(
|
||||
"mdi_icons",
|
||||
f"File {_TARGET} is not up to date. Run python3 -m script.hassfest",
|
||||
fixable=True,
|
||||
)
|
||||
|
||||
|
||||
def generate(integrations: dict[str, Integration], config: Config) -> None:
|
||||
"""Generate MDI icons file."""
|
||||
if "mdi_icons_content" not in config.cache:
|
||||
return
|
||||
target_path = config.root / _TARGET
|
||||
target_path.write_text(config.cache["mdi_icons_content"])
|
||||
@@ -0,0 +1,274 @@
|
||||
"""Tests for the MDI icons checker."""
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
import astroid
|
||||
from pylint.testutils import UnittestLinter
|
||||
from pylint.utils.ast_walker import ASTWalker
|
||||
from pylint_home_assistant.checkers.mdi_icons import MdiIconsChecker
|
||||
from pylint_home_assistant.helpers.icons import clear_icons_cache
|
||||
import pytest
|
||||
|
||||
from . import assert_no_messages
|
||||
|
||||
|
||||
@pytest.fixture(name="mdi_checker")
|
||||
def mdi_checker_fixture(linter: UnittestLinter) -> MdiIconsChecker:
|
||||
"""Fixture to provide an MDI icons checker."""
|
||||
clear_icons_cache()
|
||||
checker = MdiIconsChecker(linter)
|
||||
checker.open()
|
||||
return checker
|
||||
|
||||
|
||||
def _make_integration(tmp_path: Path, icons: dict | None = None) -> Path:
|
||||
"""Create a fake integration with optional icons.json."""
|
||||
integration_dir = tmp_path / "homeassistant" / "components" / "test_int"
|
||||
integration_dir.mkdir(parents=True)
|
||||
if icons is not None:
|
||||
(integration_dir / "icons.json").write_text(json.dumps(icons))
|
||||
return integration_dir
|
||||
|
||||
|
||||
# --- Python code tests ---
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"code",
|
||||
[
|
||||
pytest.param(
|
||||
'icon="mdi:thermometer"',
|
||||
id="valid_icon",
|
||||
),
|
||||
pytest.param(
|
||||
'icon="mdi:lightning-bolt"',
|
||||
id="valid_icon_with_hyphen",
|
||||
),
|
||||
pytest.param(
|
||||
'ICON = "mdi:home"',
|
||||
id="valid_icon_constant",
|
||||
),
|
||||
pytest.param(
|
||||
'device_class = "temperature"',
|
||||
id="non_mdi_string",
|
||||
),
|
||||
pytest.param(
|
||||
'icon = "mdi:%s" % icon_name',
|
||||
id="percent_format_template",
|
||||
),
|
||||
pytest.param(
|
||||
'icon = "mdi:{}".format(icon_name)',
|
||||
id="str_format_template",
|
||||
),
|
||||
pytest.param(
|
||||
'icon = f"mdi:{icon_name}"',
|
||||
id="fstring_template",
|
||||
),
|
||||
pytest.param(
|
||||
'icon = "mdi:fan-speed-" + suffix',
|
||||
id="partial_with_concatenation",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_python_no_warning(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
code: str,
|
||||
) -> None:
|
||||
"""Test that valid MDI icons in Python code pass."""
|
||||
root_node = astroid.parse(code, "homeassistant.components.test_integration.sensor")
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
|
||||
with assert_no_messages(linter):
|
||||
walker.walk(root_node)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("icon", "code"),
|
||||
[
|
||||
pytest.param(
|
||||
"mdi:nonexistent-icon-name",
|
||||
'icon="mdi:nonexistent-icon-name"',
|
||||
id="nonexistent_icon",
|
||||
),
|
||||
pytest.param(
|
||||
"mdi:typo-thremometer",
|
||||
'ICON = "mdi:typo-thremometer"',
|
||||
id="typo_in_icon",
|
||||
),
|
||||
pytest.param(
|
||||
"mdi:bad_icon",
|
||||
'icon = "mdi:bad_icon"',
|
||||
id="underscore_in_name",
|
||||
),
|
||||
pytest.param(
|
||||
"mdi:Bad-Icon",
|
||||
'icon = "mdi:Bad-Icon"',
|
||||
id="uppercase_in_name",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_python_invalid_icon_flagged(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
icon: str,
|
||||
code: str,
|
||||
) -> None:
|
||||
"""Test that invalid MDI icons in Python code are flagged."""
|
||||
root_node = astroid.parse(code, "homeassistant.components.test_integration.sensor")
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
walker.walk(root_node)
|
||||
|
||||
messages = linter.release_messages()
|
||||
assert len(messages) == 1
|
||||
assert messages[0].msg_id == "home-assistant-mdi-icon-not-found"
|
||||
assert icon in messages[0].args[0]
|
||||
|
||||
|
||||
def test_python_not_integration_ignored(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
) -> None:
|
||||
"""Test that non-integration modules are ignored."""
|
||||
root_node = astroid.parse(
|
||||
'ICON = "mdi:nonexistent-icon"',
|
||||
"tests.components.test_integration",
|
||||
)
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
|
||||
with assert_no_messages(linter):
|
||||
walker.walk(root_node)
|
||||
|
||||
|
||||
# --- icons.json tests ---
|
||||
|
||||
|
||||
def test_icons_json_valid(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Test that valid icons.json passes."""
|
||||
integration_dir = _make_integration(
|
||||
tmp_path,
|
||||
{
|
||||
"entity": {
|
||||
"sensor": {
|
||||
"temperature": {"default": "mdi:thermometer"},
|
||||
}
|
||||
},
|
||||
"services": {
|
||||
"my_service": {"service": "mdi:cog"},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
root_node = astroid.parse(
|
||||
"DOMAIN = 'test_int'",
|
||||
"homeassistant.components.test_int.__init__",
|
||||
)
|
||||
root_node.file = str(integration_dir / "__init__.py")
|
||||
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
|
||||
with assert_no_messages(linter):
|
||||
walker.walk(root_node)
|
||||
|
||||
|
||||
def test_icons_json_invalid_flagged(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Test that invalid icons in icons.json are flagged."""
|
||||
integration_dir = _make_integration(
|
||||
tmp_path,
|
||||
{
|
||||
"entity": {
|
||||
"sensor": {
|
||||
"temperature": {"default": "mdi:nonexistent-sensor-icon"},
|
||||
}
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
root_node = astroid.parse(
|
||||
"DOMAIN = 'test_int'",
|
||||
"homeassistant.components.test_int.__init__",
|
||||
)
|
||||
root_node.file = str(integration_dir / "__init__.py")
|
||||
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
walker.walk(root_node)
|
||||
|
||||
messages = linter.release_messages()
|
||||
assert len(messages) == 1
|
||||
assert messages[0].msg_id == "home-assistant-mdi-icon-json-not-found"
|
||||
assert "nonexistent-sensor-icon" in messages[0].args[0]
|
||||
|
||||
|
||||
def test_icons_json_no_file_no_warning(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Test that missing icons.json doesn't cause warnings."""
|
||||
integration_dir = _make_integration(tmp_path)
|
||||
|
||||
root_node = astroid.parse(
|
||||
"DOMAIN = 'test_int'",
|
||||
"homeassistant.components.test_int.__init__",
|
||||
)
|
||||
root_node.file = str(integration_dir / "__init__.py")
|
||||
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
|
||||
with assert_no_messages(linter):
|
||||
walker.walk(root_node)
|
||||
|
||||
|
||||
def test_icons_json_nested_invalid_flagged(
|
||||
linter: UnittestLinter,
|
||||
mdi_checker: MdiIconsChecker,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Test that deeply nested invalid icons are caught."""
|
||||
integration_dir = _make_integration(
|
||||
tmp_path,
|
||||
{
|
||||
"entity": {
|
||||
"light": {
|
||||
"my_light": {
|
||||
"state_attributes": {
|
||||
"effect": {
|
||||
"state": {
|
||||
"sparkle": "mdi:does-not-exist",
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
root_node = astroid.parse(
|
||||
"DOMAIN = 'test_int'",
|
||||
"homeassistant.components.test_int.__init__",
|
||||
)
|
||||
root_node.file = str(integration_dir / "__init__.py")
|
||||
|
||||
walker = ASTWalker(linter)
|
||||
walker.add_checker(mdi_checker)
|
||||
walker.walk(root_node)
|
||||
|
||||
messages = linter.release_messages()
|
||||
assert len(messages) == 1
|
||||
assert "does-not-exist" in messages[0].args[0]
|
||||
Reference in New Issue
Block a user