From c5bfd1e87763ca140986ecb11b68fb46eb86ba04 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 5 Jul 2023 15:19:24 -0400 Subject: [PATCH] Allow descriptor instantiations in dataclass fields (#5537) ## Summary Per the Python documentation, dataclasses are allowed to instantiate descriptors, like so: ```python class IntConversionDescriptor: def __init__(self, *, default): self._default = default def __set_name__(self, owner, name): self._name = "_" + name def __get__(self, obj, type): if obj is None: return self._default return getattr(obj, self._name, self._default) def __set__(self, obj, value): setattr(obj, self._name, int(value)) @dataclass class InventoryItem: quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100) ``` Closes https://github.com/astral-sh/ruff/issues/4451. --- .../resources/test/fixtures/ruff/RUF009.py | 28 ++++++++++- .../function_call_in_dataclass_default.rs | 3 +- crates/ruff/src/rules/ruff/rules/helpers.rs | 21 +++++++- ..._rules__ruff__tests__RUF009_RUF009.py.snap | 48 +++++++++---------- 4 files changed, 72 insertions(+), 28 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF009.py b/crates/ruff/resources/test/fixtures/ruff/RUF009.py index 3ba1aad6bed96..f1cc836ffc5aa 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF009.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF009.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import ClassVar, NamedTuple + def default_function() -> list[int]: return [] @@ -25,12 +26,13 @@ class A: fine_timedelta: datetime.timedelta = datetime.timedelta(hours=7) fine_tuple: tuple[int] = tuple([1]) fine_regex: re.Pattern = re.compile(r".*") - fine_float: float = float('-inf') + fine_float: float = float("-inf") fine_int: int = int(12) fine_complex: complex = complex(1, 2) fine_str: str = str("foo") fine_bool: bool = bool("foo") - fine_fraction: Fraction = Fraction(1,2) + fine_fraction: Fraction = Fraction(1, 2) + DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES = ImmutableType(40) DEFAULT_A_FOR_ALL_DATACLASSES = A([1, 2, 3]) @@ -45,3 +47,25 @@ class B: okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES fine_dataclass_function: list[int] = field(default_factory=list) + + +class IntConversionDescriptor: + def __init__(self, *, default): + self._default = default + + def __set_name__(self, owner, name): + self._name = "_" + name + + def __get__(self, obj, type): + if obj is None: + return self._default + + return getattr(obj, self._name, self._default) + + def __set__(self, obj, value): + setattr(obj, self._name, int(value)) + + +@dataclass +class InventoryItem: + quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100) diff --git a/crates/ruff/src/rules/ruff/rules/function_call_in_dataclass_default.rs b/crates/ruff/src/rules/ruff/rules/function_call_in_dataclass_default.rs index 93961572ae5c5..3b52cac788e38 100644 --- a/crates/ruff/src/rules/ruff/rules/function_call_in_dataclass_default.rs +++ b/crates/ruff/src/rules/ruff/rules/function_call_in_dataclass_default.rs @@ -8,7 +8,7 @@ use ruff_python_semantic::analyze::typing::is_immutable_func; use crate::checkers::ast::Checker; use crate::rules::ruff::rules::helpers::{ - is_class_var_annotation, is_dataclass, is_dataclass_field, + is_class_var_annotation, is_dataclass, is_dataclass_field, is_descriptor_class, }; /// ## What it does @@ -98,6 +98,7 @@ pub(crate) fn function_call_in_dataclass_default( if !is_class_var_annotation(annotation, checker.semantic()) && !is_immutable_func(func, checker.semantic(), &extend_immutable_calls) && !is_dataclass_field(func, checker.semantic()) + && !is_descriptor_class(func, checker.semantic()) { checker.diagnostics.push(Diagnostic::new( FunctionCallInDataclassDefaultArgument { diff --git a/crates/ruff/src/rules/ruff/rules/helpers.rs b/crates/ruff/src/rules/ruff/rules/helpers.rs index b70c6918e17ab..8ba7e01624ad8 100644 --- a/crates/ruff/src/rules/ruff/rules/helpers.rs +++ b/crates/ruff/src/rules/ruff/rules/helpers.rs @@ -1,7 +1,7 @@ use rustpython_parser::ast::{self, Expr}; use ruff_python_ast::helpers::map_callable; -use ruff_python_semantic::SemanticModel; +use ruff_python_semantic::{BindingKind, SemanticModel}; /// Return `true` if the given [`Expr`] is a special class attribute, like `__slots__`. /// @@ -64,3 +64,22 @@ pub(super) fn is_pydantic_model(class_def: &ast::StmtClassDef, semantic: &Semant }) }) } + +/// Returns `true` if the given function is an instantiation of a class that implements the +/// descriptor protocol. +/// +/// See: +pub(super) fn is_descriptor_class(func: &Expr, semantic: &SemanticModel) -> bool { + semantic.lookup_attribute(func).map_or(false, |id| { + let BindingKind::ClassDefinition(scope_id) = semantic.binding(id).kind else { + return false; + }; + + // Look for `__get__`, `__set__`, and `__delete__` methods. + ["__get__", "__set__", "__delete__"].iter().any(|method| { + semantic.scopes[scope_id].get(method).map_or(false, |id| { + semantic.binding(id).kind.is_function_definition() + }) + }) + }) +} diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap index ea6bb93008a3a..64571bbe2d9f3 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap @@ -1,44 +1,44 @@ --- source: crates/ruff/src/rules/ruff/mod.rs --- -RUF009.py:19:41: RUF009 Do not perform function call `default_function` in dataclass defaults +RUF009.py:20:41: RUF009 Do not perform function call `default_function` in dataclass defaults | -17 | @dataclass() -18 | class A: -19 | hidden_mutable_default: list[int] = default_function() +18 | @dataclass() +19 | class A: +20 | hidden_mutable_default: list[int] = default_function() | ^^^^^^^^^^^^^^^^^^ RUF009 -20 | class_variable: typing.ClassVar[list[int]] = default_function() -21 | another_class_var: ClassVar[list[int]] = default_function() +21 | class_variable: typing.ClassVar[list[int]] = default_function() +22 | another_class_var: ClassVar[list[int]] = default_function() | -RUF009.py:41:41: RUF009 Do not perform function call `default_function` in dataclass defaults +RUF009.py:43:41: RUF009 Do not perform function call `default_function` in dataclass defaults | -39 | @dataclass -40 | class B: -41 | hidden_mutable_default: list[int] = default_function() +41 | @dataclass +42 | class B: +43 | hidden_mutable_default: list[int] = default_function() | ^^^^^^^^^^^^^^^^^^ RUF009 -42 | another_dataclass: A = A() -43 | not_optimal: ImmutableType = ImmutableType(20) +44 | another_dataclass: A = A() +45 | not_optimal: ImmutableType = ImmutableType(20) | -RUF009.py:42:28: RUF009 Do not perform function call `A` in dataclass defaults +RUF009.py:44:28: RUF009 Do not perform function call `A` in dataclass defaults | -40 | class B: -41 | hidden_mutable_default: list[int] = default_function() -42 | another_dataclass: A = A() +42 | class B: +43 | hidden_mutable_default: list[int] = default_function() +44 | another_dataclass: A = A() | ^^^ RUF009 -43 | not_optimal: ImmutableType = ImmutableType(20) -44 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +45 | not_optimal: ImmutableType = ImmutableType(20) +46 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES | -RUF009.py:43:34: RUF009 Do not perform function call `ImmutableType` in dataclass defaults +RUF009.py:45:34: RUF009 Do not perform function call `ImmutableType` in dataclass defaults | -41 | hidden_mutable_default: list[int] = default_function() -42 | another_dataclass: A = A() -43 | not_optimal: ImmutableType = ImmutableType(20) +43 | hidden_mutable_default: list[int] = default_function() +44 | another_dataclass: A = A() +45 | not_optimal: ImmutableType = ImmutableType(20) | ^^^^^^^^^^^^^^^^^ RUF009 -44 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES -45 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES +46 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +47 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES |