From f03e7edb877782fa0d57fd3a24592c9631784116 Mon Sep 17 00:00:00 2001 From: Kai Harder Date: Mon, 2 Mar 2026 07:36:29 +0000 Subject: [PATCH 1/3] Improve B018 to include useless calls Closes #546 --- README.rst | 13 +++--- bugbear.py | 50 ++++++++++++++++------ tests/eval_files/b018_calls.py | 24 +++++++++++ tests/eval_files/b023.py | 78 +++++++++++++++++----------------- 4 files changed, 107 insertions(+), 58 deletions(-) create mode 100644 tests/eval_files/b018_calls.py diff --git a/README.rst b/README.rst index 8ace44a..6371e6f 100644 --- a/README.rst +++ b/README.rst @@ -173,6 +173,7 @@ using ``pytest.raises``), or use the context manager form with a target .. _B018: **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``. 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. @@ -292,16 +293,16 @@ second usage. Save the result to a list if the result is needed multiple times. .. _B042: -**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. -Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. -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 -`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. -Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. +**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. +Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. +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 +`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. +Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. If you define `__str__/__reduce__` in super classes this check is unable to detect it, and we advise disabling it. .. _B043: -**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``. +**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``. There is no additional safety in using ``delattr`` if you know the attribute name ahead of time. diff --git a/bugbear.py b/bugbear.py index 3c7e217..1e9d228 100644 --- a/bugbear.py +++ b/bugbear.py @@ -42,6 +42,22 @@ ast.GeneratorExp, ) FUNCTION_NODES = (ast.AsyncFunctionDef, ast.FunctionDef, ast.Lambda) +FUNCTIONS_WITHOUT_SIDE_EFFECTS = ( + "all", + "any", + "dict", + "frozenset", + "isinstance", + "issubclass", + "len", + "max", + "min", + "repr", + "set", + "sorted", + "str", + "tuple", +) B908_pytest_functions = {"raises", "warns"} B908_unittest_methods = { "assertRaises", @@ -1466,19 +1482,27 @@ def check_for_b903(self, node: ast.ClassDef) -> None: def check_for_b018(self, node: ast.AST) -> None: if not isinstance(node, ast.Expr): return - if isinstance( - node.value, - ( - ast.List, - ast.Set, - ast.Dict, - ast.Tuple, - ), - ) or ( - isinstance(node.value, ast.Constant) - and ( - isinstance(node.value.value, (int, float, complex, bytes, bool)) - or node.value.value is None + if ( + isinstance( + node.value, + ( + ast.List, + ast.Set, + ast.Dict, + ast.Tuple, + ), + ) + or ( + isinstance(node.value, ast.Constant) + and ( + isinstance(node.value.value, (int, float, complex, bytes, bool)) + or node.value.value is None + ) + ) + or ( + isinstance(node.value, ast.Call) + and isinstance(node.value.func, ast.Name) + and node.value.func.id in FUNCTIONS_WITHOUT_SIDE_EFFECTS ) ): self.add_error("B018", node, node.value.__class__.__name__) diff --git a/tests/eval_files/b018_calls.py b/tests/eval_files/b018_calls.py new file mode 100644 index 0000000..1882264 --- /dev/null +++ b/tests/eval_files/b018_calls.py @@ -0,0 +1,24 @@ +""" +Should emit: +B018 - on lines 11-24 +""" + +something() +assert issubclass(object, int) +x = sorted(foo) + +# calls to be found +all() # B018: 0, "Call" +any() # B018: 0, "Call" +dict() # B018: 0, "Call" +frozenset() # B018: 0, "Call" +isinstance() # B018: 0, "Call" +issubclass() # B018: 0, "Call" +len() # B018: 0, "Call" +max() # B018: 0, "Call" +min() # B018: 0, "Call" +repr() # B018: 0, "Call" +set() # B018: 0, "Call" +sorted() # B018: 0, "Call" +str() # B018: 0, "Call" +tuple() # B018: 0, "Call" diff --git a/tests/eval_files/b023.py b/tests/eval_files/b023.py index e4fff45..2e4b9c4 100644 --- a/tests/eval_files/b023.py +++ b/tests/eval_files/b023.py @@ -111,50 +111,50 @@ def myfunc(x): # argument or in a consumed `filter()` (even if a comprehension is better style) for x in range(2): # It's not a complete get-out-of-linting-free construct - these should fail: - min([None, lambda: x], key=repr) # B023: 23, "x" - sorted([None, lambda: x], key=repr) # B023: 26, "x" - any(filter(bool, [None, lambda: x])) # B023: 36, "x" - list(filter(bool, [None, lambda: x])) # B023: 37, "x" - all(reduce(bool, [None, lambda: x])) # B023: 36, "x" + _ = min([None, lambda: x], key=repr) # B023: 27, "x" + _ = sorted([None, lambda: x], key=repr) # B023: 30, "x" + _ = any(filter(bool, [None, lambda: x])) # B023: 40, "x" + _ = list(filter(bool, [None, lambda: x])) # B023: 41, "x" + _ = all(reduce(bool, [None, lambda: x])) # B023: 40, "x" # But all these ones should be OK: - min(range(3), key=lambda y: x * y) - max(range(3), key=lambda y: x * y) - sorted(range(3), key=lambda y: x * y) - - any(map(lambda y: x < y, range(3))) - all(map(lambda y: x < y, range(3))) - set(map(lambda y: x < y, range(3))) - list(map(lambda y: x < y, range(3))) - tuple(map(lambda y: x < y, range(3))) - sorted(map(lambda y: x < y, range(3))) - frozenset(map(lambda y: x < y, range(3))) - - any(filter(lambda y: x < y, range(3))) - all(filter(lambda y: x < y, range(3))) - set(filter(lambda y: x < y, range(3))) - list(filter(lambda y: x < y, range(3))) - tuple(filter(lambda y: x < y, range(3))) - sorted(filter(lambda y: x < y, range(3))) - frozenset(filter(lambda y: x < y, range(3))) - - any(reduce(lambda y: x | y, range(3))) - all(reduce(lambda y: x | y, range(3))) - set(reduce(lambda y: x | y, range(3))) - list(reduce(lambda y: x | y, range(3))) - tuple(reduce(lambda y: x | y, range(3))) - sorted(reduce(lambda y: x | y, range(3))) - frozenset(reduce(lambda y: x | y, range(3))) + _ = min(range(3), key=lambda y: x * y) + _ = max(range(3), key=lambda y: x * y) + _ = sorted(range(3), key=lambda y: x * y) + + _ = any(map(lambda y: x < y, range(3))) + _ = all(map(lambda y: x < y, range(3))) + _ = set(map(lambda y: x < y, range(3))) + _ = list(map(lambda y: x < y, range(3))) + _ = tuple(map(lambda y: x < y, range(3))) + _ = sorted(map(lambda y: x < y, range(3))) + _ = frozenset(map(lambda y: x < y, range(3))) + + _ = any(filter(lambda y: x < y, range(3))) + _ = all(filter(lambda y: x < y, range(3))) + _ = set(filter(lambda y: x < y, range(3))) + _ = list(filter(lambda y: x < y, range(3))) + _ = tuple(filter(lambda y: x < y, range(3))) + _ = sorted(filter(lambda y: x < y, range(3))) + _ = frozenset(filter(lambda y: x < y, range(3))) + + _ = any(reduce(lambda y: x | y, range(3))) + _ = all(reduce(lambda y: x | y, range(3))) + _ = set(reduce(lambda y: x | y, range(3))) + _ = list(reduce(lambda y: x | y, range(3))) + _ = tuple(reduce(lambda y: x | y, range(3))) + _ = sorted(reduce(lambda y: x | y, range(3))) + _ = frozenset(reduce(lambda y: x | y, range(3))) import functools - any(functools.reduce(lambda y: x | y, range(3))) - all(functools.reduce(lambda y: x | y, range(3))) - set(functools.reduce(lambda y: x | y, range(3))) - list(functools.reduce(lambda y: x | y, range(3))) - tuple(functools.reduce(lambda y: x | y, range(3))) - sorted(functools.reduce(lambda y: x | y, range(3))) - frozenset(functools.reduce(lambda y: x | y, range(3))) + _ = any(functools.reduce(lambda y: x | y, range(3))) + _ = all(functools.reduce(lambda y: x | y, range(3))) + _ = set(functools.reduce(lambda y: x | y, range(3))) + _ = list(functools.reduce(lambda y: x | y, range(3))) + _ = tuple(functools.reduce(lambda y: x | y, range(3))) + _ = sorted(functools.reduce(lambda y: x | y, range(3))) + _ = frozenset(functools.reduce(lambda y: x | y, range(3))) # OK because the lambda which references a loop variable is defined in a `return` From 5f965d87e4fc2177b8a560986bc912bd6cb37c93 Mon Sep 17 00:00:00 2001 From: Kai Harder Date: Mon, 2 Mar 2026 07:38:24 +0000 Subject: [PATCH 2/3] Fix --- README.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index 6371e6f..b3235ba 100644 --- a/README.rst +++ b/README.rst @@ -293,16 +293,16 @@ second usage. Save the result to a list if the result is needed multiple times. .. _B042: -**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. -Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. -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 -`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. -Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. +**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. +Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. +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 +`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. +Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. If you define `__str__/__reduce__` in super classes this check is unable to detect it, and we advise disabling it. .. _B043: -**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``. +**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``. There is no additional safety in using ``delattr`` if you know the attribute name ahead of time. From f0cf49428ee772eda3ccba591c5b94f580499990 Mon Sep 17 00:00:00 2001 From: Kai Harder Date: Tue, 3 Mar 2026 06:32:16 +0000 Subject: [PATCH 3/3] Add changelog --- README.rst | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/README.rst b/README.rst index b3235ba..d8cfd18 100644 --- a/README.rst +++ b/README.rst @@ -293,16 +293,16 @@ second usage. Save the result to a list if the result is needed multiple times. .. _B042: -**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. -Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. -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 -`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. -Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. +**B042**: Exception classes with a custom `__init__` should pass all args to `super().__init__()` to work correctly with `copy.copy` and `pickle`. +Both `BaseException.__reduce__` and `BaseException.__str__` rely on the `args` attribute being set correctly, which is set in `BaseException.__new__` and `BaseException.__init__`. +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 +`BaseException.__new__` and `BaseException.__init__` ignore. It's also important that `__init__` not accept any keyword-only parameters. +Alternately you can define both `__str__` and `__reduce__` to bypass the need for correct handling of `args`. If you define `__str__/__reduce__` in super classes this check is unable to detect it, and we advise disabling it. .. _B043: -**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``. +**B043**: Do not call ``delattr(x, 'attr')``, instead use ``del x.attr``. There is no additional safety in using ``delattr`` if you know the attribute name ahead of time. @@ -495,6 +495,11 @@ MIT Change Log ---------- +UNRELEASED +~~~~~~~~~~ + +* B018: handle also useless calls such as `isinstance(x, int)` without assigning or using the result + 25.11.29 ~~~~~~~~