When upgrading, some false positives were found for B023 (implimented in https://github.com/PyCQA/flake8-bugbear/pull/265).

Example

"""B023.py"""
def filter_values(values: list[list[int]], max_percentage: float):
    for value in values:
        filter_val = max(value) * max_percentage
        yield list(filter(lambda x: x < filter_val, value))

$ flake8 B023.py
bugbear_me.py:4:41: B023 Function definition does not bind loop variable 'filter_val' is valid, and doesn't fall into the 

A silly example here, but one where the use of filter_val is valid.

cc @Zac-HD In the PR you mentioned some hard to detect false positives that you were okay with. Was this kind of pattern one of the ones you had in mind?

Thanks for this check btw, I think it's a good idea. I've fallen for this in the past!

Similar situation to #20, there are conflicts across codes B301-B306.

https://github.com/openstack/bandit:

The following tests were discovered and loaded:
  ...
  B301  pickle
  B302  marshal
  B303  md5
  B304  ciphers
  B305  cipher_modes
  B306  mktemp_q

• https://github.com/tylerwince/flake8-bandit is "a flake8 wrapper around [bandit]"

In my situation:

• When both are installed, bandit is still available while bugbear is deactivated
• If I uninstall bandit, bugbear is activated and works as expected

Newly updated bugbear 21.4.1 fails with AttributeError:

21:06:51 Traceback (most recent call last):
21:06:51   File "/usr/lib/python3.6/multiprocessing/pool.py", line 119, in worker
21:06:51     result = (True, func(*args, **kwds))
21:06:51   File "/usr/lib/python3.6/multiprocessing/pool.py", line 44, in mapstar
21:06:51     return list(map(*args))
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/flake8/checker.py", line 676, in _run_checks
21:06:51     return checker.run_checks()
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/flake8/checker.py", line 589, in run_checks
21:06:51     self.run_ast_checks()
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/flake8/checker.py", line 494, in run_ast_checks
21:06:51     for (line_number, offset, text, _) in runner:
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 36, in run
21:06:51     visitor.visit(self.tree)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 165, in visit
21:06:51     super().visit(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 253, in visit
21:06:51     return visitor(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 261, in generic_visit
21:06:51     self.visit(item)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 165, in visit
21:06:51     super().visit(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 253, in visit
21:06:51     return visitor(node)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 306, in visit_ClassDef
21:06:51     self.generic_visit(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 261, in generic_visit
21:06:51     self.visit(item)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 165, in visit
21:06:51     super().visit(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 253, in visit
21:06:51     return visitor(node)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 302, in visit_FunctionDef
21:06:51     self.generic_visit(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 261, in generic_visit
21:06:51     self.visit(item)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 165, in visit
21:06:51     super().visit(node)
21:06:51   File "/usr/lib/python3.6/ast.py", line 253, in visit
21:06:51     return visitor(node)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 321, in visit_With
21:06:51     self.check_for_b017(node)
21:06:51   File "/src/.tox/flake8-py36/lib/python3.6/site-packages/bugbear.py", line 442, in check_for_b017
21:06:51     hasattr(item_context.func, "attr")
21:06:51 AttributeError: 'Name' object has no attribute 'func'
21:06:51 """

See https://integration.wikimedia.org/ci/job/pywikibot-core-tox-docker/14952/console

Python 3 allows mixing of return-with-value and yield in the same function. I got bitten by this behavior multiple times, so I want to write a flake8 plugin for it. I wonder if I should add the lint to this package instead of making a separate one. What do you think?

Having an inline attribute doc string or a module variable docstring, it is wrongly marked as B018 (sample from sphinx doc):

module_level_variable2 = 98765
"""int: Module level variable documented inline.

The docstring may span multiple lines. The type may optionally be specified
on the first line, separated by a colon.
"""

B001 and Flake8/pycodestyle E722 both check for bare_except, but B001 misses a lot of cases that E722 finds.

We noticed this when we tried running an internal tool that looks for overlaps in checks -- we are writing some of our own and don't want to overlap -- and found that every time B001 fires E722 also fires; but the reverse is not true.

Tool output: B001 (6786) <-> E722 (34093): 13572 occurred at same line+path B001 implies E722: 100% E722 implies B001: 19%

I took a look at the implementations for B001 and E722 and found that bugbear uses the AST and E722 uses a regex:

Bugbear implementation uses AST.

def visit_ExceptHandler(self, node):
    if node.type is None:
        self.errors.append(B001(node.lineno, node.col_offset))
    self.generic_visit(node)

Flake8 implementation uses regex.

def bare_except(logical_line, noqa):
    if noqa:
        return

    regex = re.compile(r"except\s*:")
    match = regex.match(logical_line)
    if match:
        yield match.start(), "E722 do not use bare 'except'"

From the implementation, it looks like B001 and E722 should hit the same locations every time.

We have a platform that lets us run static analysis over a bunch of open source repositories at the same time, so I ran vanilla flake8 and bugbear at the same time to see if there was a pattern, but one wasn't immediately obvious.

I thought this might be related to forgetting to call visit or something like that (I've been bitten by that before!) but the reason for this disparity wasn't clear to me... so I'm making this issue. Feel free to reach out to me if you have any other questions!

Here are some examples:

As described in #217, using functools.cache/functools.lru_cache on class methods can potentially cause their instances to live longer than expected.

I took a stab at adding a rule to catch this pattern but I have a few questions:

1. flake8-bugbear's error messages generally try to propose a solution to make the linting error go away (besides ignoring the line), but I'm not sure about the verbiage to include. For reference, the starting point is the following:

Use of functools.lru_cache or functools.cache on class methods can lead to memory leaks. The cache may retain instance references, preventing garbage collection.

2. Should this be restricted to caches that are explicitly unbounded, rather than any use of a built-in cache with a class method? i.e. this would only raise for functools.cache and functools.lru_cache(maxsize=None). This seems to more specifically address the issue though some still may not want to potentially keep these instances alive, bounded cache or not.

Related reading:

• https://bugs.python.org/issue19859
• https://bugs.python.org/issue44310
• https://docs.python.org/3/faq/programming.html#how-do-i-cache-method-calls

Resolves: #217 Resolves: #215 Resolves: #212 Resolves: #221

Closes #180, by adding a new check ~~B018~~ B904 to recommend exception chaining. For example,

try:
    assert False
except AssertionError:
    raise RuntimeError  # B904: Within an except clause, use `raise ... from err` ...

This turned out to be remarkably easy to add to the existing code 😁

Instead of

try:
    do_something()
except RuntimeError:
    pass

suggest using

with contextlib.suppress(RuntimeError):
    do_something()

What do you think?

Resolves #190. Introduces a simple version of context management, though more detailed analysis might need something a little bit specialized in the future (e.g stuff like this).

I haven't been able to make the recently released feature extend-immutable-calls to work.

Have I configured it wrong?

I have verified that flake8 recognizes the config, by setting the line-length to 10 and verified flake8 warnings about line length.

flake8 --version

4.0.1 (flake8-bugbear: 21.11.29, mccabe: 0.6.1, pycodestyle: 2.8.0, pyflakes: 2.4.0) CPython 3.10.0 on
Darwin

example.py

import fastapi


def example_fn(data: str = fastapi.Header(None)):
    pass

setup.cfg

[flake8]
max-line-length = 120
extend-immutable-calls = ["fastapi.Header"]

flake8 --show-source

./example.py:4:28: B008 Do not perform function calls in argument defaults.  The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call.  If this is intended, assign the function call to a module-level variable and use that variable as a default value.
def example_fn(data: str = fastapi.Header(None)):
                           ^

I am suggesting a new check to recommend replacing defaultdict(int) with Counter(), both of which are from the standard library collections, due to the former having a major gotcha with memory usage.

Consider this test case:

from collections import Counter
from collections import defaultdict

# Profiled using;
# $ mprof run defaultdict_leak.py && mprof plot
if True:
    # Apparent memory leak
    # Over 700MB using Python 3.9.13 on macOS
    # Over 900MB using Python 3.9.15 on Linux
    counts = defaultdict(int)
else:
    # Low and flat memory
    # about 16MB using Python 3.9.13 on macOS
    # about 19MB using Python 3.9.15 on Linux
    counts = Counter()

# Setup some sparse data
for x in [12,34,56,78,90]:
    counts[str(x)] = 123 + x

threshold = 100
excesses = 0
for x in range(int(1e7)):
    if threshold < counts[str(x)]:
        excesses += counts[str(x)]
print(f"Sum of entries exceeding the threshold {threshold} is {excesses}")

The apparent memory leak with defaultdict is due to the documented behaviour of recording the missing keys in the dictionary with the default value, https://docs.python.org/3/library/collections.html#collections.defaultdict says:

... provide a default value for the given key, this value is inserted in the dictionary for the key, and returned.

...

When each key is encountered for the first time, it is not already in the mapping; so an entry is automatically created using the default_factory ...

I assume I'm not the only person to have used defaultdict without appreciating this, and in the case of defaultdict(int) there is good alternative in Counter()

I've seen some code lately where a reminder about the !r format specifier would probably have helped - instead of displaying the repr, single-quotes were manually added outside the braces as in f"No value found at '{path}'". This OK, until path contains a single-quote character and then it can get confusing fast.

I'd therefore propose warning about this case, which is conveniently easy to reliably detect using the JoinedStr and FormattedValue nodes - within a JoinedStr, warn if there is a sequence of Constant/FormattedValue/Constant, where the first constant ends and the second starts with a single or double quote character, and the formatter value has no conversion code.

Separately, I used the regex ".*('\{[^}]+\}').*" to confirm that this is common enough to be worth linting for. It's not especially rare, and IMO pretty useful to teach people about format specifiers.

Consider the following code

dct = {
    'a': 1,
}
dct['b']: 2
print(dct)  # {'a': 1}

This is actually valid python syntax, because it is typing the 'b' entry as a 2 type. But we can assume the user meant to assign an element rather than type it (especially if there's no equal on this line).

(Recommended I post this to bugbear by pyflakes maintainers here: https://github.com/PyCQA/pyflakes/issues/756)

Short question on the reason behind newly added B027: If we have an ABC

from abc import ABC, abstractmethod

class MyABC(ABC):
    
    def method(self):
        """
        Some method that does nothing by default. Derived classes can choose to override it.
        """
        pass

    @abstractmethod
    def method2(self):
        """
        Another method. Derived classes MUST define it.
        """

B027 will raise on the first method for not being declared abstract.

Is there a way of allowing it to be empty despite the class being abstract? (Besides noqa and besides overriding them with an empty method in derived classes.) I would argue that there is reason to have empty methods provided by the ABC (interface) where the default implementation is to do nothing, but that derived classes can choose to override (e.g. if they need to perform some initialization / tear down of servers in my case), as opposed to methods that must be overridden.

Calling warnings.warn() without a stacklevel, or a stacklevel value of <2, is nearly always a mistake. Doing so flags the warning inside the called function, rather than the caller, making it hard to fix the warning.

flake8-bugbear could detect such invocations of warnings.warn().

This issue inspired by @asottile's video of the day: https://www.youtube.com/watch?v=CtFdXBEwYfk . This is also something I've seen recur as a review point in Django PR's.