Skip to content

Commit c38ec21

Browse files
thejchapcarljm
thejchap
authored and
carljm
committed
[ty] Refactor frozen dataclass diagnostic
1 parent c9d1aaf commit c38ec21

File tree

2 files changed

+136
-70
lines changed

2 files changed

+136
-70
lines changed

‎crates/ty_python_semantic/resources/mdtest/dataclasses.md‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -369,24 +369,64 @@ To do
369369

370370
### `frozen`
371371

372-
If true (the default is False), assigning to fields will generate a diagnostic. If `__setattr__()` or
373-
`__delattr__()` is defined in the class, we should emit a diagnostic.
372+
If true (the default is False), assigning to fields will generate a diagnostic.
374373

375374
```py
376375
from dataclasses import dataclass
377376

378377
@dataclass(frozen=True)
379-
class Frozen:
378+
class MyFrozenClass:
379+
x: int
380+
381+
frozen_instance = MyFrozenClass(1)
382+
frozen_instance.x = 2 # error: [invalid-assignment]
383+
```
384+
385+
If `__setattr__()` or `__delattr__()` is defined in the class, we should emit a diagnostic.
386+
387+
```py
388+
from dataclasses import dataclass
389+
390+
@dataclass(frozen=True)
391+
class MyFrozenClass:
380392
x: int
381393

382394
# TODO: Emit a diagnostic here
383395
def __setattr__(self, name: str, value: object) -> None: ...
384396

385397
# TODO: Emit a diagnostic here
386398
def __delattr__(self, name: str) -> None: ...
399+
```
400+
401+
This also works for generic dataclasses:
402+
403+
```toml
404+
[environment]
405+
python-version = "3.12"
406+
```
407+
408+
```py
409+
from dataclasses import dataclass
410+
411+
@dataclass(frozen=True)
412+
class MyFrozenGeneric[T]:
413+
x: T
414+
415+
frozen_instance = MyFrozenGeneric[int](1)
416+
frozen_instance.x = 2 # TODO
417+
```
418+
419+
When attempting to mutate an unresolved attribute on a frozen dataclass, only `unresolved-attribute`
420+
is emitted:
421+
422+
```py
423+
from dataclasses import dataclass
424+
425+
@dataclass(frozen=True)
426+
class MyFrozenClass: ...
387427

388-
frozen = Frozen(1)
389-
frozen.x = 2 # error: [invalid-assignment]
428+
frozen = MyFrozenClass()
429+
frozen.x = 2 # error: [unresolved-attribute]
390430
```
391431

392432
### `match_args`

‎crates/ty_python_semantic/src/types/infer.rs‎

