refactor: Fix boolean traps in signatures #287
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the Ruff rule FBT002, which detects positional boolean arguments in signatures that have a default value.
The affected function parameters are all changed to be keyword-only, so that we don't have hard-to-understand function invocations in the codebase wherein some positional bool decides the behavior of something, i.e.:
When it should be:
Adding this rule has highlighted that I am way too fond of this pattern, and I should be more mindful of how I write functions in the future. Does every function need some switch that enabled/disables some behavior? Looking at the codebase, probably not. These boolean toggles are rarely/never used.
FBT001 & FBT003
We cannot activate the FBT001 and FBT003 rules yet, since we use a lot of bool annotated positional args, as well as positional bool defaults for Typer options in commands. We need to migrate to the
Annotated
approach in order to use these without adding a bunch of# noqa
comments all over the codebase (which is not worth it for essentially 0 gain).