2003412 - Fix ruff 'warning-rules' warnings and upgrade them to errors

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Alex Hochheiden [:ahochheiden] [he/him]]

When adding new lints, various warnings/categories were downgraded from errors to warnings. This was because in some cases, there's thousands of violations and it was too time consuming to fix them all. We made a sacrifice on some rules (downgrading them to warnings) so we could get others as errors.

Ideally, most/all of these warnings should be errors. This task is to burn down the list of warning-rules so that we may one day be in the ideal state.

With the use of tools like Claude, we should be able to automate most of this.

Comment 1

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix UP008 warnings: Use super() call instead of super(__class__, self) r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/super-call-with-parameters/

Comment 2

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix UP028 warnings: Use yield from instead of yield in for loop r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/yield-in-for-loop/

Comment 3

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix UP030 warnings: Use implicit references for positional format fields r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/format-literals/

Comment 4

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Update `tools/lint/test/test_ruff.py::test_lint_fix_withotu_warning[ruff]` to account for PLR rule auto fixes r?sylvestre,emilio

With PLR rules now enabled as errors, auto-fix removes the useless
return statement (PLR1711), so we expect 3 fixes instead of 2.

Comment 5

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLR1714 warnings: Use in operator for repeated equality comparisons r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/repeated-equality-comparison/

Comment 6

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLR1722 warnings: Use sys.exit() instead of exit()/quit() r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/sys-exit-alias/

Comment 7

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLR1704 warnings: Avoid redefining argument with local variable r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/redefined-argument-from-local/

Comment 8

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLR0133 warnings: Remove comparison of constant with itself r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/comparison-of-constant/

Comment 9

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLR1730 warnings: Replace if statement with min()/max() call r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/if-stmt-min-max/

Comment 10

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - PLW warnings auto-fixed by `./mach lint --fix .` r?sylvestre,emilio

Comment 11

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW0128 warnings: Remove redeclared variable in assignment r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/redeclared-assigned-name/

Comment 12

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW0642 warnings: Avoid reassigning self/cls variable in method r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/self-or-cls-assignment/

Comment 13

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW0711 warnings: Use tuple for multiple exception types r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/binary-op-exception/

Comment 14

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW0127 warnings: Remove self-assigning variable r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/self-assigning-variable/

Comment 15

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW1510 warnings: Add explicit check argument to subprocess.run r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/subprocess-run-without-check/

Comment 16

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW1508 warnings: Use string default for os.getenv r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/invalid-envvar-default/

Comment 17

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW0211 warnings: Remove self/cls as first argument in staticmethod r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/bad-staticmethod-argument/

Comment 18

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW0602 warnings: Remove unnecessary global declaration r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/global-variable-not-assigned/

Comment 19

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLW1641 warnings: Implement __hash__ for classes defining __eq__ r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/eq-without-hash/

Comment 20

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Fix PLR5501 warnings: Use elif instead of else with nested if r?sylvestre,emilio

https://docs.astral.sh/ruff/rules/collapsible-else-if/

Comment 21

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - Upgrade fixed warnings to errors r?ahal,sylvestre

