Update bugbear

[henryiii]

Looks like bugbear has new rules:

• B028: No explicit stacklevel keyword argument found. The warn method from the warnings module uses a stacklevel of 1 by default. This will only show a stack trace for the line on which the warn method is called. It is therefore recommended to use a stacklevel of 2 or greater to provide more information to the user.
• B029: Using except (): with an empty tuple does not handle/catch anything. Add exceptions to handle.
• B030: Except handlers should only be exception classes or tuples of exception classes.
• B031: Using the generator returned from itertools.groupby() more than once will do nothing on the second usage. Save the result to a list if the result is needed multiple times.
• B032: Possible unintentional type annotation (using :). Did you mean to assign (using =)?

Also, some of the B9 rules are nice if opted into explicitly and seem to be missing (B90x except B901 (Python 2 related) & B907 which is nice but poorly implemented in bugbear) - only B904 is present.

I don't actually need these for something specific, just noticed this due to an incorrect noqa and thought a tracking issue would be useful.

Opinionated checks:

• B901: Using return x in a generator function. (Somewhat bad reasoning in description, talks about Python 2, see comments below.)
• B902: Invalid first argument used for method.
• B903: Use collections.namedtuple (or typing.NamedTuple) for data classes that only set attributes in an init method, and do nothing else. (Probably should include dataclasses recommendation? NamedTuple injects extra tuple methods and is meant for backward compat, not a data class replacement. That's dataclasses nowadays.)
• B906: visit_ function with no further call to a visit function.

[charliermarsh]

(Good rules if anyone's looking for a first issue :))

[charliermarsh]

B029 implemented in #3068.

[charliermarsh]

B032 implemented in #3085.

[sjdemartini]

Would it be possible to add B901 to the checklist above? (I'm not sure I understand the original caveat about it in the description.)

I haven't contributed to ruff before (or used Rust), but might be interested in trying. That rule is the only one keeping me from migrating to ruff from flake8/flake8-bugbear. Thanks!

[henryiii]

B901 is really a Python 2 compatibly rule, so I'd assume it is really low priority (and when Python 2 is gone, there's no need to outlaw perfectly valid Python 3 syntax for compatibility with Python 2 anymore).

[sjdemartini]

@henryiii Hm, I don't think that fully captures the utility of the rule. It's valid syntax in Python 3 as you say, but like most of the B9 opinionated rules in bugbear, the point is that the syntax (return x within a generator) can be misused and lead to difficult-to-discover bugs. Here's one example (all python 3):

from collections.abc import Iterable
from pathlib import Path

def get_file_paths(file_types: Iterable[str] | None = None) -> Iterable[Path]:
    dir_path = Path('.')
    if file_types is None:
        return dir_path.glob("*")

    for file_type in file_types:
        yield from dir_path.glob(f"*.{file_type}")

While one might assume that the above will give an iterable of all paths in the directory if file_types is None, instead it passes the returned argument to StopIteration, so it yields no results:

>>> list(get_file_paths(file_types=["cfg", "toml"]))
[PosixPath('.bumpversion.cfg'),
 PosixPath('setup.cfg'),
 PosixPath('pyproject.toml')]
>>> list(get_file_paths())
[]

B901 catches this, since the syntax/behavior that the author of the code likely intended was:

def get_file_paths_fixed(file_types: Iterable[str] | None = None) -> Iterable[Path]:
    dir_path = Path('.')
    if file_types is None:
        yield from dir_path.glob("*")
    else:
        for file_type in file_types:
            yield from dir_path.glob(f"*.{file_type}")

which works:

>>> list(get_file_paths_fixed())
[PosixPath('.pylintrc'),
 PosixPath('.DS_Store'),
 PosixPath('pyproject.toml'),
 PosixPath('.bumpversion.cfg'),
 PosixPath('tests'),
 PosixPath('README.md'),
 ...]

[henryiii]

Thanks for the explanation - then the original explanation for B001 isn't ideal. I've added the opinionated rules above, including B001. B007 would actually be nice too if Ruff was smart enough to only include it if the target Python version was high enough - flake8 & bugbear can't do this, as they don't have a target Python setting like Ruff has.

[sjdemartini]

Thanks for discussing; agreed that the bugbear README's B901 explanation isn't great. One note: looks like you meant B901-B907 in your latest comment and edits to the checklist (rather than B001-B007, which are actually already implemented in Ruff).

[aacunningham]

Not sure where best to discuss it, but I've been looking at B030. How closely do we need to implement the solution from the base package? I looked over bugbear's code for B030 and maybe found a bug in it(?), so not sure whether to implement the bug or try to make ruff a little smarter than that (with big emphasis on little smarter).

[dhruvmanila]

I'll take up the last rule (B031) 🚀

[sjdemartini]

@charliermarsh it looks like this issue was closed prematurely with the merging of the B031 rule PR. There are still 4 opinionated rules listed in the description which haven't yet been implemented.

[charliermarsh]

Let's open a new issue for those. To me the goal here was to regain parity based on bugbear updates.

I will mention that the challenge with including opinionated rules is that they'll be enabled by default by any users that have select = ["B"] in their configuration, unlike in flake8, where plugins are turned on implicitly by way of being installed in the environment (and so users have to explicitly enable the B900 rules, since they're not part of the flake8-bugbear defaults -- at least, that's my understanding). We don't really have a way to require users to explicitly opt-in to a rule beyond opting in to a plugin. So, in some contexts, this will feel like a divergence from flake8-bugbear.

For me, right now that means potentially being a bit more picky about which of the opinionated rules we include...

[sjdemartini]

Sounds good, and that makes sense 👍 I opened a separate issue to track the opinionated rules here #3758

[henryiii]

IIUC, Flake8 makes all X9** opt-in only, so you have to do B,B9 to get both the base and B9xx rules. (It's possible this is configurable by the plugin, and it just happens that most of them use X9xx, but I think it's automatic?). If so, then maybe a way forward in Ruff would be to duplicate that behavior - B would not select B9xx, and you'd have to specify B9 or (better) the individual codes to get them? I feel like, even it wasn't automatic (which, given there are no Ruff plugins, wouldn't be needed), there's likely enough in the current config system to split B and B9?