Skip to content
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

Add end_lineno and end_col_offset to nodes #1258

Merged
merged 14 commits into from
Nov 21, 2021
Prev Previous commit
Next Next commit
Fix tests
  • Loading branch information
cdce8p committed Nov 20, 2021
commit 493956fc65d5ec0b21c84e356cfcf76e35137c16
1 change: 1 addition & 0 deletions astroid/const.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import enum
import sys

PY38 = sys.version_info[:2] == (3, 8)
PY37_PLUS = sys.version_info >= (3, 7)
PY38_PLUS = sys.version_info >= (3, 8)
PY39_PLUS = sys.version_info >= (3, 9)
Expand Down
25 changes: 21 additions & 4 deletions astroid/rebuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

from astroid import nodes
from astroid._ast import ParserModule, get_parser_module, parse_function_type_comment
from astroid.const import PY37_PLUS, PY38_PLUS, Context
from astroid.const import PY37_PLUS, PY38, PY38_PLUS, Context
from astroid.manager import AstroidManager
from astroid.nodes import NodeNG

Expand Down Expand Up @@ -830,6 +830,15 @@ def visit_arguments(self, node: "ast.arguments", parent: NodeNG) -> nodes.Argume
if node.kwarg:
kwarg = node.kwarg.arg
kwargannotation = self.visit(node.kwarg.annotation, newnode)

if PY38:
# In Python 3.8 'end_lineno' and 'end_col_offset'
# for 'kwonlyargs' don't include the annotation.
for arg in node.kwonlyargs:
if arg.annotation is not None:
arg.end_lineno = arg.annotation.end_lineno
arg.end_col_offset = arg.annotation.end_col_offset

kwonlyargs = [self.visit(child, newnode) for child in node.kwonlyargs]
kw_defaults = [self.visit(child, newnode) for child in node.kw_defaults]
annotations = [self.visit(arg.annotation, newnode) for arg in node.args]
Expand Down Expand Up @@ -1401,7 +1410,7 @@ def visit_extslice(
self, node: "ast.ExtSlice", parent: nodes.Subscript
) -> nodes.Tuple:
"""visit an ExtSlice node by returning a fresh instance of Tuple"""
# TODO: add line and column info?
# ExtSlice doesn't have lineno or col_offset information
newnode = nodes.Tuple(ctx=Context.Load, parent=parent)
newnode.postinit([self.visit(dim, newnode) for dim in node.dims]) # type: ignore[attr-defined]
return newnode
Expand Down Expand Up @@ -2046,8 +2055,16 @@ def visit_setcomp(self, node: "ast.SetComp", parent: NodeNG) -> nodes.SetComp:

