-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix class/def line reporting and ignoring for 3.8+. #6753
Changes from 3 commits
974d5ae
0cebf4b
516f728
7846eff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -520,13 +520,23 @@ def do_func_def(self, n: Union[ast3.FunctionDef, ast3.AsyncFunctionDef], | |
# Before 3.8, [typed_]ast the line number points to the first decorator. | ||
# In 3.8, it points to the 'def' line, where we want it. | ||
lineno += len(n.decorator_list) | ||
end_lineno = None | ||
else: | ||
# It's okay that end_lineno is *before* lineno here. | ||
# lineno is where the error is reported, and end_lineno defines | ||
# the opposite end of the range to search for "# type: ignore" | ||
# comments in. This reports the correct line in the event of an | ||
# error, but keeps old pre-3.8 "# type: ignore" comments working | ||
# correctly. | ||
lineno = n.lineno | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you even need this? It seems this was already set on line 432 above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch. I believe that's left over from an earlier iteration of this change. |
||
end_lineno = n.decorator_list[0].lineno + len(n.decorator_list) | ||
|
||
var = Var(func_def.name()) | ||
var.is_ready = False | ||
var.set_line(lineno) | ||
|
||
func_def.is_decorated = True | ||
func_def.set_line(lineno, n.col_offset) | ||
func_def.set_line(lineno, n.col_offset, end_lineno) | ||
func_def.body.set_line(lineno) # TODO: Why? | ||
|
||
deco = Decorator(func_def, self.translate_expr_list(n.decorator_list), var) | ||
|
@@ -629,15 +639,18 @@ def visit_ClassDef(self, n: ast3.ClassDef) -> ClassDef: | |
metaclass=dict(keywords).get('metaclass'), | ||
keywords=keywords) | ||
cdef.decorators = self.translate_expr_list(n.decorator_list) | ||
if n.decorator_list and sys.version_info >= (3, 8): | ||
# Before 3.8, n.lineno points to the first decorator; in | ||
# 3.8, it points to the 'class' statement. We always make | ||
# it point to the first decorator. (The node structure | ||
# here is different than for decorated functions.) | ||
cdef.line = n.decorator_list[0].lineno | ||
cdef.column = n.col_offset | ||
# It's okay if end_lineno is *before* lineno here. | ||
# lineno is where the error is reported, and end_lineno defines the | ||
# opposite end of the range to search for "# type: ignore" comments in. | ||
# This reports the correct line in the event of an error, but keeps old | ||
# pre-3.8 "# type: ignore" comments working correctly. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar comment (except I wasn't really confused here, since I had just figured out the previous one :-). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above: # Set end_lineno to the old mypy 0.700 lineno, in order to keep
# existing "# type: ignore" comments working: |
||
if sys.version_info < (3, 8): | ||
cdef.line = n.lineno + len(n.decorator_list) | ||
cdef.end_line = n.lineno | ||
else: | ||
self.set_line(cdef, n) | ||
cdef.line = n.lineno | ||
cdef.end_line = n.decorator_list[0].lineno if n.decorator_list else None | ||
cdef.column = n.col_offset | ||
self.class_and_function_stack.pop() | ||
return cdef | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -754,7 +754,7 @@ class C: | |
pass | ||
[out] | ||
MypyFile:1( | ||
ClassDef:1( | ||
ClassDef:2( | ||
C | ||
Decorators( | ||
CallExpr:1( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment confused me. The key thing to note here is that we want to point
lineno
to the line that has thedef
keyword, while we want to pointend_lineno
to the line that would have been used for Python 3.7 and before. And we must computeend_lineno
(i.e. the "old start line") using the crufty way given here since the old way of computing it (i.e.n.lineno + len(n.decorator_list)
won't have the same result in 3.8.Sorry that this is still so confusing. The code is right!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you going to update these comments? I hope so!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to reword this to be more concise. Maybe something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good -- also for the other location!