[ty] apply KW_ONLY sentinel only to local fields (#19986)
Some checks are pending
CI / cargo test (linux) (push) Blocked by required conditions
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run

fix https://github.com/astral-sh/ty/issues/1047

## Summary

This PR fixes how `KW_ONLY` is applied in dataclasses. Previously, the
sentinel leaked into subclasses and incorrectly marked their fields as
keyword-only; now it only affects fields declared in the same class.

```py
from dataclasses import dataclass, KW_ONLY

@dataclass
class D:
    x: int
    _: KW_ONLY
    y: str

@dataclass
class E(D):
    z: bytes

# This should work: x=1 (positional), z=b"foo" (positional), y="foo" (keyword-only)
E(1, b"foo", y="foo")

reveal_type(E.__init__)  # revealed: (self: E, x: int, z: bytes, *, y: str) -> None
```

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

<!-- How was it tested? -->
mdtests
This commit is contained in:
Eric Jolibois 2025-08-19 20:01:35 +02:00 committed by GitHub
parent c6dcfe36d0
commit 58efd19f11
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 93 additions and 45 deletions

View file

@ -988,6 +988,28 @@ class D: # error: [duplicate-kw-only]
z: float
```
`KW_ONLY` should only affect fields declared after it within the same class, not fields in
subclasses:
```py
from dataclasses import dataclass, KW_ONLY
@dataclass
class D:
x: int
_: KW_ONLY
y: str
@dataclass
class E(D):
z: bytes
# This should work: x=1 (positional), z=b"foo" (positional), y="foo" (keyword-only)
E(1, b"foo", y="foo")
reveal_type(E.__init__) # revealed: (self: E, x: int, z: bytes, *, y: str) -> None
```
## Other special cases
### `dataclasses.dataclass`

View file

@ -49,6 +49,22 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.
35 | y: str
36 | _2: KW_ONLY
37 | z: float
38 | from dataclasses import dataclass, KW_ONLY
39 |
40 | @dataclass
41 | class D:
42 | x: int
43 | _: KW_ONLY
44 | y: str
45 |
46 | @dataclass
47 | class E(D):
48 | z: bytes
49 |
50 | # This should work: x=1 (positional), z=b"foo" (positional), y="foo" (keyword-only)
51 | E(1, b"foo", y="foo")
52 |
53 | reveal_type(E.__init__) # revealed: (self: E, x: int, z: bytes, *, y: str) -> None
```
# Diagnostics
@ -141,3 +157,15 @@ info: `KW_ONLY` fields: `_1`, `_2`
info: rule `duplicate-kw-only` is enabled by default
```
```
info[revealed-type]: Revealed type
--> src/mdtest_snippet.py:53:13
|
51 | E(1, b"foo", y="foo")
52 |
53 | reveal_type(E.__init__) # revealed: (self: E, x: int, z: bytes, *, y: str) -> None
| ^^^^^^^^^^ `(self: E, x: int, z: bytes, *, y: str) -> None`
|
```

View file

@ -1179,6 +1179,16 @@ pub(crate) struct Field<'db> {
pub(crate) kw_only: Option<bool>,
}
impl<'db> Field<'db> {
/// Returns true if this field is a `dataclasses.KW_ONLY` sentinel.
/// <https://docs.python.org/3/library/dataclasses.html#dataclasses.KW_ONLY>
pub(crate) fn is_kw_only_sentinel(&self, db: &'db dyn Db) -> bool {
self.declared_ty
.into_nominal_instance()
.is_some_and(|instance| instance.class(db).is_known(db, KnownClass::KwOnly))
}
}
/// Representation of a class definition statement in the AST: either a non-generic class, or a
/// generic class that has not been specialized.
///
@ -1932,10 +1942,9 @@ impl<'db> ClassLiteral<'db> {
Type::instance(db, self.apply_optional_specialization(db, specialization));
let signature_from_fields = |mut parameters: Vec<_>, return_ty: Option<Type<'db>>| {
let mut kw_only_field_seen = false;
for (
field_name,
Field {
field @ Field {
declared_ty: mut field_ty,
mut default_ty,
init_only: _,
@ -1949,14 +1958,10 @@ impl<'db> ClassLiteral<'db> {
continue;
}
if field_ty
.into_nominal_instance()
.is_some_and(|instance| instance.class(db).is_known(db, KnownClass::KwOnly))
{
if field.is_kw_only_sentinel(db) {
// Attributes annotated with `dataclass.KW_ONLY` are not present in the synthesized
// `__init__` method; they are used to indicate that the following parameters are
// keyword-only.
kw_only_field_seen = true;
continue;
}
@ -2006,9 +2011,7 @@ impl<'db> ClassLiteral<'db> {
}
let is_kw_only = name == "__replace__"
|| kw_only.unwrap_or(
has_dataclass_param(DataclassParams::KW_ONLY) || kw_only_field_seen,
);
|| kw_only.unwrap_or(has_dataclass_param(DataclassParams::KW_ONLY));
let mut parameter = if is_kw_only {
Parameter::keyword_only(field_name)
@ -2307,6 +2310,7 @@ impl<'db> ClassLiteral<'db> {
let table = place_table(db, class_body_scope);
let use_def = use_def_map(db, class_body_scope);
let mut kw_only_sentinel_field_seen = false;
for (symbol_id, declarations) in use_def.all_end_of_scope_symbol_declarations() {
// Here, we exclude all declarations that are not annotated assignments. We need this because
// things like function definitions and nested classes would otherwise be considered dataclass
@ -2351,16 +2355,25 @@ impl<'db> ClassLiteral<'db> {
kw_only = field.kw_only(db);
}
attributes.insert(
symbol.name().clone(),
Field {
let mut field = Field {
declared_ty: attr_ty.apply_optional_specialization(db, specialization),
default_ty,
init_only: attr.is_init_var(),
init,
kw_only,
},
);
};
// Check if this is a KW_ONLY sentinel and mark subsequent fields as keyword-only
if field.is_kw_only_sentinel(db) {
kw_only_sentinel_field_seen = true;
}
// If no explicit kw_only setting and we've seen KW_ONLY sentinel, mark as keyword-only
if field.kw_only.is_none() && kw_only_sentinel_field_seen {
field.kw_only = Some(true);
}
attributes.insert(symbol.name().clone(), field);
}
}

View file

@ -90,7 +90,7 @@ use crate::semantic_index::{
ApplicableConstraints, EnclosingSnapshotResult, SemanticIndex, place_table, semantic_index,
};
use crate::types::call::{Binding, Bindings, CallArguments, CallError, CallErrorKind};
use crate::types::class::{CodeGeneratorKind, Field, MetaclassErrorKind};
use crate::types::class::{CodeGeneratorKind, MetaclassErrorKind};
use crate::types::diagnostic::{
self, CALL_NON_CALLABLE, CONFLICTING_DECLARATIONS, CONFLICTING_METACLASS,
CYCLIC_CLASS_DEFINITION, DIVISION_BY_ZERO, DUPLICATE_KW_ONLY, INCONSISTENT_MRO,
@ -1405,31 +1405,16 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
CodeGeneratorKind::from_class(self.db(), class)
{
let specialization = None;
let mut kw_only_field_names = vec![];
for (
name,
Field {
declared_ty: field_ty,
..
},
) in class.fields(self.db(), specialization, field_policy)
{
let Some(instance) = field_ty.into_nominal_instance() else {
continue;
};
let kw_only_sentinel_fields: Vec<_> = class
.fields(self.db(), specialization, field_policy)
.into_iter()
.filter_map(|(name, field)| {
field.is_kw_only_sentinel(self.db()).then_some(name)
})
.collect();
if !instance
.class(self.db())
.is_known(self.db(), KnownClass::KwOnly)
{
continue;
}
kw_only_field_names.push(name);
}
if kw_only_field_names.len() > 1 {
if kw_only_sentinel_fields.len() > 1 {
// TODO: The fields should be displayed in a subdiagnostic.
if let Some(builder) = self
.context
@ -1441,7 +1426,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
diagnostic.info(format_args!(
"`KW_ONLY` fields: {}",
kw_only_field_names
kw_only_sentinel_fields
.iter()
.map(|name| format!("`{name}`"))
.join(", ")