From bb38892246fc11e588be24c4d0d28a27aacdae62 Mon Sep 17 00:00:00 2001 From: Joost Lekkerkerker Date: Sun, 30 Aug 2026 20:37:34 +0200 Subject: [PATCH] Add pylint checker to use json helpers (#180829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lucas Mindêllo de Andrade --- pylint/plugins/README.md | 32 +++++ .../checkers/json_fixture.py | 98 ++++++++++++++ tests/pylint/test_json_fixture.py | 125 ++++++++++++++++++ 3 files changed, 255 insertions(+) create mode 100644 pylint/plugins/pylint_home_assistant/checkers/json_fixture.py create mode 100644 tests/pylint/test_json_fixture.py diff --git a/pylint/plugins/README.md b/pylint/plugins/README.md index 9b9942ded199..eae640b333b5 100644 --- a/pylint/plugins/README.md +++ b/pylint/plugins/README.md @@ -138,6 +138,7 @@ Every check has a code following the | `W7431` | [`home-assistant-options-flow-field-not-translated`](#w7431-home-assistant-options-flow-field-not-translated) | Options flow form field missing translation in `strings.json` | | `W7432` | [`home-assistant-subentry-flow-field-not-translated`](#w7432-home-assistant-subentry-flow-field-not-translated) | Subentry flow form field missing translation in `strings.json` | | `W7433` | [`home-assistant-missing-test-before-configure`](#w7433-home-assistant-missing-test-before-configure) | Config flow should test the connection before creating an entry | +| `W7435` | [`home-assistant-json-fixture`](#w7435-home-assistant-json-fixture) | Use a JSON fixture helper instead of parsing a loaded fixture | ## `home_assistant_logger` checker @@ -951,3 +952,34 @@ websocket command, which is only registered when the `usb` integration is set up. The selector therefore requires `usb` as a hard dependency (`"dependencies": ["usb"]`); `after_dependencies` is not sufficient because it does not force `usb` to be set up. + + +## `home_assistant_json_fixture` checker + +Detects tests that load a fixture and then parse it as JSON, instead of +using the dedicated JSON fixture helpers from `tests.common`. Only runs on +test modules. `tests.common` itself is exempt, since it defines the JSON +fixture helpers, which legitimately parse a loaded fixture. + +### `W7435`: `home-assistant-json-fixture` + +A fixture loader (`load_fixture`, `load_fixture_bytes`, or +`async_load_fixture`) is wrapped in a JSON-parsing call (`json.loads`, +`json.load`, or the `json_loads` / `json_loads_array` / `json_loads_object` +helpers), e.g.: + +```python +data = json.loads(load_fixture("data.json", DOMAIN)) +data = json_loads_object(await async_load_fixture(hass, "data.json")) +``` + +Use the dedicated helper that loads and parses in one step instead: + +- `load_json_value_fixture` / `async_load_json_object_fixture` for a JSON value, +- `load_json_array_fixture` / `async_load_json_array_fixture` for a JSON array, +- `load_json_object_fixture` / `async_load_json_object_fixture` for a JSON object. + +```python +data = load_json_object_fixture("data.json", DOMAIN) +data = await async_load_json_object_fixture(hass, "data.json", DOMAIN) +``` diff --git a/pylint/plugins/pylint_home_assistant/checkers/json_fixture.py b/pylint/plugins/pylint_home_assistant/checkers/json_fixture.py new file mode 100644 index 000000000000..4ece464d13b7 --- /dev/null +++ b/pylint/plugins/pylint_home_assistant/checkers/json_fixture.py @@ -0,0 +1,98 @@ +"""Checker for JSON-parsing a fixture instead of using the JSON fixture helpers.""" + +from astroid import nodes +from pylint.checkers import BaseChecker +from pylint.lint import PyLinter + +from pylint_home_assistant.helpers.module_info import is_test_module + +# JSON-parsing helpers imported as bare names (from homeassistant.util.json). +_JSON_PARSE_NAMES = frozenset( + { + "json_loads", + "json_loads_array", + "json_loads_object", + } +) + +# Attribute-form JSON parsers, only when called on the ``json`` module. +_JSON_PARSE_ATTRS = frozenset({"loads", "load"}) + +# Fixture loaders whose result is a raw string/bytes. +_FIXTURE_LOADER_NAMES = frozenset( + { + "load_fixture", + "load_fixture_bytes", + "async_load_fixture", + } +) + + +def _is_json_parse_call(node: nodes.Call) -> bool: + """Return True if the call parses JSON.""" + func = node.func + if isinstance(func, nodes.Attribute): + return ( + func.attrname in _JSON_PARSE_ATTRS + and isinstance(func.expr, nodes.Name) + and func.expr.name == "json" + ) + if isinstance(func, nodes.Name): + return func.name in _JSON_PARSE_NAMES + return False + + +def _is_fixture_loader(node: nodes.NodeNG) -> bool: + """Return True if the node is a call to a fixture loader.""" + if isinstance(node, nodes.Await): + node = node.value + if not isinstance(node, nodes.Call): + return False + func = node.func + if isinstance(func, nodes.Attribute): + return func.attrname in _FIXTURE_LOADER_NAMES + if isinstance(func, nodes.Name): + return func.name in _FIXTURE_LOADER_NAMES + return False + + +class HassJsonFixtureChecker(BaseChecker): + """Checker for JSON-parsing a loaded fixture.""" + + name = "home_assistant_json_fixture" + priority = -1 + msgs = { + "W7435": ( + "Use a JSON fixture helper (e.g. load_json_object_fixture) instead of " + "parsing a loaded fixture", + "home-assistant-json-fixture", + "Used when a fixture is loaded and then parsed as JSON instead of using " + "the dedicated JSON fixture helpers", + ), + } + options = () + + _in_test_module: bool + + def visit_module(self, node: nodes.Module) -> None: + """Visit a module definition.""" + # ``tests.common`` defines the JSON fixture helpers themselves, which + # legitimately parse a loaded fixture. + self._in_test_module = is_test_module(node.name) and node.name != "tests.common" + + def visit_call(self, node: nodes.Call) -> None: + """Check for JSON parsing of a loaded fixture.""" + if ( + not self._in_test_module + or not _is_json_parse_call(node) + or not node.args + or not _is_fixture_loader(node.args[0]) + ): + return + + self.add_message("home-assistant-json-fixture", node=node) + + +def register(linter: PyLinter) -> None: + """Register the checker.""" + linter.register_checker(HassJsonFixtureChecker(linter)) diff --git a/tests/pylint/test_json_fixture.py b/tests/pylint/test_json_fixture.py new file mode 100644 index 000000000000..c4383f13e8e9 --- /dev/null +++ b/tests/pylint/test_json_fixture.py @@ -0,0 +1,125 @@ +"""Tests for the JSON fixture checker.""" + +import astroid +from pylint.testutils import MessageTest, UnittestLinter +from pylint_home_assistant.checkers.json_fixture import HassJsonFixtureChecker +import pytest + +from . import assert_adds_messages, assert_no_messages, walk_checker + + +@pytest.fixture(name="json_fixture_checker") +def json_fixture_checker_fixture( + linter: UnittestLinter, +) -> HassJsonFixtureChecker: + """Fixture to provide a JSON fixture checker.""" + return HassJsonFixtureChecker(linter) + + +@pytest.mark.parametrize( + "code", + [ + pytest.param( + "value = json.loads(load_fixture('data.json', 'my_integration'))", + id="json_loads_load_fixture", + ), + pytest.param( + "value = json_loads(load_fixture('data.json'))", + id="json_loads_helper", + ), + pytest.param( + "value = json_loads_object(load_fixture('data.json'))", + id="json_loads_object", + ), + pytest.param( + "value = json_loads_array(load_fixture_bytes('data.json'))", + id="json_loads_array_bytes", + ), + pytest.param( + "value = json.loads(await async_load_fixture(hass, 'data.json'))", + id="json_loads_async_load_fixture", + ), + ], +) +def test_flagged( + linter: UnittestLinter, + json_fixture_checker: HassJsonFixtureChecker, + code: str, +) -> None: + """Test cases that should be flagged.""" + root_node = astroid.parse(code, "tests.components.my_integration.test_sensor") + call_node = next(root_node.nodes_of_class(astroid.nodes.Call)) + + with assert_adds_messages( + linter, + MessageTest( + msg_id="home-assistant-json-fixture", + node=call_node, + line=call_node.lineno, + col_offset=call_node.col_offset, + end_line=call_node.end_lineno, + end_col_offset=call_node.end_col_offset, + ), + ): + walk_checker(linter, json_fixture_checker, root_node) + + +@pytest.mark.parametrize( + "code", + [ + pytest.param( + "value = load_json_object_fixture('data.json', 'my_integration')", + id="json_object_fixture_helper", + ), + pytest.param( + "value = json.loads(some_string)", + id="json_loads_non_fixture", + ), + pytest.param( + "value = json.dumps(load_fixture('data.json'))", + id="json_dumps_load_fixture", + ), + pytest.param( + "value = load_fixture('data.json')", + id="load_fixture_only", + ), + ], +) +def test_not_flagged( + linter: UnittestLinter, + json_fixture_checker: HassJsonFixtureChecker, + code: str, +) -> None: + """Test cases that should not be flagged.""" + root_node = astroid.parse(code, "tests.components.my_integration.test_sensor") + + with assert_no_messages(linter): + walk_checker(linter, json_fixture_checker, root_node) + + +def test_not_flagged_outside_test_module( + linter: UnittestLinter, + json_fixture_checker: HassJsonFixtureChecker, +) -> None: + """Test that non-test modules are ignored.""" + root_node = astroid.parse( + "value = json.loads(load_fixture('data.json'))", + "homeassistant.components.my_integration.sensor", + ) + + with assert_no_messages(linter): + walk_checker(linter, json_fixture_checker, root_node) + + +def test_not_flagged_in_tests_common( + linter: UnittestLinter, + json_fixture_checker: HassJsonFixtureChecker, +) -> None: + """Test that the fixture helper definitions in tests.common are ignored.""" + root_node = astroid.parse( + "value = json_loads_object(load_fixture('data.json'))", + "tests.common", + ) + + with assert_no_messages(linter): + walk_checker(linter, json_fixture_checker, root_node)