Review feedback
This commit is contained in:
@@ -208,7 +208,7 @@ static_assert(not is_disjoint_from(type[Foo], type[Bar]))
|
||||
|
||||
## Using dynamic classes with `super()`
|
||||
|
||||
Dynamic classes can be used as pivot in `super()`:
|
||||
Dynamic classes can be used as the pivot class in `super()` calls:
|
||||
|
||||
```py
|
||||
class Base:
|
||||
@@ -256,10 +256,10 @@ child = ChildCls()
|
||||
reveal_type(child) # revealed: ChildCls
|
||||
reveal_type(child.base_attr) # revealed: int
|
||||
|
||||
# Child instances are subtypes of Parent instances.
|
||||
# Child instances are subtypes of `Parent` instances.
|
||||
def takes_parent(x: Parent) -> None: ...
|
||||
|
||||
takes_parent(child) # No error - ChildCls is a subtype of Parent
|
||||
takes_parent(child) # No error - `ChildCls` is a subtype of `Parent`
|
||||
```
|
||||
|
||||
## Dataclass transform inheritance
|
||||
@@ -397,11 +397,19 @@ Other numbers of arguments are invalid:
|
||||
|
||||
```py
|
||||
# error: [no-matching-overload] "No overload of class `type` matches arguments"
|
||||
type("Foo", ())
|
||||
reveal_type(type("Foo", ())) # revealed: Unknown
|
||||
|
||||
# TODO: keyword arguments should be supported for `__init_subclass__` calls
|
||||
# TODO: the keyword arguments for `Foo`/`Bar`/`Baz` here are invalid
|
||||
# (you cannot pass `metaclass=` to `type()`, and none of them have
|
||||
# base classes with `__init_subclass__` methods),
|
||||
# but `type[Unknown]` would be better than `Unknown` here
|
||||
#
|
||||
# error: [no-matching-overload] "No overload of class `type` matches arguments"
|
||||
type("Foo", (), {}, weird_other_arg=42)
|
||||
reveal_type(type("Foo", (), {}, weird_other_arg=42)) # revealed: Unknown
|
||||
# error: [no-matching-overload] "No overload of class `type` matches arguments"
|
||||
reveal_type(type("Bar", (int,), {}, weird_other_arg=42)) # revealed: Unknown
|
||||
# error: [no-matching-overload] "No overload of class `type` matches arguments"
|
||||
reveal_type(type("Baz", (), {}, metaclass=type)) # revealed: Unknown
|
||||
```
|
||||
|
||||
The following calls are also invalid, due to incorrect argument types:
|
||||
@@ -521,6 +529,15 @@ def make_class(name: str):
|
||||
cls = type(name, (), {})
|
||||
reveal_type(cls) # revealed: <class '<unknown>'>
|
||||
return cls
|
||||
|
||||
def make_classes(name1: str, name2: str):
|
||||
cls1 = type(name1, (), {})
|
||||
cls2 = type(name2, (), {})
|
||||
|
||||
def inner(x: cls1): ...
|
||||
|
||||
# error: [invalid-argument-type] "Argument to function `inner` is incorrect: Expected `mdtest_snippet.<locals of function 'make_classes'>.<unknown> @ src/mdtest_snippet.py:8`, found `mdtest_snippet.<locals of function 'make_classes'>.<unknown> @ src/mdtest_snippet.py:9`"
|
||||
inner(cls2())
|
||||
```
|
||||
|
||||
When the name comes from a union of string literals, we also use a placeholder name:
|
||||
@@ -563,7 +580,7 @@ def make_class(bases: tuple[type, ...]):
|
||||
return cls
|
||||
```
|
||||
|
||||
When bases is a module-level variable holding a tuple of class literals, we can extract the base
|
||||
When `bases` is a module-level variable holding a tuple of class literals, we can extract the base
|
||||
classes:
|
||||
|
||||
```py
|
||||
@@ -596,6 +613,31 @@ Cls2 = type("Cls2", (Base,), {**namespace})
|
||||
reveal_type(Cls2) # revealed: <class 'Cls2'>
|
||||
```
|
||||
|
||||
When `*args` or `**kwargs` fill an unknown number of parameters, we cannot determine which overload
|
||||
of `type()` is being called:
|
||||
|
||||
```py
|
||||
def f(*args, **kwargs):
|
||||
# Completely dynamic: could be 1-arg or 3-arg form
|
||||
A = type(*args, **kwargs)
|
||||
reveal_type(A) # revealed: type[Unknown]
|
||||
|
||||
# Has a string first arg, but unknown additional args from *args
|
||||
B = type("B", *args, **kwargs)
|
||||
# TODO: `type[Unknown]` would cause fewer false positives
|
||||
reveal_type(B) # revealed: <class 'str'>
|
||||
|
||||
# Has string and tuple, but unknown additional args
|
||||
C = type("C", (), *args, **kwargs)
|
||||
# TODO: `type[Unknown]` would cause fewer false positives
|
||||
reveal_type(C) # revealed: type
|
||||
|
||||
# All three positional args provided, only **kwargs unknown
|
||||
D = type("D", (), {}, **kwargs)
|
||||
# TODO: `type[Unknown]` would cause fewer false positives
|
||||
reveal_type(D) # revealed: type
|
||||
```
|
||||
|
||||
## Explicit type annotations
|
||||
|
||||
When an explicit type annotation is provided, the inferred type is checked against it:
|
||||
@@ -693,12 +735,12 @@ reveal_type(ValidExtension) # revealed: <class 'ValidExtension'>
|
||||
|
||||
## `__init_subclass__` keyword arguments
|
||||
|
||||
When a base class defines `__init_subclass__` with required keyword arguments, those should be
|
||||
passed to `type()`. This is not yet supported:
|
||||
When a base class defines `__init_subclass__` with required arguments, those should be passed to
|
||||
`type()`. This is not yet supported:
|
||||
|
||||
```py
|
||||
class Base:
|
||||
def __init_subclass__(cls, *, required_arg: str, **kwargs):
|
||||
def __init_subclass__(cls, required_arg: str, **kwargs):
|
||||
super().__init_subclass__(**kwargs)
|
||||
cls.config = required_arg
|
||||
|
||||
|
||||
@@ -926,9 +926,9 @@ from ty_extensions import has_member, static_assert
|
||||
|
||||
DynamicWithDict = type("DynamicWithDict", (), {"custom_attr": 42})
|
||||
|
||||
# Namespace dict attributes are not available for autocomplete
|
||||
static_assert(not has_member(DynamicWithDict, "custom_attr"))
|
||||
static_assert(not has_member(DynamicWithDict(), "custom_attr"))
|
||||
# TODO: these should pass -- namespace dict attributes are not yet available for autocomplete
|
||||
static_assert(has_member(DynamicWithDict, "custom_attr")) # error: [static-assert-error]
|
||||
static_assert(has_member(DynamicWithDict(), "custom_attr")) # error: [static-assert-error]
|
||||
```
|
||||
|
||||
Dynamic classes inheriting from classes with custom metaclasses get metaclass members:
|
||||
@@ -960,10 +960,10 @@ class Base:
|
||||
DynamicSingle = type("DynamicSingle", (Base,), {})
|
||||
instance = DynamicSingle()
|
||||
|
||||
# TODO: Instance members should be available but currently are not
|
||||
static_assert(not has_member(instance, "base_attr"))
|
||||
static_assert(not has_member(instance, "__repr__"))
|
||||
static_assert(not has_member(instance, "__hash__"))
|
||||
# TODO: these should pass; instance members should be available
|
||||
static_assert(has_member(instance, "base_attr")) # error: [static-assert-error]
|
||||
static_assert(has_member(instance, "__repr__")) # error: [static-assert-error]
|
||||
static_assert(has_member(instance, "__hash__")) # error: [static-assert-error]
|
||||
```
|
||||
|
||||
### Attributes not available at runtime
|
||||
|
||||
@@ -596,12 +596,11 @@ impl<'db> ClassLiteral<'db> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the metaclass instance type for this class.
|
||||
/// Return a type representing "the set of all instances of the metaclass of this class".
|
||||
pub(crate) fn metaclass_instance_type(self, db: &'db dyn Db) -> Type<'db> {
|
||||
match self {
|
||||
Self::Static(class) => class.metaclass_instance_type(db),
|
||||
Self::Dynamic(class) => class.metaclass(db),
|
||||
}
|
||||
self.metaclass(db)
|
||||
.to_instance(db)
|
||||
.expect("`Type::to_instance()` should always return `Some()` when called on the type of a metaclass")
|
||||
}
|
||||
|
||||
/// Returns whether this class is type-check only.
|
||||
@@ -635,6 +634,8 @@ impl<'db> ClassLiteral<'db> {
|
||||
}
|
||||
|
||||
/// Returns whether this class is final.
|
||||
// TODO: Support `@final` on dynamic classes, e.g. `X = final(type("X", (), {}))`.
|
||||
// We should either recognize and track this, or emit a diagnostic if unsupported.
|
||||
pub(crate) fn is_final(self, db: &'db dyn Db) -> bool {
|
||||
self.as_static().is_some_and(|class| class.is_final(db))
|
||||
}
|
||||
@@ -2585,14 +2586,6 @@ impl<'db> StaticClassLiteral<'db> {
|
||||
.unwrap_or_else(|_| SubclassOfType::subclass_of_unknown())
|
||||
}
|
||||
|
||||
/// Return a type representing "the set of all instances of the metaclass of this class".
|
||||
pub(super) fn metaclass_instance_type(self, db: &'db dyn Db) -> Type<'db> {
|
||||
self
|
||||
.metaclass(db)
|
||||
.to_instance(db)
|
||||
.expect("`Type::to_instance()` should always return `Some()` when called on the type of a metaclass")
|
||||
}
|
||||
|
||||
/// Return the metaclass of this class, or an error if the metaclass cannot be inferred.
|
||||
#[salsa::tracked(cycle_initial=try_metaclass_cycle_initial,
|
||||
heap_size=ruff_memory_usage::heap_size,
|
||||
|
||||
@@ -3205,7 +3205,7 @@ pub(crate) fn report_invalid_argument_number_to_special_form(
|
||||
pub(crate) fn report_bad_argument_to_get_protocol_members(
|
||||
context: &InferContext,
|
||||
call: &ast::ExprCall,
|
||||
class: StaticClassLiteral,
|
||||
class: ClassLiteral,
|
||||
) {
|
||||
let Some(builder) = context.report_lint(&INVALID_ARGUMENT_TYPE, call) else {
|
||||
return;
|
||||
|
||||
@@ -1737,9 +1737,7 @@ impl KnownFunction {
|
||||
if class.is_protocol(db) {
|
||||
return;
|
||||
}
|
||||
if let Some(class) = class.as_static() {
|
||||
report_bad_argument_to_get_protocol_members(context, call_expression, class);
|
||||
}
|
||||
report_bad_argument_to_get_protocol_members(context, call_expression, *class);
|
||||
}
|
||||
|
||||
KnownFunction::RevealProtocolInterface => {
|
||||
|
||||
@@ -659,6 +659,8 @@ fn c3_merge(mut sequences: Vec<VecDeque<ClassBase>>) -> Option<Mro> {
|
||||
}
|
||||
|
||||
/// Error for dynamic class MRO computation with fallback MRO.
|
||||
///
|
||||
/// Separate from [`MroError`] because dynamic classes can only have a subset of MRO errors.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize, salsa::Update)]
|
||||
pub(crate) struct DynamicMroError<'db> {
|
||||
kind: DynamicMroErrorKind<'db>,
|
||||
@@ -679,7 +681,7 @@ impl<'db> DynamicMroError<'db> {
|
||||
|
||||
/// Error kinds for dynamic class MRO computation.
|
||||
///
|
||||
/// These mirror the relevant variants from `MroErrorKind` for regular classes.
|
||||
/// These mirror the relevant variants from `MroErrorKind` for static classes.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, get_size2::GetSize, salsa::Update)]
|
||||
pub(crate) enum DynamicMroErrorKind<'db> {
|
||||
/// The class has duplicate bases in its bases tuple.
|
||||
|
||||
Reference in New Issue
Block a user