Repository navigation
Function line number information is not always correct #3871
Description
Currently, the line number of the function definition is not correct leading to issues such as #3869.
In the Python ast (as well as type-ast) the FunctionDef node we use to, for example, report missing return annotations gives a line number of the beginning of the first decorator (if any). Therefore it can incorrectly report the line number information we need to report errors.
Our currently logic increments the line number of any decorators, however, we cannot depend on this to be consistent. Consider this problematic (but completely valid!) example:
@decorate(
"multi",
"line"
)
def foo(
a: int,
b: str,
):
passThe FunctionDef.lineno would be 1, so mypy will report test.py:2: error: Function is missing a return type annotation. However, the return should be on line 8! Note that blanks space between the body and end of the function definition node means we cannot decrement based on the first item in the body of the function.
As a first possible solution, we could modify typed_ast to give the line number of the returns if the function is not typed (currently it just puts the attribute as None), so that the information is known, but it clearly indicates that the node is empty. This is certainly not an ideal solution.
Corresponding issue opened in python/typed_ast#50
Activity
gvanrossum commentedon Aug 25, 2017
emmatyping commentedon Aug 25, 2017
Hm, that is probably true now that I think about it more. That would also have the added bonus of better error messages if a function name is redefined. I was thinking that since on larger functions it is common to split arguments over multiple lines, it would be nice to give an exact line number of where to put the annotation, but now that I think about it, if we point to the name, it shouldn't be too bad for the user to modify the return of that function.
Perhaps we could add a
def_linenoattribute toFunctionDefnodes in typed_ast? Then refactor mypy to use that of course.gvanrossum commentedon Aug 25, 2017
That sounds reasonable.
ilevkivskyi commentedon Aug 25, 2017
I think we should do the same for classes (i.e. add
class_lineno), mypy currently provides special support to only one class decorator@runtime, but IIRC @JelleZijlstra already got confused once when I put a# type: ignoreon the class decorator line.JelleZijlstra commentedon Aug 25, 2017
I think I would have been confused too if that ignore comment had not been on the decorator line. :)
emmatyping commentedon Aug 25, 2017
I agree with @ilevkivskyi adding a
class_linenois also probably a good idea.If nothing else, it will correctly report name re-definitions better.
If there are no objections, I will start on a patch to typed_ast.I got busy with a PEP, so this is up for grabs
emmatyping commentedon Apr 5, 2018
I should mention there is an open bpo issue about this exact problem. https://bugs.python.org/issue33211
emmatyping commentedon Apr 26, 2018
So if we do this, it will require a change from:
to
everywhere, which is semantically more correct, but shows up a fair amount in typeshed (cc @JelleZijlstra, I'm happy to work on changing the occurrences of this).
That also means pytype will need to make a similar patch. @matthiaskramm, since Google already patches their ast, I hope it wouldn't hurt to patch it a bit more? The totality of changes to the ast are a handful of lines (see emmatyping/typed_ast@51b8d3e and note only the first file is the patch you'd need).
gvanrossum commentedon Apr 27, 2018
For pytype, Matthias is no longer on that project, @rchen152 is his replacement.
rchen152 commentedon Apr 27, 2018
So if I understand correctly, the problem is that in typeshed, "# type: ignore" comments are going to move from the first line of the first decorator to the line on which "def" appears, so pytype will need to change its pyi parsing to handle this?
If that's it, then everything is fine! Pytype ignores "# type: ignore" in a pyi file.
emmatyping commentedon Apr 27, 2018
Ah, I didn't know that! Apologies.
Yes exactly.
Excellent! I will work on the changes tomorrow then.
gvanrossum commentedon Apr 28, 2019
Should we do this or not? I worry that just changing the lineno attribute of FunctionDef and ClassDef nodes will break tons of users' code since it changes where
# type: ignorecomments would have to go. So maybe let sleeping dogs lie?OTOH in Python 3.8 the ast module moves the lineno attributes of these nodes, so in the long run we may have no choice (and to do it we would have to update typed_ast first, see python/typed_ast#50).
brandtbucher commentedon Apr 29, 2019
I recently had a PR merged (#6648) that recognizes
# type: ignores on any line of a multi-line expression in Python 3.8; it would probably be fairly straightforward to use the same approach here for classes and functions. It would allow the correct lines to be reported, while also respecting ignores occurring on any line in the scope (such as between the first decorator and thefinal colondef/classline).If this seems like a viable solution, I'd be happy to write a PR.
gvanrossum commentedon Apr 29, 2019
But that will only affect 3.8, right? Maybe that’s okay and we should just keep the existing behavior for older versions—and not modify typed-ast.
brandtbucher commentedon Apr 29, 2019
Correct, 3.8 only. It would probably make any planned transition smoother too, since 3.8 users will have the option of the "correct" line (i.e. in new 3.8 code) or the "compatibility" line (i.e. in typeshed).
gvanrossum commentedon Apr 29, 2019
Yeah, this sounds like a good thing to have.
brandtbucher commentedon Apr 29, 2019
Awesome, I'll begin work on this.
Fix class/def line reporting and ignoring for 3.8+ (#6753)