Implement flake8-bugbear

[charliermarsh]

This is a big set of rules but flake8-bugbear is extremely popular (~1.5M downloads per month).

[tgross35]

Link to bugbear warnings for reference: https://github.com/PyCQA/flake8-bugbear#list-of-warnings

B001, B002, B006, B007, B008, B017, B023, and B904 are the most useful for catching real bugs imho, out of the warnings that are not yet implemented.

B950 covers line length - it is not really necessary since ruff is already designed to work with black. However, it may be good thing to mention somewhere since black mentions configuring it explicitly.

(awesome project by the way!)

[charliermarsh]

Cool, those are all pretty straightforward! Will prioritize them over the others.

[harupy]

I'm working on B006 implementation.

[tgross35]

Quick checklist for convenience:

• B001: bare except
• B002: ++ -> +=
• B003: os.environ assigning
• B004: hasattr(x, '__call__') to test if x is callable
• B005: .strip() with multi-character strings
• B006: mutable data structures for argument defaults
• B007: Loop control variable not used within the loop body
• B008: function calls in argument defaults
• B009: getattr(x, 'attr') -> x.attr
• B010: setattr(x, 'attr', val) -> x.attr = val
• B011: assert False calls
• B012: break, continue or return inside finally blocks
• B013: length-one tuple literal is redundant
• B014: Redundant exception types in except (Exception, TypeError)
• B015: Pointless comparison
• B016: Cannot raise a literal
• B017: self.assertRaises(Exception)
• B018: Found useless expression
• B019: Use of functools.lru_cache or functools.cache on methods
• B020: Loop control variable overrides iterable it iterates
• B021: f-string used as docstring
• B022: arguments passed to contextlib.suppress
• B023: functions defined inside a loop using variables redefined in the loop
• B024: Abstract base class with no abstract method
• B025: try-except block with duplicate exceptions
• B026: Star-arg unpacking after a keyword argument is strongly discouraged
• B027: Empty method in abstract base class

Opinionated:

• B901: return x in a generator function
• B902: invalid first argument for method. Use self for instance methods, and cls for class methods
• B903: Use collections.namedtuple (or typing.NamedTuple) for data classes that only set attributes in an init method
• B904: Within an except clause, raise exceptions with raise ... from err or raise ... from None
• B950: Line too long, generous option

[charliermarsh]

That's awesome, thank you for collating it!

I'm tentatively marking B001 and B950 as complete, since IIUC B001 is the same as E722, and B950 is close enough to E501 that we may not support it independently (I know they're not exactly the same).

[charliermarsh]

Just updated the list as @harupy is making awesome progress here. We're now at 50% coverage.

[grillazz]

@charliermarsh for B008 is there any simple way to setup extend-immutable-calls = fastapi.Depends, fastapi.params.Depends ?

[edgarrmondragon]

@grillazz It's rather straightforward to add. I'm working on it. PR is #706 🙂

[grillazz]

Test 0.0.120 and problem solved. Thx

[charliermarsh]

@harupy is dangerously close to implementing the entire plugin ;)

[harupy]

Nice to see a task list with lots of ✅ :)

[charliermarsh]

I think if we implement B023, we should feel comfortable closing this.

[harupy]

I think if we implement B023, we should feel comfortable closing this.

Agreed! Feel free to implement it if anyone wants to :)

[charliermarsh]

@harupy - Sounds good, I can give it a try!

[harupy]

Awesome, thank you!

[harupy]

Is B902 covered by N804 and N805 in pep8-namiing?

[charliermarsh]

@harupy - Yeah I believe so.

[g-as]

B905 was just added:

B905: zip() without an explicit strict= parameter set. strict=True causes the resulting iterator to raise a ValueError if the arguments are exhausted at differing lengths. The strict= argument was added in Python 3.10, so don't enable this flag for code that should work on <3.10. For more information: https://peps.python.org/pep-0618/

[charliermarsh]

@g-as -- Added in #1122.

[SRv6d]

@charliermarsh Are there still plans on supporting B950 ? Enforcing a line length while at the same time giving some headroom is often a pretty good compromise between sane line length and unnecessarily ugly code resulting from a hard limit.

[charliermarsh]

We could support it. I was a bit hesitant but we could. Can you talk me through the rationale of using B950 vs. just configuring E501 to use a 10% increase? I think they're exactly equivalent, so I'm guessing it's more of a philosophical thing?

[SRv6d]

I wouldn't call it philosophical, it mostly comes down to personal preference and experience. I think a line length of 88 is a great middle ground, but personally find my code to often be right around that or slightly over. Reformatting those lines of code just because they are a couple of characters over the limit often looks silly and increasing the limit by 10% we are getting close to 100 which results in an entirely different looking codebase just for a couple of outliers.

[charliermarsh]

Sorry -- didn't intend "philosophical" to come off as pejorative so apologies if it did. Was mostly trying to confirm that, functionally, using B950 with line-length of 88 is identical to using E501 with a line-length of 96.8, right?

Assuming that's correct, is the difference here that if you treat 88 as the line length (when in reality, the enforced limit is 96.8), then folks set that as the line-length in their editors, tend to format around it, Black gets configured with 88, and so on?

[SRv6d]

No offense taken whatsoever. The latter is exactly what I meant to say.

[SRv6d]

@charliermarsh Would you be open to implementing B950 ? It's the only thing missing for me coming from flake8. I could give it a try, but I have no rust experience (yet?).

[jamesbraza]

@SRv6d to make E501 behave like B950 I am just using this:

[tool.ruff]

# Line length to use when enforcing long-lines violations (like `E501`).
line-length = 97  # ceil(1.1 * 88) makes `E501` equivalent to `B950`