Repository navigation
Don't report errors inside unannotated functions #1334
Description
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.)
Activity
ddfisher commentedon Apr 6, 2016
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 gettingAnytyped). @JukkaL, do you know when we currently do this?gvanrossum commentedon Apr 6, 2016
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 casename_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 commentedon Apr 6, 2016
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.
drewhaven commentedon Apr 6, 2016
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 commentedon Apr 6, 2016
drewhaven commentedon Apr 6, 2016
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 commentedon Apr 7, 2016
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.
[-]Error on conditional import inside unannotated function[/-][+]Don't report errors inside unannotated functions[/+]Suppress errors from semantic analysis in dynamic (unannotated) funct…
Suppress errors from semantic analysis in dynamic (unannotated) funct…
Suppress errors from semantic analysis in dynamic (unannotated) funct…