pylint
pylint copied to clipboard
False Positive W0621: Redefining name %r from outer scope when using pytest fixtures
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
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
We should probably fix the crash, but the issue of handling pytest fixtures should probably be delegated to pytest-pylint.
We should probably fix the crash
I can take a look.
https://github.com/PyCQA/pylint/pull/6532
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
Related : https://github.com/PyCQA/pylint/issues/1535
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.
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.
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...
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?
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.
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.
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.
Relevant discussion here: https://github.com/PyCQA/meta/issues/56