Page MenuHomePhabricator

[lint] Find function calls in default arguments
ClosedPublic

Authored by PiRK on Fri, Nov 7, 15:55.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC21d397cf14e4: [lint] Find function calls in default arguments
Summary

Replace the existing python mutable linter with a ruff check.

The new check is much more complete than the old one, is checks more than just lists, sets or dict literals. In particular, it also detects function calls whose result will be used as a default argument. In python, the function would be executed only once at function definition time, which in itself might be confusing, but then the returned mutable arg could be changed and affect future calls.

Fix the detected infractions by either:

  • making the arg optional and default to None, and defining it in the body of the function if None
  • making the arg non-optional if all callsites already specify it

This is a backport of core#30553

See core#30543 for a particularly nasty example of bug that this check should prevent.

Depends on D18906

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK published this revision for review.Fri, Nov 7, 16:06
PiRK added inline comments.
electrum/electrumabc_gui/qt/main_window.py
2268 ↗(On Diff #56519)

never called without specifying a title

test/functional/abc-mempool-coherence-on-activations.py
103 ↗(On Diff #56519)

these and the next few are never called without specifying a tx, so we can make the tx mandatory

test/functional/test_framework/avatools.py
488 ↗(On Diff #56519)

this is evidence that the previous linter didn't do its job.

test/functional/test_framework/chronik/token_tx.py
15 ↗(On Diff #56519)

this is evidence that the previous linter didn't do its job.

PiRK retitled this revision from lint: Find function calls in default arguments to [lint] Find function calls in default arguments.Fri, Nov 7, 16:06
This revision is now accepted and ready to land.Mon, Nov 10, 09:49