Skip to content

Commit

Permalink
Allow descriptor instantiations in dataclass fields (#5537)
Browse files Browse the repository at this point in the history
## 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 #4451.
  • Loading branch information
charliermarsh authored Jul 5, 2023
1 parent 9e1039f commit c5bfd1e
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 28 deletions.
28 changes: 26 additions & 2 deletions crates/ruff/resources/test/fixtures/ruff/RUF009.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from pathlib import Path
from typing import ClassVar, NamedTuple


def default_function() -> list[int]:
return []

Expand All @@ -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])
Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
21 changes: 20 additions & 1 deletion crates/ruff/src/rules/ruff/rules/helpers.rs
Original file line number Diff line number Diff line change
@@ -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__`.
///
Expand Down Expand Up @@ -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: <https://docs.python.org/3.10/reference/datamodel.html#descriptors>
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()
})
})
})
}
Original file line number Diff line number Diff line change
@@ -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
|


0 comments on commit c5bfd1e

Please sign in to comment.