[ty] Unify Type::is_subtype_of() and Type::is_assignable_to()

[AlexWaygood]

Summary

There's lots of duplication between Type::is_subtype_of() and Type::is_assignable_to(), and we've had multiple bugs in the past because we've remembered to update Type::is_subtype_of() but have not realised that Type::is_assignable_to() also needed to be updated. This PR unifies the two methods: they are now both thin wrappers over a Type::has_assignability_relation method.

Test Plan

• Existing tests
• New tests.
  • Some of these cover latent bugs that were accidentally fixed in this refactor
  • Some of them cover bugs that early versions of this PR introduced but were revealed by mypy_primer and have since been fixed.
• Property tests
• mypy_primer

[github-actions]

mypy_primer results

Changes were detected when running on open source projects

aioredis (https://github.com/aio-libs/aioredis)
- error[invalid-parameter-default] aioredis/connection.py:1541:9: Default value of type `<class 'LifoQueue'>` is not assignable to annotated parameter type `type[Queue]`
- Found 26 diagnostics
+ Found 25 diagnostics

graphql-core (https://github.com/graphql-python/graphql-core)
+ error[invalid-assignment] src/graphql/execution/execute.py:180:5: Object of type `staticmethod` is not assignable to `(Any, /) -> bool`
- Found 440 diagnostics
+ Found 441 diagnostics

pydantic (https://github.com/pydantic/pydantic)
+ error[invalid-assignment] pydantic/json_schema.py:1681:9: Object of type `Any | list[Any]` is not assignable to `ConfigDict`
- Found 761 diagnostics
+ Found 762 diagnostics

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- error[invalid-argument-type] src/hydra_zen/structured_configs/_implementations.py:2952:54: Argument to function `parse_strict_dataclass_options` is incorrect: Expected `Mapping[str, Any]`, found `DataclassOptions`
- error[invalid-argument-type] src/hydra_zen/structured_configs/_implementations.py:3229:21: Argument is incorrect: Expected `InitVar[ZenConvert | None]`, found `ZenConvert`
- error[invalid-argument-type] src/hydra_zen/structured_configs/_implementations.py:3241:21: Argument is incorrect: Expected `InitVar[ZenConvert | None]`, found `ZenConvert`
- error[invalid-argument-type] src/hydra_zen/structured_configs/_implementations.py:3311:13: Argument to function `parse_strict_dataclass_options` is incorrect: Expected `Mapping[str, Any]`, found `DataclassOptions`
- Found 613 diagnostics
+ Found 609 diagnostics

pwndbg (https://github.com/pwndbg/pwndbg)
- error[invalid-assignment] pwndbg/dbg/lldb/repl/readline.py:24:9: Object of type `(*args) -> Unknown` is not assignable to attribute `set_completion_display_matches_hook` on type `<module 'readline'> & ~<Protocol with members 'set_completion_display_matches_hook'>`
- Found 2301 diagnostics
+ Found 2300 diagnostics

static-frame (https://github.com/static-frame/static-frame)
- error[invalid-argument-type] static_frame/test/property/test_archive_npy.py:36:13: Argument to function `get_frame` is incorrect: Expected `type[Index]`, found `<class 'IndexDate'>`
- error[invalid-argument-type] static_frame/test/property/test_archive_npy.py:38:13: Argument to function `get_frame` is incorrect: Expected `type[Index]`, found `<class 'IndexDate'>`
- error[invalid-argument-type] static_frame/test/property/test_index.py:22:25: Argument to function `property_values` is incorrect: Expected `type[Index]`, found `<class 'IndexGO'>`
- error[invalid-argument-type] static_frame/test/property/test_index.py:34:25: Argument to function `property_values` is incorrect: Expected `type[Index]`, found `<class 'IndexGO'>`
- error[invalid-argument-type] static_frame/test/property/test_index.py:46:38: Argument to function `property_loc_to_iloc_element` is incorrect: Expected `type[Index]`, found `<class 'IndexGO'>`
- error[invalid-argument-type] static_frame/test/property/test_index.py:63:36: Argument to function `property_loc_to_iloc_slice` is incorrect: Expected `type[Index]`, found `<class 'IndexGO'>`
- Found 1931 diagnostics
+ Found 1925 diagnostics

pytest (https://github.com/pytest-dev/pytest)
- error[invalid-argument-type] testing/typing_checks.py:48:25: Argument to bound method `setitem` is incorrect: Expected `Mapping[Literal["x"], Literal[2]]`, found `Foo`
- error[invalid-argument-type] testing/typing_checks.py:49:25: Argument to bound method `delitem` is incorrect: Expected `Mapping[Literal["y"], Unknown]`, found `Foo`
- Found 778 diagnostics
+ Found 776 diagnostics

discord.py (https://github.com/Rapptz/discord.py)
- error[invalid-argument-type] discord/message.py:2436:40: Argument to bound method `from_dict` is incorrect: Expected `Mapping[str, Any]`, found `Embed`
+ warning[unused-ignore-comment] discord/state.py:1151:70: Unused blanket `type: ignore` directive

meson (https://github.com/mesonbuild/meson)
- error[invalid-argument-type] mesonbuild/interpreter/interpreter.py:3398:50: Argument to function `extract_as_list` is incorrect: Expected `dict[Literal["dependencies"], Unknown]`, found `Executable | StaticLibrary | SharedLibrary | SharedModule | Jar`
- error[invalid-argument-type] mesonbuild/modules/gnome.py:2223:56: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'CustomTargetHolder'>`
- error[invalid-argument-type] mesonbuild/modules/gnome.py:2224:62: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'CustomTargetHolder'>`
- error[invalid-argument-type] mesonbuild/modules/gnome.py:2225:50: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'CustomTargetHolder'>`
- error[invalid-argument-type] mesonbuild/modules/gnome.py:2226:54: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'CustomTargetHolder'>`
- error[invalid-argument-type] mesonbuild/modules/gnome.py:2227:51: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'CustomTargetHolder'>`
- error[invalid-argument-type] mesonbuild/modules/hotdoc.py:485:53: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'HotdocTargetHolder'>`
- error[invalid-argument-type] mesonbuild/modules/python.py:160:45: Argument to function `extract_as_list` is incorrect: Expected `dict[Literal["dependencies"], Unknown]`, found `ExtensionModuleKw`
- error[invalid-argument-type] mesonbuild/modules/python.py:180:51: Argument to function `extract_as_list` is incorrect: Expected `dict[Literal["c_args"], Unknown]`, found `ExtensionModuleKw`
- error[invalid-argument-type] mesonbuild/modules/python.py:184:53: Argument to function `extract_as_list` is incorrect: Expected `dict[Literal["cpp_args"], Unknown]`, found `ExtensionModuleKw`
- error[invalid-argument-type] mesonbuild/modules/python.py:209:58: Argument to function `extract_as_list` is incorrect: Expected `dict[Literal["link_args"], Unknown]`, found `ExtensionModuleKw`
- error[invalid-argument-type] mesonbuild/modules/python.py:558:62: Argument to bound method `append_holder_map` is incorrect: Expected `type[ObjectHolder]`, found `<class 'PythonInstallation'>`
- Found 1322 diagnostics
+ Found 1310 diagnostics

openlibrary (https://github.com/internetarchive/openlibrary)
- error[invalid-argument-type] openlibrary/core/lists/model.py:465:27: Argument to bound method `__init__` is incorrect: Expected `Thing | str | AnnotatedSeed`, found `str | ThingReferenceDict | AnnotatedSeedDict`
- Found 726 diagnostics
+ Found 725 diagnostics

aiohttp (https://github.com/aio-libs/aiohttp)
- error[invalid-raise] aiohttp/connector.py:1164:19: Cannot raise object of type `ClientConnectorCertificateError` (must be a `BaseException` subclass or instance)
- error[invalid-raise] aiohttp/connector.py:1166:19: Cannot raise object of type `ClientConnectorSSLError` (must be a `BaseException` subclass or instance)
- error[invalid-raise] aiohttp/connector.py:1273:19: Cannot raise object of type `ClientConnectorCertificateError` (must be a `BaseException` subclass or instance)
- error[invalid-raise] aiohttp/connector.py:1275:19: Cannot raise object of type `ClientConnectorSSLError` (must be a `BaseException` subclass or instance)
- Found 178 diagnostics
+ Found 174 diagnostics

prefect (https://github.com/PrefectHQ/prefect)
+ warning[unused-ignore-comment] src/prefect/utilities/callables.py:321:67: Unused blanket `type: ignore` directive
- Found 4482 diagnostics
+ Found 4483 diagnostics

zulip (https://github.com/zulip/zulip)
- error[invalid-assignment] corporate/lib/remote_billing_util.py:155:5: Object of type `RemoteBillingIdentityDict | LegacyServerIdentityDict | None` is not assignable to `LegacyServerIdentityDict | None`
- error[invalid-argument-type] corporate/views/upgrade.py:222:66: Argument to function `render` is incorrect: Expected `Mapping[str, Any] | None`, found `UpgradePageContext | None`
- error[invalid-argument-type] corporate/views/upgrade.py:257:66: Argument to function `render` is incorrect: Expected `Mapping[str, Any] | None`, found `UpgradePageContext | None`
- error[invalid-argument-type] corporate/views/upgrade.py:292:66: Argument to function `render` is incorrect: Expected `Mapping[str, Any] | None`, found `UpgradePageContext | None`
- error[invalid-argument-type] zerver/actions/message_delete.py:94:33: Argument to function `send_event_on_commit` is incorrect: Expected `Mapping[str, Any]`, found `DeleteMessagesEvent`
- error[invalid-argument-type] zerver/actions/message_edit.py:864:33: Argument to function `send_event_on_commit` is incorrect: Expected `Mapping[str, Any]`, found `DeleteMessagesEvent`
- error[invalid-argument-type] zerver/views/users.py:888:34: Argument to function `json_success` is incorrect: Expected `Mapping[str, Any]`, found `APIUserDict`
- Found 6933 diagnostics
+ Found 6926 diagnostics

[AlexWaygood]

+ error[invalid-assignment] src/graphql/execution/execute.py:180:5: Object of type `staticmethod` is not assignable to `(Any, /) -> bool`

I'm not totally sure why this diagnostic is being added by this PR. Note that staticmethod.__call__ was added in Python 3.10; I'm not sure what Python version we're inferring for this project in CI. I do know that we don't really understand the stub for builtins.staticmethod at all right now, because of the fact that it's generic over a legacy ParamSpec, which seems to do quite odd things to our inference (we don't even think that's valid right now).

- error[invalid-assignment] pwndbg/dbg/lldb/repl/readline.py:24:9: Object of type `(*args) -> Unknown` is not assignable to attribute `set_completion_display_matches_hook` on type `<module 'readline'> & ~<Protocol with members 'set_completion_display_matches_hook'>`

This error goes away because we now infer the type of the module readline in this branch as being Never, since we know that the module readline always has a set_completion_display_matches_hook attribute:

import readline

if not hasattr(readline, 'set_completion_display_matches_hook'):
    reveal_type(readline)

This seems like we're doing a better job now according to the information we've been given from typeshed's stubs.

+ warning[possibly-unbound-attribute] ddtrace/propagation/_database_monitoring.py:74:51: Attribute `tracer` on type `<module 'ddtrace'>` is possibly unbound
+ warning[possibly-unbound-attribute] ddtrace/propagation/_database_monitoring.py:76:17: Attribute `tracer` on type `<module 'ddtrace'>` is possibly unbound

This is a pre-existing bug: astral-sh/ty#578. The reason it's showing up now is that we didn't previously ever consider module-literal types to be subtypes of protocol-instance types, but now we do.

[AlexWaygood]

(I don't love the name Type::has_assignability_relation. Better suggestions are welcome!)

[jelle-openai]

Did you steal this from https://github.com/JelleZijlstra/pycroscope/blob/385397ac151199638c65d298997fd8298eb1abb2/pycroscope/relations.py#L154

[AlexWaygood]

great minds think alike 😜

in all seriousness, though -- no, I hadn't seen that before writing this!

[dcreager]

I love this overall! My main questions are about whether we should go all the way and add has_assignability_relation_to to all of the subsidiary Type types

[dcreager]

I don't disagree, but is this just so that you don't have to perform a similar refactoring on ClassType?

[AlexWaygood]

Hmm, I don't think so -- we're comparing between two different variants here rather than between two instances of the same variant

[AlexWaygood]

I think I now see what you mean -- does 0385c77 make the changes you were looking for here?

[AlexWaygood]

I love this overall! My main questions are about whether we should go all the way and add has_assignability_relation_to to all of the subsidiary Type types

I think we should! I deliberately held off from that here though, as it was already a large change that was tricky to get right, and I didn't want to make it harder to review. I can make that change as part of this PR, but I feel like I'd prefer to do it as a followup?

[dcreager]

I think we should! I deliberately held off from that here though, as it was already a large change that was tricky to get right, and I didn't want to make it harder to review. I can make that change as part of this PR, but I feel like I'd prefer to do it as a followup?

I have a slight preference for doing it all in one PR, though I'm happy to defer to you as the person doing the work! (It's a bit easier to turn off my brain and look for ~every call site to become has_assignability_relation, instead of having to decide for each hunk what the right change should be.)

Also note that Signature is already refactored like this (though it takes in a callback closure not the new enum), so it might not be as much extra effort.

But again, happy to defer to you!

(I don't love the name Type::has_assignability_relation. Better suggestions are welcome!)

It's called is_assignable_to_impl in Signature:

ruff/crates/ty_python_semantic/src/types/signatures.rs

Line 127 in 0079cc6

fn is_assignable_to_impl<F>(

[carljm]

This is awesome!

Not taking the time to do a full detailed review, since @dcreager is on it; just a few comments on the tests, and on naming.

[AlexWaygood]

Okay, I think that's all the review comments addressed! There are still one or two Type::is_subtype_of and Type::is_assignable_to methods on this branch that are not thin wrappers around Type::has_relation_to methods. But I'm unwilling to make further refactors as part of this PR, since some of these methods are in areas of ty's codebase that I'm not an expert in (e.g. signature subtyping).

[dhruvmanila]

But I'm unwilling to make further refactors as part of this PR, since some of these methods are in areas of ty's codebase that I'm not an expert in (e.g. signature subtyping).

I'm happy to take this up as a follow-up unless someone else is interested in this :)

[dhruvmanila]

I haven't looked too closely as there are already 2 reviewers but a couple of things I noticed while glancing through the changes.

[AlexWaygood]

+ warning[possibly-unbound-attribute] ddtrace/propagation/_database_monitoring.py:74:51: Attribute `tracer` on type `<module 'ddtrace'>` is possibly unbound
+ warning[possibly-unbound-attribute] ddtrace/propagation/_database_monitoring.py:76:17: Attribute `tracer` on type `<module 'ddtrace'>` is possibly unbound

This is a pre-existing bug: astral-sh/ty#578. The reason it's showing up now is that we didn't previously ever consider module-literal types to be subtypes of protocol-instance types, but now we do.

Confirmed that this has now gone from the mypy_primer report following #18466, which fixed astral-sh/ty#578!

[carljm]

This is great!