Mypy: Don't report errors inside unannotated functions

[gnprice]

When typechecking the following file, we give an error:

$ cat /tmp/foo.py
def f(foo):
  if foo is not None:
    dumps = foo.dumps
  else:
    from json import dumps

$ mypy /tmp/foo.py
/tmp/foo.py: note: In function "f":
/tmp/foo.py:5: error: Name 'dumps' already defined

Because the function f doesn't have a type annotation, we ought to let this code be without complaining about it -- that makes it easier to start using mypy in parts of a large file, and pay the cost of fixing up code like this to work with mypy incrementally.

(Thanks to @varenc for reporting this issue -- he tried putting an annotation on one new function in a 3kLOC file with 188 existing functions, and it had this one error which stopped him.)

[ddfisher]

I think we should fix this issue not by fixing this particular case but by entirely not type checking unannotated functions. This would also speed up mypy on codebases that still have a significant fraction of functions unannotated.

I think there may be some cases (related to classes?) where we derive information from unannotated functions (maybe just __init__?). My inclination would be to address these cases by _not_ gathering that information, if possible (e.g. resulting in classes with unannotated __init__ methods getting Any typed). @JukkaL, do you know when we currently do this?

[gvanrossum]

I'm just speculating, but I wonder if this particular error is happening due to the slightly ad-hoc way we try to deal with conditional imports. The general rule is indeed that we don't type-check code inside un-annotated functions, but this particular error may come out of the semantic analysis, which always runs (or which may be triggered by the presence of the import in one of the branches).

Debugging tip: if you want to know where an error is generated, put a debug breakpoint (e.g. import pdb; pdb.set_trace()) in the code that generates the error (in this case name_already_defined() in semanal.py) and go up the stack a few frames to find out where it's being called and why. I've learned a ton this way.

[JukkaL]

This error is coming from semantic analysis, so skipping type checking wouldn't help here. We can probably not run type checking for unannotated functions if we move some things to an earlier phase. The reason we run type checking for all functions is actually so that mypy can export better type information for all AST nodes. These types were used by a compiler prototype that I wrote back in the day, but the information isn't very useful any more, though future features like IDE integration might still benefit from it.

Some behavior might change if we didn't run type checking for unannotated code. Nested annotated functions / classes wouldn't get type checked. We could perhaps special case these, but this seems like a pretty marginal use case. Also, not sure how attribute definitions would behave. In any case, we could infer attribute types during semantic analysis in unannotated functions, as the type would always be Any. We could also tweak semantic analysis to not complain about certain things such as name already defined in an unannotated function.

The best way to fix the original problem would be to support the idiom both in annotated and unannotated functions.

[ahaven]

I like the idea of avoiding unannotated functions entirely -- our codebase has a number of errors showing up in unannotated functions. (For example, issue #984)

[gvanrossum]

What you wrote seems ambiguous. Did you mean --disallow-untyped-defs? Or
did you mean you don't want any errors to be reported for unannotated
functions?

[ahaven]

I was liking @ddfisher's suggestion of not reporting errors for unannotated functions. It would get us much closer to having mypy running without errors on our codebase.

[gvanrossum]

David and I discussed this a bit and we decided that (without looking at the code) the most reasonable approach is to keep doing the semantic analysis for unannotated functions, but to somehow suppress errors from it.