Skip to content

[py] Adopt selected ruff rule families surfaced by ruff 0.16 #17989

Description

@titusfortner

Background

Ruff 0.16 expanded its default rule set from 59 rules to 413. Because py/pyproject.toml
only set extend-select, the project silently inherited that new default and
bazel run //py:ruff-check reported 192 errors on an unchanged tree.

PR #17937 resolved the immediate breakage by changing extend-select to select, so the
enforced set is now stated explicitly rather than inherited. That was deliberately a
no-op on enforcement: the resolved rule set is identical to what ruff 0.15.21 enforced
(206 rules), plus 3 new RUF rules the project already opts into and passes.

This issue tracks adopting the parts of the new default that are actually worth having,
deliberately and in tranches, rather than all at once via a dependency bump.

What the 192 findings look like

Family Count Notes
B (bugbear) 35 High value, but 17 of 19 B018 hits are a false positive (see below)
BLE (blind-except) 30 Almost all deliberate best-effort cleanup
SIM (simplify) 28 Mostly cosmetic; SIM115 is worthwhile
PLW (pylint warn) 16 Contains 3 real bugs
S (bandit) 15 14 of 15 are S110 try/except/pass
TRY 11 Opinionated exception style
FURB 10 Modernization, mostly auto-fixable
PYI 9 PYI041 redundant-numeric-union is real
PIE 8 Contains a real bug (PIE796)
EXE 6 Shebang / executable-bit mismatches
C4 (comprehensions) 6 Mechanical and safe
G (logging-format) 5 G201 is reasonable
PLR 4 All assert 1 == 0 "should have thrown" idioms
DTZ 4 All naive datetime.now() used for elapsed-time deltas
PERF 2 Trivial and real
W, PLE, FLY 3 One each, all worth fixing

Recommendation

Tranche 1 — adopt now (~37 findings, mostly auto-fixable)

W, PLE, FLY, PERF, C4, EXE, PIE, FURB, PLR

Low noise, high signal, and largely --fix-able. PLE and PIE each catch a genuine
bug (see the companion bugs issue). PLR's four hits are all assert 1 == 0, "should have thrown an exception", which should become pytest.raises regardless.

Tranche 2 — adopt with scoping (~44 findings)

B, PYI

B is the highest-value family here (B006, B008, B015, B017 all flag real
problems), but it needs one carve-out: 17 of the 19 B018 "useless expression"
hits are alert.text inside a with pytest.raises(...) block
— the idiomatic way to
assert that a property access raises. Adopting B without ignoring B018 under
py/test/** would mean sprinkling _ = alert.text, which is strictly worse. Suggest:

[tool.ruff.lint.per-file-ignores]
"py/test/**" = ["B018"]

Tranche 3 — discuss (~44 findings)

SIM, TRY, G

Defensible either way. SIM115 (context manager for file opens) is the one with real
substance; the rest is style. Worth a maintainer opinion before spending the churn.

Decline for now (~45 findings)

BLE (30), S (15), DTZ (4)

  • BLE001 and S110 are the same story: broad except / try-except-pass in browser
    teardown and test cleanup, where swallowing is usually deliberate. Adopting means 45
    individual judgment calls plus permanent # noqa churn, for little signal.
  • All four DTZ005 hits are datetime.now() used to measure elapsed time in
    w3c_interaction_tests.py. Naive datetimes are correct for deltas; the rule is
    aimed at a problem this code doesn't have.

Note on mechanics

Adopting a family means adding it to select in py/pyproject.toml. Keeping select
(rather than reverting to extend-select) is what prevents the next ruff release from
re-introducing this whole situation.

Cross-binding impact

None. This is Python lint configuration only.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

C-pyPython BindingsI-cleanupSomething needs to be tidied

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions