Look into ruff rule RUF012

[DimitriPapadopoulos]

How would this feature be useful?

RUF012 checks for mutable default values in class attributes:

Mutable default values share state across all instances of the class, while not being obvious. This can lead to bugs when the attributes are changed in one instance, as those changes will unexpectedly affect all other instances.

$ ruff check --extend-select RUF --ignore RUF005
src/pipx/animate.py:24:20: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
src/pipx/pipx_metadata_file.py:40:28: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
src/pipx/pipx_metadata_file.py:41:29: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
src/pipx/pipx_metadata_file.py:42:44: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
src/pipx/pipx_metadata_file.py:43:56: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
Found 5 errors.
$ 

Introduced in #610:

pipx/src/pipx/animate.py

Lines 23 to 24 in 5eed479

	class _CursorInfo(ctypes.Structure):
	_fields_ = [("size", ctypes.c_int), ("visible", ctypes.c_byte)]

Introduced in #222:

pipx/src/pipx/pipx_metadata_file.py

Lines 29 to 44 in 5eed479

	class PackageInfo(NamedTuple):
	package: Optional[str]
	package_or_url: Optional[str]
	pip_args: List[str]
	include_dependencies: bool
	include_apps: bool
	apps: List[str]
	app_paths: List[Path]
	apps_of_dependencies: List[str]
	app_paths_of_dependencies: Dict[str, List[Path]]
	package_version: str
	man_pages: List[str] = []
	man_paths: List[Path] = []
	man_pages_of_dependencies: List[str] = []
	man_paths_of_dependencies: Dict[str, List[Path]] = {}
	suffix: str = ""

Describe the solution you'd like

As a first step, I'd like to understand whether this is on purpose or not.

I suspect the relevant classes are designed to have one instance, in which case this is not an issue in practice. Nevertheless, it's good practice to avoid mutable default values, so that future maintainers don't have to ask themselves again.

Describe alternatives you've considered

[chrysle]

I suspect the relevant classes are designed to have one instance, in which case this is not an issue in practice.

As for PackageInfo, there are several instances created for the different types of packages pipx recognizes (e.g. injected, main, ..). BTW, I think that should be made a dataclass.

Nevertheless, it's good practice to avoid mutable default values, so that future maintainers don't have to ask themselves again.

Agreed.

[huxuan]

I think it is actually a bug, we did not encounter problem mostly because pipx is a cli tool and most functions are only called once before exit.

BTW, I notice that you have proposed many ruff rules, just wonder whehter there is any recommended rule set? AFAIK, there is no preset.

[DimitriPapadopoulos]

While ruff enables the following set of rules by default, other rules are indeed useful:

By default, Ruff enables Flake8's F rules, along with a subset of the E rules, omitting any stylistic rules that overlap with the use of a formatter, like ruff format or Black.

It is hard to suggest a default set of rules. There is no wide enough consensus to balance the individual preferences of maintainers. I've seen maintainers revert from ruff to flake8 because ruff is not totally a drop-in replacement of flake8, and other maintainers embrace ruff and enable ALL rules except specific rules they disable.

The rules I have suggested so far reveal actual bugs or result undeniably(?) in more readable or faster code.

[huxuan]

The rules I have suggested so far reveal actual bugs or result undeniably(?) in more readable or faster code.

Do not get me wrong, I am trying to learn about a recommended rule set. :-)