Skip to content

Commit 4d54ecb

Browse files
authored
Improve B018 to include useless calls (#547)
* Improve B018 to include useless calls Closes #546 * Fix * Add changelog
1 parent 22aac1f commit 4d54ecb

File tree

4 files changed

+112
-58
lines changed

4 files changed

+112
-58
lines changed

README.rst

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ using ``pytest.raises``), or use the context manager form with a target
173173
.. _B018:
174174

175175
**B018**: Found useless expression. Either assign it to a variable or remove it.
176+
The check also considers function calls without side-effects such as ``isinstance``.
176177
Note that dangling commas will cause things to be interpreted as useless tuples.
177178
For example, in the statement ``print(".."),`` is the same as ``(print(".."),)``
178179
which is an unassigned tuple. Simply remove the comma to clear the error.
@@ -292,16 +293,16 @@ second usage. Save the result to a list if the result is needed multiple times.
292293

293294
.. _B042:
294295

295-
**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`.
296-
Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`.
297-
If you define `__init__` yourself without passing all arguments to `super().__init__` it is very easy to break pickling, especially if they pass keyword arguments which both
298-
`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters.
299-
Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`.
296+
**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`.
297+
Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`.
298+
If you define `__init__` yourself without passing all arguments to `super().__init__` it is very easy to break pickling, especially if they pass keyword arguments which both
299+
`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters.
300+
Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`.
300301
If you define `__str__/__reduce__` in super classes this check is unable to detect it, and we advise disabling it.
301302

302303
.. _B043:
303304

304-
**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``.
305+
**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``.
305306
There is no additional safety in using ``delattr`` if you know the attribute name ahead of time.
306307

307308

@@ -494,6 +495,11 @@ MIT
494495
Change Log
495496
----------
496497

498+
UNRELEASED
499+
~~~~~~~~~~
500+
501+
* B018: handle also useless calls such as `isinstance(x, int)` without assigning or using the result
502+
497503
25.11.29
498504
~~~~~~~~
499505

