Add pylint checker to use json helpers (#180829)

Co-authored-by: Lucas Mindêllo de Andrade <lucas@mindello.com.br>

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)
+```

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))

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)