python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.22k stars 2.78k forks source link

Inference from unannotated functions #4455

Open elazarg opened 6 years ago

elazarg commented 6 years ago

Mypy uses the body of unannotated functions for inference and to understand what attributes an object has. For example:

d = {}
e = {}

def f():
    d['x'] = 1

d['y'] = 2
e['y'] = 2

reveal_type(d)  # dict[Any, Any]
reveal_type(e)  # dict[str, int]

I couldn't find any mention of it on the docs. I think this behaviour needs to be documented. Perhaps as a separate section, to make it searchable.

Related: #4434 #4409

ilevkivskyi commented 6 years ago

I would actually say this is a bug. We shouldn't use Any types (even explicit probably) for partial types, and require an annotation instead (if there are no other assignments in scope). This can lead to false negatives:

d = {}  # We should require an annotation here, if the second assignment below is absent...

def g():
    d['x'] = 1

d[1] = 'x'  # ...instead of inferring 'Dict[Any, Any]' that may be unsafe.
elazarg commented 6 years ago

IMO it is unclear that the error is at the last line. I would expect that line to define the type, and the error to be inside the function, since it is unannotated and therefore assumed to be too clever for the type checker.

I am confused though. I agree that we shouldn't use (implicit) Any for partial types, but from the discussion on #4434 I understood that this is by design.

This issue was meant to be more general: I am trying to understand how things should behave. What are the rules.

ilevkivskyi commented 6 years ago

IMO it is unclear that the error is at the last line.

I was probably not very clear in my example.

At least one of the two assignments is probably erroneous. But it would be safer to use the second for inference. The only downside of this is that after annotating the function, an error will appear in previously valid code. But this is better than the current (clearly unsafe) behavior. In case if there is no second assignment we should give a normal Need type annotation for variable error.

I am confused though. I agree that we shouldn't use (implicit) Any for partial types, but from the discussion on #4434 I understood that this is by design.

I don't think current behavior is really intentional, @JukkaL may correct me.

JukkaL commented 6 years ago

The current behavior seems accidental or at least not very well thought out. I think that it would be better to require all parts of a partial type definition to be located within a single scope. So this shouldn't be valid:

d = {}  # We should require an annotation here

def f():
    # This is a different scope from the initial assignment so can't affect the partial type
    d['x'] = 1

In the original example, we should probably use the second assignment for inference (d['y'] = 2). There probably shouldn't be an error, since the function is unannotated and we can just give d type Any within the function, independent of whether it has a partial type or not. If the function is annotated, we can defer the function and re-check it after we've finished with the module top level. This would conform to how fine-grained incremental works, I think.

elazarg commented 6 years ago

Jukka, I agree. So - what does mypy do inside unannotated functions?

JukkaL commented 6 years ago

Here are some things (probably incomplete):

Much of this isn't very principled, to be honest.

elazarg commented 6 years ago

Seems like the not-inconsistent part is straight-forward semantic analysis.