bugbear.py

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,22 @@
4242
ast.GeneratorExp,
4343
)
4444
FUNCTION_NODES = (ast.AsyncFunctionDef, ast.FunctionDef, ast.Lambda)
45+
FUNCTIONS_WITHOUT_SIDE_EFFECTS = (
46+
"all",
47+
"any",
48+
"dict",
49+
"frozenset",
50+
"isinstance",
51+
"issubclass",
52+
"len",
53+
"max",
54+
"min",
55+
"repr",
56+
"set",
57+
"sorted",
58+
"str",
59+
"tuple",
60+
)
4561
B908_pytest_functions = {"raises", "warns"}
4662
B908_unittest_methods = {
4763
"assertRaises",
@@ -1466,19 +1482,27 @@ def check_for_b903(self, node: ast.ClassDef) -> None:
14661482
def check_for_b018(self, node: ast.AST) -> None:
14671483
if not isinstance(node, ast.Expr):
14681484
return
1469-
if isinstance(
1470-
node.value,
1471-
(
1472-
ast.List,
1473-
ast.Set,
1474-
ast.Dict,
1475-
ast.Tuple,
1476-
),
1477-
) or (
1478-
isinstance(node.value, ast.Constant)
1479-
and (
1480-
isinstance(node.value.value, (int, float, complex, bytes, bool))
1481-
or node.value.value is None
1485+
if (
1486+
isinstance(
1487+
node.value,
1488+
(
1489+
ast.List,
1490+
ast.Set,
1491+
ast.Dict,
1492+
ast.Tuple,
1493+
),
1494+
)
1495+
or (
1496+
isinstance(node.value, ast.Constant)
1497+
and (
1498+
isinstance(node.value.value, (int, float, complex, bytes, bool))
1499+
or node.value.value is None
1500+
)
1501+
)
1502+
or (
1503+
isinstance(node.value, ast.Call)
1504+
and isinstance(node.value.func, ast.Name)
1505+
and node.value.func.id in FUNCTIONS_WITHOUT_SIDE_EFFECTS
14821506
)
14831507
):
14841508
self.add_error("B018", node, node.value.__class__.__name__)

tests/eval_files/b018_calls.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
"""
2+
Should emit:
3+
B018 - on lines 11-24
4+
"""
5+
6+
something()
7+
assert issubclass(object, int)
8+
x = sorted(foo)
9+
10+
# calls to be found
11+
all() # B018: 0, "Call"
12+
any() # B018: 0, "Call"
13+
dict() # B018: 0, "Call"
14+
frozenset() # B018: 0, "Call"
15+
isinstance() # B018: 0, "Call"
16+
issubclass() # B018: 0, "Call"
17+
len() # B018: 0, "Call"
18+
max() # B018: 0, "Call"
19+
min() # B018: 0, "Call"
20+
repr() # B018: 0, "Call"
21+
set() # B018: 0, "Call"
22+
sorted() # B018: 0, "Call"
23+
str() # B018: 0, "Call"
24+
tuple() # B018: 0, "Call"

tests/eval_files/b023.py

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -111,50 +111,50 @@ def myfunc(x):
111111
# argument or in a consumed `filter()` (even if a comprehension is better style)
112112
for x in range(2):
113113
# It's not a complete get-out-of-linting-free construct - these should fail:
114-
min([None, lambda: x], key=repr) # B023: 23, "x"
115-
sorted([None, lambda: x], key=repr) # B023: 26, "x"
116-
any(filter(bool, [None, lambda: x])) # B023: 36, "x"
117-
list(filter(bool, [None, lambda: x])) # B023: 37, "x"
118-
all(reduce(bool, [None, lambda: x])) # B023: 36, "x"
114+
_ = min([None, lambda: x], key=repr) # B023: 27, "x"
115+
_ = sorted([None, lambda: x], key=repr) # B023: 30, "x"
116+
_ = any(filter(bool, [None, lambda: x])) # B023: 40, "x"
117+
_ = list(filter(bool, [None, lambda: x])) # B023: 41, "x"
118+
_ = all(reduce(bool, [None, lambda: x])) # B023: 40, "x"
119119

120120
# But all these ones should be OK:
121-
min(range(3), key=lambda y: x * y)
122-
max(range(3), key=lambda y: x * y)
123-
sorted(range(3), key=lambda y: x * y)
124-
125-
any(map(lambda y: x < y, range(3)))
126-
all(map(lambda y: x < y, range(3)))
127-
set(map(lambda y: x < y, range(3)))
128-
list(map(lambda y: x < y, range(3)))
129-
tuple(map(lambda y: x < y, range(3)))
130-
sorted(map(lambda y: x < y, range(3)))
131-
frozenset(map(lambda y: x < y, range(3)))
132-
133-
any(filter(lambda y: x < y, range(3)))
134-
all(filter(lambda y: x < y, range(3)))
135-
set(filter(lambda y: x < y, range(3)))
136-
list(filter(lambda y: x < y, range(3)))
137-
tuple(filter(lambda y: x < y, range(3)))
138-
sorted(filter(lambda y: x < y, range(3)))
139-
frozenset(filter(lambda y: x < y, range(3)))
140-
141-
any(reduce(lambda y: x | y, range(3)))
142-
all(reduce(lambda y: x | y, range(3)))
143-
set(reduce(lambda y: x | y, range(3)))
144-
list(reduce(lambda y: x | y, range(3)))
145-
tuple(reduce(lambda y: x | y, range(3)))
146-
sorted(reduce(lambda y: x | y, range(3)))
147-
frozenset(reduce(lambda y: x | y, range(3)))
121+
_ = min(range(3), key=lambda y: x * y)
122+
_ = max(range(3), key=lambda y: x * y)
123+
_ = sorted(range(3), key=lambda y: x * y)
124+
125+
_ = any(map(lambda y: x < y, range(3)))
126+
_ = all(map(lambda y: x < y, range(3)))
127+
_ = set(map(lambda y: x < y, range(3)))
128+
_ = list(map(lambda y: x < y, range(3)))
129+
_ = tuple(map(lambda y: x < y, range(3)))
130+
_ = sorted(map(lambda y: x < y, range(3)))
131+
_ = frozenset(map(lambda y: x < y, range(3)))
132+
133+
_ = any(filter(lambda y: x < y, range(3)))
134+
_ = all(filter(lambda y: x < y, range(3)))
135+
_ = set(filter(lambda y: x < y, range(3)))
136+
_ = list(filter(lambda y: x < y, range(3)))
137+
_ = tuple(filter(lambda y: x < y, range(3)))
138+
_ = sorted(filter(lambda y: x < y, range(3)))
139+
_ = frozenset(filter(lambda y: x < y, range(3)))
140+
141+
_ = any(reduce(lambda y: x | y, range(3)))
142+
_ = all(reduce(lambda y: x | y, range(3)))
143+
_ = set(reduce(lambda y: x | y, range(3)))
144+
_ = list(reduce(lambda y: x | y, range(3)))
145+
_ = tuple(reduce(lambda y: x | y, range(3)))
146+
_ = sorted(reduce(lambda y: x | y, range(3)))
147+
_ = frozenset(reduce(lambda y: x | y, range(3)))
148148

149149
import functools
150150

151-
any(functools.reduce(lambda y: x | y, range(3)))
152-
all(functools.reduce(lambda y: x | y, range(3)))
153-
set(functools.reduce(lambda y: x | y, range(3)))
154-
list(functools.reduce(lambda y: x | y, range(3)))
155-
tuple(functools.reduce(lambda y: x | y, range(3)))
156-
sorted(functools.reduce(lambda y: x | y, range(3)))
157-
frozenset(functools.reduce(lambda y: x | y, range(3)))
151+
_ = any(functools.reduce(lambda y: x | y, range(3)))
152+
_ = all(functools.reduce(lambda y: x | y, range(3)))
153+
_ = set(functools.reduce(lambda y: x | y, range(3)))
154+
_ = list(functools.reduce(lambda y: x | y, range(3)))
155+
_ = tuple(functools.reduce(lambda y: x | y, range(3)))
156+
_ = sorted(functools.reduce(lambda y: x | y, range(3)))
157+
_ = frozenset(functools.reduce(lambda y: x | y, range(3)))
158158

159159

160160
# OK because the lambda which references a loop variable is defined in a `return`

0 commit comments

Comments
 (0)