diff --git a/pylint/plugins/README.md b/pylint/plugins/README.md index 5cb82dbeb8f0..1de594ff635f 100644 --- a/pylint/plugins/README.md +++ b/pylint/plugins/README.md @@ -98,6 +98,7 @@ Every check has a code following the | `W7406` | [`home-assistant-unique-id-ip-based`](#w7406-home-assistant-unique-id-ip-based) | Unique ID should not be based on IP/hostname | | `W7407` | [`home-assistant-config-flow-polling-field`](#w7407-home-assistant-config-flow-polling-field) | Config flow should not include polling interval fields | | `W7408` | [`home-assistant-config-flow-name-field`](#w7408-home-assistant-config-flow-name-field) | Config flow should not include name fields | +| `R7402` | [`home-assistant-unused-test-fixture-argument`](#r7402-home-assistant-unused-test-fixture-argument) | Unused test function argument should use `@pytest.mark.usefixtures` | ## `home_assistant_logger` checker @@ -324,3 +325,18 @@ Config flow should not include a name field. Users should not set names in config flows; they come automatically from the device or are set by the integration. + +## `home_assistant_unused_test_fixture_args` checker + +**Disabled by default** while existing violations are being cleaned up. + + +### `R7402`: `home-assistant-unused-test-fixture-argument` + +Test functions that receive a fixture argument but never reference it in +the function body should use `@pytest.mark.usefixtures("name")` instead. +This keeps the function signature clean and makes it clear the fixture is +only needed for its side effects. + +This rule only applies to `test_*` functions, not to fixture functions. + diff --git a/pylint/plugins/pylint_home_assistant/checkers/unused_test_fixture_args.py b/pylint/plugins/pylint_home_assistant/checkers/unused_test_fixture_args.py new file mode 100644 index 000000000000..013a0cd28842 --- /dev/null +++ b/pylint/plugins/pylint_home_assistant/checkers/unused_test_fixture_args.py @@ -0,0 +1,77 @@ +"""Checker for unused fixture arguments in test functions. + +Test functions that receive a fixture argument but never reference it in the +function body should use ``@pytest.mark.usefixtures("name")`` instead. This +keeps the function signature clean and makes it clear the fixture is only +needed for its side effects. + +This rule only applies to ``test_*`` functions, not to fixture functions. +""" + +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 + + +class UnusedTestFixtureArgsChecker(BaseChecker): + """Checker for unused fixture arguments in test functions.""" + + name = "home_assistant_unused_test_fixture_args" + priority = -1 + msgs = { + "R7402": ( + "Argument '%s' is not used in %s, use " + '`@pytest.mark.usefixtures("%s")` instead', + "home-assistant-unused-test-fixture-argument", + "Used when a test function has a fixture argument that is never " + "referenced in the function body. Use @pytest.mark.usefixtures " + "to declare the dependency instead.", + ), + } + options = () + + _in_test_module: bool + + def visit_module(self, node: nodes.Module) -> None: + """Track whether we are in a test module.""" + self._in_test_module = is_test_module(node.name) + + def visit_functiondef(self, node: nodes.FunctionDef) -> None: + """Check test functions for unused fixture arguments.""" + if not self._in_test_module: + return + + if not node.name.startswith("test_"): + return + + # Only check top-level test functions (not nested) + if not isinstance(node.parent, nodes.Module): + return + + # Collect all argument names (skip 'self' for methods) + arg_names = {arg.name for arg in node.args.args if arg.name != "self"} + + if not arg_names: + return + + # Collect all Name references in the function body + used_names: set[str] = set() + for child in node.nodes_of_class(nodes.Name): + used_names.add(child.name) + + for arg_name in sorted(arg_names - used_names): + arg_node = next(arg for arg in node.args.args if arg.name == arg_name) + self.add_message( + "home-assistant-unused-test-fixture-argument", + node=arg_node, + args=(arg_name, node.name, arg_name), + ) + + visit_asyncfunctiondef = visit_functiondef + + +def register(linter: PyLinter) -> None: + """Register the checker.""" + linter.register_checker(UnusedTestFixtureArgsChecker(linter)) diff --git a/pyproject.toml b/pyproject.toml index 099c2508b59b..cd63b499e5f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -176,6 +176,9 @@ disable = [ "consider-using-assignment-expr", "possibly-used-before-assignment", + # Disabled while existing violations are being cleaned up + "home-assistant-unused-test-fixture-argument", + # Handled by ruff # Ref: "await-outside-async", # PLE1142 diff --git a/tests/pylint/test_unused_test_fixture_args.py b/tests/pylint/test_unused_test_fixture_args.py new file mode 100644 index 000000000000..60bd2ff94f99 --- /dev/null +++ b/tests/pylint/test_unused_test_fixture_args.py @@ -0,0 +1,163 @@ +"""Tests for the unused test fixture arguments checker.""" + +import astroid +from pylint.testutils import UnittestLinter +from pylint.utils.ast_walker import ASTWalker +from pylint_home_assistant.checkers.unused_test_fixture_args import ( + UnusedTestFixtureArgsChecker, +) +import pytest + +from . import assert_no_messages + + +@pytest.fixture(name="unused_args_checker") +def unused_args_checker_fixture( + linter: UnittestLinter, +) -> UnusedTestFixtureArgsChecker: + """Fixture to provide an unused test fixture args checker.""" + return UnusedTestFixtureArgsChecker(linter) + + +@pytest.mark.parametrize( + "code", + [ + pytest.param( + """ +def test_something(hass: HomeAssistant) -> None: + assert hass.state +""", + id="all_args_used", + ), + pytest.param( + """ +def test_something(hass: HomeAssistant, config_entry: MockConfigEntry) -> None: + hass.states.async_set("sensor.test", "on") + config_entry.add_to_hass(hass) +""", + id="multiple_args_all_used", + ), + pytest.param( + """ +@pytest.fixture +def my_fixture(enable_bluetooth: None) -> None: + pass +""", + id="fixture_function_ignored", + ), + pytest.param( + """ +def helper_function(unused_arg: str) -> None: + pass +""", + id="non_test_function_ignored", + ), + pytest.param( + """ +def test_empty() -> None: + pass +""", + id="no_args", + ), + ], +) +def test_no_warning( + linter: UnittestLinter, + unused_args_checker: UnusedTestFixtureArgsChecker, + code: str, +) -> None: + """Test cases that should not trigger a warning.""" + root_node = astroid.parse(code, "tests.components.test_integration.test_init") + walker = ASTWalker(linter) + walker.add_checker(unused_args_checker) + + with assert_no_messages(linter): + walker.walk(root_node) + + +def test_unused_single_arg( + linter: UnittestLinter, + unused_args_checker: UnusedTestFixtureArgsChecker, +) -> None: + """Test that unused fixture arg is flagged.""" + root_node = astroid.parse( + """ +def test_something(hass: HomeAssistant, enable_bluetooth: None) -> None: + assert hass.state +""", + "tests.components.test_integration.test_init", + ) + walker = ASTWalker(linter) + walker.add_checker(unused_args_checker) + walker.walk(root_node) + + messages = linter.release_messages() + assert len(messages) == 1 + assert messages[0].msg_id == "home-assistant-unused-test-fixture-argument" + assert messages[0].args == ( + "enable_bluetooth", + "test_something", + "enable_bluetooth", + ) + + +def test_unused_multiple_args( + linter: UnittestLinter, + unused_args_checker: UnusedTestFixtureArgsChecker, +) -> None: + """Test that multiple unused fixture args are all flagged.""" + root_node = astroid.parse( + """ +def test_something(hass: HomeAssistant, enable_bluetooth: None, socket_enabled: None) -> None: + assert hass.state +""", + "tests.components.test_integration.test_init", + ) + walker = ASTWalker(linter) + walker.add_checker(unused_args_checker) + walker.walk(root_node) + + messages = linter.release_messages() + assert len(messages) == 2 + assert messages[0].args[0] == "enable_bluetooth" + assert messages[1].args[0] == "socket_enabled" + + +def test_not_test_module( + linter: UnittestLinter, + unused_args_checker: UnusedTestFixtureArgsChecker, +) -> None: + """Test that non-test modules are ignored.""" + root_node = astroid.parse( + """ +def test_something(unused: str) -> None: + pass +""", + "homeassistant.components.test_integration", + ) + walker = ASTWalker(linter) + walker.add_checker(unused_args_checker) + + with assert_no_messages(linter): + walker.walk(root_node) + + +def test_async_test_function( + linter: UnittestLinter, + unused_args_checker: UnusedTestFixtureArgsChecker, +) -> None: + """Test that async test functions are also checked.""" + root_node = astroid.parse( + """ +async def test_something(hass: HomeAssistant, enable_bluetooth: None) -> None: + assert hass.state +""", + "tests.components.test_integration.test_init", + ) + walker = ASTWalker(linter) + walker.add_checker(unused_args_checker) + walker.walk(root_node) + + messages = linter.release_messages() + assert len(messages) == 1 + assert messages[0].args[0] == "enable_bluetooth"