def visit_slice(self, node: "ast.Slice", parent: nodes.Subscript) -> nodes.Slice:
"""visit a Slice node by returning a fresh instance of it"""
# TODO add lineno and col offset info?
newnode = nodes.Slice(parent=parent)
if sys.version_info >= (3, 9):
newnode = nodes.Slice(
lineno=node.lineno,
col_offset=node.col_offset,
end_lineno=node.end_lineno,
end_col_offset=node.end_col_offset,
parent=parent,
)
else:
newnode = nodes.Slice(parent=parent)
newnode.postinit(
lower=self.visit(node.lower, newnode),
upper=self.visit(node.upper, newnode),
Expand Down
67 changes: 53 additions & 14 deletions tests/unittest_nodes_lineno.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import pytest

from astroid import builder, nodes
from astroid.const import PY38_PLUS
from astroid.const import PY38_PLUS, PY39_PLUS, PY310_PLUS


@pytest.mark.skipif(
Expand Down Expand Up @@ -160,8 +160,10 @@ def test_end_lineno_call() -> None:
assert (c1.args[0].lineno, c1.args[0].col_offset) == (1, 5)
assert (c1.args[0].end_lineno, c1.args[0].end_col_offset) == (1, 9)

assert (c1.keywords[0].lineno, c1.keywords[0].col_offset) == (1, 11)
assert (c1.keywords[0].end_lineno, c1.keywords[0].end_col_offset) == (1, 21)
if PY39_PLUS:
# 'lineno' and 'col_offset' information only added in Python 3.9
assert (c1.keywords[0].lineno, c1.keywords[0].col_offset) == (1, 11)
assert (c1.keywords[0].end_lineno, c1.keywords[0].end_col_offset) == (1, 21)
assert (c1.keywords[0].value.lineno, c1.keywords[0].value.col_offset) == (1, 16)
assert (
c1.keywords[0].value.end_lineno,
Expand Down Expand Up @@ -762,7 +764,7 @@ def test_end_lineno_subscript() -> None:
"""
var[0] #@
var[1:2:1] #@
var[1, 2] #@
var[1:2, 2] #@
"""
).strip()
ast_nodes = builder.extract_node(code)
Expand Down Expand Up @@ -798,9 +800,11 @@ def test_end_lineno_subscript() -> None:
assert isinstance(s3, nodes.Subscript)
assert isinstance(s3.slice, nodes.Tuple)
assert (s3.lineno, s3.col_offset) == (3, 0)
assert (s3.end_lineno, s3.end_col_offset) == (3, 9)
assert (s3.slice.lineno, s3.slice.col_offset) == (3, 4)
assert (s3.slice.end_lineno, s3.slice.end_col_offset) == (3, 8)
assert (s3.end_lineno, s3.end_col_offset) == (3, 11)
if PY39_PLUS:
# 'lineno' and 'col_offset' information only added in Python 3.9
assert (s3.slice.lineno, s3.slice.col_offset) == (3, 4)
assert (s3.slice.end_lineno, s3.slice.end_col_offset) == (3, 10)

@staticmethod
def test_end_lineno_import() -> None:
Expand Down Expand Up @@ -916,10 +920,14 @@ def test_end_lineno_string() -> None:
"""FormattedValue, JoinedStr."""
code = textwrap.dedent(
"""
f"Hello World: {42.1234:02d}"
f"Hello World: {42.1234:02d}" #@
f"Hello: {name=}" #@
"""
).strip()
s1 = builder.extract_node(code)
ast_nodes = builder.extract_node(code)
assert isinstance(ast_nodes, list) and len(ast_nodes) == 2

s1 = ast_nodes[0]
assert isinstance(s1, nodes.JoinedStr)
assert isinstance(s1.values[0], nodes.Const)
assert (s1.lineno, s1.col_offset) == (1, 0)
Expand All @@ -932,13 +940,42 @@ def test_end_lineno_string() -> None:
assert (s2.lineno, s2.col_offset) == (1, 0)
assert (s2.end_lineno, s2.end_col_offset) == (1, 29)
assert isinstance(s2.value, nodes.Const) # 42.1234
assert (s2.value.lineno, s2.value.col_offset) == (1, 16)
assert (s2.value.end_lineno, s2.value.end_col_offset) == (1, 23)
assert isinstance(s2.format_spec, nodes.JoinedStr) # 02d
if PY39_PLUS:
assert (s2.value.lineno, s2.value.col_offset) == (1, 16)
assert (s2.value.end_lineno, s2.value.end_col_offset) == (1, 23)
else:
# Bug in Python 3.8
# https://bugs.python.org/issue44885
assert (s2.value.lineno, s2.value.col_offset) == (1, 1)
assert (s2.value.end_lineno, s2.value.end_col_offset) == (1, 8)
assert isinstance(s2.format_spec, nodes.JoinedStr) # '02d'
assert (s2.format_spec.lineno, s2.format_spec.col_offset) == (1, 0)
assert (s2.format_spec.end_lineno, s2.format_spec.end_col_offset) == (1, 29)

s3 = ast_nodes[1]
assert isinstance(s3, nodes.JoinedStr)
assert isinstance(s3.values[0], nodes.Const)
assert (s3.lineno, s3.col_offset) == (2, 0)
assert (s3.end_lineno, s3.end_col_offset) == (2, 17)
assert (s3.values[0].lineno, s3.values[0].col_offset) == (2, 0)
assert (s3.values[0].end_lineno, s3.values[0].end_col_offset) == (2, 17)

s4 = s3.values[1]
assert isinstance(s4, nodes.FormattedValue)
assert (s4.lineno, s4.col_offset) == (2, 0)
assert (s4.end_lineno, s4.end_col_offset) == (2, 17)
assert isinstance(s4.value, nodes.Name) # 'name'
if PY39_PLUS:
assert (s4.value.lineno, s4.value.col_offset) == (2, 10)
assert (s4.value.end_lineno, s4.value.end_col_offset) == (2, 14)
else:
# Bug in Python 3.8
# https://bugs.python.org/issue44885
assert (s4.value.lineno, s4.value.col_offset) == (2, 1)
assert (s4.value.end_lineno, s4.value.end_col_offset) == (2, 5)

@staticmethod
@pytest.mark.skipif(not PY310_PLUS, reason="pattern matching was added in PY310")
def test_end_lineno_match() -> None:
"""Match, MatchValue, MatchSingleton, MatchSequence, MatchMapping,
MatchClass, MatchStar, MatchOr, MatchAs.
Expand Down Expand Up @@ -1140,7 +1177,9 @@ class X(Parent, var=42):
assert (c1.decorators.end_lineno, c1.decorators.end_col_offset) == (2, 11)
assert (c1.bases[0].lineno, c1.bases[0].col_offset) == (3, 8)
assert (c1.bases[0].end_lineno, c1.bases[0].end_col_offset) == (3, 14)
assert (c1.keywords[0].lineno, c1.keywords[0].col_offset) == (3, 16)
assert (c1.keywords[0].end_lineno, c1.keywords[0].end_col_offset) == (3, 22)
if PY39_PLUS:
# 'lineno' and 'col_offset' information only added in Python 3.9
assert (c1.keywords[0].lineno, c1.keywords[0].col_offset) == (3, 16)
assert (c1.keywords[0].end_lineno, c1.keywords[0].end_col_offset) == (3, 22)
assert (c1.body[0].lineno, c1.body[0].col_offset) == (4, 4)
assert (c1.body[0].end_lineno, c1.body[0].end_col_offset) == (4, 8)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reviewed the content of the test, not the expected column. But it comes from the ast, if it's wrong there isn't a lot we can do, right ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, at least not without significant effort. One area I was able to fix fairly easily was Arguments. See the snipped below. We might also be able to address the AssignName issue I mentioned by parsing the source code ourselves but that is definitely an issue for another time.

Usually the worse that can happen is that the lineno and col_offset also include the parent node. This is still correct just not as accurate as we would like it to be for error highlighting.

The true value of these tests is to know when a new version changed something. Like knowing that Keywords and Slice only include end_lineno in 3.9, or the issue with Arguments.

--
https://github.com/PyCQA/astroid/blob/7dd55fe8d96846617b2827e5272582ffc865190f/astroid/rebuilder.py#L834-L840