Added rules for pytest fixtures: (reverbc#17)

* Added rules to pytest fixtures:
- rule for deprecated fixture scope as positional argument
- rule for useless pytest.mark.usefixtures for fixtures
- added tests for these checks

* added to readme

* Fixed comment in PR:
- used check for all `@pytest.mark.*` decorators
- edited error message for warnning W6403
- added py39 for testing

README.md

@@ -99,6 +99,31 @@ def yield_fixture():
     yield
 ```
 
+### W6402 `useless-pytest-mark-decorator`
+Raise when using every `@pytest.mark.*` for the fixture ([ref](https://docs.pytest.org/en/stable/reference.html#marks))
+```python
+import pytest
+
+@pytest.fixture
+def awesome_fixture():
+    ...
+
+@pytest.fixture
+@pytest.mark.usefixtures("awesome_fixture")
+def another_awesome_fixture():
+    ...
+```
+
+### W6403 `deprecated-positional-argument-for-pytest-fixture`
+Raise when using deprecated positional arguments for fixture decorator
+```python
+import pytest
+
+@pytest.fixture("module")
+def awesome_fixture():
+    ...
+```
+
 ## Changelog
 
 See [CHANGELOG](CHANGELOG.md).

pylint_pytest/checkers/fixture.py

@@ -5,7 +5,13 @@
 from pylint.checkers.variables import VariablesChecker
 from pylint.interfaces import IAstroidChecker
 import pytest
-from ..utils import _can_use_fixture, _is_same_module, _is_pytest_mark_usefixtures, _is_pytest_fixture
+from ..utils import (
+    _can_use_fixture,
+    _is_pytest_mark,
+    _is_pytest_mark_usefixtures,
+    _is_pytest_fixture,
+    _is_same_module,
+)
 from . import BasePytestChecker
 
 
@@ -25,6 +31,20 @@ class FixtureChecker(BasePytestChecker):
             'deprecated-pytest-yield-fixture',
             'Used when using a deprecated pytest decorator that has been deprecated in pytest-3.0'
         ),
+        'W6402': (
+            'Using useless `@pytest.mark.*` decorator for fixtures',
+            'useless-pytest-mark-decorator',
+            (
+                '@pytest.mark.* decorators can\'t by applied to fixtures. '
+                'Take a look at: https://docs.pytest.org/en/stable/reference.html#marks'
+            ),
+        ),
+        'W6403': (
+            'Using a deprecated positional arguments for fixture',
+            'deprecated-positional-argument-for-pytest-fixture',
+            'Pass scope as a kwarg, not positional arg, which is deprecated in future pytest.'
+            'Take a look at: https://docs.pytest.org/en/stable/deprecations.html#pytest-fixture-arguments-are-keyword-only',
+        ),
     }
 
     _pytest_fixtures = {}
@@ -91,6 +111,41 @@ def visit_module(self, node):
             # restore output devices
             sys.stdout, sys.stderr = stdout, stderr
 
+    def visit_decorators(self, node):
+        """
+        Walk through all decorators on functions.
+        Tries to find cases:
+            When uses `@pytest.fixture` with `scope` as positional argument (deprecated)
+            https://docs.pytest.org/en/stable/deprecations.html#pytest-fixture-arguments-are-keyword-only
+                >>> @pytest.fixture("module")
+                >>> def awesome_fixture(): ...
+                Instead
+                >>> @pytest.fixture(scope="module")
+                >>> def awesome_fixture(): ...
+            When uses `@pytest.mark.usefixtures` for fixture (useless because didn't work)
+            https://docs.pytest.org/en/stable/reference.html#marks
+                >>> @pytest.mark.usefixtures("another_fixture")
+                >>> @pytest.fixture
+                >>> def awesome_fixture(): ...
+        Parameters
+        ----------
+        node : astroid.scoped_nodes.Decorators
+        """
+        uses_fixture_deco, uses_mark_deco = False, False
+        for decorator in node.nodes:
+            try:
+                if _is_pytest_fixture(decorator) and isinstance(decorator, astroid.Call) and decorator.args:
+                    self.add_message(
+                        'deprecated-positional-argument-for-pytest-fixture', node=decorator
+                    )
+                uses_fixture_deco |= _is_pytest_fixture(decorator)
+                uses_mark_deco |= _is_pytest_mark(decorator)
+            except AttributeError:
+                # ignore any parse exceptions
+                pass
+        if uses_mark_deco and uses_fixture_deco:
+            self.add_message("useless-pytest-mark-decorator", node=node)
+
     def visit_functiondef(self, node):
         '''
         - save invoked fixtures for later use

pylint_pytest/utils.py

@@ -15,6 +15,17 @@ def _is_pytest_mark_usefixtures(decorator):
     return False
 
 
+def _is_pytest_mark(decorator):
+    try:
+        deco = decorator  # as attribute `@pytest.mark.trylast`
+        if isinstance(decorator, astroid.Call):
+            deco = decorator.func  # as function `@pytest.mark.skipif(...)`
+        if deco.expr.attrname == 'mark' and deco.expr.expr.name == 'pytest':
+            return True
+    except AttributeError:
+        return False
+
+
 def _is_pytest_fixture(decorator, fixture=True, yield_fixture=True):
     attr = None
     to_check = set()

tests/input/deprecated-positional-argument-for-pytest-fixture/with_args_scope.py

@@ -0,0 +1,6 @@
+import pytest
+
+
+@pytest.fixture('function')
+def some_fixture():
+    return 'ok'

tests/input/deprecated-positional-argument-for-pytest-fixture/with_kwargs_scope.py

@@ -0,0 +1,6 @@
+import pytest
+
+
+@pytest.fixture(scope='function')
+def some_fixture():
+    return 'ok'

tests/input/deprecated-positional-argument-for-pytest-fixture/without_scope.py

@@ -0,0 +1,6 @@
+import pytest
+
+
+@pytest.fixture
+def some_fixture():
+    return 'ok'

tests/input/useless-pytest-mark-decorator/mark_usefixture_using_for_class.py

@@ -0,0 +1,16 @@
+import pytest
+
+
+@pytest.fixture
+def first():
+    return "OK"
+
+
+@pytest.mark.usefixtures("first")
+class TestFirst:
+    @staticmethod
+    def do():
+        return True
+
+    def test_first(self):
+        assert self.do() is True

tests/input/useless-pytest-mark-decorator/mark_usefixture_using_for_fixture_attribute.py

@@ -0,0 +1,18 @@
+import pytest
+
+
+@pytest.fixture
+def first():
+    return "OK"
+
+
+@pytest.fixture
+@pytest.mark.usefixtures("first")
+def second():
+    return "NOK"
+
+
+@pytest.mark.usefixtures("first")
+@pytest.fixture
+def third():
+    return "NOK"

tests/input/useless-pytest-mark-decorator/mark_usefixture_using_for_fixture_function.py

@@ -0,0 +1,18 @@
+import pytest
+
+
+@pytest.fixture(scope="session")
+def first():
+    return "OK"
+
+
+@pytest.fixture(scope="function")
+@pytest.mark.usefixtures("first")
+def second():
+    return "NOK"
+
+
+@pytest.mark.usefixtures("first")
+@pytest.fixture(scope="function")
+def third():
+    return "NOK"

tests/input/useless-pytest-mark-decorator/mark_usefixture_using_for_test.py

@@ -0,0 +1,11 @@
+import pytest
+
+
+@pytest.fixture
+def first():
+    return "OK"
+
+
+@pytest.mark.usefixtures("first")
+def test_first():
+    pass

tests/input/useless-pytest-mark-decorator/other_marks_using_for_fixture.py

@@ -0,0 +1,26 @@
+import os
+import pytest
+
+
+@pytest.mark.trylast
+@pytest.fixture
+def fixture():
+    return "Not ok"
+
+
+@pytest.mark.parametrize("id", range(2))
+@pytest.fixture
+def fixture_with_params(id):
+    return "{} not OK".format(id)
+
+
+@pytest.mark.custom_mark
+@pytest.fixture
+def fixture_with_custom_mark():
+    return "NOT OK"
+
+
+@pytest.mark.skipif(os.getenv("xXx"))
+@pytest.fixture
+def fixture_with_conditional_mark():
+    return "NOK"

tests/test_pytest_fixture_positional_arguments.py

@@ -0,0 +1,19 @@
+from base_tester import BasePytestTester
+from pylint_pytest.checkers.fixture import FixtureChecker
+
+
+class TestDeprecatedPytestFixtureScopeAsPositionalParam(BasePytestTester):
+    CHECKER_CLASS = FixtureChecker
+    MSG_ID = 'deprecated-positional-argument-for-pytest-fixture'
+
+    def test_with_args_scope(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(1)
+
+    def test_with_kwargs_scope(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(0)
+
+    def test_without_scope(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(0)

tests/test_pytest_mark_for_fixtures.py

@@ -0,0 +1,27 @@
+from base_tester import BasePytestTester
+from pylint_pytest.checkers.fixture import FixtureChecker
+
+
+class TestPytestMarkUsefixtures(BasePytestTester):
+    CHECKER_CLASS = FixtureChecker
+    MSG_ID = 'useless-pytest-mark-decorator'
+
+    def test_mark_usefixture_using_for_test(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(0)
+
+    def test_mark_usefixture_using_for_class(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(0)
+
+    def test_mark_usefixture_using_for_fixture_attribute(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(2)
+
+    def test_mark_usefixture_using_for_fixture_function(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(2)
+
+    def test_other_marks_using_for_fixture(self):
+        self.run_linter(enable_plugin=True)
+        self.verify_messages(4)

tox.ini

@@ -1,5 +1,5 @@
 [tox]
-envlist = py27,py35,py36,py37,py38
+envlist = py36,py37,py38,py39
 skipsdist = True
 
 [testenv]