From bb0b523a8a2cf2b40022c51781ad9b99a4a058a1 Mon Sep 17 00:00:00 2001 From: Nate Parsons Date: Wed, 2 Nov 2022 09:03:55 -0700 Subject: [PATCH 1/3] removes more false positives for unused-argument --- pylint_pytest/checkers/fixture.py | 17 ++++++++----- .../func_param_as_fixture_arg.py | 24 +++++++++++++++++++ tests/test_unused_argument.py | 5 ++++ 3 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 tests/input/unused-argument/func_param_as_fixture_arg.py diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index c2a00a0..51c1a94 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -110,10 +110,10 @@ def visit_module(self, node): is_test_module = True break + stdout, stderr = sys.stdout, sys.stderr try: with open(os.devnull, 'w') as devnull: # suppress any future output from pytest - stdout, stderr = sys.stdout, sys.stderr sys.stderr = sys.stdout = devnull # run pytest session with customized plugin to collect fixtures @@ -238,11 +238,16 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, return # check W0613 unused-argument - if msgid == 'unused-argument' and \ - _can_use_fixture(node.parent.parent) and \ - isinstance(node.parent, astroid.Arguments) and \ - node.name in FixtureChecker._pytest_fixtures: - return + if msgid == 'unused-argument': + if _can_use_fixture(node.parent.parent): + if isinstance(node.parent, astroid.Arguments): + if node.name in FixtureChecker._pytest_fixtures: + return # argument is used as a fixture + else: + fixnames = (arg.name for arg in node.parent.args if arg.name in FixtureChecker._pytest_fixtures) + for fixname in fixnames: + if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames: + return # argument is used by a fixture # check W0621 redefined-outer-name if msgid == 'redefined-outer-name' and \ diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py new file mode 100644 index 0000000..2ef5f10 --- /dev/null +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -0,0 +1,24 @@ +""" +This module illustrates a situation in which unused-argument should be +suppressed, but is not. +""" + +import pytest + + +@pytest.fixture +def myfix(arg): + """A fixture that requests a function param""" + print("arg is ", arg) + return True + + +@pytest.mark.parametrize("arg", [1, 2, 3]) +def test_myfix(myfix, arg): + """A test function that uses the param through a fixture""" + assert myfix + +@pytest.mark.parametrize("narg", [4, 5, 6]) +def test_nyfix(narg): # unused-argument + """A test function that does not use its param""" + assert True diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index 7ab2747..529e764 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -28,3 +28,8 @@ def test_caller_not_a_test_func(self, enable_plugin): def test_args_and_kwargs(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(2) + + @pytest.mark.parametrize('enable_plugin', [True, False]) + def test_func_param_as_fixture_arg(self, enable_plugin): + self.run_linter(enable_plugin) + self.verify_messages(1 if enable_plugin else 2) From 360461c01214aa9f4f605f4212b4063ccbcedb91 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 23 Oct 2023 11:43:03 +0300 Subject: [PATCH 2/3] Make `nsp/nate/better-unused-argument` merge-able with `origin/master` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- pylint_pytest/checkers/fixture.py | 28 +++++++++++-------- .../func_param_as_fixture_arg.py | 1 + tests/test_unused_argument.py | 12 ++++---- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/pylint_pytest/checkers/fixture.py b/pylint_pytest/checkers/fixture.py index 51c1a94..0d4362a 100644 --- a/pylint_pytest/checkers/fixture.py +++ b/pylint_pytest/checkers/fixture.py @@ -196,7 +196,7 @@ def visit_functiondef(self, node): for arg in node.args.args: self._invoked_with_func_args.add(arg.name) - # pylint: disable=protected-access,bad-staticmethod-argument + # pylint: disable=bad-staticmethod-argument,too-many-branches # The function itself is an if-return logic. @staticmethod def patch_add_message(self, msgid, line=None, node=None, args=None, confidence=None, col_offset=None): @@ -238,16 +238,22 @@ def patch_add_message(self, msgid, line=None, node=None, args=None, return # check W0613 unused-argument - if msgid == 'unused-argument': - if _can_use_fixture(node.parent.parent): - if isinstance(node.parent, astroid.Arguments): - if node.name in FixtureChecker._pytest_fixtures: - return # argument is used as a fixture - else: - fixnames = (arg.name for arg in node.parent.args if arg.name in FixtureChecker._pytest_fixtures) - for fixname in fixnames: - if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames: - return # argument is used by a fixture + if ( + msgid == "unused-argument" + and _can_use_fixture(node.parent.parent) + and isinstance(node.parent, astroid.Arguments) + ): + if node.name in FixtureChecker._pytest_fixtures: + # argument is used as a fixture + return + + fixnames = ( + arg.name for arg in node.parent.args if arg.name in FixtureChecker._pytest_fixtures + ) + for fixname in fixnames: + if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames: + # argument is used by a fixture + return # check W0621 redefined-outer-name if msgid == 'redefined-outer-name' and \ diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py index 2ef5f10..43f01dd 100644 --- a/tests/input/unused-argument/func_param_as_fixture_arg.py +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -18,6 +18,7 @@ def test_myfix(myfix, arg): """A test function that uses the param through a fixture""" assert myfix + @pytest.mark.parametrize("narg", [4, 5, 6]) def test_nyfix(narg): # unused-argument """A test function that does not use its param""" diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index 529e764..8ce8a8b 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -7,29 +7,29 @@ class TestUnusedArgument(BasePytestTester): CHECKER_CLASS = FixtureChecker IMPACTED_CHECKER_CLASSES = [VariablesChecker] - MSG_ID = 'unused-argument' + MSG_ID = "unused-argument" - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_smoke(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(0 if enable_plugin else 2) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_caller_yield_fixture(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(0 if enable_plugin else 1) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_caller_not_a_test_func(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(1) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_args_and_kwargs(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(2) - @pytest.mark.parametrize('enable_plugin', [True, False]) + @pytest.mark.parametrize("enable_plugin", [True, False]) def test_func_param_as_fixture_arg(self, enable_plugin): self.run_linter(enable_plugin) self.verify_messages(1 if enable_plugin else 2) From 8756cc4627a66f150197519a2588512511f0c4f3 Mon Sep 17 00:00:00 2001 From: Stavros Ntentos <133706+stdedos@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:34:32 +0200 Subject: [PATCH 3/3] Extend coverage for `if node.name in FixtureChecker._pytest_fixtures[fixname][0].argnames:` Signed-off-by: Stavros Ntentos <133706+stdedos@users.noreply.github.com> --- tests/input/unused-argument/func_param_as_fixture_arg.py | 6 ++++++ tests/test_unused_argument.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/input/unused-argument/func_param_as_fixture_arg.py b/tests/input/unused-argument/func_param_as_fixture_arg.py index 43f01dd..003d194 100644 --- a/tests/input/unused-argument/func_param_as_fixture_arg.py +++ b/tests/input/unused-argument/func_param_as_fixture_arg.py @@ -23,3 +23,9 @@ def test_myfix(myfix, arg): def test_nyfix(narg): # unused-argument """A test function that does not use its param""" assert True + + +@pytest.mark.parametrize("arg", [1, 2, 3]) +def test_narg_is_used_nowhere(myfix, narg): + """A test function that uses the param through a fixture""" + assert myfix diff --git a/tests/test_unused_argument.py b/tests/test_unused_argument.py index a5224e1..a164672 100644 --- a/tests/test_unused_argument.py +++ b/tests/test_unused_argument.py @@ -33,4 +33,4 @@ def test_args_and_kwargs(self, enable_plugin): @pytest.mark.parametrize("enable_plugin", [True, False]) def test_func_param_as_fixture_arg(self, enable_plugin): self.run_linter(enable_plugin) - self.verify_messages(1 if enable_plugin else 2) + self.verify_messages(2 if enable_plugin else 3)