Repository navigation
False positive: B018 #3831
Description
Ruff 0.0.260
Reproducer:
(sphinx) PS I:\Development\sphinx> pytest -vvv bug_b018.py
========================================================================================================= test session starts =========================================================================================================
platform win32 -- Python 3.11.0, pytest-7.2.2, pluggy-1.0.0 -- I:\Mambaforge\envs\sphinx\python.exe
cachedir: .pytest_cache
rootdir: I:\Development\sphinx, configfile: pyproject.toml
plugins: xdist-3.2.0
collected 1 item
bug_b018.py::test_attribute_error PASSED [100%]
========================================================================================================== 1 passed in 0.01s ==========================================================================================================
(sphinx) PS I:\Development\sphinx> type bug_b018.py
import pytest
def test_attribute_error():
obj = object()
with pytest.raises(AttributeError):
obj.nonexistent
(sphinx) PS I:\Development\sphinx> ruff --isolated --show-source --select B018 bug_b018.py
bug_b018.py:7:9: B018 Found useless expression. Either assign it to a variable or remove it.
|
7 | obj.nonexistent
| ^^^^^^^^^^^^^^^ B018
|
Found 1 error.
(sphinx) PS I:\Development\sphinx>
I think that this may be related to #3453 and #3455, as this is a regression from 0.0.259 to 0.0.260.
In tests it can be useful to check that an attribute does not exist, or some behaviour relating to side effects on accessing an attribute. Perhaps if unittest or pytest context managers are detected, B018 should not be raised?
Thanks,
Adam
Reacted by Arne de Laat, Manon Marchand and Pierre Sassoulas
Activity
charliermarsh commentedon Mar 31, 2023
Yeah I considered not flagging any expressions that use attributes or subscript accesses as "useless". It would've limited the rule significantly, but it also would help us avoid a class of false-positives...
leiserfg commentedon Apr 1, 2023
I will prefer if it stays as is, it is better having to add a
noqathan having a dangling attribute reference, that either means it's a non-required expression or is a descriptor with secondary effects. The first one should be deleted, the second one should be avoided and both require human intervention.AA-Turner commentedon Apr 1, 2023
I would tend to agree in the general case, my specific request was that:
Would you agree with this, specifically in a testing context?
Thanks,
Adam
charliermarsh commentedon Apr 2, 2023
I think the request is reasonable but I'm hesitant to act on it. I see it as similarly to how we handle unused imports within
ModuleNotFoundError-like try-except blocks. We don't ignore them, since it can be surprising to users and hard to understand the intent explicitly. But we do have a custom message that points users in the right direction for a more preferable pattern.In this case, my preference would be to encourage a pattern like:
To me, that signals the intent that the variable is intentionally unused (and IIRC we should avoid flagging that as an unused local).
spaceone commentedon Jun 10, 2023
_is often used as gettext translation variable.leiserfg commentedon Jun 10, 2023
It can be any var matching "dummy-variable-rgx" like
_noqa = obj.nonexistent.lafrech commentedon Mar 27, 2024
I came here with the same issue and I believe the underscore suggestion is a nice solution.
charliermarsh commentedon Mar 27, 2024
Closing this for now as we don't intend to change it, and I recently added the underscore suggestion to the docs (but may not be released yet, can't remember off-hand).