diff --git a/.github/workflows/act-validation.yml b/.github/workflows/act-validation.yml index d5b157e..b982578 100644 --- a/.github/workflows/act-validation.yml +++ b/.github/workflows/act-validation.yml @@ -27,7 +27,7 @@ jobs: python-version: '3.13' - name: Install uv - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 - name: Install act diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index e9bdbf5..5a2094a 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -30,7 +30,7 @@ jobs: python-version: '3.13' - name: Install uv - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 - name: Audit dependencies diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86e2640..14e8360 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: python-version: '3.13' - name: Install uv - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 - name: Install CLI tools diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index febca53..8051a30 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -39,7 +39,7 @@ jobs: python-version: '3.13' - name: Install uv - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 - name: Generate coverage diff --git a/README.md b/README.md index b079065..5759337 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ ______________________________________________________________________ ## Features -Twelve pylint messages: +Thirteen pylint messages: - `prefer-structural-pattern-matching` (R9101) — `isinstance` dispatch on one subject should be a `match` statement with class patterns. @@ -98,7 +98,9 @@ Twelve pylint messages: - `prefer-snapshot-assertion` (R9108) and `prefer-snapshot-substring` (R9109) — tests asserting against large inline literals or repeatedly probing substrings should use a syrupy snapshot. -- `prefer-type-statement` (R9111) — module-level type aliases should use +- `prefer-slots-for-dataclass` (R9111) — closed standard-library dataclasses + should request generated slots or declare an explicit slot layout. +- `prefer-type-statement` (R9112) — module-level type aliases should use the PEP 695 `type` statement on a 3.12+ baseline. - `redundant-future-annotations` (C9112) — `from __future__ import annotations` should be removed on a 3.14+ baseline, where deferred evaluation is the diff --git a/df12_python_lints/__init__.py b/df12_python_lints/__init__.py index 18ea23a..71f87a9 100644 --- a/df12_python_lints/__init__.py +++ b/df12_python_lints/__init__.py @@ -1,7 +1,7 @@ """df12-python-lints: pylint checkers for df12 Python conventions. -The package is a pylint plugin. Loading it registers nine checkers -providing twelve messages: +The package is a pylint plugin. Loading it registers ten checkers +providing thirteen messages: - ``prefer-structural-pattern-matching`` (R9101) flags ``isinstance`` dispatch chains better expressed as ``match`` statements; @@ -22,7 +22,9 @@ - ``prefer-snapshot-assertion`` (R9108) with ``prefer-snapshot-substring`` (R9109) flag test assertions better expressed as syrupy snapshots; and -- ``prefer-type-statement`` (R9111) flags module-level type aliases +- ``prefer-slots-for-dataclass`` (R9111) flags closed standard-library + dataclasses that do not request or declare slots; +- ``prefer-type-statement`` (R9112) flags module-level type aliases better declared with the PEP 695 ``type`` statement, while ``redundant-future-annotations`` (C9112) flags ``from __future__ import annotations`` on a 3.14+ baseline; both respect pylint's @@ -46,6 +48,7 @@ from .assert_messages import AssertMessageChecker from .constant_chain import ConstantChainChecker +from .dataclass_slots import DataclassSlotsChecker from .future_annotations import FutureAnnotationsChecker from .match_dispatch import MatchDispatchChecker from .reexports import ReexportAssignmentChecker @@ -60,6 +63,7 @@ __all__ = [ "AssertMessageChecker", "ConstantChainChecker", + "DataclassSlotsChecker", "FutureAnnotationsChecker", "MatchDispatchChecker", "ReexportAssignmentChecker", @@ -83,6 +87,7 @@ def register(linter: PyLinter) -> None: linter.register_checker(MatchDispatchChecker(linter)) linter.register_checker(AssertMessageChecker(linter)) linter.register_checker(ConstantChainChecker(linter)) + linter.register_checker(DataclassSlotsChecker(linter)) linter.register_checker(TrivialWrapperChecker(linter)) linter.register_checker(ReexportAssignmentChecker(linter)) linter.register_checker(SuppressionCommentChecker(linter)) diff --git a/df12_python_lints/_dataclass_analysis.py b/df12_python_lints/_dataclass_analysis.py new file mode 100644 index 0000000..0e3d608 --- /dev/null +++ b/df12_python_lints/_dataclass_analysis.py @@ -0,0 +1,400 @@ +"""Decide when generated dataclass slots are safe and meaningful. + +The checker delegates conservative analysis here to avoid unsafe suggestions. +""" + +from __future__ import annotations + +import contextlib +import typing as typ + +from astroid import bases, exceptions, nodes + +from ._dataclass_decorators import ( + decorator_target, + expression_origin, + find_dataclass_decorator, + has_literal_slots, + subscript_target, +) +from ._dataclass_inference import ( + VARIABLE_LENGTH_BUILTIN_QNAMES, + Layout, + inferred_class, +) +from ._dataclass_state import ( + declared_instance_state, + has_declared_instance_fields, + has_local_slots, + local_slot_names, +) + +if typ.TYPE_CHECKING: + import collections.abc as cabc + +_MIN_MULTIPLE_BASES = 2 + + +def _direct_nodes(root: nodes.NodeNG) -> cabc.Iterator[nodes.NodeNG]: + """Yield descendants without entering nested executable scopes.""" + for child in root.get_children(): + if isinstance( + child, + (nodes.ClassDef, nodes.FunctionDef, nodes.AsyncFunctionDef, nodes.Lambda), + ): + continue + yield child + yield from _direct_nodes(child) + + +def _direct_methods(node: nodes.ClassDef) -> cabc.Iterator[nodes.FunctionDef]: + """Yield direct instance methods declared by *node*.""" + for statement in node.body: + if not isinstance(statement, (nodes.FunctionDef, nodes.AsyncFunctionDef)): + continue + if statement.type == "method" and statement.argnames(): + yield statement + + +def _decorated_with(method: nodes.FunctionDef, origin: str) -> bool: + """Return whether *method* has a decorator imported from *origin*.""" + return method.decorators is not None and any( + expression_origin(decorator_target(decorator)) == origin + for decorator in method.decorators.nodes + ) + + +def _is_unshadowed_builtin(node: nodes.Name, name: str) -> bool: + """Return whether *node* is the unshadowed builtin called *name*.""" + if node.name != name: + return False + _, assignments = node.lookup(name) + if len(assignments) != 1: + return False + assignment = assignments[0] + return isinstance(assignment, (nodes.ClassDef, nodes.FunctionDef)) and ( + assignment.qname() == f"builtins.{name}" + ) + + +def _is_instance_name(node: nodes.NodeNG, parameter: str) -> bool: + """Return whether *node* is the current method's instance parameter.""" + return isinstance(node, nodes.Name) and node.name == parameter + + +def _call_requires_dictionary(call: nodes.Call, parameter: str) -> bool: + """Return whether *call* demonstrates dynamic instance state.""" + match call: + case nodes.Call(func=nodes.Name() as function, args=[first, *_]) if ( + _is_instance_name(first, parameter) + ): + return any( + _is_unshadowed_builtin(function, name) + for name in ("vars", "setattr", "delattr") + ) + return False + + +def _object_setattr_is_open( + call: nodes.Call, parameter: str, declared_state: frozenset[str] +) -> bool: + """Return whether an ``object.__setattr__`` call names undeclared state.""" + match call: + case nodes.Call( + func=nodes.Attribute( + expr=nodes.Name() as object_name, + attrname="__setattr__", + ), + args=[instance, name_node, *_], + ) if _is_unshadowed_builtin(object_name, "object") and _is_instance_name( + instance, parameter + ): + pass + case _: + return False + return not ( + isinstance(name_node, nodes.Const) + and isinstance(name_node.value, str) + and name_node.value in declared_state + ) + + +def _is_instance_dictionary(node: nodes.NodeNG, parameter: str) -> bool: + """Return whether *node* reads the instance dictionary.""" + return ( + isinstance(node, nodes.Attribute) + and node.attrname == "__dict__" + and _is_instance_name(node.expr, parameter) + ) + + +def _attribute_mutation_is_open( + node: nodes.NodeNG, parameter: str, declared_state: frozenset[str] +) -> bool: + """Return whether *node* mutates an undeclared instance attribute.""" + if not isinstance(node, (nodes.AssignAttr, nodes.DelAttr)): + return False + return _is_instance_name(node.expr, parameter) and ( + node.attrname not in declared_state + ) + + +def _node_requires_open_state( + node: nodes.NodeNG, parameter: str, declared_state: frozenset[str] +) -> bool: + """Return whether one method descendant demonstrates open instance state.""" + if _is_instance_dictionary(node, parameter): + return True + if not isinstance(node, nodes.Call): + return _attribute_mutation_is_open(node, parameter, declared_state) + return any(( + _call_requires_dictionary(node, parameter), + _object_setattr_is_open(node, parameter, declared_state), + )) + + +def _method_requires_open_state( + method: nodes.FunctionDef, declared_state: frozenset[str] +) -> bool: + """Return whether one direct method visibly depends on open state.""" + parameter = method.argnames()[0] + if _decorated_with(method, "functools.cached_property"): + return True + return any( + _node_requires_open_state(child, parameter, declared_state) + for child in _direct_nodes(method) + ) + + +def _is_zero_argument_super(node: nodes.NodeNG) -> bool: + """Return whether *node* calls the unshadowed ``super`` without arguments.""" + match node: + case nodes.Call( + func=nodes.Name() as function, + args=[], + keywords=[], + ): + return _is_unshadowed_builtin(function, "super") + return False + + +def _uses_class_cell(method: nodes.FunctionDef) -> bool: + """Return whether *method*'s subtree uses a replacement-class hazard.""" + for child in method.nodes_of_class( + (nodes.Name, nodes.Call), skip_klass=nodes.ClassDef + ): + if isinstance(child, nodes.Name) and child.name == "__class__": + return True + if _is_zero_argument_super(child): + return True + return False + + +def _has_extension_base(node: nodes.ClassDef) -> bool: + """Return whether *node* directly names a known extension boundary.""" + return any( + expression_origin(subscript_target(base)) + in {"abc.ABC", "typing.Protocol", "typing_extensions.Protocol"} + for base in node.bases + ) + + +def _has_class_header_boundary(node: nodes.ClassDef) -> bool: + """Return whether explicit header configuration affects class creation.""" + if node.keywords: + return True + try: + return node.declared_metaclass() is not None + except exceptions.InferenceError: + return True + + +def _methods_have_class_hazard(methods: tuple[nodes.FunctionDef, ...]) -> bool: + """Return whether a direct method makes replacement-class slots unsafe.""" + return any( + _decorated_with(method, "abc.abstractmethod") or _uses_class_cell(method) + for method in methods + ) + + +def _methods_require_open_state( + methods: tuple[nodes.FunctionDef, ...], state: frozenset[str] +) -> bool: + """Return whether direct methods demonstrate deliberately open state.""" + return any(_method_requires_open_state(method, state) for method in methods) + + +def _has_unsafe_inner_decorator(node: nodes.ClassDef, decorator: nodes.NodeNG) -> bool: + """Return whether an inner decorator may retain the original class.""" + if node.decorators is None: + return True + decorator_index = node.decorators.nodes.index(decorator) + return any( + expression_origin(decorator_target(inner)) != "typing.final" + for inner in node.decorators.nodes[decorator_index + 1 :] + ) + + +def has_local_slots_hazard( + node: nodes.ClassDef, + decorator: nodes.NodeNG, + state_cache: dict[nodes.ClassDef, frozenset[str]] | None = None, +) -> bool: + """Return whether local class evidence makes generated slots unsafe.""" + methods = tuple(_direct_methods(node)) + checks = ( + lambda: _has_class_header_boundary(node), + lambda: _has_extension_base(node), + lambda: "__init_subclass__" in node.locals, + lambda: _methods_have_class_hazard(methods), + lambda: _methods_require_open_state( + methods, declared_instance_state(node, state_cache) + ), + ) + return any(check() for check in checks) or _has_unsafe_inner_decorator( + node, decorator + ) + + +def _local_dataclass_base( + base: nodes.NodeNG | bases.Proxy, module: nodes.Module +) -> nodes.ClassDef | None: + """Return a local dataclass named by *base*, when inference is unambiguous.""" + inferred = inferred_class(base) + if inferred is None or inferred.root() is not module: + return None + return inferred if find_dataclass_decorator(inferred) is not None else None + + +def _multiple_inheritance_dataclass_bases( + child: nodes.ClassDef, + module: nodes.Module, + base_layout: cabc.Callable[[nodes.NodeNG | bases.Proxy], Layout], +) -> tuple[nodes.ClassDef, ...]: + """Return local dataclass bases in a conflicting slot-layout shape.""" + if len(child.bases) < _MIN_MULTIPLE_BASES: + return () + layouts = tuple(base_layout(base) for base in child.bases) + if layouts.count(Layout.SLOTTED) <= 1: + return () + return tuple( + inferred + for base, layout in zip(child.bases, layouts, strict=True) + if layout is Layout.SLOTTED + and (inferred := _local_dataclass_base(base, module)) is not None + ) + + +class LayoutAnalyzer: + """Cache conservative layout and reverse-inheritance decisions per module.""" + + def __init__(self, module: nodes.Module) -> None: + """Build inheritance facts in two phases. + + ``_find_multiple_bases`` runs conflict-free; clear ``_eligibility`` after. + """ + self.module = module + self._eligibility: dict[nodes.ClassDef, bool] = {} + self._layouts: dict[nodes.ClassDef, Layout] = {} + self._declared_states: dict[nodes.ClassDef, frozenset[str]] = {} + self._visiting: set[nodes.ClassDef] = set() + self._multiple_bases: frozenset[nodes.ClassDef] = frozenset() + self._multiple_bases = self._find_multiple_bases() + self._eligibility.clear() + self._layouts.clear() + + def _find_multiple_bases(self) -> frozenset[nodes.ClassDef]: + """Find local dataclass bases used in direct multiple inheritance.""" + unsafe: set[nodes.ClassDef] = set() + for child in self.module.nodes_of_class(nodes.ClassDef): + unsafe.update( + _multiple_inheritance_dataclass_bases( + child, self.module, self._base_layout + ) + ) + return frozenset(unsafe) + + @staticmethod + def _field_layout(node: nodes.ClassDef) -> Layout: + """Return the layout contribution from *node*'s declared fields.""" + return Layout.SLOTTED if has_declared_instance_fields(node) else Layout.NEUTRAL + + def _inherited_layout(self, node: nodes.ClassDef) -> Layout: + """Combine the provable layout inherited from *node*'s direct bases.""" + layouts = tuple(self._base_layout(base) for base in node.bases) + if Layout.UNSAFE in layouts or layouts.count(Layout.SLOTTED) > 1: + return Layout.UNSAFE + return Layout.SLOTTED if Layout.SLOTTED in layouts else Layout.NEUTRAL + + def _local_layout(self, node: nodes.ClassDef) -> Layout: + """Classify a base declared in the linted module.""" + inherited_layout = self._inherited_layout(node) + if inherited_layout is Layout.UNSAFE: + return Layout.UNSAFE + if (slot_names := local_slot_names(node)) is not None: + if "__dict__" in slot_names: + return Layout.UNSAFE + own_layout = Layout.SLOTTED if slot_names else Layout.NEUTRAL + return max(inherited_layout, own_layout) + decorator = find_dataclass_decorator(node) + if decorator is None: + return Layout.UNSAFE + if has_literal_slots(decorator) or self.is_eligible(node): + own_layout = self._field_layout(node) + else: + return Layout.UNSAFE + return max(inherited_layout, own_layout) + + @contextlib.contextmanager + def _visiting_node(self, node: nodes.ClassDef) -> cabc.Iterator[None]: + """Mark *node* as visiting for the duration of recursive analysis.""" + self._visiting.add(node) + try: + yield + finally: + self._visiting.remove(node) + + @staticmethod + def _external_layout(node: nodes.ClassDef) -> Layout: + """Classify an inferred base outside the linted module.""" + try: + slots = node.slots() + except exceptions.InferenceError: + return Layout.UNSAFE + if slots is None: + return Layout.UNSAFE + return Layout.SLOTTED if slots else Layout.NEUTRAL + + def _base_layout(self, base: nodes.NodeNG | bases.Proxy) -> Layout: + """Classify one explicit base lineage.""" + inferred = inferred_class(base) + if inferred is None: + return Layout.UNSAFE + if inferred.qname() == "builtins.object": + return Layout.NEUTRAL + if inferred.qname() in VARIABLE_LENGTH_BUILTIN_QNAMES: + return Layout.UNSAFE + if inferred.root() is self.module: + if inferred not in self._layouts: + self._layouts[inferred] = self._local_layout(inferred) + return self._layouts[inferred] + return self._external_layout(inferred) + + def is_eligible(self, node: nodes.ClassDef) -> bool: + """Return whether *node* may safely receive generated slots.""" + if node in self._eligibility: + return self._eligibility[node] + if node in self._visiting: + return False + with self._visiting_node(node): + decorator = find_dataclass_decorator(node) + result = ( + decorator is not None + and not has_literal_slots(decorator) + and not has_local_slots(node) + and node not in self._multiple_bases + and not has_local_slots_hazard(node, decorator, self._declared_states) + and self._inherited_layout(node) is not Layout.UNSAFE + ) + self._eligibility[node] = result + return result diff --git a/df12_python_lints/_dataclass_decorators.py b/df12_python_lints/_dataclass_decorators.py new file mode 100644 index 0000000..ef69173 --- /dev/null +++ b/df12_python_lints/_dataclass_decorators.py @@ -0,0 +1,86 @@ +"""Recognize standard-library dataclass decorators and slot arguments. + +The :class:`~df12_python_lints.dataclass_slots.DataclassSlotsChecker` uses +these small classifiers to identify the real imported decorator and literal +``slots=True`` without executing the linted module or trusting a name alone. +""" + +from __future__ import annotations + +from astroid import bases, nodes + + +def _import_binding_origin( + names: list[tuple[str, str | None]], bound_name: str +) -> str | None: + """Return the module path an ``import`` binds to *bound_name*.""" + for original, alias in names: + if (alias or original.split(".", maxsplit=1)[0]) == bound_name: + return original if alias else original.split(".", maxsplit=1)[0] + return None + + +def _assignment_origin(assignment: nodes.NodeNG, bound_name: str) -> str | None: + """Return the import origin represented by one lexical binding.""" + match assignment: + case nodes.Import(names=names): + return _import_binding_origin(names, bound_name) + case nodes.ImportFrom(modname=modname, names=names): + for original, alias in names: + if (alias or original) == bound_name: + return f"{modname}.{original}" + return None + + +def imported_origin(name_node: nodes.Name) -> str | None: + """Resolve the import origin of *name_node*'s active lexical binding.""" + _, assignments = name_node.lookup(name_node.name) + origins = { + origin + for assignment in assignments + if (origin := _assignment_origin(assignment, name_node.name)) is not None + } + return origins.pop() if len(origins) == 1 and len(assignments) == 1 else None + + +def expression_origin(node: nodes.NodeNG | bases.Proxy) -> str | None: + """Resolve an imported dotted expression without executing linted code.""" + match node: + case nodes.Name(): + return imported_origin(node) + case nodes.Attribute(expr=expr, attrname=attrname): + prefix = expression_origin(expr) + return f"{prefix}.{attrname}" if prefix is not None else None + return None + + +def subscript_target(node: nodes.NodeNG | bases.Proxy) -> nodes.NodeNG | bases.Proxy: + """Return the expression subscripted by *node*, or *node* itself.""" + return node.value if isinstance(node, nodes.Subscript) else node + + +def decorator_target(decorator: nodes.NodeNG) -> nodes.NodeNG: + """Return the callable expression underlying *decorator*.""" + return decorator.func if isinstance(decorator, nodes.Call) else decorator + + +def find_dataclass_decorator(node: nodes.ClassDef) -> nodes.NodeNG | None: + """Return *node*'s real stdlib dataclass decorator, when present.""" + if node.decorators is None: + return None + for decorator in node.decorators.nodes: + if expression_origin(decorator_target(decorator)) == "dataclasses.dataclass": + return decorator + return None + + +def has_literal_slots(decorator: nodes.NodeNG) -> bool: + """Return whether *decorator* contains the lexical pair ``slots=True``.""" + if not isinstance(decorator, nodes.Call): + return False + return any( + keyword.arg == "slots" + and isinstance(keyword.value, nodes.Const) + and keyword.value.value is True + for keyword in decorator.keywords + ) diff --git a/df12_python_lints/_dataclass_inference.py b/df12_python_lints/_dataclass_inference.py new file mode 100644 index 0000000..8f6090e --- /dev/null +++ b/df12_python_lints/_dataclass_inference.py @@ -0,0 +1,47 @@ +"""Represent and resolve inherited layouts for dataclass slot analysis. + +The dataclass analyzer uses the ordered :class:`Layout` severity and the +conservative Astroid candidate resolver when classifying every base lineage. +""" + +from __future__ import annotations + +import enum +import itertools + +from astroid import bases, exceptions, nodes, util + +VARIABLE_LENGTH_BUILTIN_QNAMES = frozenset({ + "builtins.bytearray", + "builtins.bytes", + "builtins.dict", + "builtins.list", + "builtins.set", + "builtins.str", + "builtins.tuple", +}) + + +class Layout(enum.IntEnum): + """Describe a base lineage with explicit severity ordering.""" + + NEUTRAL = 0 + SLOTTED = 1 + UNSAFE = 2 + + +def inferred_class(base: nodes.NodeNG | bases.Proxy) -> nodes.ClassDef | None: + """Infer one unambiguous class for *base*, or return ``None``.""" + try: + inferred = tuple(itertools.islice(base.infer(), 2)) + except exceptions.InferenceError: + return None + if len(inferred) != 1 or inferred[0] is util.Uninferable: + return None + candidate = inferred[0] + if isinstance(candidate, bases.Instance): + # Astroid exposes no public route from an inferred Instance to its + # underlying ClassDef; keep this private unwrapping covered by the + # supported Pylint range and a focused regression test. + candidate = candidate._proxied + return candidate if isinstance(candidate, nodes.ClassDef) else None diff --git a/df12_python_lints/_dataclass_state.py b/df12_python_lints/_dataclass_state.py new file mode 100644 index 0000000..974f4de --- /dev/null +++ b/df12_python_lints/_dataclass_state.py @@ -0,0 +1,165 @@ +"""Classify dataclass fields and explicit runtime slot declarations. + +The dataclass-slots analysis uses these helpers to distinguish storage-backed +instance state from ``ClassVar`` and ``InitVar`` pseudo-fields, including state +inherited through local dataclass and manually slotted lineages. +""" + +from __future__ import annotations + +import itertools +import typing as typ + +from astroid import exceptions, nodes, util + +from ._dataclass_decorators import ( + expression_origin, + find_dataclass_decorator, + subscript_target, +) + +if typ.TYPE_CHECKING: + import collections.abc as cabc + +_PSEUDO_FIELD_ORIGINS = frozenset({ + "dataclasses.InitVar", + "dataclasses.KW_ONLY", + "typing.ClassVar", + "typing_extensions.ClassVar", +}) + + +def _dataclass_field_name(statement: nodes.NodeNG) -> str | None: + """Return one real dataclass field name declared by *statement*.""" + if not isinstance(statement, nodes.AnnAssign) or not isinstance( + statement.target, nodes.AssignName + ): + return None + origin = expression_origin(subscript_target(statement.annotation)) + return None if origin in _PSEUDO_FIELD_ORIGINS else statement.target.name + + +def dataclass_field_names(node: nodes.ClassDef) -> frozenset[str]: + """Return real dataclass fields declared directly by *node*.""" + return frozenset( + field_name + for statement in node.body + if (field_name := _dataclass_field_name(statement)) is not None + ) + + +def _slots_value(statement: nodes.NodeNG) -> nodes.NodeNG | None: + """Return the runtime value from a direct ``__slots__`` assignment.""" + match statement: + case nodes.Assign(targets=[nodes.AssignName(name="__slots__")], value=value): + return value + case nodes.AnnAssign( + target=nodes.AssignName(name="__slots__"), value=value + ) if value is not None: + return value + return None + + +def _inferred_slot_value(value: nodes.NodeNG) -> nodes.NodeNG | None: + """Return one unambiguous inferred value for a slots expression.""" + try: + inferred = tuple(itertools.islice(value.infer(), 2)) + except exceptions.InferenceError: + return None + if len(inferred) != 1 or inferred[0] is util.Uninferable: + return None + if not isinstance(inferred[0], nodes.NodeNG): + return None + return inferred[0] + + +def _slot_name(value: nodes.NodeNG) -> str | None: + """Return one valid, unambiguously inferred slot name.""" + inferred = _inferred_slot_value(value) + if not isinstance(inferred, nodes.Const) or not isinstance(inferred.value, str): + return None + return inferred.value if inferred.value.isidentifier() else None + + +def _node_elements(values: cabc.Iterable[object]) -> tuple[nodes.NodeNG, ...] | None: + """Return *values* when every slot element is an Astroid node.""" + elements = tuple(values) + if not all(isinstance(element, nodes.NodeNG) for element in elements): + return None + return typ.cast("tuple[nodes.NodeNG, ...]", elements) + + +def _slot_elements(value: nodes.NodeNG) -> tuple[nodes.NodeNG, ...] | None: + """Return the elements from one statically supported slot value.""" + if isinstance(value, nodes.Const): + return (value,) + if isinstance(value, nodes.Dict): + elements = tuple(key for key, _ in value.items if key is not None) + return _node_elements(elements) if len(elements) == len(value.items) else None + if isinstance(value, (nodes.List, nodes.Set, nodes.Tuple)): + return _node_elements(value.elts) + return None + + +def _validated_slot_names( + elements: tuple[nodes.NodeNG, ...], +) -> frozenset[str] | None: + """Validate every element and return the complete set of slot names.""" + names: set[str] = set() + for element in elements: + if (name := _slot_name(element)) is None: + return None + names.add(name) + return frozenset(names) + + +def _resolved_slot_names(value: nodes.NodeNG) -> frozenset[str] | None: + """Resolve and validate one complete ``__slots__`` value.""" + inferred = _inferred_slot_value(value) + if inferred is None or (elements := _slot_elements(inferred)) is None: + return None + return _validated_slot_names(elements) + + +def local_slot_names(node: nodes.ClassDef) -> frozenset[str] | None: + """Return validated names from *node*'s sole direct slots assignment.""" + values = tuple( + value + for statement in node.body + if (value := _slots_value(statement)) is not None + ) + return _resolved_slot_names(values[0]) if len(values) == 1 else None + + +def has_local_slots(node: nodes.ClassDef) -> bool: + """Return whether *node* has one valid, locally resolved slots value.""" + return local_slot_names(node) is not None + + +def _local_instance_state(node: nodes.ClassDef) -> cabc.Iterator[str]: + """Yield instance state declared directly by one class.""" + if find_dataclass_decorator(node) is not None: + yield from dataclass_field_names(node) + if (slot_names := local_slot_names(node)) is not None: + yield from slot_names + + +def declared_instance_state( + node: nodes.ClassDef, + cache: dict[nodes.ClassDef, frozenset[str]] | None = None, +) -> frozenset[str]: + """Return visible dataclass fields and slots, caching each lineage prefix.""" + state_cache = {} if cache is None else cache + if node in state_cache: + return state_cache[node] + state = set(_local_instance_state(node)) + for ancestor in node.ancestors(recurs=False): + if isinstance(ancestor, nodes.ClassDef): + state.update(declared_instance_state(ancestor, state_cache)) + state_cache[node] = frozenset(state) + return state_cache[node] + + +def has_declared_instance_fields(node: nodes.ClassDef) -> bool: + """Return whether *node* visibly declares real dataclass fields.""" + return bool(dataclass_field_names(node)) diff --git a/df12_python_lints/dataclass_slots.py b/df12_python_lints/dataclass_slots.py new file mode 100644 index 0000000..d28cd0f --- /dev/null +++ b/df12_python_lints/dataclass_slots.py @@ -0,0 +1,95 @@ +"""Require explicit slot layouts for closed standard-library dataclasses. + +The plugin reports R9111 when a real :func:`dataclasses.dataclass` appears to +describe closed instance state but omits both ``slots=True`` and a manual +``__slots__`` layout. Conservative analysis suppresses advice when generated +slots could be unsafe or ineffective. Load the package plugin with Pylint:: + + pylint --load-plugins=df12_python_lints my_package + +Loading :mod:`df12_python_lints` registers this checker alongside the package's +other df12 house-policy checks. +""" + +from __future__ import annotations + +import typing as typ + +from pylint import checkers + +from ._dataclass_analysis import LayoutAnalyzer +from ._dataclass_decorators import find_dataclass_decorator + +if typ.TYPE_CHECKING: + from astroid import nodes + from pylint.lint import PyLinter + from pylint.typing import MessageDefinitionTuple + + override = typ.override +else: # The project's pylint gate runs the plugin under PyPy 3.11. + override = getattr(typ, "override", lambda method: method) + +_MSGS: typ.Final[dict[str, MessageDefinitionTuple]] = { + "R9111": ( + "Dataclass %r should declare slots=True", + "prefer-slots-for-dataclass", + ( + "Emitted when a standard-library dataclass appears to define closed " + "instance state but neither requests generated slots nor declares an " + "explicit __slots__ layout. Use dataclass(slots=True), adding " + "weakref_slot=True when weak references are required. Intentional " + "open-state or compatibility exceptions require an explained local " + "suppression." + ), + ), +} + + +class DataclassSlotsChecker(checkers.BaseChecker): + """Report closed standard-library dataclasses without slots. + + Attributes + ---------- + name : str + The checker identifier, ``df12-dataclass-slots``. + msgs : dict[str, MessageDefinitionTuple] + The R9111 ``prefer-slots-for-dataclass`` message. + + Examples + -------- + Enable alongside the plugin and run Pylint as usual:: + + pylint --load-plugins=df12_python_lints my_module.py + """ + + name = "df12-dataclass-slots" + msgs = _MSGS + + @override + # pylint: disable-next=useless-return # Keep the terminal return explicit. + def __init__(self, linter: PyLinter) -> None: + """Initialize without retaining analysis across modules.""" + super().__init__(linter) + self._analyzer: LayoutAnalyzer | None = None + return # ruff:ignore[useless-return] # Keep the terminal return explicit. + + # pylint: disable-next=useless-return # Keep the terminal return explicit. + def visit_module(self, node: nodes.Module) -> None: + """Prepare cached reverse-inheritance analysis for *node*.""" + self._analyzer = LayoutAnalyzer(node) + return # ruff:ignore[useless-return] # Keep the terminal return explicit. + + def visit_classdef(self, node: nodes.ClassDef) -> None: + """Check *node* when it is a real standard-library dataclass.""" + decorator = find_dataclass_decorator(node) + if decorator is None: + return + if self._analyzer is None or self._analyzer.module is not node.root(): + self._analyzer = LayoutAnalyzer(node.root()) + if self._analyzer.is_eligible(node): + self.add_message( + "prefer-slots-for-dataclass", + node=decorator, + args=(node.name,), + ) + return diff --git a/df12_python_lints/type_aliases.py b/df12_python_lints/type_aliases.py index ebcbc99..f0ebb1b 100644 --- a/df12_python_lints/type_aliases.py +++ b/df12_python_lints/type_aliases.py @@ -39,7 +39,7 @@ from pylint.typing import MessageDefinitionTuple _MSGS: typ.Final[dict[str, MessageDefinitionTuple]] = { - "R9111": ( + "R9112": ( "Declare type alias %r with a 'type' statement", "prefer-type-statement", ( @@ -238,7 +238,7 @@ class TypeAliasChecker(checkers.BaseChecker): name : str The checker identifier, ``df12-type-aliases``. msgs : dict[str, MessageDefinitionTuple] - The R9111 ``prefer-type-statement`` message. + The R9112 ``prefer-type-statement`` message. Examples -------- diff --git a/docs/adr-001-conservative-dataclass-layout-analysis.md b/docs/adr-001-conservative-dataclass-layout-analysis.md new file mode 100644 index 0000000..6fb08e6 --- /dev/null +++ b/docs/adr-001-conservative-dataclass-layout-analysis.md @@ -0,0 +1,59 @@ +# Architectural decision record (ADR) 001: Conservative dataclass layout analysis + +## Status + +Accepted. R9111 uses conservative source analysis with per-module caches and a +bounded Pylint compatibility range. + +## Date + +2026-08-03. + +## Context and problem statement + +`dataclass(slots=True)` returns a replacement class and can be unsafe or +ineffective when decorators retain the original class, methods capture its +class cell, or inherited layouts provide dictionaries or conflicting slots. +R9111 must report ordinary closed value types without speculating when Astroid +cannot prove that the replacement layout is safe. Transitive base analysis must +also remain efficient for modules containing deep inheritance chains. + +## Decision drivers + +- Prefer false negatives to unsafe slot suggestions. +- Do not import, execute, or mutate the linted program. +- Keep repeated base-layout analysis linear in the local class graph. +- Make reliance on Astroid's private `Instance._proxied` bridge explicit. + +## Options considered + +- Infer optimistically and report unless a known incompatibility is found. +- Recompute each complete base lineage for every class visit. +- Analyse conservatively and memoize eligibility and local inherited layouts. + +## Decision outcome + +In the context of R9111 analysis for standard-library dataclasses, facing +replacement-class hazards, incomplete inference, and deep inheritance chains, +conservative source analysis with per-module eligibility and layout caches was +chosen over optimistic inference or repeated lineage traversal, to achieve safe +diagnostics with linear repeated layout classification, accepting additional +false negatives and cache invalidation after the provisional +reverse-inheritance pass. + +The package supports `pylint>=3.3,<5`. A new Pylint major version must preserve +the covered Astroid `Instance._proxied` behaviour before the upper bound moves. + +## Consequences + +- Unknown or ambiguous bases suppress R9111 rather than guessing. +- `LayoutAnalyzer` clears provisional eligibility and layout caches before + final class visits incorporate reverse multiple-inheritance conflicts. +- Regression tests cover the private inference bridge and linear deep-chain + layout classification. + +## Known risks and limitations + +- Cross-module reverse inheritance remains unknowable to a normal Pylint pass. +- Conservative inference may require an explained local suppression for safe + classes whose layout cannot be proven. diff --git a/docs/contents.md b/docs/contents.md index 23a773a..bd41a9f 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -12,6 +12,11 @@ documentation set. - [Documentation style guide](documentation-style-guide.md) defines the spelling, structure, Markdown, Architecture Decision Record (ADR), Request for Comments (RFC), and roadmap conventions used by this documentation set. +- [Version 0.2.0 migration guide](migration-0.2.0.md) explains the new + dataclass-slots rule, the reassigned message identifiers, and the required + pylint configuration changes. +- [ADR 001](adr-001-conservative-dataclass-layout-analysis.md) records the + conservative, cached layout analysis and supported Pylint range for R9111. ## Engineering practice diff --git a/docs/developers-guide.md b/docs/developers-guide.md index f57a7d8..0bc4704 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -6,18 +6,19 @@ This guide explains the contributor workflow for the generated project. Pylint discovers the plugin through `register()` in `df12_python_lints/__init__.py`, the entry point pylint calls when -`load-plugins` names the package. It instantiates and registers the nine +`load-plugins` names the package. It instantiates and registers the ten checkers, each defined in its own module: `MatchDispatchChecker`, `AssertMessageChecker`, `ConstantChainChecker`, `TrivialWrapperChecker`, `ReexportAssignmentChecker`, `SuppressionCommentChecker`, -`SnapshotAssertionChecker`, `TypeAliasChecker`, and `FutureAnnotationsChecker`. -Between them they expose twelve messages. The last two are gated on pylint's -`py-version` option: the type-alias check needs a 3.12+ baseline (PEP 695) and -the future-annotations check a 3.14+ baseline (PEP 749 deferred evaluation), so -the end-to-end shim tests pass `--py-version=3.14` explicitly — the shim runs -under PyPy, whose interpreter version would otherwise gate both checks off. +`SnapshotAssertionChecker`, `DataclassSlotsChecker`, `TypeAliasChecker`, and +`FutureAnnotationsChecker`. Between them, they expose thirteen messages. The +last two are gated on pylint's `py-version` option: the type-alias check needs +a 3.12+ baseline (PEP 695) and the future-annotations check a 3.14+ baseline +(PEP 749 deferred evaluation), so the end-to-end shim tests pass +`--py-version=3.14` explicitly — the shim runs under PyPy, whose interpreter +version would otherwise gate both checks off. -Logic shared by more than one checker lives in two private helper modules: +Reusable or substantial checker analysis lives in private helper modules: - `_chains.py` holds the traversal used by the dispatch-oriented checkers to walk a head `if` statement and its `elif` chain. Its pure selection kernels @@ -27,6 +28,59 @@ Logic shared by more than one checker lives in two private helper modules: - `_expressions.py` holds the attribute-chain and name-binding helpers — for example resolving the base `Name` of a pure `name.attr.deeper` chain — used by the wrapper and re-export checkers. +- `_dataclass_decorators.py` resolves `dataclasses.dataclass` and related + decorators from active lexical import bindings. It deliberately avoids + qualified-name spelling alone, inference that imports the linted program, and + ambiguous lookup chains. This strict resolver is intentionally distinct from + the type-alias checker's import recognition: a dataclass decorator is valid + only when one unambiguous active binding proves its identity, while the + type-alias checker conservatively classifies a name when its lookup chain + contains a supported import. Keep the shared primitives policy-neutral if + these implementations are consolidated; do not weaken either checker's + ambiguity contract merely to remove similar traversal code. +- `_dataclass_state.py` distinguishes runtime `__slots__` assignments and real + dataclass fields from class-only names. It is the shared source-state + boundary for slot-layout and direct-method mutation analysis; keep + inheritance and replacement-class decisions out of this module. +- `_dataclass_inference.py` resolves Astroid base candidates to unambiguous + class definitions and defines the ordered layout classification used by + inherited-layout and replacement-class hazard analysis. +- `_dataclass_analysis.py` classifies direct-method state evidence, + replacement-class hazards, and inherited layouts for `DataclassSlotsChecker`. + A `LayoutAnalyzer` is created once per module. It caches eligibility and + per-class inherited layouts, and performs a reverse inheritance pass before + class visits, so local dataclass bases later combined through multiple + inheritance are suppressed before either base can report. The provisional + caches are cleared after that reverse pass so final decisions include every + discovered conflict. + +The runtime dependency is bounded to `pylint>=3.3,<5`. Dataclass base inference +uses Astroid's private `Instance._proxied` bridge because no public API exposes +the underlying `ClassDef`; a new Pylint major version therefore requires the +focused inference and inherited-layout tests to be revalidated before widening +the range. [ADR 001](adr-001-conservative-dataclass-layout-analysis.md) records +the conservative analysis, caching, and compatibility decision. + +The dataclass-slots decorator pass preserves source order. Decorators below +`dataclass` run first and suppress the check unless they are a proven +identity-preserving marker; decorators above it see the replacement class and +do not suppress. Direct-method analysis uses each method's first instance +parameter and ignores static methods. Open-state checks do not enter nested +executable scopes. Replacement-class checks scan nested executable scopes but +stop at nested `ClassDef` bodies: nested helper classes own their class cells, +so a helper's zero-argument `super()` is not an outer dataclass hazard. +Inference ambiguity is a reason to stay silent. + +Inherited-layout analysis is transitive, including through a local dataclass +that already requests generated slots. `object` and proven empty-slot marker +bases are neutral; a single non-empty slotted lineage is safe; unslotted, +unknown, variable-length, or conflicting lineages suppress. Reverse analysis +suppresses local dataclass bases only when a direct multiple-inheritance shape +would combine more than one prospectively non-empty slot lineage. A local +dataclass that is itself eligible for R9111 is treated as prospectively +slotted, allowing a safe single-inheritance chain to report every missing +declaration in one run. The checker never imports or executes the linted +program and never mutates its AST. `SuppressionCommentChecker` is token-based rather than AST-based: it inspects comment tokens to find suppression pragmas and the explanations that may diff --git a/docs/migration-0.2.0.md b/docs/migration-0.2.0.md new file mode 100644 index 0000000..d45256d --- /dev/null +++ b/docs/migration-0.2.0.md @@ -0,0 +1,60 @@ +# Migrate to version 0.2.0 + +Version 0.2.0 adds a rule for closed standard-library dataclasses and +reassigns the message identifier previously allocated to +`prefer-type-statement`. Projects that configure pylint messages by identifier +must update that configuration when upgrading from version 0.1.0. + +## Adopt the dataclass-slots rule + +The new `prefer-slots-for-dataclass` rule uses R9111. It reports a +standard-library dataclass when the source provides neither a literal +`slots=True` argument nor a runtime `__slots__` assignment, unless the checker +finds concrete evidence that generated slots would be unsafe or ineffective. + +Prefer an explicit closed layout for ordinary value objects: + +```python +import dataclasses + + +@dataclasses.dataclass(slots=True) +class Coordinate: + latitude: float + longitude: float +``` + +Review each new R9111 diagnostic before changing the class. Generated slots +return a replacement class and can interact with inheritance, decorators, +class-cell closures, and dictionary-backed state. When compatibility requires +an unslotted class, retain it with a narrow, explained suppression: + +```python +# Compatibility: consumers attach adapter state dynamically. +@dataclasses.dataclass # pylint: disable=prefer-slots-for-dataclass +class LegacyRecord: + value: str +``` + +## Update message identifiers + +In version 0.1.0, R9111 identified `prefer-type-statement`. Version 0.2.0 +assigns R9111 to `prefer-slots-for-dataclass` and moves +`prefer-type-statement` to R9112. + +Replace R9111 with R9112 wherever a pylint `enable` or `disable` list intends +to select `prefer-type-statement`. For example: + +```toml +[tool.pylint.messages_control] +enable = ["R9112"] +``` + +Configuration that uses the stable symbol `prefer-type-statement` needs no +change. Prefer symbols over numeric identifiers in new configuration so future +identifier changes remain explicit at the plugin boundary. + +After updating the configuration, run pylint across the normal source and test +targets. Resolve any new `prefer-slots-for-dataclass` findings or add explained +local suppressions where a compatibility requirement cannot be inferred from +the source. diff --git a/docs/users-guide.md b/docs/users-guide.md index ddc13a8..5fd1373 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -15,7 +15,7 @@ or from the command line: pylint --load-plugins=df12_python_lints my_package ``` -Loading the plugin registers twelve messages. +Loading the plugin registers thirteen messages. ### `prefer-structural-pattern-matching` (R9101) @@ -167,7 +167,73 @@ Comparisons with names (an `expected` fixture or parameter), small literals, and asserts outside test functions are never reported. Leaf counting works on the AST, so reformatting a literal does not change whether it fires. -### `prefer-type-statement` (R9111) +### `prefer-slots-for-dataclass` (R9111) + +Standard-library dataclasses that describe closed instance state should request +generated slots: + +```python +import dataclasses + + +@dataclasses.dataclass(frozen=True, slots=True) +class Coordinate: + latitude: float + longitude: float +``` + +The rule recognizes the real `dataclasses.dataclass` through lexical import +bindings, including module and direct-import aliases. A local function named +`dataclass`, a shadowed import, pydantic, attrs, msgspec, and +`dataclass_transform`-based frameworks are outside its scope. + +Only a lexically visible `slots=True` satisfies the generated-layout form. +`slots=False`, `slots=1`, a named constant, or `**options` still report because +the class layout should not vary through configuration or indirection. A local +runtime assignment to `__slots__` suppresses R9111 only when the checker +validates a valid, locally resolved slot value. Annotation-only declarations, +invalid values, unresolved names, and ambiguous values do not qualify. Use +`weakref_slot=True` alongside `slots=True` when instances require weak +references.[^1] + +The checker holds its tongue when the source contains hard evidence that +generated slots would be unsafe, ineffective, or misleading: + +- a direct instance method requires dictionary-backed or undeclared state + through `cached_property`, `__dict__`, `vars`, dynamic attribute operations, + or assignment to an undeclared instance attribute; +- the class is an explicit extension boundary through `abc.ABC`, + `typing.Protocol`, `abstractmethod`, `__init_subclass__`, an explicit + metaclass, or other class-header keywords; +- a decorator below `dataclass` might retain the original class object; +- a direct method uses zero-argument `super()` or closes over `__class__` on + the supported Python 3.12 and 3.13 runtimes; or +- an inherited layout is unknown, already supplies an instance dictionary, + cannot accept non-empty slots, or would create conflicting non-empty slot + lineages through multiple inheritance. + +Assignments to actual dataclass fields, including `field(init=False)` values +populated in `__post_init__`, and to explicit inherited slots remain +slot-compatible. Plain class attributes, `ClassVar`, and `InitVar` declarations +do not create instance storage. An outer decorator is also safe because it sees +the replacement class returned by `dataclass(slots=True)`. + +Public naming, export through `__all__`, and the absence of `typing.final` do +not suppress the message. Keep an intentionally open or compatibility-bound +class unslotted with a narrow, explained suppression beside the decorator: + +```python +# Compatibility: consumers attach adapter state dynamically. +@dataclasses.dataclass # pylint: disable=prefer-slots-for-dataclass +class LegacyRecord: + value: str +``` + +The `lint-suppression-without-explanation` rule requires that local reason. See +Python's dataclass and slot-layout documentation for the replacement-class and +inheritance constraints.[^1][^2] + +### `prefer-type-statement` (R9112) Module-level type aliases should use the PEP 695 `type` statement, which names the intent and defers evaluation of the aliased expression: @@ -204,6 +270,9 @@ which runtime annotation consumers can observe. The check respects pylint's `py-version` option; projects whose configured baseline still includes 3.13 or older keep the import without noise. +[^1]: [Python 3.12 `dataclasses.dataclass`](https://docs.python.org/3.12/library/dataclasses.html#dataclasses.dataclass) +[^2]: [Python data model notes on `__slots__`](https://docs.python.org/3.12/reference/datamodel.html#slots) + ## The ambrleaks Snapshot Scanner The package also ships `ambrleaks`, a standalone scanner for syrupy `.ambr` diff --git a/pyproject.toml b/pyproject.toml index 6ec1e19..2ef7ae4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ description = "df12-python-lints package" readme = "README.md" requires-python = ">=3.12" license = { text = "ISC" } -dependencies = ["pylint>=3.3"] +dependencies = ["pylint>=3.3,<5"] [project.scripts] ambrleaks = "df12_python_lints.ambrleaks.cli:main" @@ -318,4 +318,3 @@ build-backend = "hatchling.build" [tool.hatch.build.targets.wheel] packages = ["df12_python_lints"] - diff --git a/tests/dataclass_slots_support.py b/tests/dataclass_slots_support.py new file mode 100644 index 0000000..64f50e6 --- /dev/null +++ b/tests/dataclass_slots_support.py @@ -0,0 +1,82 @@ +"""Shared test harness for dataclass-slots checker cases.""" + +from __future__ import annotations + +import textwrap + +import astroid +from astroid import nodes +from pylint import testutils + +from df12_python_lints.dataclass_slots import DataclassSlotsChecker + + +def parse_module(code: str) -> nodes.Module: + """Parse dedented *code* as one synthetic module.""" + return astroid.parse(textwrap.dedent(code)) + + +def module_classes(module: nodes.Module) -> tuple[nodes.ClassDef, ...]: + """Return all classes in source order, including nested classes.""" + return tuple(module.nodes_of_class(nodes.ClassDef)) + + +def _message(node: nodes.ClassDef) -> testutils.MessageTest: + """Build the expected decorator-attached diagnostic for *node*.""" + decorator = next( + ( + candidate + for candidate in (node.decorators.nodes if node.decorators else ()) + if _looks_like_dataclass(candidate) + ), + None, + ) + if decorator is None: + message = f"expected a dataclass decorator on {node.name}" + raise AssertionError(message) + return testutils.MessageTest( + "prefer-slots-for-dataclass", + line=decorator.fromlineno, + node=decorator, + args=(node.name,), + col_offset=decorator.col_offset, + end_line=decorator.end_lineno, + end_col_offset=decorator.end_col_offset, + ) + + +def _looks_like_dataclass(decorator: nodes.NodeNG) -> bool: + """Recognize test-fixture dataclass spelling independently of production.""" + target = decorator.func if isinstance(decorator, nodes.Call) else decorator + if isinstance(target, nodes.Attribute): + return target.attrname == "dataclass" + return isinstance(target, nodes.Name) and target.name in {"dataclass", "record"} + + +class DataclassSlotsTestCase(testutils.CheckerTestCase): + """Provide whole-module assertions for dataclass-slots cases.""" + + CHECKER_CLASS = DataclassSlotsChecker + + def assert_reports(self, code: str, *names: str) -> None: + """Assert that exactly the named dataclasses report.""" + module = parse_module(code) + classes = module_classes(module) + classes_by_name = {class_node.name: class_node for class_node in classes} + unresolved = set(names) - classes_by_name.keys() + if unresolved: + message = f"requested classes were not parsed: {sorted(unresolved)}" + raise AssertionError(message) + expected = tuple(_message(classes_by_name[name]) for name in names) + with self.assertAddsMessages(*expected): + self.checker.visit_module(module) + for class_node in classes: + self.checker.visit_classdef(class_node) + + def assert_silent(self, code: str) -> None: + """Assert that no class in *code* reports.""" + module = parse_module(code) + with self.assertNoMessages(): + self.checker.visit_module(module) + for class_node in module_classes(module): + self.checker.visit_classdef(class_node) diff --git a/tests/test_dataclass_slots.py b/tests/test_dataclass_slots.py new file mode 100644 index 0000000..62f28b1 --- /dev/null +++ b/tests/test_dataclass_slots.py @@ -0,0 +1,389 @@ +"""Recognition and state tests for the closed-dataclass slots checker.""" + +from __future__ import annotations + +import pytest +from pylint import testutils + +from df12_python_lints.dataclass_slots import _MSGS, DataclassSlotsChecker +from tests.dataclass_slots_support import ( + DataclassSlotsTestCase, + module_classes, + parse_module, +) + + +class TestDataclassSlotsChecker(DataclassSlotsTestCase): + """Exercise decorator recognition and closed-state evidence.""" + + @pytest.mark.parametrize( + ("class_var_import", "annotation"), + [ + ("import typing_extensions", "typing_extensions.ClassVar[int]"), + ("from typing_extensions import ClassVar as CV", "CV[int]"), + ], + ) + def test_typing_extensions_classvar_is_class_only_state( + self, class_var_import: str, annotation: str + ) -> None: + """Backport ClassVar imports do not declare slotted instance state.""" + self.assert_silent( + f""" + import dataclasses + {class_var_import} + + @dataclasses.dataclass + class Record: + value: int + cache: {annotation} = 0 + + def reset(self): + self.cache = 1 + """ + ) + + def test_diagnostic_contract_includes_decorator_location(self) -> None: + """The stable diagnostic payload identifies the class and decorator.""" + module = parse_module( + """ + import dataclasses + + @dataclasses.dataclass + class Record: + value: int + """ + ) + class_node = module_classes(module)[0] + linter = testutils.UnittestLinter() + checker = DataclassSlotsChecker(linter) + checker.visit_module(module) + checker.visit_classdef(class_node) + message = linter.release_messages()[0] + assert { + "symbol": message.msg_id, + "message": _MSGS["R9111"][0] % message.args, + "class_argument": message.args, + "line": message.line, + "column": message.col_offset, + } == { + "symbol": "prefer-slots-for-dataclass", + "message": "Dataclass 'Record' should declare slots=True", + "class_argument": ("Record",), + "line": 4, + "column": 1, + }, "the diagnostic contract or decorator attachment changed" + + @pytest.mark.parametrize( + "decorator", + [ + "dataclasses.dataclass", + "dataclasses.dataclass()", + "dataclasses.dataclass(slots=False)", + "dataclasses.dataclass(slots=1)", + "dataclasses.dataclass(slots=SLOTS)", + "dataclasses.dataclass(**DATACLASS_OPTIONS)", + "dataclasses.dataclass(frozen=True)", + "dataclasses.dataclass(order=True)", + "dataclasses.dataclass(eq=False)", + "dataclasses.dataclass(kw_only=True)", + "dataclasses.dataclass(weakref_slot=True)", + ], + ) + def test_reports_non_literal_slots(self, decorator: str) -> None: + """Only the singleton literal ``True`` requests generated slots.""" + self.assert_reports( + f""" + import dataclasses + SLOTS = True + DATACLASS_OPTIONS = {{"slots": True}} + + @{decorator} + class Record: + value: int + """, + "Record", + ) + + @pytest.mark.parametrize( + ("import_line", "decorator"), + [ + ("import dataclasses as dc", "dc.dataclass"), + ("from dataclasses import dataclass as record", "record"), + ], + ) + def test_reports_import_aliases(self, import_line: str, decorator: str) -> None: + """Module and direct decorator aliases retain their stdlib identity.""" + self.assert_reports( + f""" + {import_line} + + @{decorator} + class Record: + value: int + """, + "Record", + ) + + def test_reports_public_private_nested_and_zero_field_classes(self) -> None: + """Naming, nesting, exports, and field count do not create exemptions.""" + self.assert_reports( + """ + import dataclasses + __all__ = ["Public"] + + @dataclasses.dataclass + class Public: + value: int + + @dataclasses.dataclass + class _Private: + pass + + class Namespace: + @dataclasses.dataclass + class Nested: + value: int + """, + "Public", + "_Private", + "Nested", + ) + + def test_reports_safe_decorator_orderings(self) -> None: + """Outer decorators and an identity-preserving inner final are safe.""" + self.assert_reports( + """ + import dataclasses + import typing + + def register(cls): + return cls + + @register + @dataclasses.dataclass + class Outer: + value: int + + @dataclasses.dataclass + @typing.final + class Final: + value: int + """, + "Outer", + "Final", + ) + + @pytest.mark.parametrize( + "decorator", + [ + "dataclasses.dataclass(slots=True)", + "dataclasses.dataclass(slots=True, weakref_slot=True)", + ], + ) + def test_literal_slots_is_silent(self, decorator: str) -> None: + """Literal generated slots satisfy the policy.""" + self.assert_silent( + f""" + import dataclasses + + @{decorator} + class Record: + value: int + """ + ) + + @pytest.mark.parametrize( + "declaration", + [ + '__slots__ = "value"', + '__slots__ = ("value", "__dict__")', + '__slots__ = ["value"]', + '__slots__ = {"value"}', + '__slots__ = {"value": "field documentation"}', + '__slots__: typing.ClassVar[tuple[str, ...]] = ("value",)', + '_SLOT_NAMES = ("value",)\n__slots__ = _SLOT_NAMES', + ], + ) + def test_manual_slots_is_silent(self, declaration: str) -> None: + """Valid literal and locally resolved slot layouts satisfy the rule.""" + declaration = declaration.replace("\n", "\n ") + self.assert_silent( + f""" + import dataclasses + import typing + + @dataclasses.dataclass + class Record: + {declaration} + value: int + """ + ) + + def test_annotation_only_slots_still_reports(self) -> None: + """A slots annotation without a runtime value creates no layout.""" + self.assert_reports( + """ + import dataclasses + import typing + + @dataclasses.dataclass + class Record: + __slots__: typing.ClassVar[tuple[str, ...]] + value: int + """, + "Record", + ) + + @pytest.mark.parametrize( + "slot_value", + [ + "42", + '("value", 42)', + '"not a valid identifier"', + "SLOT_NAMES", + '("value",) if condition else ("other",)', + ], + ids=["integer", "mixed", "invalid-name", "unresolved", "ambiguous"], + ) + def test_unvalidated_manual_slots_still_reports(self, slot_value: str) -> None: + """Invalid, unresolved, and ambiguous slot values do not qualify.""" + self.assert_reports( + f""" + import dataclasses + + @dataclasses.dataclass + class Record: + __slots__ = {slot_value} + value: int + """, + "Record", + ) + + @pytest.mark.parametrize( + "source", + [ + """ + def dataclass(cls): return cls + @dataclass + class Record: pass + """, + """ + import dataclasses + dataclasses = factory() + @dataclasses.dataclass + class Record: pass + """, + """ + import pydantic.dataclasses + @pydantic.dataclasses.dataclass + class Record: pass + """, + """ + import attrs + @attrs.define + class Record: pass + """, + """ + import msgspec + class Record(msgspec.Struct): pass + """, + """ + import typing + @typing.dataclass_transform() + def model(cls): return cls + @model + class Record: pass + """, + ], + ) + def test_unrelated_decorators_are_silent(self, source: str) -> None: + """Spelling and dataclass-like frameworks cannot trigger the rule.""" + self.assert_silent(source) + + @pytest.mark.parametrize( + "method", + [ + "def reveal(this): return this.__dict__", + "def reveal(this): return vars(this)", + "def mutate(this, name): setattr(this, name, 1)", + "def mutate(this, name): delattr(this, name)", + "def mutate(this): this.extra = 1", + "def mutate(this): this.extra += 1", + "def mutate(this): del this.extra", + "def mutate(this, name): object.__setattr__(this, name, 1)", + 'def mutate(this): object.__setattr__(this, "extra", 1)', + ], + ) + def test_open_state_evidence_is_silent(self, method: str) -> None: + """Direct method evidence of dictionary-backed state suppresses.""" + self.assert_silent( + f""" + import dataclasses + + @dataclasses.dataclass + class Record: + value: int + {method} + """ + ) + + def test_cached_property_is_silent(self) -> None: + """The real dictionary-backed cached property suppresses.""" + self.assert_silent( + """ + import dataclasses + import functools + + @dataclasses.dataclass + class Record: + value: int + + @functools.cached_property + def doubled(self): + return self.value * 2 + """ + ) + + def test_declared_init_false_field_assignment_still_reports(self) -> None: + """Assignments to declared fields remain slot-compatible.""" + self.assert_reports( + """ + import dataclasses + + @dataclasses.dataclass + class Record: + value: int + cached: int = dataclasses.field(init=False) + + def __post_init__(instance): + instance.cached = instance.value + """, + "Record", + ) + + @pytest.mark.parametrize( + "declaration", + [ + "cache = 0", + "cache: typing.ClassVar[int] = 0", + "cache: dataclasses.InitVar[int] = 0", + ], + ) + def test_class_only_attribute_assignment_is_open_state( + self, declaration: str + ) -> None: + """Assigning through an instance to class-only state requires a dict.""" + self.assert_silent( + f""" + import dataclasses + import typing + + @dataclasses.dataclass + class Record: + value: int + {declaration} + + def reset(self): + self.cache = 1 + """ + ) diff --git a/tests/test_dataclass_slots_class_cells.py b/tests/test_dataclass_slots_class_cells.py new file mode 100644 index 0000000..30cdf22 --- /dev/null +++ b/tests/test_dataclass_slots_class_cells.py @@ -0,0 +1,29 @@ +"""Focused class-cell boundary tests for dataclass slot analysis.""" + +from __future__ import annotations + +from tests.dataclass_slots_support import DataclassSlotsTestCase + + +class TestDataclassSlotsClassCells(DataclassSlotsTestCase): + """Exercise nested class boundaries in replacement-class analysis.""" + + def test_nested_helper_class_super_still_reports(self) -> None: + """A helper class's class cell does not belong to the outer dataclass.""" + self.assert_reports( + """ + import dataclasses + + @dataclasses.dataclass + class Record: + value: int + + def make_helper(self): + class Helper: + def method(self): + return super().method() + + return Helper() + """, + "Record", + ) diff --git a/tests/test_dataclass_slots_layout.py b/tests/test_dataclass_slots_layout.py new file mode 100644 index 0000000..273231d --- /dev/null +++ b/tests/test_dataclass_slots_layout.py @@ -0,0 +1,41 @@ +"""Focused inherited-layout tests for manual dataclass slots.""" + +from __future__ import annotations + +from df12_python_lints._dataclass_analysis import LayoutAnalyzer +from df12_python_lints._dataclass_inference import Layout +from tests.dataclass_slots_support import ( + DataclassSlotsTestCase, + module_classes, + parse_module, +) + + +class TestDataclassSlotsLayout(DataclassSlotsTestCase): + """Exercise explicit layouts that retain an instance dictionary.""" + + def test_manual_dict_slot_base_makes_child_unsafe(self) -> None: + """An inherited explicit dictionary cannot become a closed layout.""" + source = """ + import dataclasses + + @dataclasses.dataclass + class Base: + __slots__ = ("__dict__",) + value: int + + @dataclasses.dataclass + class Child(Base): + label: str + """ + module = parse_module(source) + child = next(node for node in module_classes(module) if node.name == "Child") + analyzer = LayoutAnalyzer(module) + + assert analyzer._base_layout(child.bases[0]) is Layout.UNSAFE, ( + "the inherited __dict__ slot must classify the base layout as unsafe" + ) + assert not analyzer.is_eligible(child), ( + "the child with an inherited instance dictionary must not be eligible" + ) + self.assert_silent(source) diff --git a/tests/test_dataclass_slots_performance.py b/tests/test_dataclass_slots_performance.py new file mode 100644 index 0000000..5337627 --- /dev/null +++ b/tests/test_dataclass_slots_performance.py @@ -0,0 +1,102 @@ +"""Scaling regression tests for dataclass inherited-layout analysis.""" + +from __future__ import annotations + +import types +import typing as typ + +import df12_python_lints._dataclass_state as dataclass_state +from df12_python_lints._dataclass_analysis import LayoutAnalyzer +from df12_python_lints._dataclass_inference import inferred_class +from tests.dataclass_slots_support import module_classes, parse_module + +if typ.TYPE_CHECKING: + import collections.abc as cabc + + import pytest + from astroid import nodes + + from df12_python_lints._dataclass_inference import Layout + + +def test_deep_layout_chain_is_classified_once_per_base( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Memoized layouts keep a deep single-inheritance pass linear.""" + depth = 80 + declarations = ["@dataclasses.dataclass\nclass Node0:\n value_0: int"] + declarations.extend( + f"@dataclasses.dataclass\nclass Node{index}(Node{index - 1}):" + f"\n value_{index}: int" + for index in range(1, depth) + ) + module = parse_module("import dataclasses\n\n" + "\n\n".join(declarations)) + classes = module_classes(module) + local_layout_calls = 0 + original_local_layout = LayoutAnalyzer._local_layout + + def counting_local_layout(analyzer: LayoutAnalyzer, node: nodes.ClassDef) -> Layout: + """Count uncached local-layout classifications.""" + nonlocal local_layout_calls + local_layout_calls += 1 + return original_local_layout(analyzer, node) + + monkeypatch.setattr(LayoutAnalyzer, "_local_layout", counting_local_layout) + analyzer = LayoutAnalyzer(module) + + assert all(analyzer.is_eligible(node) for node in reversed(classes)) + assert local_layout_calls <= depth - 1, ( + f"expected linear layout analysis, got {local_layout_calls} calls" + ) + + +def test_deep_layout_chain_resolves_state_once_per_class( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Memoized state prevents repeated ancestor walks in deep lineages.""" + depth = 80 + declarations = ["@dataclasses.dataclass\nclass Node0:\n value_0: int"] + declarations.extend( + f"@dataclasses.dataclass\nclass Node{index}(Node{index - 1}):" + f"\n value_{index}: int" + for index in range(1, depth) + ) + module = parse_module("import dataclasses\n\n" + "\n\n".join(declarations)) + local_state_calls = 0 + original_local_instance_state = dataclass_state._local_instance_state + + def counting_local_instance_state( + node: nodes.ClassDef, + ) -> cabc.Iterator[str]: + """Count the local state work underlying inherited state analysis.""" + nonlocal local_state_calls + local_state_calls += 1 + yield from original_local_instance_state(node) + + monkeypatch.setattr( + dataclass_state, "_local_instance_state", counting_local_instance_state + ) + analyzer = LayoutAnalyzer(module) + + assert all(analyzer.is_eligible(node) for node in reversed(module_classes(module))) + assert local_state_calls <= depth + 1, ( + "expected linear state analysis including the shared object base, " + f"got {local_state_calls} local walks" + ) + + +def test_ambiguous_inference_stops_after_two_candidates() -> None: + """Ambiguity detection does not consume an unbounded inference stream.""" + yielded = 0 + + def many_candidates() -> cabc.Iterator[nodes.NodeNG]: + """Yield enough opaque candidates to expose eager materialization.""" + nonlocal yielded + while True: + yielded += 1 + yield typ.cast("nodes.NodeNG", object()) + + base = typ.cast("nodes.NodeNG", types.SimpleNamespace(infer=many_candidates)) + + assert inferred_class(base) is None + assert yielded == 2, f"expected two inference candidates, consumed {yielded}" diff --git a/tests/test_dataclass_slots_safety.py b/tests/test_dataclass_slots_safety.py new file mode 100644 index 0000000..6fe7e85 --- /dev/null +++ b/tests/test_dataclass_slots_safety.py @@ -0,0 +1,388 @@ +"""Safety-exemption and inheritance tests for dataclass slots.""" + +from __future__ import annotations + +import pytest +from astroid import nodes + +import df12_python_lints._dataclass_analysis as dataclass_analysis +from df12_python_lints._dataclass_analysis import LayoutAnalyzer +from df12_python_lints._dataclass_inference import inferred_class +from tests.dataclass_slots_support import ( + DataclassSlotsTestCase, + module_classes, + parse_module, +) + + +class TestDataclassSlotsSafety(DataclassSlotsTestCase): + """Exercise replacement-class and inherited-layout hold-tongue rules.""" + + def test_generic_protocol_is_silent(self) -> None: + """A subscripted Protocol base remains an extension boundary.""" + self.assert_silent( + """ + import dataclasses + import typing + + T = typing.TypeVar("T") + + @dataclasses.dataclass + class Config(typing.Protocol[T]): + value: T + """ + ) + + def test_instance_inference_unwraps_to_class_definition(self) -> None: + """Astroid Instance base candidates retain their defining class.""" + module = parse_module( + """ + class Base: pass + class Child(Base()): pass + """ + ) + child = next(item for item in module_classes(module) if item.name == "Child") + inferred = inferred_class(child.bases[0]) + assert isinstance(inferred, nodes.ClassDef), ( + f"expected a ClassDef, got {inferred!r}" + ) + assert inferred.name == "Base", f"expected Base, got {inferred.name!r}" + + def test_failed_eligibility_does_not_leave_visiting_state( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An analysis exception cannot poison later recursion detection.""" + module = parse_module( + """ + import dataclasses + @dataclasses.dataclass + class Record: + value: int + """ + ) + class_node = module_classes(module)[0] + analyzer = LayoutAnalyzer(module) + + def raise_analysis_error(*_args: object) -> bool: + """Raise a synthetic failure during local hazard analysis.""" + error = RuntimeError("synthetic analysis failure") + raise error + + monkeypatch.setattr( + dataclass_analysis, "has_local_slots_hazard", raise_analysis_error + ) + with pytest.raises(RuntimeError, match="synthetic analysis failure"): + analyzer.is_eligible(class_node) + assert class_node not in analyzer._visiting, ( + "failed analysis must clear recursive-visit state" + ) + + @pytest.mark.parametrize( + "source", + [ + """ + import abc + import dataclasses + @dataclasses.dataclass + class Record(abc.ABC): pass + """, + """ + import dataclasses + import typing + @dataclasses.dataclass + class Record(typing.Protocol): pass + """, + """ + import abc + import dataclasses + @dataclasses.dataclass + class Record: + @abc.abstractmethod + def value(self): ... + """, + """ + import dataclasses + @dataclasses.dataclass + class Record: + def __init_subclass__(cls): pass + """, + """ + import dataclasses + @dataclasses.dataclass + class Record(metaclass=type): pass + """, + """ + import dataclasses + @dataclasses.dataclass + class Record(flag=True): pass + """, + ], + ) + def test_extension_boundaries_are_silent(self, source: str) -> None: + """Explicit extension and class-creation boundaries suppress.""" + self.assert_silent(source) + + def test_unknown_inner_decorator_is_silent(self) -> None: + """A decorator below dataclass may retain the original class.""" + self.assert_silent( + """ + import dataclasses + @dataclasses.dataclass + @register + class Record: + value: int + """ + ) + + @pytest.mark.parametrize( + "method", + [ + "def method(self): return super().method()", + "def method(self): return __class__.__name__", + ], + ) + def test_class_cell_hazards_are_silent(self, method: str) -> None: + """Replacement-class closure hazards suppress on supported runtimes.""" + self.assert_silent( + f""" + import dataclasses + @dataclasses.dataclass + class Record: + value: int + {method} + """ + ) + + def test_nested_zero_argument_super_is_silent(self) -> None: + """A nested closure using zero-argument super makes replacement unsafe.""" + self.assert_silent( + """ + import dataclasses + + @dataclasses.dataclass + class Record: + value: int + + def method(self): + def nested(): + return super().method() + + return nested() + """ + ) + + @pytest.mark.parametrize( + ("protocol_import", "base"), + [ + ("import typing_extensions", "typing_extensions.Protocol"), + ("from typing_extensions import Protocol as Proto", "Proto"), + ], + ) + def test_typing_extensions_protocol_is_silent( + self, protocol_import: str, base: str + ) -> None: + """Backport Protocol imports remain explicit extension boundaries.""" + self.assert_silent( + f""" + import dataclasses + {protocol_import} + + @dataclasses.dataclass + class Record({base}): + value: int + """ + ) + + def test_two_argument_super_still_reports(self) -> None: + """Explicit two-argument super does not close over the class cell.""" + self.assert_reports( + """ + import dataclasses + @dataclasses.dataclass + class Record: + value: int + def method(self): return super(Record, self).__repr__() + """, + "Record", + ) + + @pytest.mark.parametrize("base", ["UnknownBase", "list", "tuple"]) + def test_unsafe_or_unknown_base_is_silent(self, base: str) -> None: + """Unprovable and variable-length inherited layouts suppress.""" + self.assert_silent( + f""" + import dataclasses + @dataclasses.dataclass + class Record({base}): + value: int + """ + ) + + def test_unslotted_local_base_is_silent(self) -> None: + """An ordinary unslotted base already contributes a dictionary.""" + self.assert_silent( + """ + import dataclasses + class Base: pass + @dataclasses.dataclass + class Record(Base): + value: int + """ + ) + + def test_reports_prospectively_slotted_single_inheritance(self) -> None: + """A local dataclass chain can be made slot-only in one lint run.""" + self.assert_reports( + """ + import dataclasses + + @dataclasses.dataclass + class Base: + value: int + + @dataclasses.dataclass + class Child(Base): + label: str + """, + "Base", + "Child", + ) + + def test_explicit_slotted_base_allows_child_report(self) -> None: + """A child of a proven local slotted base remains eligible.""" + self.assert_reports( + """ + import dataclasses + + class Marker: + __slots__ = () + + @dataclasses.dataclass + class Record(Marker): + value: int + """, + "Record", + ) + + def test_slotted_dataclass_preserves_inherited_dictionary(self) -> None: + """Generated slots cannot remove an unslotted ancestor's dictionary.""" + self.assert_silent( + """ + import dataclasses + + class OpenBase: + pass + + @dataclasses.dataclass(slots=True) + class SlottedBase(OpenBase): + value: int + + @dataclasses.dataclass + class Child(SlottedBase): + label: str + """ + ) + + def test_empty_slot_marker_lineages_are_neutral(self) -> None: + """Several empty-slot bases contribute no conflicting layout.""" + self.assert_reports( + """ + import dataclasses + + class LeftMarker: + __slots__ = () + + class RightMarker: + __slots__ = () + + @dataclasses.dataclass + class Record(LeftMarker, RightMarker): + value: int + """, + "Record", + ) + + def test_transitive_inherited_field_assignment_still_reports(self) -> None: + """A grandparent field remains declared slot-compatible state.""" + self.assert_reports( + """ + import dataclasses + + @dataclasses.dataclass + class Base: + value: int + + @dataclasses.dataclass + class Middle(Base): + label: str + + @dataclasses.dataclass + class Child(Middle): + def reset(self): + self.value = 0 + """, + "Base", + "Middle", + "Child", + ) + + def test_non_dataclass_annotation_is_not_inherited_state(self) -> None: + """An ordinary base annotation does not create instance storage.""" + self.assert_silent( + """ + import dataclasses + + class Base: + __slots__ = () + cache: int + + @dataclasses.dataclass + class Child(Base): + value: int + + def reset(self): + self.cache = 0 + """ + ) + + def test_multiple_inheritance_suppresses_bases_and_child(self) -> None: + """Reverse analysis avoids independently slotting combined bases.""" + self.assert_silent( + """ + import dataclasses + @dataclasses.dataclass + class Left: + left: int + @dataclasses.dataclass + class Right: + right: int + @dataclasses.dataclass + class Combined(Left, Right): + value: int + + @dataclasses.dataclass + class Chained(Left): + label: str + """ + ) + + def test_empty_slot_marker_does_not_suppress_dataclass_lineage(self) -> None: + """One non-empty lineage cannot create a slot-layout conflict.""" + self.assert_reports( + """ + import dataclasses + + @dataclasses.dataclass + class Base: + value: int + + class EmptySlotsMarker: + __slots__ = () + + @dataclasses.dataclass + class Child(Base, EmptySlotsMarker): + label: str + """, + "Base", + "Child", + ) diff --git a/tests/test_e2e_shim.py b/tests/test_e2e_shim.py index d0cece6..c71f44d 100644 --- a/tests/test_e2e_shim.py +++ b/tests/test_e2e_shim.py @@ -15,6 +15,7 @@ import re import shutil import subprocess # ruff:ignore[suspicious-subprocess-import] # fixed argv, no shell +import textwrap import typing as typ import pytest @@ -34,6 +35,7 @@ "trivial-alias-wrapper", "prefer-type-statement", "redundant-future-annotations", + "prefer-slots-for-dataclass", }) _FIXTURE = '''\ @@ -41,6 +43,7 @@ from __future__ import annotations import collections.abc as cabc +import dataclasses import os.path join = os.path.join @@ -48,6 +51,13 @@ Clock = cabc.Callable[[], float] +@dataclasses.dataclass +class Record: + """Trigger prefer-slots-for-dataclass.""" + + value: int + + def walk(value): """Trigger prefer-structural-pattern-matching.""" if isinstance(value, dict): @@ -108,6 +118,20 @@ def test_report(output): assert "footer" in output, "has footer" ''' +_CLEAN_FIXTURE = '''\ + """A module the df12 checkers have nothing to say about.""" + import dataclasses + from os.path import join + + @dataclasses.dataclass(slots=True) + class Record: + """A closed value object with an explicit layout.""" + + value: int + + __all__ = ["Record", "join"] +''' + def _shim_reference() -> str: """Read the pinned shim ref from the Makefile to avoid drift.""" @@ -179,12 +203,7 @@ def test_clean_module_is_silent_under_the_shim( ) -> None: """A module with no violations produces no plugin messages.""" fixture = tmp_path / "fixture_clean.py" - fixture.write_text( - '"""A module the df12 checkers have nothing to say about."""\n' - "from os.path import join\n\n" - '__all__ = ["join"]\n', - encoding="utf-8", - ) + fixture.write_text(textwrap.dedent(_CLEAN_FIXTURE), encoding="utf-8") messages = _run_shim_pylint(fixture) plugin_messages = [ message for message in messages if message["symbol"] in _EXPECTED_SYMBOLS diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 03179db..a8665a4 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -16,6 +16,7 @@ def test_register_adds_all_checkers() -> None: "df12-match-dispatch", "df12-assert-message", "df12-constant-chain", + "df12-dataclass-slots", "df12-trivial-wrapper", "df12-reexport-assignment", "df12-suppression-comments", @@ -25,3 +26,23 @@ def test_register_adds_all_checkers() -> None: } missing = expected - names assert not missing, f"checkers failed to register: {sorted(missing)}" + + +def test_message_ids_remain_unique_after_r9111_integration() -> None: + """Dataclass slots owns R9111 and type statements move to R9112.""" + linter = PyLinter() + df12_python_lints.register(linter) + message_ids = [definition.msgid for definition in linter.msgs_store.messages] + assert len(message_ids) == len(set(message_ids)), ( + f"duplicate message identifiers: {message_ids!r}" + ) + by_symbol = { + definition.symbol: definition.msgid + for definition in linter.msgs_store.messages + if definition.symbol in {"prefer-slots-for-dataclass", "prefer-type-statement"} + } + expected = { + "prefer-slots-for-dataclass": "R9111", + "prefer-type-statement": "R9112", + } + assert by_symbol == expected, f"unexpected message identifiers: {by_symbol!r}" diff --git a/tests/test_properties.py b/tests/test_properties.py index 3913faf..87b693f 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -10,6 +10,7 @@ import io import math +import operator import tokenize import typing as typ @@ -20,11 +21,13 @@ from pylint.utils import ASTWalker from df12_python_lints._chains import narrowing_prefix, repeated_subject +from df12_python_lints._dataclass_analysis import LayoutAnalyzer from df12_python_lints.ambrleaks.scanner import shannon_entropy from df12_python_lints.constant_chain import ConstantChainChecker from df12_python_lints.match_dispatch import MatchDispatchChecker from df12_python_lints.snapshot_asserts import SnapshotAssertionChecker from df12_python_lints.suppressions import SuppressionCommentChecker +from tests.dataclass_slots_support import module_classes, parse_module if typ.TYPE_CHECKING: from pylint.checkers import BaseChecker @@ -35,6 +38,13 @@ _OTHER_NAMES = st.from_regex(r"w_[a-z]{1,6}", fullmatch=True) _BREAKERS = st.from_regex(r"x_[a-z]{1,6}", fullmatch=True) _WORDS = st.from_regex(r"[a-z]{2,8}", fullmatch=True) +_DATACLASS_KEYWORDS = ( + ("eq", "False"), + ("frozen", "True"), + ("kw_only", "True"), + ("order", "True"), + ("unsafe_hash", "True"), +) def _walk_symbols(checker_class: type[BaseChecker], code: str) -> list[str]: @@ -66,6 +76,49 @@ def _constant_chain(subject: str, constants: list[int]) -> str: return f"def handle({subject}):\n{''.join(branches)} return -1\n" +@st.composite +def _dataclass_keyword_order( + draw: st.DrawFn, +) -> tuple[tuple[str, str], ...]: + """Generate unique dataclass keywords in arbitrary lexical order.""" + irrelevant = draw( + st.lists( + st.sampled_from(_DATACLASS_KEYWORDS), + unique_by=operator.itemgetter(0), + ) + ) + slot_value = draw(st.sampled_from([None, "True", "False", "1", "SLOTS"])) + keywords = irrelevant + ([] if slot_value is None else [("slots", slot_value)]) + return tuple(draw(st.permutations(keywords))) + + +class TestDataclassSlotsProperties: + """Decorator keyword selection honours the lexical slots contract.""" + + @settings(deadline=None) + @given(keywords=_dataclass_keyword_order()) + def test_only_literal_slots_true_is_silent( + self, keywords: tuple[tuple[str, str], ...] + ) -> None: + """Irrelevant options and ordering cannot change slot eligibility.""" + arguments = ", ".join(f"{name}={value}" for name, value in keywords) + module = parse_module( + f""" + import dataclasses + SLOTS = True + @dataclasses.dataclass({arguments}) + class Record: + value: int + """ + ) + class_node = module_classes(module)[0] + is_eligible = LayoutAnalyzer(module).is_eligible(class_node) + expected = ("slots", "True") not in keywords + assert is_eligible is expected, ( + f"eligibility mismatch: is_eligible={is_eligible}, keywords={keywords!r}" + ) + + class TestConstantChainProperties: """Constant chains of any length behave uniformly.""" diff --git a/typos.local.toml b/typos.local.toml index 782430a..c8af0cd 100644 --- a/typos.local.toml +++ b/typos.local.toml @@ -9,6 +9,12 @@ stems = [] accepted = [ # The AST library pylint is built on; not a misspelling of "asteroid". "astroid", + # Public helper name used in implementation and developer documentation. + "Analyzer", + "LayoutAnalyzer", + # Terms retained verbatim when documenting external APIs and command options. + "artifact", + "color", ] [words.corrections] diff --git a/typos.toml b/typos.toml index 50a29c2..692fda2 100644 --- a/typos.toml +++ b/typos.toml @@ -33,11 +33,12 @@ extend-ignore-re = [ "(?s)```.*?```", "Center \\| Microsoft Learn", "\\brust-analyzer\\b", - "`[^`\\n]+`", ] [default.extend-words] "ASO" = "ASO" +"Analyzer" = "Analyzer" +"LayoutAnalyzer" = "LayoutAnalyzer" "absolutisable" = "absolutizable" "absolutisation" = "absolutization" "absolutisations" = "absolutizations" @@ -146,6 +147,7 @@ extend-ignore-re = [ "apologizers" = "apologizers" "apologizes" = "apologizes" "apologizing" = "apologizing" +"artifact" = "artifact" "astroid" = "astroid" "atomisable" = "atomizable" "atomisation" = "atomization" @@ -309,6 +311,7 @@ extend-ignore-re = [ "colonizers" = "colonizers" "colonizes" = "colonizes" "colonizing" = "colonizing" +"color" = "color" "colourisable" = "colourizable" "colourisation" = "colourization" "colourisations" = "colourizations" diff --git a/uv.lock b/uv.lock index ee240c2..f7f5a4a 100644 --- a/uv.lock +++ b/uv.lock @@ -237,7 +237,7 @@ dev = [ ] [package.metadata] -requires-dist = [{ name = "pylint", specifier = ">=3.3" }] +requires-dist = [{ name = "pylint", specifier = ">=3.3,<5" }] [package.metadata.requires-dev] dev = [