Skip to content

Commit 04355b9

Browse files
thejchapthejchap
committed
[ty] Refactor frozen dataclass diagnostic
1 parent f873f42 commit 04355b9

2 files changed

Lines changed: 136 additions & 70 deletions

File tree

‎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
@@ -3066,23 +3066,19 @@ impl<'db> TypeInferenceBuilder<'db> {
30663066
| Type::TypeVar(..)
30673067
| Type::AlwaysTruthy
30683068
| Type::AlwaysFalsy => {
3069-
if let Type::NominalInstance(instance) = object_ty {
3070-
let dataclass_params = match instance.class() {
3069+
let dataclass_params = match object_ty {
3070+
Type::NominalInstance(instance) => match instance.class() {
30713071
ClassType::NonGeneric(cls) => cls.dataclass_params(self.db()),
3072-
ClassType::Generic(_) => None,
3073-
};
3074-
let frozen = dataclass_params
3075-
.is_some_and(|params| params.contains(DataclassParams::FROZEN));
3076-
if frozen && emit_diagnostics {
3077-
if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target)
3078-
{
3079-
builder.into_diagnostic(format_args!(
3080-
"Property `{attribute}` defined in `{ty}` is read-only",
3081-
ty = object_ty.display(self.db()),
3082-
));
3072+
ClassType::Generic(cls) => {
3073+
cls.origin(self.db()).dataclass_params(self.db())
30833074
}
3084-
}
3085-
}
3075+
},
3076+
_ => None,
3077+
};
3078+
3079+
let read_only =
3080+
dataclass_params.is_some_and(|params| params.contains(DataclassParams::FROZEN));
3081+
30863082
match object_ty.class_member(db, attribute.into()) {
30873083
meta_attr @ SymbolAndQualifiers { .. } if meta_attr.is_class_var() => {
30883084
if emit_diagnostics {
@@ -3102,68 +3098,83 @@ impl<'db> TypeInferenceBuilder<'db> {
31023098
symbol: Symbol::Type(meta_attr_ty, meta_attr_boundness),
31033099
qualifiers: _,
31043100
} => {
3105-
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
3106-
meta_attr_ty.class_member(db, "__set__".into()).symbol
3107-
{
3108-
let successful_call = meta_dunder_set
3109-
.try_call(
3110-
db,
3111-
&CallArgumentTypes::positional([
3112-
meta_attr_ty,
3113-
object_ty,
3114-
value_ty,
3115-
]),
3116-
)
3117-
.is_ok();
3118-
3119-
if !successful_call && emit_diagnostics {
3101+
if read_only {
3102+
if emit_diagnostics {
31203103
if let Some(builder) =
31213104
self.context.report_lint(&INVALID_ASSIGNMENT, target)
31223105
{
3123-
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
31243106
builder.into_diagnostic(format_args!(
3125-
"Invalid assignment to data descriptor attribute \
3126-
`{attribute}` on type `{}` with custom `__set__` method",
3127-
object_ty.display(db)
3107+
"Property `{attribute}` defined in `{ty}` is read-only",
3108+
ty = object_ty.display(self.db()),
31283109
));
31293110
}
31303111
}
3131-
3132-
successful_call
3112+
false
31333113
} else {
3134-
ensure_assignable_to(meta_attr_ty)
3135-
};
3114+
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
3115+
meta_attr_ty.class_member(db, "__set__".into()).symbol
3116+
{
3117+
let successful_call = meta_dunder_set
3118+
.try_call(
3119+
db,
3120+
&CallArgumentTypes::positional([
3121+
meta_attr_ty,
3122+
object_ty,
3123+
value_ty,
3124+
]),
3125+
)
3126+
.is_ok();
31363127

3137-
let assignable_to_instance_attribute = if meta_attr_boundness
3138-
== Boundness::PossiblyUnbound
3139-
{
3140-
let (assignable, boundness) =
3141-
if let Symbol::Type(instance_attr_ty, instance_attr_boundness) =
3142-
object_ty.instance_member(db, attribute).symbol
3143-
{
3144-
(
3145-
ensure_assignable_to(instance_attr_ty),
3128+
if !successful_call && emit_diagnostics {
3129+
if let Some(builder) =
3130+
self.context.report_lint(&INVALID_ASSIGNMENT, target)
3131+
{
3132+
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
3133+
builder.into_diagnostic(format_args!(
3134+
"Invalid assignment to data descriptor attribute \
3135+
`{attribute}` on type `{}` with custom `__set__` method",
3136+
object_ty.display(db)
3137+
));
3138+
}
3139+
}
3140+
3141+
successful_call
3142+
} else {
3143+
ensure_assignable_to(meta_attr_ty)
3144+
};
3145+
3146+
let assignable_to_instance_attribute =
3147+
if meta_attr_boundness == Boundness::PossiblyUnbound {
3148+
let (assignable, boundness) = if let Symbol::Type(
3149+
instance_attr_ty,
31463150
instance_attr_boundness,
3147-
)
3148-
} else {
3149-
(true, Boundness::PossiblyUnbound)
3150-
};
3151+
) =
3152+
object_ty.instance_member(db, attribute).symbol
3153+
{
3154+
(
3155+
ensure_assignable_to(instance_attr_ty),
3156+
instance_attr_boundness,
3157+
)
3158+
} else {
3159+
(true, Boundness::PossiblyUnbound)
3160+
};
31513161

3152-
if boundness == Boundness::PossiblyUnbound {
3153-
report_possibly_unbound_attribute(
3154-
&self.context,
3155-
target,
3156-
attribute,
3157-
object_ty,
3158-
);
3159-
}
3162+
if boundness == Boundness::PossiblyUnbound {
3163+
report_possibly_unbound_attribute(
3164+
&self.context,
3165+
target,
3166+
attribute,
3167+
object_ty,
3168+
);
3169+
}
31603170

3161-
assignable
3162-
} else {
3163-
true
3164-
};
3171+
assignable
3172+
} else {
3173+
true
3174+
};
31653175

3166-
assignable_to_meta_attr && assignable_to_instance_attribute
3176+
assignable_to_meta_attr && assignable_to_instance_attribute
3177+
}
31673178
}
31683179

31693180
SymbolAndQualifiers {
@@ -3182,7 +3193,22 @@ impl<'db> TypeInferenceBuilder<'db> {
31823193
);
31833194
}
31843195

3185-
ensure_assignable_to(instance_attr_ty)
3196+
if read_only {
3197+
// TODO(thejchap): illustrating missing diagnostic
3198+
// if emit_diagnostics {
3199+
// if let Some(builder) =
3200+
// self.context.report_lint(&INVALID_ASSIGNMENT, target)
3201+
// {
3202+
// builder.into_diagnostic(format_args!(
3203+
// "Property `{attribute}` defined in `{ty}` is read-only",
3204+
// ty = object_ty.display(self.db()),
3205+
// ));
3206+
// }
3207+
// }
3208+
false
3209+
} else {
3210+
ensure_assignable_to(instance_attr_ty)
3211+
}
31863212
} else {
31873213
let result = object_ty.try_call_dunder_with_policy(
31883214
db,

0 commit comments

Comments
 (0)
⚡