Skip to content

Commit 4892268

Browse files
thejchapthejchap
committed
[ty] Refactor frozen dataclass diagnostic
1 parent d3a8d73 commit 4892268

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
@@ -2924,23 +2924,19 @@ impl<'db> TypeInferenceBuilder<'db> {
29242924
| Type::TypeVar(..)
29252925
| Type::AlwaysTruthy
29262926
| Type::AlwaysFalsy => {
2927-
if let Type::NominalInstance(instance) = object_ty {
2928-
let dataclass_params = match instance.class() {
2927+
let dataclass_params = match object_ty {
2928+
Type::NominalInstance(instance) => match instance.class() {
29292929
ClassType::NonGeneric(cls) => cls.dataclass_params(self.db()),
2930-
ClassType::Generic(_) => None,
2931-
};
2932-
let frozen = dataclass_params
2933-
.is_some_and(|params| params.contains(DataclassParams::FROZEN));
2934-
if frozen && emit_diagnostics {
2935-
if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target)
2936-
{
2937-
builder.into_diagnostic(format_args!(
2938-
"Property `{attribute}` defined in `{ty}` is read-only",
2939-
ty = object_ty.display(self.db()),
2940-
));
2930+
ClassType::Generic(cls) => {
2931+
cls.origin(self.db()).dataclass_params(self.db())
29412932
}
2942-
}
2943-
}
2933+
},
2934+
_ => None,
2935+
};
2936+
2937+
let read_only =
2938+
dataclass_params.is_some_and(|params| params.contains(DataclassParams::FROZEN));
2939+
29442940
match object_ty.class_member(db, attribute.into()) {
29452941
meta_attr @ SymbolAndQualifiers { .. } if meta_attr.is_class_var() => {
29462942
if emit_diagnostics {
@@ -2960,68 +2956,83 @@ impl<'db> TypeInferenceBuilder<'db> {
29602956
symbol: Symbol::Type(meta_attr_ty, meta_attr_boundness),
29612957
qualifiers: _,
29622958
} => {
2963-
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
2964-
meta_attr_ty.class_member(db, "__set__".into()).symbol
2965-
{
2966-
let successful_call = meta_dunder_set
2967-
.try_call(
2968-
db,
2969-
&CallArgumentTypes::positional([
2970-
meta_attr_ty,
2971-
object_ty,
2972-
value_ty,
2973-
]),
2974-
)
2975-
.is_ok();
2976-
2977-
if !successful_call && emit_diagnostics {
2959+
if read_only {
2960+
if emit_diagnostics {
29782961
if let Some(builder) =
29792962
self.context.report_lint(&INVALID_ASSIGNMENT, target)
29802963
{
2981-
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
29822964
builder.into_diagnostic(format_args!(
2983-
"Invalid assignment to data descriptor attribute \
2984-
`{attribute}` on type `{}` with custom `__set__` method",
2985-
object_ty.display(db)
2965+
"Property `{attribute}` defined in `{ty}` is read-only",
2966+
ty = object_ty.display(self.db()),
29862967
));
29872968
}
29882969
}
2989-
2990-
successful_call
2970+
false
29912971
} else {
2992-
ensure_assignable_to(meta_attr_ty)
2993-
};
2972+
let assignable_to_meta_attr = if let Symbol::Type(meta_dunder_set, _) =
2973+
meta_attr_ty.class_member(db, "__set__".into()).symbol
2974+
{
2975+
let successful_call = meta_dunder_set
2976+
.try_call(
2977+
db,
2978+
&CallArgumentTypes::positional([
2979+
meta_attr_ty,
2980+
object_ty,
2981+
value_ty,
2982+
]),
2983+
)
2984+
.is_ok();
29942985

2995-
let assignable_to_instance_attribute = if meta_attr_boundness
2996-
== Boundness::PossiblyUnbound
2997-
{
2998-
let (assignable, boundness) =
2999-
if let Symbol::Type(instance_attr_ty, instance_attr_boundness) =
3000-
object_ty.instance_member(db, attribute).symbol
3001-
{
3002-
(
3003-
ensure_assignable_to(instance_attr_ty),
2986+
if !successful_call && emit_diagnostics {
2987+
if let Some(builder) =
2988+
self.context.report_lint(&INVALID_ASSIGNMENT, target)
2989+
{
2990+
// TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
2991+
builder.into_diagnostic(format_args!(
2992+
"Invalid assignment to data descriptor attribute \
2993+
`{attribute}` on type `{}` with custom `__set__` method",
2994+
object_ty.display(db)
2995+
));
2996+
}
2997+
}
2998+
2999+
successful_call
3000+
} else {
3001+
ensure_assignable_to(meta_attr_ty)
3002+
};
3003+
3004+
let assignable_to_instance_attribute =
3005+
if meta_attr_boundness == Boundness::PossiblyUnbound {
3006+
let (assignable, boundness) = if let Symbol::Type(
3007+
instance_attr_ty,
30043008
instance_attr_boundness,
3005-
)
3006-
} else {
3007-
(true, Boundness::PossiblyUnbound)
3008-
};
3009+
) =
3010+
object_ty.instance_member(db, attribute).symbol
3011+
{
3012+
(
3013+
ensure_assignable_to(instance_attr_ty),
3014+
instance_attr_boundness,
3015+
)
3016+
} else {
3017+
(true, Boundness::PossiblyUnbound)
3018+
};
30093019

3010-
if boundness == Boundness::PossiblyUnbound {
3011-
report_possibly_unbound_attribute(
3012-
&self.context,
3013-
target,
3014-
attribute,
3015-
object_ty,
3016-
);
3017-
}
3020+
if boundness == Boundness::PossiblyUnbound {
3021+
report_possibly_unbound_attribute(
3022+
&self.context,
3023+
target,
3024+
attribute,
3025+
object_ty,
3026+
);
3027+
}
30183028

3019-
assignable
3020-
} else {
3021-
true
3022-
};
3029+
assignable
3030+
} else {
3031+
true
3032+
};
30233033

3024-
assignable_to_meta_attr && assignable_to_instance_attribute
3034+
assignable_to_meta_attr && assignable_to_instance_attribute
3035+
}
30253036
}
30263037

30273038
SymbolAndQualifiers {
@@ -3040,7 +3051,22 @@ impl<'db> TypeInferenceBuilder<'db> {
30403051
);
30413052
}
30423053

3043-
ensure_assignable_to(instance_attr_ty)
3054+
if read_only {
3055+
// TODO(thejchap): illustrating missing diagnostic
3056+
// if emit_diagnostics {
3057+
// if let Some(builder) =
3058+
// self.context.report_lint(&INVALID_ASSIGNMENT, target)
3059+
// {
3060+
// builder.into_diagnostic(format_args!(
3061+
// "Property `{attribute}` defined in `{ty}` is read-only",
3062+
// ty = object_ty.display(self.db()),
3063+
// ));
3064+
// }
3065+
// }
3066+
false
3067+
} else {
3068+
ensure_assignable_to(instance_attr_ty)
3069+
}
30443070
} else {
30453071
let result = object_ty.try_call_dunder_with_policy(
30463072
db,

0 commit comments

Comments
 (0)