[ty] Validate metaclass __new__/__prepare__/__init__, and meta-metaclass __call__, as well as __init_subclass__#24550
[ty] Validate metaclass __new__/__prepare__/__init__, and meta-metaclass __call__, as well as __init_subclass__#24550AlexWaygood wants to merge 1 commit intomainfrom
__new__/__prepare__/__init__, and meta-metaclass __call__, as well as __init_subclass__#24550Conversation
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
from
c950304 to
7244ec4
Compare
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 87.94%. The percentage of expected errors that received a diagnostic held steady at 83.36%. The number of fully passing files held steady at 79/133. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
45 | 0 | 0 |
too-many-positional-arguments |
42 | 0 | 0 |
unknown-argument |
6 | 0 | 0 |
| Total | 93 | 0 | 0 |
Raw diff (93 changes)
hydpy (https://github.com/hydpy-dev/hydpy)
+ hydpy/auxs/iuhtools.py:129:10 error[invalid-argument-type] Argument to constructor `MetaIUH.__new__` is incorrect: Expected `tuple[type[Any]]`, found `tuple[()]`
manticore (https://github.com/trailofbits/manticore)
+ server/manticore_server/ManticoreServer_pb2.pyi:151:19 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["HookType"]`
+ server/manticore_server/ManticoreServer_pb2.pyi:151:19 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ server/manticore_server/ManticoreServer_pb2.pyi:315:22 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["StateAction"]`
+ server/manticore_server/ManticoreServer_pb2.pyi:315:22 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
mypy-protobuf (https://github.com/dropbox/mypy-protobuf)
+ test/generated/testproto/nested/nested_pb2.pyi:51:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum"]`
+ test/generated/testproto/nested/nested_pb2.pyi:51:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/nested/nested_pb2.pyi:70:26 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum2"]`
+ test/generated/testproto/nested/nested_pb2.pyi:70:26 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/readme_enum_pb2.pyi:28:13 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["MyEnum"]`
+ test/generated/testproto/readme_enum_pb2.pyi:28:13 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test3_pb2.pyi:33:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated/testproto/test3_pb2.pyi:33:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test3_pb2.pyi:69:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated/testproto/test3_pb2.pyi:69:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/nested/nested_pb2.pyi:51:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum"]`
+ test/generated_async_only/testproto/nested/nested_pb2.pyi:51:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/nested/nested_pb2.pyi:70:26 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum2"]`
+ test/generated_async_only/testproto/nested/nested_pb2.pyi:70:26 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/readme_enum_pb2.pyi:28:13 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["MyEnum"]`
+ test/generated_async_only/testproto/readme_enum_pb2.pyi:28:13 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test3_pb2.pyi:33:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated_async_only/testproto/test3_pb2.pyi:33:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test3_pb2.pyi:69:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated_async_only/testproto/test3_pb2.pyi:69:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/nested/nested_pb2.pyi:51:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum"]`
+ test/generated_concrete/testproto/nested/nested_pb2.pyi:51:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/nested/nested_pb2.pyi:70:26 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum2"]`
+ test/generated_concrete/testproto/nested/nested_pb2.pyi:70:26 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/readme_enum_pb2.pyi:28:13 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["MyEnum"]`
+ test/generated_concrete/testproto/readme_enum_pb2.pyi:28:13 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test3_pb2.pyi:33:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated_concrete/testproto/test3_pb2.pyi:33:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test3_pb2.pyi:69:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated_concrete/testproto/test3_pb2.pyi:69:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/nested/nested_pb2.pyi:51:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum"]`
+ test/generated_sync_only/testproto/nested/nested_pb2.pyi:51:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/nested/nested_pb2.pyi:70:26 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NestedEnum2"]`
+ test/generated_sync_only/testproto/nested/nested_pb2.pyi:70:26 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/readme_enum_pb2.pyi:28:13 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["MyEnum"]`
+ test/generated_sync_only/testproto/readme_enum_pb2.pyi:28:13 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test3_pb2.pyi:33:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated_sync_only/testproto/test3_pb2.pyi:33:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test3_pb2.pyi:69:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated_sync_only/testproto/test3_pb2.pyi:69:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test_pb2.pyi:46:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated/testproto/test_pb2.pyi:46:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test_pb2.pyi:62:22 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NamingConflicts"]`
+ test/generated/testproto/test_pb2.pyi:62:22 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test_pb2.pyi:92:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["DeprecatedEnum"]`
+ test/generated/testproto/test_pb2.pyi:92:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test_pb2.pyi:117:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated/testproto/test_pb2.pyi:117:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated/testproto/test_pb2.pyi:351:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["_r_finally"]`
+ test/generated/testproto/test_pb2.pyi:351:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test_pb2.pyi:46:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated_async_only/testproto/test_pb2.pyi:46:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test_pb2.pyi:62:22 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NamingConflicts"]`
+ test/generated_async_only/testproto/test_pb2.pyi:62:22 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test_pb2.pyi:92:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["DeprecatedEnum"]`
+ test/generated_async_only/testproto/test_pb2.pyi:92:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test_pb2.pyi:117:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated_async_only/testproto/test_pb2.pyi:117:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_async_only/testproto/test_pb2.pyi:351:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["_r_finally"]`
+ test/generated_async_only/testproto/test_pb2.pyi:351:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test_pb2.pyi:46:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated_concrete/testproto/test_pb2.pyi:46:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test_pb2.pyi:62:22 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NamingConflicts"]`
+ test/generated_concrete/testproto/test_pb2.pyi:62:22 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test_pb2.pyi:92:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["DeprecatedEnum"]`
+ test/generated_concrete/testproto/test_pb2.pyi:92:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test_pb2.pyi:117:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated_concrete/testproto/test_pb2.pyi:117:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_concrete/testproto/test_pb2.pyi:351:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["_r_finally"]`
+ test/generated_concrete/testproto/test_pb2.pyi:351:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test_pb2.pyi:46:16 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["OuterEnum"]`
+ test/generated_sync_only/testproto/test_pb2.pyi:46:16 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test_pb2.pyi:62:22 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["NamingConflicts"]`
+ test/generated_sync_only/testproto/test_pb2.pyi:62:22 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test_pb2.pyi:92:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["DeprecatedEnum"]`
+ test/generated_sync_only/testproto/test_pb2.pyi:92:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test_pb2.pyi:117:20 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["InnerEnum"]`
+ test/generated_sync_only/testproto/test_pb2.pyi:117:20 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
+ test/generated_sync_only/testproto/test_pb2.pyi:351:21 error[invalid-argument-type] Argument to `_EnumTypeWrapper.__init__` is incorrect: Expected `EnumDescriptor`, found `Literal["_r_finally"]`
+ test/generated_sync_only/testproto/test_pb2.pyi:351:21 error[too-many-positional-arguments] Too many positional arguments to `_EnumTypeWrapper.__init__`: expected 2, got 4
spack (https://github.com/spack/spack)
+ lib/spack/spack/vendor/typing_extensions.py:2206:39 error[unknown-argument] Argument `_root` does not match any known parameter of function `object.__init_subclass__`
+ lib/spack/spack/vendor/typing_extensions.py:1649:47 error[unknown-argument] Argument `_root` does not match any known parameter of function `object.__init_subclass__`
+ lib/spack/spack/vendor/typing_extensions.py:1953:49 error[unknown-argument] Argument `_root` does not match any known parameter of function `object.__init_subclass__`
+ lib/spack/spack/vendor/typing_extensions.py:2066:47 error[unknown-argument] Argument `_root` does not match any known parameter of function `object.__init_subclass__`
+ lib/spack/spack/vendor/typing_extensions.py:2453:46 error[unknown-argument] Argument `_root` does not match any known parameter of function `object.__init_subclass__`
+ lib/spack/spack/vendor/typing_extensions.py:2597:44 error[unknown-argument] Argument `_root` does not match any known parameter of function `object.__init_subclass__`
strawberry (https://github.com/strawberry-graphql/strawberry)
+ strawberry/types/auto.py:69:21 error[invalid-argument-type] Argument to `StrawberryAutoMeta.__init__` is incorrect: Expected `str`, found `dict[str, Any]`
+ strawberry/types/auto.py:69:21 error[invalid-argument-type] Argument to `StrawberryAutoMeta.__init__` is incorrect: Expected `str`, found `tuple[()]`
|
The "regressions" in the conformance suite are all due to invalidly defined classes in the conformance tests that would fail to be created at runtime. I've filed python/typing#2258 to fix that. |
Ecosystem analysisLots of Exactly the same issue is also the root cause of the The
The These all seem like true positives, therefore; I think the only reason that there are so many is that no other type checker has tried to properly emulate the runtime semantics here before, so these have all gone unnoticed by prior type checkers. |
AlexWaygood
requested review from
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
AlexWaygood
force-pushed
the
alex/init-subclass-none
branch
from
9708615 to
52a07c9
Compare
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
2 times, most recently
from
9fbfb17 to
b958a6f
Compare
(All gone now that we've updated our conformance-suite pin to a commit that includes python/typing@4c02514) |
AlexWaygood
force-pushed
the
alex/init-subclass-none
branch
from
52a07c9 to
447364e
Compare
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
from
b958a6f to
ae44af2
Compare
AlexWaygood
force-pushed
the
alex/init-subclass-none
branch
from
447364e to
8075ab1
Compare
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
2 times, most recently
from
fb90234 to
0c1c88d
Compare
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
5 times, most recently
from
789e561 to
bdf657c
Compare
|
Hi @AlexWaygood, thanks for the work on this. Similar to the spec about constructors, I'm wondering if it would make sense to incorporate part of the logic here into the spec (things like In Pydantic, we allow class arguments to be used (e.g. Does this make sense? I'd be happy to take a stab at it, opening a DPO discussion first. |
|
Thanks @Viicos! Yes, it could definitely make sense to standardise some of the rules here. Can you paste some links to some of these classes that have |
|
So in our case the metaclass' @dataclass_transform(kw_only_default=True, field_specifiers=(PydanticModelField, PydanticModelPrivateAttr, NoInitField))
class ModelMetaclass(ABCMeta):
def __new__(
mcs,
cls_name: str,
bases: tuple[type[Any], ...],
namespace: dict[str, Any],
__pydantic_generic_metadata__: PydanticGenericMetadata | None = None,
__pydantic_reset_parent_namespace__: bool = True,
_create_model_module: str | None = None,
**kwargs: Any,
) -> type:And the if TYPE_CHECKING:
def __init_subclass__(cls, **kwargs: Unpack[ConfigDict]):
...The idea for us would be to remove the Footnotes
|
|
I see. So would it be correct to say that:
|
| class Valid(Base, meta_arg=5, metaclass=Meta): ... | ||
| class MissingArg(Base, metaclass=Meta): ... # error: [missing-argument] | ||
| class InvalidType(Base, meta_arg="foo", metaclass=Meta): ... # error: [invalid-argument-type] |
There was a problem hiding this comment.
Worth also testing here that sub_arg is an unknown parameter?
| def __prepare__(mcs, name: str, bases: tuple[type, ...], *, prep_arg: int = 0, **kwargs: Any) -> dict[str, Any]: | ||
| return {} | ||
|
|
||
| def __new__(mcs, name: str, bases: tuple[type, ...], namespace: dict[str, Any], **kwargs: Any): |
There was a problem hiding this comment.
Because this signature of __new__ accepts everything __prepare__ does, this test doesn't establish that we are validating against both signatures.
| 12 | return super().__new__(mcs, name, bases, namespace) | ||
| 13 | | ||
| 14 | class Foo(metaclass=Meta): ... # error: [invalid-argument-type] | ||
| | ^^^^^^^^^^^^^^^^ Expected `MyNamespace`, found `dict[str, Any]` |
There was a problem hiding this comment.
I guess ideally we'd point to the return annotation on __new__ to explain where dict[str, Any] came from?
There was a problem hiding this comment.
I think you mean the return annotation on __prepare__? And my answer would be "yes, ideally", but I think figuring out exactly how to attach that diagnostic from deep inside our call-binding machinery might be one of those "I'll need a research team and five years" problems (https://xkcd.com/1425/)
| // If the `metaclass=` keyword argument is not passed an instance of `type`, | ||
| // the runtime just calls it as-is without trying to resolve the appropriate metaclass | ||
| // with respect to the class's base classes. So, just use the `metaclass=` argument directly | ||
| // in that case. If the `metaclass=` argument was passed an instance of `type`, however, | ||
| // the class's actual metaclass won't necessarily be the `metaclass=` argument: if you pass | ||
| // `metaclass=object`, the class's actual metaclass will be `type`, because all classes have | ||
| // `object` as a supertype and the metaclass of `object` is `type`, and `object`/`type` have | ||
| // a subclass relationship. | ||
| let metaclass = if let Some(explicit_metaclass) = explicit_metaclass | ||
| && !explicit_metaclass.is_subtype_of(db, KnownClass::Type.to_instance(db)) | ||
| { | ||
| explicit_metaclass | ||
| } else { | ||
| class.metaclass(db) | ||
| }; |
There was a problem hiding this comment.
I feel like this logic is somewhat iffy and at least deserves a TODO. It is also, I think, the cause of the regression in handling of metaclass=Any, because typing.Any is a class (thus an instance of type), but it is not something that try_metaclass is able to recognize as a metaclass, so it just gets reduced to Unknown.
I think that regression illustrates the issue with this logic: the mismatch between "metaclasses that are actually recognized by try_metaclass" vs "things we consider a subtype of type" leaves a gap where invalid metaclasses can silently become Unknown and not be validated at all here.
I suspect that instead of implementing our own separate logic here, we should maybe be calling try_metaclass directly, and use explicit_metaclass if it returns an error?
There was a problem hiding this comment.
I suspect that instead of implementing our own separate logic here, we should maybe be calling
try_metaclassdirectly, and useexplicit_metaclassif it returns an error?
That's a great suggestion. Thanks!
| } else if !metaclass.is_subtype_of(db, KnownClass::Type.to_subclass_of(db)) { | ||
| // `ClassLiteral::metaclass()` doesn't give an accurate result currently if the metaclass | ||
| // was inherited from a superclass and is not an instance of `type` (which is anyway very rare), | ||
| // so we don't check either `__init_subclass__` or the call to the metaclass in that case right now. | ||
| (explicit_metaclass.is_some(), false) |
There was a problem hiding this comment.
It would be easier to understand the logic if we clarified in what way ClassLiteral::metaclass() gives wrong results today in the mentioned case. Does it error and so .metaclass() just returns type[Unknown]?
Given that we actually perform our metaclass-call checks below against metaclass, not explicit_metaclass, it feels like we are assuming here that if metaclass is not a subclass of type but explicit_metaclass.is_some(), that means we fell into the "unrecognizable metaclass" case above, and metaclass == explicit_metaclass? But I'm not sure if that's the intent or not... can we preserve more state from the check above and do less re-checking / assuming here?
This feels related to the comment above -- in general it seems like we're doing a fair amount of "working around" try_metaclass, and it would be a lot clearer if it could make the necessary distinctions and centralize them in one place.
| // We pass in a tuple where all elements are `type` rather than passing in the actual bases, | ||
| // because `Generic[T]` is not actually an instance of `type`, but most metaclass `__new__` | ||
| // and `__prepare__` methods are annotated as accepting `tuple[type, ...]`, which leads to | ||
| // too many false-positive errors. |
There was a problem hiding this comment.
This workaround can also cause false positives, though I assume much less likely (or you'd have seen it in the ecosystem):
from typing import Any
class A: ...
class Meta(type):
def __new__(
mcls,
name: str,
bases: tuple[type[A]],
namespace: dict[str, Any],
):
return super().__new__(mcls, name, bases, namespace)
class Good(A, metaclass=Meta): ...This should be fine, but it fails because we replace the actual bases with type.
Maybe this is an acceptable limitation, and we can just document it with a TODO?
Or it seems like we might be able to just recognize the cases (Protocol, Generic) that run into this, and special-case them? (Mirroring the __mro_entries__ magic at runtime.)
There was a problem hiding this comment.
__mro_entries__ could return a tuple of length >=1, so there's sort-of no hope that we can actually model all this correctly. (You know, we could try looking up the return type of __mro_entries__ for base classes that are not instances of type, but I really doubt that anybody using __mro_entries__ is precisely annotating the return type in the hope that a type checker will understand it.)
>>> class Foo: ...
...
>>> class Bar: ...
...
>>> class Baz:
... def __mro_entries__(self, *args, **kwargs):
... return Foo, Bar
...
>>> class Spam(Baz()): ...
...
>>> Spam.__bases__
(<class '__main__.Foo'>, <class '__main__.Bar'>)but if all items in the bases list are instances of type, then we know that __mro_entries__ will never be called on any of those items... so in that instance, it should be safe to pass in the bases tuple as-is...
There was a problem hiding this comment.
Yeah, sorry, I wasn't clear here. I didn't mean trying to fully model __mro_entries__ behavior, just recognize the commonly used classes that we know have it, and special-case them enough to make it work. The tuple[Any, ...] solution we discussed in-person should eliminate false positives, I think?
Essentially yes, our policy is to comply to the spec and support type checkers that comply to the spec
Thankfully, even if Pyright's behavior is inconsistent when it comes to typing extra kwargs to the metaclass' |
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
from
17ef206 to
52e0fec
Compare
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
2 times, most recently
from
a4b2932 to
fbe0562
Compare
…metaclass `__call__`, as well as `__init_subclass__`
AlexWaygood
force-pushed
the
alex/metaclass-new-prepare
branch
from
fbe0562 to
2d243ac
Compare
4 participants
Summary
Currently, if a
classstatement has keyword arguments, we verify those keyword arguments against the signature of the class's__init_subclass__method. But this isn't necessarily correct! If the class has a metaclass that defines a custom__new__, the keywords passed in theclassstatement might never get to the__init_subclass__method -- arbitrary keyword arguments could be consumed or injected by the metaclass's__new__method. If the class has a custom metaclass that overrides__new__, the keyword arguments passed to theclassstatement should instead be verified against the__new__method on the metaclass.This PR fixes that, and a variety of other edge cases, including:
__init__as well as, or instead of,__new__?__init__and__new__and the meta-metaclass defines__call__?__prepare__method of the metaclass doesn't return the namespace type expected by the__new__method of the metaclass?Test Plan
lots of mdtests and snapshots