Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow subscripted aliases at function scope #4000

Merged
merged 7 commits into from Oct 10, 2017
Merged

Allow subscripted aliases at function scope #4000

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4c9b023
Allow subscripted aliases at function scope; prohibit bound typevar r…
ilevkivskyi Sep 24, 2017
a464a53
Improve one test
ilevkivskyi Sep 24, 2017
e4c1ef4
Merge remote-tracking branch 'upstream/master' into fix-qualified-ali…
Sep 27, 2017
38ecc68
Merge branch 'master' into fix-qualified-aliases
ilevkivskyi Oct 6, 2017
178826c
Merge branch 'fix-qualified-aliases' of https://github.com/ilevkivsky…
Oct 6, 2017
358ca96
Update tests after merge
Oct 6, 2017
5cbe056
Address CR
ilevkivskyi Oct 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion mypy/checkexpr.py
View file
Open in desktop
Expand Up @@ -650,7 +650,7 @@ def analyze_type_type_callee(self, item: Type, context: Context) -> Type:
res = type_object_type(item.type, self.named_type)
if isinstance(res, CallableType):
res = res.copy_modified(from_type_type=True)
return res
return expand_type_by_instance(res, item)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not directly related to this PR, but this was exposed by a test I updated. For example:

x: Type[C[int]]
x()  # this should be 'C[int]', not 'C[<nothing>]'

y: Type[List]
y()  # this should be 'List[Any]', not 'List[<nothing>]'

By the way, this use case is not mentioned in PEP 484, but there are tests for this, for example testClassValuedAttributesGeneric. There are of course subtleties related to Type[...], but I think this change doesn't add unsafety, but removes some false positives.

if isinstance(item, UnionType):
return UnionType([self.analyze_type_type_callee(item, context)
for item in item.relevant_items()], item.line)
Expand Down
15 changes: 8 additions & 7 deletions mypy/semanal.py
View file
Open in desktop
Expand Up @@ -1732,11 +1732,10 @@ def alias_fallback(self, tp: Type) -> Instance:
return Instance(fb_info, [])

def analyze_alias(self, rvalue: Expression,
allow_unnormalized: bool) -> Tuple[Optional[Type], List[str]]:
warn_bound_tvar: bool = False) -> Tuple[Optional[Type], List[str]]:
"""Check if 'rvalue' represents a valid type allowed for aliasing
(e.g. not a type variable). If yes, return the corresponding type and a list of
qualified type variable names for generic aliases.
If 'allow_unnormalized' is True, allow types like builtins.list[T].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this PR also affect whether list[x] is accepted in stubs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just noticed that this parameter is unused in the function body when I added the new one.

"""
dynamic = bool(self.function_stack and self.function_stack[-1].is_dynamic())
global_scope = not self.type and not self.function_stack
Expand All @@ -1751,7 +1750,8 @@ def analyze_alias(self, rvalue: Expression,
self.is_typeshed_stub_file,
allow_unnormalized=True,
in_dynamic_func=dynamic,
global_scope=global_scope)
global_scope=global_scope,
warn_bound_tvar=warn_bound_tvar)
if res:
alias_tvars = [name for (name, _) in
res.accept(TypeVariableQuery(self.lookup_qualified, self.tvar_scope))]
Expand All @@ -1770,11 +1770,12 @@ def check_and_set_up_type_alias(self, s: AssignmentStmt) -> None:
lvalue = s.lvalues[0]
if not isinstance(lvalue, NameExpr):
return
if (len(s.lvalues) == 1 and not self.is_func_scope() and
not (self.type and isinstance(s.rvalue, NameExpr) and lvalue.is_def)
if (len(s.lvalues) == 1 and
not ((self.type or self.is_func_scope()) and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is hard to understand. Add a comment describing what it does at a high level and maybe also give some concrete examples. It's also worth trying to simplify the nested boolean expression. Ideas:

  • Make the top-level expression of form e1 and (...) and not e2 instead of e1 and not (...) and not e2 -- the first not operator makes this a bit harder to understand, I think.
  • Use a temp variable to hold the result of self.type or self.is_func_scope() and give it a semantically meaningful name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, fixed. I replaced this with three ifs and thus saved one nesting level for the rest of the body.

isinstance(s.rvalue, NameExpr) and lvalue.is_def)
and not s.type):
rvalue = s.rvalue
res, alias_tvars = self.analyze_alias(rvalue, allow_unnormalized=True)
res, alias_tvars = self.analyze_alias(rvalue, warn_bound_tvar=True)
if not res:
return
node = self.lookup(lvalue.name, lvalue)
Expand Down Expand Up @@ -3374,7 +3375,7 @@ def visit_index_expr(self, expr: IndexExpr) -> None:
elif isinstance(expr.base, RefExpr) and expr.base.kind == TYPE_ALIAS:
# Special form -- subscripting a generic type alias.
# Perform the type substitution and create a new alias.
res, alias_tvars = self.analyze_alias(expr, allow_unnormalized=self.is_stub_file)
res, alias_tvars = self.analyze_alias(expr)
expr.analyzed = TypeAliasExpr(res, alias_tvars, fallback=self.alias_fallback(res),
in_runtime=True)
expr.analyzed.line = expr.line
Expand Down
15 changes: 11 additions & 4 deletions mypy/typeanal.py
View file
Open in desktop
Expand Up @@ -63,7 +63,8 @@ def analyze_type_alias(node: Expression,
is_typeshed_stub: bool,
allow_unnormalized: bool = False,
in_dynamic_func: bool = False,
global_scope: bool = True) -> Optional[Type]:
global_scope: bool = True,
warn_bound_tvar: bool = False) -> Optional[Type]:
"""Return type if node is valid as a type alias rvalue.

Return None otherwise. 'node' must have been semantically analyzed.
Expand Down Expand Up @@ -117,7 +118,7 @@ def analyze_type_alias(node: Expression,
return None
analyzer = TypeAnalyser(lookup_func, lookup_fqn_func, tvar_scope, fail_func, note_func,
plugin, options, is_typeshed_stub, aliasing=True,
allow_unnormalized=allow_unnormalized)
allow_unnormalized=allow_unnormalized, warn_bound_tvar=warn_bound_tvar)
analyzer.in_dynamic_func = in_dynamic_func
analyzer.global_scope = global_scope
return type.accept(analyzer)
Expand Down Expand Up @@ -154,7 +155,8 @@ def __init__(self,
aliasing: bool = False,
allow_tuple_literal: bool = False,
allow_unnormalized: bool = False,
third_pass: bool = False) -> None:
third_pass: bool = False,
warn_bound_tvar: bool = False) -> None:
self.lookup = lookup_func
self.lookup_fqn_func = lookup_fqn_func
self.fail_func = fail_func
Expand All @@ -168,6 +170,7 @@ def __init__(self,
self.plugin = plugin
self.options = options
self.is_typeshed_stub = is_typeshed_stub
self.warn_bound_tvar = warn_bound_tvar
self.third_pass = third_pass

def visit_unbound_type(self, t: UnboundType) -> Type:
Expand All @@ -194,7 +197,11 @@ def visit_unbound_type(self, t: UnboundType) -> Type:
tvar_def = self.tvar_scope.get_binding(sym)
else:
tvar_def = None
if sym.kind == TVAR and tvar_def is not None:
if self.warn_bound_tvar and sym.kind == TVAR and tvar_def is not None:
self.fail('Can\'t use bound type variable "{}"'
' to define generic alias'.format(t.name), t)
return AnyType(TypeOfAny.from_error)
elif sym.kind == TVAR and tvar_def is not None:
if len(t.args) > 0:
self.fail('Type variable "{}" used with arguments'.format(
t.name), t)
Expand Down
8 changes: 4 additions & 4 deletions test-data/unit/check-classes.test
View file
Open in desktop
Expand Up @@ -2119,17 +2119,17 @@ reveal_type(C().aa) # E: Revealed type is '__main__.A'
[out]

[case testClassValuedAttributesGeneric]
from typing import Generic, TypeVar
from typing import Generic, TypeVar, Type
T = TypeVar('T')

class A(Generic[T]):
def __init__(self, x: T) -> None:
self.x = x
class B(Generic[T]):
a = A[T]
a: Type[A[T]] = A

reveal_type(B[int]().a) # E: Revealed type is 'def (x: builtins.int*) -> __main__.A[builtins.int*]'
B[int]().a('hi') # E: Argument 1 has incompatible type "str"; expected "int"
reveal_type(B[int]().a) # E: Revealed type is 'Type[__main__.A[builtins.int*]]'
B[int]().a('hi') # E: Argument 1 to "A" has incompatible type "str"; expected "int"

class C(Generic[T]):
a = A
Expand Down
26 changes: 13 additions & 13 deletions test-data/unit/check-isinstance.test
View file
Open in desktop
Expand Up @@ -1447,16 +1447,16 @@ def f(x: Union[Type[int], Type[str], Type[List]]) -> None:
x()[1] # E: Value of type "Union[int, str]" is not indexable
else:
reveal_type(x) # E: Revealed type is 'Type[builtins.list[Any]]'
reveal_type(x()) # E: Revealed type is 'builtins.list[<nothing>]'
reveal_type(x()) # E: Revealed type is 'builtins.list[Any]'
x()[1]
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
if issubclass(x, (str, (list,))):
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.str, builtins.list[Any]]'
x()[1]
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
[builtins fixtures/isinstancelist.pyi]

[case testIssubclasDestructuringUnions2]
Expand All @@ -1469,40 +1469,40 @@ def f(x: Type[Union[int, str, List]]) -> None:
x()[1] # E: Value of type "Union[int, str]" is not indexable
else:
reveal_type(x) # E: Revealed type is 'Type[builtins.list[Any]]'
reveal_type(x()) # E: Revealed type is 'builtins.list[<nothing>]'
reveal_type(x()) # E: Revealed type is 'builtins.list[Any]'
x()[1]
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
if issubclass(x, (str, (list,))):
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.str, builtins.list[Any]]'
x()[1]
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
[builtins fixtures/isinstancelist.pyi]

[case testIssubclasDestructuringUnions3]
from typing import Union, List, Tuple, Dict, Type

def f(x: Type[Union[int, str, List]]) -> None:
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
if issubclass(x, (str, (int,))):
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str]'
x()[1] # E: Value of type "Union[int, str]" is not indexable
else:
reveal_type(x) # E: Revealed type is 'Type[builtins.list[Any]]'
reveal_type(x()) # E: Revealed type is 'builtins.list[<nothing>]'
reveal_type(x()) # E: Revealed type is 'builtins.list[Any]'
x()[1]
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
if issubclass(x, (str, (list,))):
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.str, builtins.list[Any]]'
x()[1]
reveal_type(x) # E: Revealed type is 'Union[Type[builtins.int], Type[builtins.str], Type[builtins.list[Any]]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[<nothing>]]'
reveal_type(x()) # E: Revealed type is 'Union[builtins.int, builtins.str, builtins.list[Any]]'
[builtins fixtures/isinstancelist.pyi]

[case testIssubclass]
Expand Down
42 changes: 42 additions & 0 deletions test-data/unit/check-type-aliases.test
View file
Open in desktop
Expand Up @@ -96,6 +96,48 @@ GenAlias = Sequence[T]
def fun(x: Alias) -> GenAlias[int]: pass
[out]

[case testCorrectQualifiedAliasesAlsoInFunctions]
from typing import TypeVar, Generic
T = TypeVar('T')
S = TypeVar('S')

class X(Generic[T]):
A = X[S]
def f(self) -> X[T]:
pass

a: X[T]
b: A = a
c: A[T] = a
d: A[int] = a # E: Incompatible types in assignment (expression has type "X[T]", variable has type "X[int]")

def g(self) -> None:
a: X[T]
b: X.A = a
c: X.A[T] = a
d: X.A[int] = a # E: Incompatible types in assignment (expression has type "X[T]", variable has type "X[int]")

def g(arg: X[int]) -> None:
p: X[int] = arg.f()
q: X.A = arg.f()
r: X.A[str] = arg.f() # E: Incompatible types in assignment (expression has type "X[int]", variable has type "X[str]")
[out]

[case testProhibitBoundTypeVariableReuseForAliases]
from typing import TypeVar, Generic, List
T = TypeVar('T')
class C(Generic[T]):
A = List[T] # E: Can't use bound type variable "T" to define generic alias

x: C.A
reveal_type(x) # E: Revealed type is 'builtins.list[Any]'

def f(x: T) -> T:
A = List[T] # E: Can't use bound type variable "T" to define generic alias
return x
[builtins fixtures/list.pyi]
[out]

[case testTypeAliasInBuiltins]
def f(x: bytes): pass
bytes
Expand Down
9 changes: 4 additions & 5 deletions test-data/unit/check-typevar-values.test
View file
Open in desktop
Expand Up @@ -556,16 +556,15 @@ def outer(x: T) -> T:

[case testClassMemberTypeVarInFunctionBody]
from typing import TypeVar, List
S = TypeVar('S')
class C:
T = TypeVar('T', bound=int)
def f(self, x: T) -> T:
L = List[C.T] # this creates a variable, not an alias
reveal_type(L) # E: Revealed type is 'Overload(def () -> builtins.list[T`-1], def (x: typing.Iterable[T`-1]) -> builtins.list[T`-1])'
y: C.T = x
L().append(x)
L = List[S]
y: L[C.T] = [x]
C.T # E: Type variable "C.T" cannot be used as an expression
A = C.T # E: Type variable "C.T" cannot be used as an expression
return L()[0]
return y[0]

[builtins fixtures/list.pyi]

Expand Down