mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 05:44:56 +00:00
[ty] correctly ignore field specifiers when not specified (#20002)
This commit corrects the type checker's behavior when handling `dataclass_transform` decorators that don't explicitly specify `field_specifiers`. According to [PEP 681 (Data Class Transforms)](https://peps.python.org/pep-0681/#dataclass-transform-parameters), when `field_specifiers` is not provided, it defaults to an empty tuple, meaning no field specifiers are supported and `dataclasses.field`/`dataclasses.Field` calls should be ignored. Fixes https://github.com/astral-sh/ty/issues/980
This commit is contained in:
parent
1a38831d53
commit
39ee71c2a5
5 changed files with 77 additions and 3 deletions
|
@ -84,3 +84,47 @@ reveal_type(field(default=1)) # revealed: dataclasses.Field[Literal[1]]
|
||||||
reveal_type(field(default=None)) # revealed: dataclasses.Field[None]
|
reveal_type(field(default=None)) # revealed: dataclasses.Field[None]
|
||||||
reveal_type(field(default_factory=get_default)) # revealed: dataclasses.Field[str]
|
reveal_type(field(default_factory=get_default)) # revealed: dataclasses.Field[str]
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## dataclass_transform field_specifiers
|
||||||
|
|
||||||
|
If `field_specifiers` is not specified, it defaults to an empty tuple, meaning no field specifiers
|
||||||
|
are supported and `dataclasses.field` and `dataclasses.Field` should not be accepted by default.
|
||||||
|
|
||||||
|
```py
|
||||||
|
from typing_extensions import dataclass_transform
|
||||||
|
from dataclasses import field, dataclass
|
||||||
|
from typing import TypeVar
|
||||||
|
|
||||||
|
T = TypeVar("T")
|
||||||
|
|
||||||
|
@dataclass_transform()
|
||||||
|
def create_model(*, init: bool = True):
|
||||||
|
def deco(cls: type[T]) -> type[T]:
|
||||||
|
return cls
|
||||||
|
return deco
|
||||||
|
|
||||||
|
@create_model()
|
||||||
|
class A:
|
||||||
|
name: str = field(init=False)
|
||||||
|
|
||||||
|
# field(init=False) should be ignored for dataclass_transform without explicit field_specifiers
|
||||||
|
reveal_type(A.__init__) # revealed: (self: A, name: str = Unknown) -> None
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class B:
|
||||||
|
name: str = field(init=False)
|
||||||
|
|
||||||
|
# Regular @dataclass should respect field(init=False)
|
||||||
|
reveal_type(B.__init__) # revealed: (self: B) -> None
|
||||||
|
```
|
||||||
|
|
||||||
|
Test constructor calls:
|
||||||
|
|
||||||
|
```py
|
||||||
|
# This should NOT error because field(init=False) is ignored for A
|
||||||
|
A(name="foo")
|
||||||
|
|
||||||
|
# This should error because field(init=False) is respected for B
|
||||||
|
# error: [unknown-argument]
|
||||||
|
B(name="foo")
|
||||||
|
```
|
||||||
|
|
|
@ -510,6 +510,10 @@ bitflags! {
|
||||||
const KW_ONLY = 0b0000_1000_0000;
|
const KW_ONLY = 0b0000_1000_0000;
|
||||||
const SLOTS = 0b0001_0000_0000;
|
const SLOTS = 0b0001_0000_0000;
|
||||||
const WEAKREF_SLOT = 0b0010_0000_0000;
|
const WEAKREF_SLOT = 0b0010_0000_0000;
|
||||||
|
// This is not an actual argument from `dataclass(...)` but a flag signaling that no
|
||||||
|
// `field_specifiers` was specified for the `dataclass_transform`, see [1].
|
||||||
|
// [1]: https://typing.python.org/en/latest/spec/dataclasses.html#dataclass-transform-parameters
|
||||||
|
const NO_FIELD_SPECIFIERS = 0b0100_0000_0000;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -542,6 +546,11 @@ impl From<DataclassTransformerParams> for DataclassParams {
|
||||||
params.contains(DataclassTransformerParams::FROZEN_DEFAULT),
|
params.contains(DataclassTransformerParams::FROZEN_DEFAULT),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
result.set(
|
||||||
|
Self::NO_FIELD_SPECIFIERS,
|
||||||
|
!params.contains(DataclassTransformerParams::FIELD_SPECIFIERS),
|
||||||
|
);
|
||||||
|
|
||||||
result
|
result
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -900,7 +900,7 @@ impl<'db> Bindings<'db> {
|
||||||
order_default,
|
order_default,
|
||||||
kw_only_default,
|
kw_only_default,
|
||||||
frozen_default,
|
frozen_default,
|
||||||
_field_specifiers,
|
field_specifiers,
|
||||||
_kwargs,
|
_kwargs,
|
||||||
] = overload.parameter_types()
|
] = overload.parameter_types()
|
||||||
{
|
{
|
||||||
|
@ -919,6 +919,16 @@ impl<'db> Bindings<'db> {
|
||||||
params |= DataclassTransformerParams::FROZEN_DEFAULT;
|
params |= DataclassTransformerParams::FROZEN_DEFAULT;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if let Some(field_specifiers_type) = field_specifiers {
|
||||||
|
// For now, we'll do a simple check: if field_specifiers is not
|
||||||
|
// None/empty, we assume it might contain dataclasses.field
|
||||||
|
// TODO: Implement proper parsing to check for
|
||||||
|
// dataclasses.field/Field specifically
|
||||||
|
if !field_specifiers_type.is_none(db) {
|
||||||
|
params |= DataclassTransformerParams::FIELD_SPECIFIERS;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
overload.set_return_type(Type::DataclassTransformer(params));
|
overload.set_return_type(Type::DataclassTransformer(params));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -2407,9 +2407,19 @@ impl<'db> ClassLiteral<'db> {
|
||||||
let mut kw_only = None;
|
let mut kw_only = None;
|
||||||
if let Some(Type::KnownInstance(KnownInstanceType::Field(field))) = default_ty {
|
if let Some(Type::KnownInstance(KnownInstanceType::Field(field))) = default_ty {
|
||||||
default_ty = Some(field.default_type(db));
|
default_ty = Some(field.default_type(db));
|
||||||
|
if self
|
||||||
|
.dataclass_params(db)
|
||||||
|
.map(|params| params.contains(DataclassParams::NO_FIELD_SPECIFIERS))
|
||||||
|
.unwrap_or(false)
|
||||||
|
{
|
||||||
|
// This happens when constructing a `dataclass` with a `dataclass_transform`
|
||||||
|
// without defining the `field_specifiers`, meaning it should ignore
|
||||||
|
// `dataclasses.field` and `dataclasses.Field`.
|
||||||
|
} else {
|
||||||
init = field.init(db);
|
init = field.init(db);
|
||||||
kw_only = field.kw_only(db);
|
kw_only = field.kw_only(db);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
let mut field = Field {
|
let mut field = Field {
|
||||||
declared_ty: attr_ty.apply_optional_specialization(db, specialization),
|
declared_ty: attr_ty.apply_optional_specialization(db, specialization),
|
||||||
|
|
|
@ -164,6 +164,7 @@ bitflags! {
|
||||||
const ORDER_DEFAULT = 1 << 1;
|
const ORDER_DEFAULT = 1 << 1;
|
||||||
const KW_ONLY_DEFAULT = 1 << 2;
|
const KW_ONLY_DEFAULT = 1 << 2;
|
||||||
const FROZEN_DEFAULT = 1 << 3;
|
const FROZEN_DEFAULT = 1 << 3;
|
||||||
|
const FIELD_SPECIFIERS= 1 << 4;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue