diff --git a/README.rst b/README.rst index b73d85e..d3fdf88 100644 --- a/README.rst +++ b/README.rst @@ -175,10 +175,13 @@ using ``pytest.raises``), or use the context manager form with a target **B018**: Found useless expression. Either assign it to a variable or remove it. The check also considers function calls without side-effects such as ``isinstance``, -and unary and arithmetic operations such as ``-x`` or ``a * b``. Shifts, bitwise -operators and ``@`` are left alone, since libraries overload them for their side -effects, and so is an operation directly in a ``try`` or ``with`` block, where it may be -run only to see it raise. +unary and arithmetic operations such as ``-x`` or ``a * b``, generator expressions +(which are lazy, so their body never runs) and lambdas (which nothing can call), as +long as what they evaluate right away doesn't call a function, like +``(x for x in f())`` or ``lambda x=f(): x``. Shifts, bitwise operators and ``@`` are +left alone, since libraries overload them for their side effects, and so is an +operation, a generator expression or a lambda with defaults directly in a ``try`` or +``with`` block, where it may be run only to see it raise. Note that dangling commas will cause things to be interpreted as useless tuples. For example, in the statement ``print(".."),`` is the same as ``(print(".."),)`` which is an unassigned tuple. Simply remove the comma to clear the error. @@ -597,6 +600,9 @@ UNRELEASED ``for k in d: d.pop(k)`` * B012: don't flag ``break`` or ``continue`` in an ``async for`` loop inside a ``finally`` block, as for ``for`` and ``while`` loops +* B018: also report generator expressions and lambdas used as statements. A + statement like ``(print(x) for x in items)`` never runs its body, and a bare + ``lambda x: True`` is usually an assignment that lost its target (#452) 26.9.30 ~~~~~~~ diff --git a/bugbear.py b/bugbear.py index e9750cb..cb927a7 100644 --- a/bugbear.py +++ b/bugbear.py @@ -2403,11 +2403,34 @@ def check_for_b018(self, node: ast.AST) -> None: and isinstance(node.value.func, ast.Name) and node.value.func.id in FUNCTIONS_WITHOUT_SIDE_EFFECTS ) + # a lambda that is neither assigned nor passed anywhere is never called + or (isinstance(node.value, ast.Lambda) and not _lambda_defaults(node.value)) or ( - isinstance(node.value, (ast.UnaryOp, ast.BinOp)) - and _is_useless_operation(node.value) + ( + ( + isinstance(node.value, (ast.UnaryOp, ast.BinOp)) + and _is_useless_operation(node.value) + ) + # a generator expression is lazy, so its body never runs, but + # its first iterable is evaluated right away: `(x for x in f())` + # still calls `f` + or ( + isinstance(node.value, ast.GeneratorExp) + and _is_useless_operation(node.value.generators[0].iter) + ) + # and so are the defaults of a lambda: `lambda x=f(): x` + or ( + isinstance(node.value, ast.Lambda) + and all( + map(_is_useless_operation, _lambda_defaults(node.value)) + ) + ) + ) # directly in a `try` or `with` block, an operation is often run - # only to see it raise: `with pytest.raises(TypeError): "1" + 1` + # only to see it raise: `with pytest.raises(TypeError): "1" + 1`. + # The same goes for the first iterable of a generator expression, + # which `iter()` is called on, and the defaults of a lambda: + # `(x for x in 1)` or `lambda x=1 / 0: x` and not ( self.node_stack and isinstance(self.node_stack[-1], B018_RAISING_BLOCKS) @@ -3227,7 +3250,8 @@ def _is_useless_operation(node: ast.expr) -> bool: """Whether a unary or arithmetic operation, used as a statement, does nothing. Every binary operator in it has to be arithmetic, and no operand may call, - await, yield or assign: `f() + 1` still calls `f`. + await, yield or assign: `f() + 1` still calls `f`. This also checks what a + generator expression or a lambda evaluates right away. """ for child in ast.walk(node): if isinstance(child, ast.BinOp) and not isinstance( @@ -3241,6 +3265,11 @@ def _is_useless_operation(node: ast.expr) -> bool: return True +def _lambda_defaults(node: ast.Lambda) -> list[ast.expr]: + """The defaults of a lambda, which are evaluated when it is created.""" + return [d for d in node.args.defaults + node.args.kw_defaults if d is not None] + + def _dotted_name(node: ast.AST) -> str | None: """Return `"self.a.b"` for an attribute chain rooted in a name, else None.""" parts = [] diff --git a/tests/eval_files/b018_generators_lambdas.py b/tests/eval_files/b018_generators_lambdas.py new file mode 100644 index 0000000..96b2b7d --- /dev/null +++ b/tests/eval_files/b018_generators_lambdas.py @@ -0,0 +1,58 @@ +""" +Should emit: +B018 - on lines 11-19 and 58 +""" + + +def discarded(items, log, debug_func): + # a generator expression is lazy: as a statement, its body never runs (#452) + # and a lambda that isn't assigned or passed anywhere can never be called + log("start") + (print(item) for item in items) # B018: 4, "GeneratorExp" + (item.close() for item in items if item) # B018: 4, "GeneratorExp" + (x for x in items) # B018: 4, "GeneratorExp" + (y for x in items for y in log(x)) # B018: 4, "GeneratorExp" + lambda: log("done") # B018: 4, "Lambda" + lambda item, /, *rest, key=None, **kwargs: item # B018: 4, "Lambda" + lambda item=-1, *, key=len: item # B018: 4, "Lambda" + if not debug_func: + lambda x: True # B018: 8, "Lambda" + return debug_func + + +def used(items, log): + consume = list(print(item) for item in items) + log(lambda: "done") + total = sum(x for x in items) + callback = lambda: log("done") # noqa: E731 + [print(item) for item in items] + return consume, total, callback + + +def evaluated_right_away(record, refresh): + # the first iterable of a generator expression and the defaults of a lambda + # are evaluated right away, so a call in them still runs + (x for x in refresh()) + lambda arg=record(): None + lambda *, arg=record(): None + + +async def awaited_right_away(refresh, pending): + (x for x in await refresh()) + lambda arg=await pending: arg + + +def expected_to_raise(raises): + # directly in a `try` or `with` block, a generator expression or a lambda + # with defaults may be built only to see its first iterable or a default raise + with raises(TypeError): + (x for x in 1) + try: + (x for x in None) + except TypeError: + pass + with raises(ZeroDivisionError): + lambda arg=1 / 0: arg + # a lambda without defaults evaluates nothing, so it can't raise + with raises(ZeroDivisionError): + lambda: 1 / 0 # B018: 8, "Lambda" diff --git a/tests/eval_files/b023.py b/tests/eval_files/b023.py index ba40293..6d755c2 100644 --- a/tests/eval_files/b023.py +++ b/tests/eval_files/b023.py @@ -59,7 +59,7 @@ async def container_for_problems(): # Nested loops should not duplicate reports for j in range(2): for k in range(3): - lambda: j * k # error # B023: 16, "j" # B023: 20, "k" + lambda: j * k # error # B018: 8, "Lambda" # B023: 16, "j" # B023: 20, "k" for j, k, l in [(1, 2, 3)]: