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

Fix make_simplified_union interaction with Any #2714

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,17 @@ def analyze_member_access(name: str,
elif isinstance(typ, UnionType):
# The base object has dynamic type.
msg.disable_type_names += 1
old_num_messages = msg.num_messages()
results = [analyze_member_access(name, subtype, node, is_lvalue, is_super,
is_operator, builtin_type, not_ready_callback, msg,
original_type=original_type, chk=chk)
for subtype in typ.items]
msg.disable_type_names -= 1
if msg.num_messages() != old_num_messages and any(isinstance(t, AnyType)
for t in results):
# If there was an error, return AnyType to avoid generating multiple messages for the
# same error.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this not a problem before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fixes a regression in testUnionAndBinaryOperation (and another test case whose name I can't remember right now). Previously, if some of the results was Any, the result got simplified into Any. Without this change, we'd preserve the non-Any types and this would sometimes produce a duplicate, similar-looking error message about the same binary operation, which would be a regression. Now the code falls back to the old behavior in these cases, and since this only happens when we have already generated a message, it doesn't affect correctness, just the specific messages generated for type errors involving union types.

return AnyType()
return UnionType.make_simplified_union(results)
elif isinstance(typ, TupleType):
# Actually look up from the fallback instance type.
Expand Down
17 changes: 12 additions & 5 deletions mypy/meet.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,19 @@ def meet_types(s: Type, t: Type) -> Type:
return t.accept(TypeMeetVisitor(s))


def meet_simple(s: Type, t: Type, default_right: bool = True) -> Type:
def meet_simple(s: Type, t: Type) -> Type:
"""Return the type s "narrowed down" to t.

Note that this is not symmetric with respect to s and t.

TODO: Explain what this does in more detail and how this is
different from meet_types.
"""
if s == t:
return s
if isinstance(t, AnyType):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change? What effect does it have overall?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, testUnionAnyIsInstance will fail -- the type in the else block will be Union[int, Any], though it should be Any. This makes is possible to narrow down from Union[int, Any] to Any, and this is the only place where this code is used. Previously this worked (probably accidentally) because Union[int, Any] got simplified to just Any. This won't have any other effect, as far I can see.

# Anything can be narrowed down to Any.
return t
if isinstance(s, UnionType):
return UnionType.make_simplified_union([meet_types(x, t) for x in s.items])
elif not is_overlapping_types(s, t, use_promotions=True):
Expand All @@ -36,10 +46,7 @@ def meet_simple(s: Type, t: Type, default_right: bool = True) -> Type:
else:
return NoneTyp()
else:
if default_right:
return t
else:
return s
return t


def is_overlapping_types(t: Type, s: Type, use_promotions: bool = False) -> bool:
Expand Down
3 changes: 3 additions & 0 deletions mypy/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ def enable_errors(self) -> None:
def is_errors(self) -> bool:
return self.errors.is_errors()

def num_messages(self) -> int:
return self.errors.num_messages()

def report(self, msg: str, context: Context, severity: str,
file: str = None, origin: Context = None) -> None:
"""Report an error or note (unless disabled)."""
Expand Down
19 changes: 15 additions & 4 deletions mypy/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1030,17 +1030,28 @@ def make_simplified_union(items: List[Type], line: int = -1, column: int = -1) -
all_items.append(typ)
items = all_items

if any(isinstance(typ, AnyType) for typ in items):
return AnyType()

from mypy.subtypes import is_subtype
from mypy.sametypes import is_same_type

def is_any_like(typ: Type) -> bool:
return (isinstance(typ, AnyType) or
(isinstance(typ, Instance) and typ.type.fallback_to_any))

removed = set() # type: Set[int]
for i, ti in enumerate(items):
if i in removed: continue
# Keep track of the truishness info for deleted subtypes which can be relevant
cbt = cbf = False
for j, tj in enumerate(items):
if i != j and is_subtype(tj, ti):
# Attempt to only combine true subtypes by avoiding types containing Any.
# TODO: Properly exclude generics and functions.
is_either_anylike = is_any_like(ti) or is_any_like(tj)
if (i != j
and is_subtype(tj, ti)
and (not is_either_anylike or
is_same_type(ti, tj) or
(isinstance(tj, NoneTyp)
and not experiments.STRICT_OPTIONAL))):
removed.add(j)
cbt = cbt or tj.can_be_true
cbf = cbf or tj.can_be_false
Expand Down
8 changes: 4 additions & 4 deletions test-data/unit/check-dynamic-typing.test
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ n = 0
d in a # E: Unsupported right operand type for in ("A")
d and a
d or a
c = d and b # Unintuitive type inference?
c = d or b # Unintuitive type inference?
c = d and b # E: Incompatible types in assignment (expression has type "Union[Any, bool]", variable has type "C")
c = d or b # E: Incompatible types in assignment (expression has type "Union[Any, bool]", variable has type "C")

c = d + a
c = d - a
Expand Down Expand Up @@ -123,8 +123,8 @@ n = 0
a and d
a or d
c = a in d
c = b and d # Unintuitive type inference?
c = b or d # Unintuitive type inference?
c = b and d # E: Incompatible types in assignment (expression has type "Union[bool, Any]", variable has type "C")
c = b or d # E: Incompatible types in assignment (expression has type "Union[bool, Any]", variable has type "C")
b = a + d
b = a / d

Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/check-generics.test
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ if not isinstance(s, str):

z = None # type: TNode # Same as TNode[Any]
z.x
z.foo() # Any simplifies Union to Any now. This test should be updated after #2197
z.foo() # E: Some element of union has no attribute "foo"

[builtins fixtures/isinstance.pyi]

Expand Down
23 changes: 23 additions & 0 deletions test-data/unit/check-optional.test
Original file line number Diff line number Diff line change
Expand Up @@ -525,3 +525,26 @@ f = None # type: Optional[Callable[[int], None]]
f = lambda x: None
f(0)
[builtins fixtures/function.pyi]

[case testOptionalAndAnyBaseClass]
from typing import Any, Optional
class C(Any):
pass
x = None # type: Optional[C]
x.foo() # E: Some element of union has no attribute "foo"

[case testUnionSimplificationWithStrictOptional]
from typing import Any, TypeVar, Union
class C(Any): pass
T = TypeVar('T')
S = TypeVar('S')
def u(x: T, y: S) -> Union[S, T]: pass
a = None # type: Any

# Test both orders
reveal_type(u(C(), None)) # E: Revealed type is 'Union[builtins.None, __main__.C*]'
reveal_type(u(None, C())) # E: Revealed type is 'Union[__main__.C*, builtins.None]'
reveal_type(u(a, None)) # E: Revealed type is 'Union[builtins.None, Any]'
reveal_type(u(None, a)) # E: Revealed type is 'Union[Any, builtins.None]'
reveal_type(u(1, None)) # E: Revealed type is 'Union[builtins.None, builtins.int*]'
reveal_type(u(None, 1)) # E: Revealed type is 'Union[builtins.int*, builtins.None]'
6 changes: 3 additions & 3 deletions test-data/unit/check-statements.test
Original file line number Diff line number Diff line change
Expand Up @@ -630,11 +630,11 @@ try:
except BaseException as e1:
reveal_type(e1) # E: Revealed type is 'builtins.BaseException'
except (E1, BaseException) as e2:
reveal_type(e2) # E: Revealed type is 'Any'
reveal_type(e2) # E: Revealed type is 'Union[Any, builtins.BaseException]'
except (E1, E2) as e3:
reveal_type(e3) # E: Revealed type is 'Any'
reveal_type(e3) # E: Revealed type is 'Union[Any, __main__.E2]'
except (E1, E2, BaseException) as e4:
reveal_type(e4) # E: Revealed type is 'Any'
reveal_type(e4) # E: Revealed type is 'Union[Any, builtins.BaseException]'

try: pass
except E1 as e1:
Expand Down
68 changes: 66 additions & 2 deletions test-data/unit/check-unions.test
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def f(x: Union[int, str]) -> None:
[case testUnionAnyIsInstance]
from typing import Any, Union

def func(v:Union[int, Any]) -> None:
def func(v: Union[int, Any]) -> None:
if isinstance(v, int):
reveal_type(v) # E: Revealed type is 'builtins.int'
else:
Expand All @@ -61,7 +61,8 @@ y = w.y
z = w.y # E: Incompatible types in assignment (expression has type "int", variable has type "str")
w.y = 'a' # E: Incompatible types in assignment (expression has type "str", variable has type "int")
y = x.y # E: Some element of union has no attribute "y"
z = x.y # E: Some element of union has no attribute "y"
v = x.y # E: Some element of union has no attribute "y"
reveal_type(v) # E: Revealed type is 'Any'

[builtins fixtures/isinstance.pyi]

Expand Down Expand Up @@ -168,3 +169,66 @@ if foo():
def g(x: Union[int, str, bytes]) -> None: pass
else:
def g(x: Union[int, str]) -> None: pass # E: All conditional function variants must have identical signatures

[case testUnionSimplificationSpecialCases]
from typing import Any, TypeVar, Union

class C(Any): pass

T = TypeVar('T')
S = TypeVar('S')
def u(x: T, y: S) -> Union[S, T]: pass

a = None # type: Any

# Base-class-Any and None, simplify
reveal_type(u(C(), None)) # E: Revealed type is '__main__.C*'
reveal_type(u(None, C())) # E: Revealed type is '__main__.C*'

# Normal instance type and None, simplify
reveal_type(u(1, None)) # E: Revealed type is 'builtins.int*'
reveal_type(u(None, 1)) # E: Revealed type is 'builtins.int*'

# Normal instance type and base-class-Any, no simplification
reveal_type(u(C(), 1)) # E: Revealed type is 'Union[builtins.int*, __main__.C*]'
reveal_type(u(1, C())) # E: Revealed type is 'Union[__main__.C*, builtins.int*]'

# Normal instance type and Any, no simplification
reveal_type(u(1, a)) # E: Revealed type is 'Union[Any, builtins.int*]'
reveal_type(u(a, 1)) # E: Revealed type is 'Union[builtins.int*, Any]'

# Any and base-class-Any, no simplificaiton
reveal_type(u(C(), a)) # E: Revealed type is 'Union[Any, __main__.C*]'
reveal_type(u(a, C())) # E: Revealed type is 'Union[__main__.C*, Any]'

# Two normal instance types, simplify
reveal_type(u(1, object())) # E: Revealed type is 'builtins.object*'
reveal_type(u(object(), 1)) # E: Revealed type is 'builtins.object*'

# Two normal instance types, no simplification
reveal_type(u(1, '')) # E: Revealed type is 'Union[builtins.str*, builtins.int*]'
reveal_type(u('', 1)) # E: Revealed type is 'Union[builtins.int*, builtins.str*]'

[case testUnionSimplificationWithDuplicateItems]
from typing import Any, TypeVar, Union

class C(Any): pass

T = TypeVar('T')
S = TypeVar('S')
R = TypeVar('R')
def u(x: T, y: S, z: R) -> Union[R, S, T]: pass

a = None # type: Any

reveal_type(u(1, 1, 1)) # E: Revealed type is 'builtins.int*'
reveal_type(u(C(), C(), None)) # E: Revealed type is '__main__.C*'
reveal_type(u(a, a, 1)) # E: Revealed type is 'Union[builtins.int*, Any]'
reveal_type(u(a, C(), a)) # E: Revealed type is 'Union[Any, __main__.C*]'
reveal_type(u('', 1, 1)) # E: Revealed type is 'Union[builtins.int*, builtins.str*]'

[case testUnionAndBinaryOperation]
from typing import Union
class A: pass
def f(x: Union[int, str, A]):
x + object() # E: Unsupported left operand type for + (some union)