Skip to content

Commit

Permalink
Use class name as namespace for type variables (#12590)
Browse files Browse the repository at this point in the history
This avoids confusion between type variables of two classes, which can
happen at least in some edge cases. Type variables are only the same if
both the numeric id and namespace match (plus meta level).

Fixes #12588 (though the textual presentation of type variables doesn't
take the namespace into consideration yet).
  • Loading branch information
JukkaL authored Apr 19, 2022
1 parent cf6a48c commit 20b0b9b
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 13 deletions.
3 changes: 1 addition & 2 deletions mypy/checkpattern.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,7 @@ def visit_mapping_pattern(self, o: MappingPattern) -> PatternType:
if is_subtype(current_type, mapping) and isinstance(current_type, Instance):
mapping_inst = map_instance_to_supertype(current_type, mapping.type)
dict_typeinfo = self.chk.lookup_typeinfo("builtins.dict")
dict_type = fill_typevars(dict_typeinfo)
rest_type = expand_type_by_instance(dict_type, mapping_inst)
rest_type = Instance(dict_typeinfo, mapping_inst.args)
else:
object_type = self.chk.named_type("builtins.object")
rest_type = self.chk.named_generic_type("builtins.dict",
Expand Down
3 changes: 2 additions & 1 deletion mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1116,7 +1116,8 @@ def check_decorated_function_is_method(self, decorator: str,
def visit_class_def(self, defn: ClassDef) -> None:
self.statement = defn
self.incomplete_type_stack.append(not defn.info)
with self.tvar_scope_frame(self.tvar_scope.class_frame()):
namespace = self.qualified_name(defn.name)
with self.tvar_scope_frame(self.tvar_scope.class_frame(namespace)):
self.analyze_class(defn)
self.incomplete_type_stack.pop()

Expand Down
15 changes: 10 additions & 5 deletions mypy/tvar_scope.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from typing import Optional, Dict, Union
from mypy.types import TypeVarLikeType, TypeVarType, ParamSpecType, ParamSpecFlavor
from mypy.types import TypeVarLikeType, TypeVarType, ParamSpecType, ParamSpecFlavor, TypeVarId
from mypy.nodes import ParamSpecExpr, TypeVarExpr, TypeVarLikeExpr, SymbolTableNode


Expand All @@ -12,7 +12,8 @@ class TypeVarLikeScope:
def __init__(self,
parent: 'Optional[TypeVarLikeScope]' = None,
is_class_scope: bool = False,
prohibited: 'Optional[TypeVarLikeScope]' = None) -> None:
prohibited: 'Optional[TypeVarLikeScope]' = None,
namespace: str = '') -> None:
"""Initializer for TypeVarLikeScope
Parameters:
Expand All @@ -27,6 +28,7 @@ def __init__(self,
self.class_id = 0
self.is_class_scope = is_class_scope
self.prohibited = prohibited
self.namespace = namespace
if parent is not None:
self.func_id = parent.func_id
self.class_id = parent.class_id
Expand All @@ -51,22 +53,25 @@ def method_frame(self) -> 'TypeVarLikeScope':
"""A new scope frame for binding a method"""
return TypeVarLikeScope(self, False, None)

def class_frame(self) -> 'TypeVarLikeScope':
def class_frame(self, namespace: str) -> 'TypeVarLikeScope':
"""A new scope frame for binding a class. Prohibits *this* class's tvars"""
return TypeVarLikeScope(self.get_function_scope(), True, self)
return TypeVarLikeScope(self.get_function_scope(), True, self, namespace=namespace)

def bind_new(self, name: str, tvar_expr: TypeVarLikeExpr) -> TypeVarLikeType:
if self.is_class_scope:
self.class_id += 1
i = self.class_id
namespace = self.namespace
else:
self.func_id -= 1
i = self.func_id
# TODO: Consider also using namespaces for functions
namespace = ''
if isinstance(tvar_expr, TypeVarExpr):
tvar_def: TypeVarLikeType = TypeVarType(
name,
tvar_expr.fullname,
i,
TypeVarId(i, namespace=namespace),
values=tvar_expr.values,
upper_bound=tvar_expr.upper_bound,
variance=tvar_expr.variance,
Expand Down
16 changes: 12 additions & 4 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,15 @@ class TypeVarId:
# Class variable used for allocating fresh ids for metavariables.
next_raw_id: ClassVar[int] = 1

def __init__(self, raw_id: int, meta_level: int = 0) -> None:
# Fullname of class (or potentially function in the future) which
# declares this type variable (not the fullname of the TypeVar
# definition!), or ''
namespace: str

def __init__(self, raw_id: int, meta_level: int = 0, *, namespace: str = '') -> None:
self.raw_id = raw_id
self.meta_level = meta_level
self.namespace = namespace

@staticmethod
def new(meta_level: int) -> 'TypeVarId':
Expand All @@ -442,15 +448,16 @@ def __repr__(self) -> str:
def __eq__(self, other: object) -> bool:
if isinstance(other, TypeVarId):
return (self.raw_id == other.raw_id and
self.meta_level == other.meta_level)
self.meta_level == other.meta_level and
self.namespace == other.namespace)
else:
return False

def __ne__(self, other: object) -> bool:
return not (self == other)

def __hash__(self) -> int:
return hash((self.raw_id, self.meta_level))
return hash((self.raw_id, self.meta_level, self.namespace))

def is_meta_var(self) -> bool:
return self.meta_level > 0
Expand Down Expand Up @@ -524,6 +531,7 @@ def serialize(self) -> JsonDict:
'name': self.name,
'fullname': self.fullname,
'id': self.id.raw_id,
'namespace': self.id.namespace,
'values': [v.serialize() for v in self.values],
'upper_bound': self.upper_bound.serialize(),
'variance': self.variance,
Expand All @@ -535,7 +543,7 @@ def deserialize(cls, data: JsonDict) -> 'TypeVarType':
return TypeVarType(
data['name'],
data['fullname'],
data['id'],
TypeVarId(data['id'], namespace=data['namespace']),
[deserialize_type(v) for v in data['values']],
deserialize_type(data['upper_bound']),
data['variance'],
Expand Down
31 changes: 30 additions & 1 deletion test-data/unit/check-selftype.test
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,14 @@ from typing import Generic, TypeVar, Tuple
T = TypeVar('T')
S = TypeVar('S')
U = TypeVar('U')
V = TypeVar('V')

class C(Generic[T]):
def magic(self: C[Tuple[S, U]]) -> Tuple[T, S, U]: ...

reveal_type(C[Tuple[int, str]]().magic()) # N: Revealed type is "Tuple[Tuple[builtins.int, builtins.str], builtins.int, builtins.str]"
class D(Generic[V]):
def f(self) -> None:
reveal_type(C[Tuple[V, str]]().magic()) # N: Revealed type is "Tuple[Tuple[V`1, builtins.str], V`1, builtins.str]"
[builtins fixtures/tuple.pyi]

[case testSelfTypeOnUnion]
Expand Down Expand Up @@ -1167,3 +1170,29 @@ def build_wrapper_non_gen(descriptor: Descriptor[int]) -> BaseWrapper[str]:
def build_sub_wrapper_non_gen(descriptor: Descriptor[int]) -> SubWrapper[str]:
return SubWrapper.create_wrapper(descriptor) # E: Argument 1 to "create_wrapper" of "BaseWrapper" has incompatible type "Descriptor[int]"; expected "Descriptor[str]"
[builtins fixtures/classmethod.pyi]

[case testSelfTypeInGenericClassUsedFromAnotherGenericClass1]
from typing import TypeVar, Generic, Iterator, List, Tuple

_T_co = TypeVar("_T_co", covariant=True)
_T1 = TypeVar("_T1")
_T2 = TypeVar("_T2")
S = TypeVar("S")

class Z(Iterator[_T_co]):
def __new__(cls,
__iter1: List[_T1],
__iter2: List[_T2]) -> Z[Tuple[_T1, _T2]]: ...
def __iter__(self: S) -> S: ...
def __next__(self) -> _T_co: ...

T = TypeVar('T')

class C(Generic[T]):
a: List[T]
b: List[str]

def f(self) -> None:
for x, y in Z(self.a, self.b):
reveal_type((x, y)) # N: Revealed type is "Tuple[T`1, builtins.str]"
[builtins fixtures/tuple.pyi]

0 comments on commit 20b0b9b

Please sign in to comment.