Skip to content

Incorrect implementation of equivalence and subtyping for module-literal types? #790

@AlexWaygood

Description

@AlexWaygood

Summary

Consider this situation:

module.py:

import typing

main.py:

from ty_extensions import is_subtype_of, is_equivalent_to, TypeOf

import typing
from module import typing as other_typing

reveal_type(is_subtype_of(TypeOf[typing], TypeOf[other_typing]))
reveal_type(is_equivalent_to(TypeOf[typing], TypeOf[other_typing]))

https://play.ty.dev/251c9d98-2f71-4b0d-a627-8d2c4f40770b

The reveal_type calls both reveal Literal[False] here, even though typing refers to the same module as other_typing. This is because our implementation of equivalence and subtyping for module-literal types just does a naive == comparison between two ModuleLiteralTypes to determine whether one ModuleLiteralType is equivalent to another. But these two "copies" of the typing module in our representation were originally imported by different modules, so they do not compare equal, because we record the importing module on the type itself in the importing_file field on ModuleLiteralType: https://github.com/astral-sh/ruff/blob/f32f7a3b48ff11351c709c60109fafd5ca18ec6c/crates/ty_python_semantic/src/types.rs#L7499-L7510

The reason why we store the importing_file field on ModuleLiteralTypes at all is so that we can check whether any submodules of that module have been imported in the same file that imported the module. If there were, then we recognize those submodules as being available as attributes on the ModuleLiteralType. That means that for something like this, we would recognize an abc attribute as being available on other_importlib but not importlib:

module.py:

import importlib
import importlib.abc

main.py:

import importlib
from module import importlib as other_importlib

# error: Type `<module 'importlib'>` has no attribute `abc` (unresolved-attribute)
reveal_type(importlib.abc)  # revealed: Unknown

reveal_type(other_importlib.abc)  # revealed: <module 'importlib.abc'>

https://play.ty.dev/8b92c614-403a-47be-84dc-f3e3321f62c1

So perhaps there is an argument that our current implementation is actually correct -- we could treat the two ModuleLiteralTypes differently in some situations, so they shouldn't be seen as equivalent types, even if they point to the same underlying module?

In any case, we should add some tests for this, and some comments to the test explaining why this is currently the case -- I don't think we have any at the moment. No tests fail if I apply this diff to main, which changes the result of the reveal_type calls in my original snippet from Literal[False] to Literal[True]

diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs
index 71e7ab5798..1b3fe9ab5d 100644
--- a/crates/ty_python_semantic/src/types.rs
+++ b/crates/ty_python_semantic/src/types.rs
@@ -1355,6 +1355,9 @@ impl<'db> Type<'db> {
             (Type::MethodWrapper(self_method), Type::MethodWrapper(target_method)) => {
                 self_method.has_relation_to(db, target_method, relation)
             }
+            (Type::ModuleLiteral(self_module), Type::ModuleLiteral(target_module)) => {
+                self_module.module(db).file() == target_module.module(db).file()
+            }
 
             // No literal type is a subtype of any other literal type, unless they are the same
             // type (which is handled above). This case is not necessary from a correctness
@@ -1630,6 +1633,9 @@ impl<'db> Type<'db> {
             | (nominal @ Type::NominalInstance(n), Type::ProtocolInstance(protocol)) => {
                 n.class.is_object(db) && protocol.normalized(db) == nominal
             }
+            (Type::ModuleLiteral(first), Type::ModuleLiteral(second)) => {
+                first.module(db).file() == second.module(db).file()
+            }
             _ => false,
         }
     }

Metadata

Metadata

Assignees

No one assigned

    Labels

    type propertiessubtyping, assignability, equivalence, and more

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions