False Positive W0621: Redefining name %r from outer scope when using pytest fixtures

[orSolocate]

Bug description

Bug opened from SO issue

Testcase:

# test_wallet.py

@pytest.fixture
def my_wallet():
    '''Returns a Wallet instance with a zero balance'''
    return Wallet()

@pytest.mark.parametrize("earned,spent,expected", [
    (30, 10, 20),
    (20, 2, 18),
])
def test_transactions(my_wallet, earned, spent, expected):
    my_wallet.add_cash(earned)
    my_wallet.spend_cash(spent)
    assert my_wallet.balance == expected

They discuss a workaround to avoid the error and also a method of disabling the checker but this bug issue is about actually fixing it in Pylint, or at least discussing it.

Configuration

No response

Pylint output

`W0621: Redefining name %r from outer scope (line %s)'`

Expected behavior

The Pylint message should recognize this situation of pytest fixture decorator and don't raise a message.

Pylint version

pylint 2.8.4
astroid 2.5.8
Python 3.7.5

OS / Environment

Windows

Additional dependencies

No response

[Pierre-Sassoulas]

Well, apparently this code crash on 2.14.0-dev0 and 2.13.8.

Exception on node <Decorators l.2 at 0x7f1be24aaa00> in file '/home/pierre/pylint/b.py'
Traceback (most recent call last):
  File "/home/pierre/pylint/pylint/utils/ast_walker.py", line 73, in walk
    callback(astroid)
  File "/home/pierre/pylint/pylint/checkers/unsupported_version.py", line 64, in visit_decorators
    self._check_typing_final(node)
  File "/home/pierre/pylint/pylint/checkers/unsupported_version.py", line 79, in _check_typing_final
    for decorator in decorators or uninferable_final_decorators(node):
  File "/home/pierre/pylint/pylint/checkers/utils.py", line 825, in uninferable_final_decorators
    import_node = decorator.expr.lookup(decorator.expr.name)[1][0]
IndexError: tuple index out of range

[DanielNoord]

We should probably fix the crash, but the issue of handling pytest fixtures should probably be delegated to pytest-pylint.

[mbyrnepr2]

We should probably fix the crash

I can take a look.

#6532

[mbyrnepr2]

I could be wrong but regarding the "Refined name from outer scope" message. Isn't this correct of Pylint in highlighting what is also mentioned in the SO (pytest docs)?

If a fixture is used in the same module in which it is defined, the function name of the fixture will be shadowed by the function arg that requests the fixture; one way to resolve this is to name the decorated function fixture and then use @pytest.fixture(name='').

[Pierre-Sassoulas]

Related : #1535

[lqueryvg]

How about my comment here:
https://stackoverflow.com/questions/46089480/pytest-fixtures-redefining-name-from-outer-scope-pylint/54045715?noredirect=1#comment122655133_57015304
I.e. you get a similar problem with mocker.
from pytest_mock import mocker followed by def test_my_thing(mocker) gives same lint error. Seems to be a recurring theme.

[DanielNoord]

These issues are probably best reported to https://github.com/reverbc/pylint-pytest/. Due to the way pytest creates its fixtures and mocks it's difficult for pylint to infer these as it does not follow the traditional/standard pattern of Pythonic code. Therefore, such issues are better handled by a pylint plugin that is focused on understanding those particular patterns.
Similarly, there is https://github.com/PyCQA/pylint-django which does the same but then for common Django patterns. There is simply not enough maintainers-bandwidth to support all these plugins within the main pylint project.

[nicktimko]

Not sure if there's a specific solution in mind for this, but if the OPs original code

@pytest.fixture
def my_wallet():
    '''Returns a Wallet instance with a zero balance'''

@pytest.mark.parametrize("earned,spent,expected", dataset)
def test_transactions(my_wallet, earned, spent, expected):
    ...
    assert my_wallet.balance == expected

was to emit no errors, I've also had issues where I'll accidentally forget to use the fixture (as an argument), or the correct name, and then I get really confused because the code technically still runs, there's no name redefinition, you just get something strange like an AttributeError: my_wallet has no attribute 'balance', and you'll be like, "yes it does, it's the wallet from the fixture, wtf"

@pytest.fixture
def my_wallet():
    '''Returns a Wallet instance with a zero balance'''

@pytest.mark.parametrize("earned,spent,expected", dataset)
def test_transactions(earned, spent, expected):
    ...
    assert my_wallet.balance == expected  # my_wallet is the "raw" fixture, not the fixture's result

A 'fix' is to do something like

@pytest.fixture(name="my_wallet")
def fixture_my_wallet():
    ...

but it can get a little tedious. My dumb wish is that there was just a prefix you could slap on to functions (e.g. fixture_) that would make them fixtures, but get stripped off when used by the arguments. A prefix like test_ is commonly used for test functions.

# @pytest.fixture(name="my_wallet")  <-- i.e. with or w/o this line, it would have the exact same behavior
def fixture_my_wallet():
    ...

But that's all in the PyTest camp...

[orSolocate]

We should probably fix the crash

I can take a look.

#6532

@mbyrnepr2 - Hey mate should I make you an assignee for fixing the crash?

[mbyrnepr2]

Hey @orSolocate that's OK with me.
I wonder can we close this one because what @DanielNoord said is an idea I've seen before on other similar issues where Pylint can't anticipate the custom coding patterns used by arbitrary 3rd party tools.

My understanding is that for this issue the solution is to use pylint-pytest; although the status of that project is unclear.

[DanielNoord]

I agree @mbyrnepr2 (with my own earlier statements 😬).
This is code that is too difficult to understand for pylint normally, so it should be handled by a plug-in.

I have started working on my own pytest plugin and this will probably be fixed by it. I'm waiting on confirmation that the current pytest plug-in is unmaintained to start working on it again. I'll come back to this issue when there is any development. For now I think this can stay open as it helps direct users with pytest related issues.

[C0DK]

although the status of that project is unclear.

Since the last status on this issue, https://github.com/reverbc/pylint-pytest has been archived, and would probably require a new maintainer for that to be the solution.

However, I do see the very valid point that pytest simply isn't done in a pythonic way and a plugin has to fix the issue. I'm hoping someone will pick up the torch, as I'd love to both use pytest as well as pylint.

[Pierre-Sassoulas]

Relevant discussion here: PyCQA/meta#56