Lines changed: 91 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3088,23 +3088,19 @@ impl<'db> TypeInferenceBuilder<'db> {
30883088
| Type::TypeVar(..)
30893089
| Type::AlwaysTruthy
30903090
| Type::AlwaysFalsy => {
3091-
if let Type::NominalInstance(instance) = object_ty {
3092-
let dataclass_params = match instance.class() {
3091+
let dataclass_params = match object_ty {
3092+
Type::NominalInstance(instance) => match instance.class() {
30933093
ClassType::NonGeneric(cls) => cls.dataclass_params(self.db()),
3094-
ClassType::Generic(_) => None,
3095-
};
3096-
let frozen = dataclass_params
3097-
.is_some_and(|params| params.contains(DataclassParams::FROZEN));
3098-
if frozen && emit_diagnostics {
3099-
if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target)
3100-
{
3101-
builder.into_diagnostic(format_args!(
3102-
"Property `{attribute}` defined in `{ty}` is read-only",
3103-
ty = object_ty.display(self.db()),
3104-
));
3094+
ClassType::Generic(cls) => {
3095+
cls.origin(self.db()).dataclass_params(self.db())
31053096
}
3106-
}
3107-
}
3097+
},
3098+
_ => None,
3099+
};
3100+
3101+
let read_only =
3102+
dataclass_params.is_some_and(|params| params.contains(DataclassParams::FROZEN));
3103+
31083104
match object_ty.class_member(db, attribute.into()) {
31093105
meta_attr @ SymbolAndQualifiers { .. } if meta_attr.is_class_var() => {
31103106
if emit_diagnostics {
@@ -3124,68 +3120,83 @@ impl<'db> TypeInferenceBuilder<'db> {
31243120
symbol: Symbol::Type(meta_attr_ty, meta_attr_boundness),
31253121
qualifiers: _,
31263122
} => {
3127-
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
3128-
meta_attr_ty.class_member(db, "__set__".into()).symbol
3129-
{
3130-
let successful_call = meta_dunder_set
3131-
.try_call(
3132-
db,
3133-
&CallArgumentTypes::positional([
3134-
meta_attr_ty,
3135-
object_ty,
3136-
value_ty,
3137-
]),
3138-
)
3139-
.is_ok();
3140-
3141-
if !successful_call && emit_diagnostics {
3123+
if read_only {
3124+
if emit_diagnostics {
31423125
if let Some(builder) =
31433126
self.context.report_lint(&INVALID_ASSIGNMENT, target)
31443127
{
3145-
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
31463128
builder.into_diagnostic(format_args!(
3147-
"Invalid assignment to data descriptor attribute \
3148-
`{attribute}` on type `{}` with custom `__set__` method",
3149-
object_ty.display(db)
3129+
"Property `{attribute}` defined in `{ty}` is read-only",
3130+
ty = object_ty.display(self.db()),
31503131
));
31513132
}
31523133
}
3153-
3154-
successful_call
3134+
false
31553135
} else {
3156-
ensure_assignable_to(meta_attr_ty)
3157-
};
3136+
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
3137+
meta_attr_ty.class_member(db, "__set__".into()).symbol
3138+
{
3139+
let successful_call = meta_dunder_set
3140+
.try_call(
3141+
db,
3142+
&CallArgumentTypes::positional([
3143+
meta_attr_ty,
3144+
object_ty,
3145+
value_ty,
3146+
]),
3147+
)
3148+
.is_ok();
31583149

3159-
let assignable_to_instance_attribute = if meta_attr_boundness
3160-
== Boundness::PossiblyUnbound
3161-
{
3162-
let (assignable, boundness) =
3163-
if let Symbol::Type(instance_attr_ty, instance_attr_boundness) =
3164-
object_ty.instance_member(db, attribute).symbol
3165-
{
3166-
(
3167-
ensure_assignable_to(instance_attr_ty),
3150+
if !successful_call && emit_diagnostics {
3151+
if let Some(builder) =
3152+
self.context.report_lint(&INVALID_ASSIGNMENT, target)
3153+
{
3154+
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
3155+
builder.into_diagnostic(format_args!(
3156+
"Invalid assignment to data descriptor attribute \
3157+
`{attribute}` on type `{}` with custom `__set__` method",
3158+
object_ty.display(db)
3159+
));
3160+
}
3161+
}
3162+
3163+
successful_call
3164+
} else {
3165+
ensure_assignable_to(meta_attr_ty)
3166+
};
3167+
3168+
let assignable_to_instance_attribute =
3169+
if meta_attr_boundness == Boundness::PossiblyUnbound {
3170+
let (assignable, boundness) = if let Symbol::Type(
3171+
instance_attr_ty,
31683172
instance_attr_boundness,
3169-
)
3170-
} else {
3171-
(true, Boundness::PossiblyUnbound)
3172-
};
3173+
) =
3174+
object_ty.instance_member(db, attribute).symbol
3175+
{
3176+
(
3177+
ensure_assignable_to(instance_attr_ty),
3178+
instance_attr_boundness,
3179+
)
3180+
} else {
3181+
(true, Boundness::PossiblyUnbound)
3182+
};
31733183

3174-
if boundness == Boundness::PossiblyUnbound {
3175-
report_possibly_unbound_attribute(
3176-
&self.context,
3177-
target,
3178-
attribute,
3179-
object_ty,
3180-
);
3181-
}
3184+
if boundness == Boundness::PossiblyUnbound {
3185+
report_possibly_unbound_attribute(
3186+
&self.context,
3187+
target,
3188+
attribute,
3189+
object_ty,
3190+
);
3191+
}
31823192

3183-
assignable
3184-
} else {
3185-
true
3186-
};
3193+
assignable
3194+
} else {
3195+
true
3196+
};
31873197

3188-
assignable_to_meta_attr && assignable_to_instance_attribute
3198+
assignable_to_meta_attr && assignable_to_instance_attribute
3199+
}
31893200
}
31903201

31913202
SymbolAndQualifiers {
@@ -3204,7 +3215,22 @@ impl<'db> TypeInferenceBuilder<'db> {
32043215
);
32053216
}
32063217

3207-
ensure_assignable_to(instance_attr_ty)
3218+
if read_only {
3219+
// TODO(thejchap): illustrating missing diagnostic
3220+
// if emit_diagnostics {
3221+
// if let Some(builder) =
3222+
// self.context.report_lint(&INVALID_ASSIGNMENT, target)
3223+
// {
3224+
// builder.into_diagnostic(format_args!(
3225+
// "Property `{attribute}` defined in `{ty}` is read-only",
3226+
// ty = object_ty.display(self.db()),
3227+
// ));
3228+
// }
3229+
// }
3230+
false
3231+
} else {
3232+
ensure_assignable_to(instance_attr_ty)
3233+
}
32083234
} else {
32093235
let result = object_ty.try_call_dunder_with_policy(
32103236
db,

0 commit comments

Comments
 (0)