From 9c835f35b0f1690597ade799c9c6a30bc5922959 Mon Sep 17 00:00:00 2001 From: "lodyai[bot]" <251440992+lodyai[bot]@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:16:35 +0100 Subject: [PATCH 1/2] Enforce slots for closed dataclasses (#11) (#13) * Enforce slots for closed dataclasses (#11) Add binding-aware recognition and conservative safety analysis for standard-library dataclasses whose instance layout can be closed safely. Cover open-state evidence, class identity hazards, and inherited layouts with example, property, plug-in, and PyPy shim tests. Reserve `R9111` for the issue's new rule and move the existing `prefer-type-statement` refactor message to `R9112`. * Document the closed-dataclass slots policy (#11) Explain the literal slots requirement, evidence-based safety exemptions, and explicit compatibility suppression path. Record the binding, decorator-order, and cached layout analysis used by the checker. Update the public inventory to thirteen messages and identify the renumbered type-statement rule as `R9112`. * Fix transitive dataclass layout analysis (#11) Treat fields from the full inherited lineage as declared state, so safe grandparent-field assignments remain eligible for generated slots. Classify manual slots from Astroid's resolved layout rather than from the presence of an assignment, keeping multiple empty-slot marker bases layout-neutral. * Simplify dataclass safety analysis Extract the open-state, class-hazard, and reverse-inheritance decisions into focused helpers. This preserves the conservative checker behaviour while keeping the new analysis within the repository's complexity thresholds. * Fix dataclass slot safety analysis (#11) Restrict declared instance state to real dataclass fields and explicit slots, and require manual slot declarations to have a runtime value. Preserve inherited dictionaries through generated-slot bases and limit reverse multiple-inheritance suppression to genuinely conflicting slot lineages. * Harden dataclass slots analysis (#11) Preserve conservative layout and import-resolution behaviour while fixing the review findings around declared state, generic extension bases, inheritance, recursive analysis, and manual slots. Strengthen diagnostic tests, document the 0.2.0 message migration, and bind Pylint to the supported major-version range. * Recognize nested dataclass slot hazards (#11) Inspect nested executable scopes for replacement-class closures and treat `typing_extensions` ClassVar and Protocol imports like their standard-library counterparts. Document the extracted inference helper and strengthen message-ID coverage. * Cache inherited dataclass layouts (#11) Memoize local inherited-layout classifications per module and clear the provisional cache before final eligibility analysis. Add a structural scaling regression for deep single-inheritance chains. Record the conservative analysis and Pylint compatibility decisions in ADR 001 and link them from the developer documentation. * Fix ADR spelling (#11) Use the project Oxford spelling required by the documentation gate. * Bound dataclass analysis traversal (#11) Stop class-cell scans at nested class boundaries and consume at most two inference candidates when deciding ambiguity. Add focused regressions for nested helper classes and unbounded inference streams. * Validate manual dataclass slots (#11) Require manual `__slots__` declarations to resolve unambiguously to a complete valid slot value before suppressing R9111. Use the validated names for inherited layout and declared-state analysis, and document and test the rejected annotation-only, invalid, unresolved, and ambiguous forms. * Separate manual slot validation stages (#11) Split inferred value classification from element validation so each helper has one decision level while preserving the conservative R9111 behaviour. * Make dataclass checker returns explicit (#11) Add terminal bare returns to the checker lifecycle methods and narrowly suppress the conflicting redundant-return rules where necessary. * Treat manual __dict__ slots as unsafe (#11) A manual `__slots__` entry naming `__dict__` keeps the instance dictionary alive, so the layout is not closed. Classify such a layout as `Layout.UNSAFE` before it can be read as `SLOTTED`, which stops R9111 firing on a dataclass child of a local base that declares `__slots__ = ("__dict__",)`. Mark `DataclassSlotsChecker.__init__` with `typing.override`, keeping the existing `disable-next` comment immediately before the `def` so it still applies. The name is bound through a `TYPE_CHECKING` branch: type checkers see `typing.override` precisely, while the runtime fallback keeps the plugin importable under the PyPy 3.11 pylint shim, where `typing.override` is absent. Rewrite the ADR decision statement impersonally, and correct the developer guide's account of the replacement-class scan: it enters nested executable scopes but stops at nested `ClassDef` bodies, since a nested helper class owns its own class cell. Co-Authored-By: Claude Opus 5 (1M context) * Clarify unsafe manual-slot regression (#11) * Allow documented external spelling Accept the `LayoutAnalyzer` identifier and literal external API terms so the generated spelling configuration validates the documentation. * Cache inherited dataclass state (#11) Reuse each class's declared state while analysing local inheritance so deep dataclass chains avoid repeated ancestor walks. --------- Co-authored-by: leynos Co-authored-by: leynos Co-authored-by: Claude Opus 5 (1M context) --- README.md | 6 +- df12_python_lints/__init__.py | 11 +- df12_python_lints/_dataclass_analysis.py | 400 ++++++++++++++++++ df12_python_lints/_dataclass_decorators.py | 86 ++++ df12_python_lints/_dataclass_inference.py | 47 ++ df12_python_lints/_dataclass_state.py | 165 ++++++++ df12_python_lints/dataclass_slots.py | 95 +++++ df12_python_lints/type_aliases.py | 4 +- ...-conservative-dataclass-layout-analysis.md | 59 +++ docs/contents.md | 5 + docs/developers-guide.md | 70 ++- docs/migration-0.2.0.md | 60 +++ docs/users-guide.md | 73 +++- pyproject.toml | 3 +- tests/dataclass_slots_support.py | 82 ++++ tests/test_dataclass_slots.py | 389 +++++++++++++++++ tests/test_dataclass_slots_class_cells.py | 29 ++ tests/test_dataclass_slots_layout.py | 41 ++ tests/test_dataclass_slots_performance.py | 102 +++++ tests/test_dataclass_slots_safety.py | 388 +++++++++++++++++ tests/test_e2e_shim.py | 31 +- tests/test_plugin.py | 21 + tests/test_properties.py | 53 +++ typos.local.toml | 6 + typos.toml | 5 +- uv.lock | 2 +- 26 files changed, 2206 insertions(+), 27 deletions(-) create mode 100644 df12_python_lints/_dataclass_analysis.py create mode 100644 df12_python_lints/_dataclass_decorators.py create mode 100644 df12_python_lints/_dataclass_inference.py create mode 100644 df12_python_lints/_dataclass_state.py create mode 100644 df12_python_lints/dataclass_slots.py create mode 100644 docs/adr-001-conservative-dataclass-layout-analysis.md create mode 100644 docs/migration-0.2.0.md create mode 100644 tests/dataclass_slots_support.py create mode 100644 tests/test_dataclass_slots.py create mode 100644 tests/test_dataclass_slots_class_cells.py create mode 100644 tests/test_dataclass_slots_layout.py create mode 100644 tests/test_dataclass_slots_performance.py create mode 100644 tests/test_dataclass_slots_safety.py 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 = [ From c1c9a8b9907977eda02720fa4c9f62f9983f5d74 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:24:43 +0000 Subject: [PATCH 2/2] Build(deps): Bump astral-sh/setup-uv from 9.0.0 to 10.0.1 Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 9.0.0 to 10.0.1. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](https://github.com/astral-sh/setup-uv/compare/c771a70e6277c0a99b617c7a806ffedaca235ff9...20cfd1bf945f4377ade1205e4dbc17946fc9a30d) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/act-validation.yml | 2 +- .github/workflows/audit.yml | 2 +- .github/workflows/ci.yml | 2 +- .github/workflows/coverage-main.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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