ruff icon indicating copy to clipboard operation
ruff copied to clipboard

Implement more flake8-bugbear opinionated rules

Open sjdemartini opened this issue 1 year ago • 12 comments

Picking up where https://github.com/charliermarsh/ruff/issues/2954 left off, there were a few opinionated (B9xx flake8-bugbear rules) checks left to be implemented in Ruff:

  • [x] B901: Using return x in a generator function. (Somewhat bad reasoning in flake8-bugbear description, talks about Python 2, see comment here https://github.com/charliermarsh/ruff/issues/2954#issuecomment-1441162976 around its utility.)
  • [x] B902: Implemented as N804 and N805.
  • [ ] 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.~
  • [ ] B907: Consider replacing f"'{foo}'" with f"{foo!r}" which is both easier to read and will escape quotes inside foo if that would appear.
  • [x] B908: Partly implemented as PT012.

There's an open question on how these should be included, since it would deviate from flake8-bugbear to have these on by default just by turning on the rest of the bugbear rules (see comment https://github.com/charliermarsh/ruff/issues/2954#issuecomment-1483594606).

There's also one outstanding non-opinionated rule:

  • [x] B036: Found except BaseException: without re-raising (no raise in the top-level of the except block). This catches all kinds of things (Exception, SystemExit, KeyboardInterrupt...) and may prevent a program from exiting as expected. Implemented as BLE001.
  • [x] B038: Renamed to B909 in bugbear; implemented as B909 is Ruff.

sjdemartini avatar Mar 27 '23 16:03 sjdemartini

B906 is flake8 plugins specific and has a false positive when the visit function is visiting an astroid node in a pylint plugin. It would be nice to not have this FP in ruff :)

Pierre-Sassoulas avatar Mar 29 '23 11:03 Pierre-Sassoulas

Since then some new rule apppeared

  • [ ] B907: (previous B028) Consider replacing f"'{foo}'" with f"{foo!r}" which is both easier to read and will escape quotes inside foo if that would appear. The check tries to filter out any format specs that are invalid together with !r. If you're using other conversion flags then e.g. f"'{foo!a}'" can be replaced with f"{ascii(foo)!r}". Not currently implemented for python<3.8 or str.format() calls.
  • [ ] B908: (looks like PT012 to me): Contexts with exceptions assertions like with self.assertRaises or with pytest.raises should not have multiple top-level statements. Each statement should be in its own context. That way, the test ensures that the exception is raised only in the exact statement where you expect it.

mikaelarguedas avatar Jan 06 '24 17:01 mikaelarguedas

Good first issues for anyone interested :)

charliermarsh avatar Jan 06 '24 19:01 charliermarsh

Another 2 rules appeared in flake8-bugbear:

  • [x] B037: Found return <value>, yield, yield <value>, or yield from <value> in class __init__() method. No values should be returned or yielded, only bare returns are ok. (PLE0100)

  • [ ] B038: Found a mutation of a mutable loop iterable inside the loop body. Changes to the iterable of a loop such as calls to list.remove() or via del can cause unintended bugs. https://github.com/astral-sh/ruff/issues/9511

mikaelarguedas avatar Jan 14 '24 10:01 mikaelarguedas

B037 is already implemented as PLE0100, @charliermarsh maybe we should consider migrating PLE0100 to B037 though?

Skylion007 avatar Jan 14 '24 16:01 Skylion007

@Skylion007 - Yes good call -- I prefer indexing under bugbear over Pylint since it's more popular for Ruff users. I'll add a note to the 0.2.0 release list.

charliermarsh avatar Jan 14 '24 17:01 charliermarsh

@zanieb Was this closed on purpose ?

I'm under the impression that some rules listed here are not part of ruff yet: B036, B038, B901, B902, B903, B907, B908

mikaelarguedas avatar Feb 02 '24 05:02 mikaelarguedas

@mikaelarguedas - I think it was an oversight, but I'll leave it to @zanieb to confirm in the AM.

charliermarsh avatar Feb 02 '24 05:02 charliermarsh

I think B036 is covered/aliased by BLE001.

mscheifer avatar May 07 '24 22:05 mscheifer

@charliermarsh @zanieb friendly :bellhop_bell: if it was closed by mistake, is it possible to reopen and update according to current state ?

Current state: B036 -> covered by BLE001 B038 -> no implemented B901 -> no implemented B902 ->no implemented B903 ->no implemented B907 ->no implemented B908 ->no implemented

mikaelarguedas avatar May 08 '24 14:05 mikaelarguedas

Will update the list, thanks.

charliermarsh avatar May 08 '24 14:05 charliermarsh

Updated. We do actually have B038 (which bugbear moved to B909).

charliermarsh avatar May 08 '24 14:05 charliermarsh

This issue says that B902 is "Implemented as N804 and N805", but that is not accurate because N804 incorrectly says you should use cls for "metaclass class methods" (e.g. __new__, __prepare__). These methods are the metaclass-equivalent of class methods and get passed the metaclass itself as the first argument. flake8-bugbear recommends metacls, and pylint recommends mcs (which flake8-bugbear will also accept).

jnrbsn avatar Aug 23 '24 16:08 jnrbsn

Interesting task,can I help?

gagandeepp avatar Oct 07 '24 12:10 gagandeepp