Mypy errors with variable reuse with different types

[timabbott]

from typing import *

class Test(object):
    x = 1
    y = 2

for sub in [(1, 2)]:
    pass

subs = {} # type: Dict[Tuple[int, int], Test]                                                                                 
for sub in [Test()]:
    subs[(sub.x, sub.y)] = sub

gives

/home/tabbott/foo.py:11: error: Incompatible types in assignment (expression has type "Test", variable has type "Tuple[int, int]")
/home/tabbott/foo.py:12: error: Tuple[int, ...] has no attribute "x"
/home/tabbott/foo.py:12: error: Tuple[int, ...] has no attribute "y"
/home/tabbott/foo.py:12: error: Incompatible types in assignment

Arguably this is not perfect code, but probably this shouldn't be a mypy error?

[gvanrossum]

I think this is a duplicate of a common theme: variable reuse within one function body. Mypy really prefers that you don't do that. (I can't find a specific issue to point to though.)

[JukkaL]

Yeah, this one has been discussed many times though there doesn't seem to be a github issue for this.

I'm not really sure about what's the best way to approach this. Perhaps mypy should allow redefining a variable with a different type by default and provide a strict mode or option which disallows this.

A general solution would be to logically define a fresh variable each time an assignment redefines a variable on all possible code paths. This would define a new variable, since the old definition can't reach beyond the second assignment:

if c:
    x = 1
x = ''  # new variable x

This would not define a new variable:

x = 1
if c:
    x = ''  # does not define a new variable (error)
print(x)

Not sure about his:

x = 1
print(x)
if c:
    x = ''  # ??
    print(x)
# x not read here any more

A less general approach would be to only allow redefining with a new type only if the original definition didn't have a type annotation and the new type definition doesn't have a type annotation. So this would be okay:

x = 1
...
x = ''

But this would not be okay because the annotation is lying:

def f(x: int) -> None:
    x = str(x)  # error?

And this would not be okay:

x = []  # type: List[int]
...
x = 3  # error?

[matthiaskramm]

For what it's worth, pytype always allows you do redefine a variable with a different type. We're essentially doing SSA.

On the other hand, that also means that pytype considers code like this correct:

def f(x: int):
  x = "foo"

I know it looks odd, but allowing this pattern is is quite useful when adding function annotations to existing code.

[JukkaL]

I propose moving this to an earlier milestone such as 0.4.0, at least tentatively, since issues like these a little painful to refactor, and they can generate a lot of noise. This is more of a style issue than correctness issue, and I'd rather not make mypy very opinionated about style issues. An optional flag that causes mypy to complain about questionable redefinitions would be better in my opinion.

[ddfisher]

Have we run into this issue a lot internally? It looks to me like this'll be a lot of work -- we'd have to start doing a significant amount of control flow analysis, and I expect doing that properly will be somewhat difficult.

[JukkaL]

I've hit this in some open source code I've experimented with. If we run
mypy against a lot of code without annotations using --implicit-any we
should get an estimate of the prevalence via type errors.
On Thu, Apr 14, 2016 at 21:07 David Fisher notifications@github.com wrote:

Have we run into this issue a lot internally? It looks to me like this'll
be a lot of work -- we'd have to start doing a significant amount of
control flow analysis.

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1174 (comment)

[timabbott]

When you're checking this out, I'd recommend at least also checking the tests directories for the projects; my experience was that our test code had a much higher ratio of these than the rest of the codebase.

[ddfisher]

Also just FYI --implicit-any is now --check-untyped-defs.

[gvanrossum]

It's --check-untyped-defs now.

I ran this over a small corpus (under 150K LOC) and found, among a total of over 1200 errors, about 30 occurrences of this message (though 9 of these didn't have the (expression has type "xxx", variable has type "yyy") suffix -- those were from assignments into container items like x[a] = b).

A bunch (indeed most common in tests) were unrelated types, but in many cases arguably the error was due to a too narrowly inferred initial type. E.g.

x = randint()
x = randint() / 1000.0

A bunch were similar but the two types involved were different subclasses of the same superclass. Common was also assignment of differently sized tuples to a variable, e.g.

x = ()
x = (1,)
x = (1, 2)

I've also seen things like

if ...:
    iter_ = [some list]
else:
    iter_ = <some iterable>
for i in iter_: ...

Other complicating factors were that a few times the assignments occurred in except clauses or other "blocks" where the variable wasn't used after exiting the blocks.

All in all I do think this is a popular idiom that we ought to support better.

[JukkaL]

Perhaps we could start with special casing some common idioms instead of trying to support all of the possible idioms. Here my concern is that the rules that mypy follows should be easy to describe and predictable, and approaches like SSA wouldn't really fit this description.

Here are some idioms that would be easy enough to support and that I think are pretty common.

1) Redefined in the same block

Examples:

def f(x: int) -> None:
    x = str(int)
    print(x)

y = 1
print(y)
y = ''
print(y)

Variables within for, while and try (what about with?) statements would be harder as an intermediate value may escape.

Here nested blocks would be considered separate blocks, and we consider function arguments to be defined in the same block as the function body.

2) Variable never read outside statement that defines it

Examples:

for x in (1, 2) : ...
for x in ('a', 'b'): ...
# x not read outside 'for' statements

_ = 1  # variable defined but never read
_ = ''  

Also 'with'. For try/except this might already work. For module-level variables we'd take the last definition as the type of the variable, but if there is a conditional definition things get harder.

3) Conditional definitions don't agree on type

This is actually a separate issue as here we have to infer a single type for a variable. Example (from above):

if ...:
    x = [...]
else:
    x = <iterable>
y = x

[gvanrossum]

Sounds like a common theme is that if variables are defined and redefined at the same indentation level ("in the same block") and there's no local flow control in between we should check the code between the definitions using the first definition and after that use the second (last) definition. E.g.

if cond():
    x = 1
    x+1
    x = ''
else:
    x = [1, 2, 3]
    x.append(4)
    x = ''

Should work because the ultimate type in each block is the same.

However this is questionable:

while cond():
    x = f()
    if x < 0:
        break
    x = ''
# The type of x here could be int or str.

In a try block everything is questionable:

try:
    x = 0
    foo(x)
    x = ''
except Exception:
    pass
# Here x could be int or str.

A future improvement could not care if there's no use afterwards.

For branches that don't agree we could have some kind of unification (like I want for conditional expressions e.g. #1094).