Comment 22

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2003412 - PLR warnings auto-fixed by `./mach lint --fix .` r?sylvestre,emilio

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ed6dda592a18
https://hg.mozilla.org/integration/autoland/rev/4e953d3f1017
Fix UP008 warnings: Use super() call instead of super(__class__, self) r=emilio,perftest-reviewers,mozperftest-reviewers,search-reviewers,sessionstore-reviewers,places-reviewers,win-reviewers,dom-storage-reviewers,toolkit-telemetry-reviewers,layout-reviewers,dom-worker-reviewers,jdescottes,emz,jari,gstoll,scunnane,sparky,nsharpley
https://github.com/mozilla-firefox/firefox/commit/306d5da462a3
https://hg.mozilla.org/integration/autoland/rev/f30257261397
Fix UP028 warnings: Use yield from instead of yield in for loop r=emilio,mozperftest-reviewers,toolkit-telemetry-reviewers,sparky
https://github.com/mozilla-firefox/firefox/commit/2bb010077c16
https://hg.mozilla.org/integration/autoland/rev/6114fca8f0f0
Fix UP030 warnings: Use implicit references for positional format fields r=emilio,perftest-reviewers,jdescottes,sparky
https://github.com/mozilla-firefox/firefox/commit/7054a1ab49dd
https://hg.mozilla.org/integration/autoland/rev/ea036cd9dc4c
PLR warnings auto-fixed by `./mach lint --fix .` r=emilio,geckoview-reviewers,taskgraph-reviewers,releng-reviewers,toolkit-telemetry-reviewers,layout-reviewers,jdescottes,bhearsum,nalexander,perftest-reviewers,sparky
https://github.com/mozilla-firefox/firefox/commit/54c38b7e364b
https://hg.mozilla.org/integration/autoland/rev/61b4cd1506d5
Update `tools/lint/test/test_ruff.py::test_lint_fix_withotu_warning[ruff]` to account for PLR rule auto fixes r=emilio
https://github.com/mozilla-firefox/firefox/commit/9ce7ca3a62c6
https://hg.mozilla.org/integration/autoland/rev/a267ad2b09e1
Fix PLR1714 warnings: Use in operator for repeated equality comparisons r=emilio,mozperftest-reviewers,geckoview-reviewers,taskgraph-reviewers,android-reviewers,layout-reviewers,jdescottes,bhearsum,tcampbell,sparky
https://github.com/mozilla-firefox/firefox/commit/7cc77b1400dc
https://hg.mozilla.org/integration/autoland/rev/d06254b1a30d
Fix PLR1722 warnings: Use sys.exit() instead of exit()/quit() r=emilio,media-playback-reviewers,android-reviewers,padenot,tcampbell
https://github.com/mozilla-firefox/firefox/commit/040998e094c0
https://hg.mozilla.org/integration/autoland/rev/ff85eadd0dd1
Fix PLR1704 warnings: Avoid redefining argument with local variable r=emilio,perftest-reviewers,mozperftest-reviewers,taskgraph-reviewers,releng-reviewers,toolkit-telemetry-reviewers,jdescottes,bhearsum,sparky
https://github.com/mozilla-firefox/firefox/commit/4a9f17033408
https://hg.mozilla.org/integration/autoland/rev/2da9e5b17aae
Fix PLR0133 warnings: Remove comparison of constant with itself r=emilio
https://github.com/mozilla-firefox/firefox/commit/866d02765cd0
https://hg.mozilla.org/integration/autoland/rev/eb523d20a7ad
Fix PLR1730 warnings: Replace if statement with min()/max() call r=emilio,perftest-reviewers,sparky
https://github.com/mozilla-firefox/firefox/commit/74bd1f11812b
https://hg.mozilla.org/integration/autoland/rev/22255f7b9eb9
PLW warnings auto-fixed by `./mach lint --fix .` r=emilio,taskgraph-reviewers,releng-reviewers,ahal,whimboo
https://github.com/mozilla-firefox/firefox/commit/13cf285f677f
https://hg.mozilla.org/integration/autoland/rev/b6793acb31c4
Fix PLW0128 warnings: Remove redeclared variable in assignment r=emilio,tcampbell,geckoview-reviewers
https://github.com/mozilla-firefox/firefox/commit/bfd038a11385
https://hg.mozilla.org/integration/autoland/rev/195f5c54493b
Fix PLW0642 warnings: Avoid reassigning self/cls variable in method r=emilio
https://github.com/mozilla-firefox/firefox/commit/cb7d8ba09ab2
https://hg.mozilla.org/integration/autoland/rev/89d61e421573
Fix PLW0711 warnings: Use tuple for multiple exception types r=emilio
https://github.com/mozilla-firefox/firefox/commit/fa8e7abb4ad3
https://hg.mozilla.org/integration/autoland/rev/78f5953c0330
Fix PLW0127 warnings: Remove self-assigning variable r=emilio
https://github.com/mozilla-firefox/firefox/commit/4cd9582b8ef9
https://hg.mozilla.org/integration/autoland/rev/8ebe55eb0f53
Fix PLW1510 warnings: Add explicit check argument to subprocess.run r=emilio,perftest-reviewers,mozperftest-reviewers,media-playback-reviewers,android-reviewers,jdescottes,padenot,sparky,calu
https://github.com/mozilla-firefox/firefox/commit/c936a9217dfd
https://hg.mozilla.org/integration/autoland/rev/f18da5a3e155
Fix PLW1508 warnings: Use string default for os.getenv r=emilio,android-reviewers,jdescottes,tcampbell
https://github.com/mozilla-firefox/firefox/commit/672ac9af9d28
https://hg.mozilla.org/integration/autoland/rev/a8342d803801
Fix PLW0211 warnings: Remove self/cls as first argument in staticmethod r=emilio
https://github.com/mozilla-firefox/firefox/commit/491e058e06f6
https://hg.mozilla.org/integration/autoland/rev/861c705c5dd8
Fix PLW0602 warnings: Remove unnecessary global declaration r=emilio,perftest-reviewers,geckoview-reviewers,tcampbell,sparky
https://github.com/mozilla-firefox/firefox/commit/659f75038e4e
https://hg.mozilla.org/integration/autoland/rev/834a0cc175cd
Fix PLR5501 warnings: Use elif instead of else with nested if r=emilio,taskgraph-reviewers,ahal
https://github.com/mozilla-firefox/firefox/commit/b430d53c71cb
https://hg.mozilla.org/integration/autoland/rev/fe26fd151122
Upgrade fixed warnings to errors r=ahal

Comment 24

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/4e953d3f1017
https://hg.mozilla.org/mozilla-central/rev/f30257261397
https://hg.mozilla.org/mozilla-central/rev/6114fca8f0f0
https://hg.mozilla.org/mozilla-central/rev/ea036cd9dc4c
https://hg.mozilla.org/mozilla-central/rev/61b4cd1506d5
https://hg.mozilla.org/mozilla-central/rev/a267ad2b09e1
https://hg.mozilla.org/mozilla-central/rev/d06254b1a30d
https://hg.mozilla.org/mozilla-central/rev/ff85eadd0dd1
https://hg.mozilla.org/mozilla-central/rev/2da9e5b17aae
https://hg.mozilla.org/mozilla-central/rev/eb523d20a7ad
https://hg.mozilla.org/mozilla-central/rev/22255f7b9eb9
https://hg.mozilla.org/mozilla-central/rev/b6793acb31c4
https://hg.mozilla.org/mozilla-central/rev/195f5c54493b
https://hg.mozilla.org/mozilla-central/rev/89d61e421573
https://hg.mozilla.org/mozilla-central/rev/78f5953c0330
https://hg.mozilla.org/mozilla-central/rev/8ebe55eb0f53
https://hg.mozilla.org/mozilla-central/rev/f18da5a3e155
https://hg.mozilla.org/mozilla-central/rev/a8342d803801
https://hg.mozilla.org/mozilla-central/rev/861c705c5dd8
https://hg.mozilla.org/mozilla-central/rev/834a0cc175cd
https://hg.mozilla.org/mozilla-central/rev/fe26fd151122