mirror of https://github.com/astral-sh/ruff
[ty] rollback preferring declared type on invalid TypedDict creation (#21169)
## Summary Discussion with @ibraheemdev clarified that https://github.com/astral-sh/ruff/pull/21168 was incorrect. In a case of failed inference of a dict literal as a `TypedDict`, we should store the context-less inferred type of the dict literal as the type of the dict literal expression itself; the fallback to declared type should happen at the level of the overall assignment definition. The reason the latter isn't working yet is because currently we (wrongly) consider a homogeneous dict type as assignable to a `TypedDict`, so we don't actually consider the assignment itself as failed. So the "bug" I observed (and tried to fix) will naturally be fixed by implementing TypedDict assignability rules. Rollback https://github.com/astral-sh/ruff/pull/21168 except for the tests, and modify the tests to include TODOs as needed. ## Test Plan Updated mdtests.
This commit is contained in:
parent
69b4c29924
commit
9664474c51
|
|
@ -99,7 +99,8 @@ eve1a: Person = {"name": b"Eve", "age": None}
|
|||
# error: [invalid-argument-type] "Invalid argument to key "name" with declared type `str` on TypedDict `Person`"
|
||||
eve1b = Person(name=b"Eve", age=None)
|
||||
|
||||
reveal_type(eve1a) # revealed: Person
|
||||
# TODO should reveal Person (should be fixed by implementing assignability for TypedDicts)
|
||||
reveal_type(eve1a) # revealed: dict[Unknown | str, Unknown | bytes | None]
|
||||
reveal_type(eve1b) # revealed: Person
|
||||
|
||||
# error: [missing-typed-dict-key] "Missing required key 'name' in TypedDict `Person` constructor"
|
||||
|
|
@ -107,7 +108,8 @@ eve2a: Person = {"age": 22}
|
|||
# error: [missing-typed-dict-key] "Missing required key 'name' in TypedDict `Person` constructor"
|
||||
eve2b = Person(age=22)
|
||||
|
||||
reveal_type(eve2a) # revealed: Person
|
||||
# TODO should reveal Person (should be fixed by implementing assignability for TypedDicts)
|
||||
reveal_type(eve2a) # revealed: dict[Unknown | str, Unknown | int]
|
||||
reveal_type(eve2b) # revealed: Person
|
||||
|
||||
# error: [invalid-key] "Invalid key for TypedDict `Person`: Unknown key "extra""
|
||||
|
|
@ -115,7 +117,8 @@ eve3a: Person = {"name": "Eve", "age": 25, "extra": True}
|
|||
# error: [invalid-key] "Invalid key for TypedDict `Person`: Unknown key "extra""
|
||||
eve3b = Person(name="Eve", age=25, extra=True)
|
||||
|
||||
reveal_type(eve3a) # revealed: Person
|
||||
# TODO should reveal Person (should be fixed by implementing assignability for TypedDicts)
|
||||
reveal_type(eve3a) # revealed: dict[Unknown | str, Unknown | str | int]
|
||||
reveal_type(eve3b) # revealed: Person
|
||||
```
|
||||
|
||||
|
|
|
|||
|
|
@ -6239,9 +6239,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
&& let Some(typed_dict) = tcx
|
||||
.filter_union(self.db(), Type::is_typed_dict)
|
||||
.as_typed_dict()
|
||||
&& let Some(ty) = self.infer_typed_dict_expression(dict, typed_dict)
|
||||
{
|
||||
self.infer_typed_dict_expression(dict, typed_dict);
|
||||
return Type::TypedDict(typed_dict);
|
||||
return ty;
|
||||
}
|
||||
|
||||
// Avoid false positives for the functional `TypedDict` form, which is currently
|
||||
|
|
@ -6266,7 +6266,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
&mut self,
|
||||
dict: &ast::ExprDict,
|
||||
typed_dict: TypedDictType<'db>,
|
||||
) {
|
||||
) -> Option<Type<'db>> {
|
||||
let ast::ExprDict {
|
||||
range: _,
|
||||
node_index: _,
|
||||
|
|
@ -6289,7 +6289,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
|
||||
validate_typed_dict_dict_literal(&self.context, typed_dict, dict, dict.into(), |expr| {
|
||||
self.expression_type(expr)
|
||||
});
|
||||
})
|
||||
.ok()
|
||||
.map(|_| Type::TypedDict(typed_dict))
|
||||
}
|
||||
|
||||
// Infer the type of a collection literal expression.
|
||||
|
|
|
|||
|
|
@ -389,7 +389,7 @@ fn validate_from_keywords<'db, 'ast>(
|
|||
provided_keys
|
||||
}
|
||||
|
||||
/// Validates a `TypedDict` dictionary literal assignment, emitting any needed diagnostics.
|
||||
/// Validates a `TypedDict` dictionary literal assignment,
|
||||
/// e.g. `person: Person = {"name": "Alice", "age": 30}`
|
||||
pub(super) fn validate_typed_dict_dict_literal<'db>(
|
||||
context: &InferContext<'db, '_>,
|
||||
|
|
@ -397,7 +397,8 @@ pub(super) fn validate_typed_dict_dict_literal<'db>(
|
|||
dict_expr: &ast::ExprDict,
|
||||
error_node: AnyNodeRef,
|
||||
expression_type_fn: impl Fn(&ast::Expr) -> Type<'db>,
|
||||
) {
|
||||
) -> Result<OrderSet<&'db str>, OrderSet<&'db str>> {
|
||||
let mut valid = true;
|
||||
let mut provided_keys = OrderSet::new();
|
||||
|
||||
// Validate each key-value pair in the dictionary literal
|
||||
|
|
@ -410,7 +411,7 @@ pub(super) fn validate_typed_dict_dict_literal<'db>(
|
|||
|
||||
let value_type = expression_type_fn(&item.value);
|
||||
|
||||
validate_typed_dict_key_assignment(
|
||||
valid &= validate_typed_dict_key_assignment(
|
||||
context,
|
||||
typed_dict,
|
||||
key_str,
|
||||
|
|
@ -423,5 +424,11 @@ pub(super) fn validate_typed_dict_dict_literal<'db>(
|
|||
}
|
||||
}
|
||||
|
||||
validate_typed_dict_required_keys(context, typed_dict, &provided_keys, error_node);
|
||||
valid &= validate_typed_dict_required_keys(context, typed_dict, &provided_keys, error_node);
|
||||
|
||||
if valid {
|
||||
Ok(provided_keys)
|
||||
} else {
|
||||
Err(provided_keys)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue