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

Fix PyPy ClassDef.fromlino with decorators #1979

Merged
merged 1 commit into from
Jan 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Release date: TBA
* Fix issues with ``typing_extensions.TypeVar``.


* Fix ``ClassDef.fromlino`` for PyPy 3.8 (v7.3.11) if class is wrapped by a decorator.
Copy link
Member Author

Choose a reason for hiding this comment

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

I can move this to 2.14.0 if we want to skip 2.13.4.

Copy link
Member

Choose a reason for hiding this comment

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

Let's not skip, releasing is cheap (compared to fixing the milestones)



What's New in astroid 2.13.3?
=============================
Expand Down
3 changes: 3 additions & 0 deletions astroid/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
IS_PYPY = sys.implementation.name == "pypy"
IS_JYTHON = sys.implementation.name == "jython"

# pylint: disable-next=no-member
PYPY_7_3_11_PLUS = IS_PYPY and sys.pypy_version_info >= (7, 3, 11) # type: ignore[attr-defined]


class Context(enum.Enum):
Load = 1
Expand Down
5 changes: 3 additions & 2 deletions astroid/nodes/scoped_nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from astroid import bases
from astroid import decorators as decorators_mod
from astroid import util
from astroid.const import IS_PYPY, PY38, PY38_PLUS, PY39_PLUS
from astroid.const import IS_PYPY, PY38, PY38_PLUS, PY39_PLUS, PYPY_7_3_11_PLUS
from astroid.context import (
CallContext,
InferenceContext,
Expand Down Expand Up @@ -2139,9 +2139,10 @@ def _newstyle_impl(self, context: InferenceContext | None = None):
@cached_property
def fromlineno(self) -> int | None:
"""The first line that this node appears on in the source code."""
if not PY38_PLUS or PY38 and IS_PYPY:
if not PY38_PLUS or IS_PYPY and PY38 and not PYPY_7_3_11_PLUS:
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to support multiple PyPy versions? I guess it's simple enough here, although I wouldn't add any more to CI.

Copy link
Member

Choose a reason for hiding this comment

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

This feel like a lot of work but also added values. I guess on a case by case basis... ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This feel like a lot of work but also added values. I guess on a case by case basis... ?

Yeah, I was more referring to this specific case 😄

# For Python < 3.8 the lineno is the line number of the first decorator.
# We want the class statement lineno. Similar to 'FunctionDef.fromlineno'
# PyPy (3.8): Fixed with version v7.3.11
lineno = self.lineno
if self.decorators is not None:
lineno += sum(
Expand Down
22 changes: 7 additions & 15 deletions tests/unittest_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import pytest

from astroid import Instance, builder, nodes, test_utils, util
from astroid.const import IS_PYPY, PY38, PY38_PLUS, PY39_PLUS
from astroid.const import IS_PYPY, PY38, PY38_PLUS, PY39_PLUS, PYPY_7_3_11_PLUS
from astroid.exceptions import (
AstroidBuildingError,
AstroidSyntaxError,
Expand Down Expand Up @@ -130,18 +130,18 @@ def function(
def test_decorated_class_lineno() -> None:
code = textwrap.dedent(
"""
class A:
class A: # L2
...

@decorator
class B:
class B: # L6
...

@deco1
@deco2(
var=42
)
class C:
class C: # L13
...
"""
)
Expand All @@ -155,23 +155,15 @@ class C:

b = ast_module.body[1]
assert isinstance(b, nodes.ClassDef)
if PY38 and IS_PYPY:
# Not perfect, but best we can do for PyPy 3.8
assert b.fromlineno == 7
else:
assert b.fromlineno == 6
assert b.fromlineno == 6
assert b.tolineno == 7

c = ast_module.body[2]
assert isinstance(c, nodes.ClassDef)
if not PY38_PLUS:
# Not perfect, but best we can do for Python 3.7
if not PY38_PLUS or IS_PYPY and PY38 and not PYPY_7_3_11_PLUS:
# Not perfect, but best we can do for Python 3.7 and PyPy 3.8 (< v7.3.11).
# Can't detect closing bracket on new line.
assert c.fromlineno == 12
elif PY38 and IS_PYPY:
# Not perfect, but best we can do for PyPy 3.8
# Can't detect closing bracket on new line.
assert c.fromlineno == 16
else:
assert c.fromlineno == 13
assert c.tolineno == 14
Expand Down
9 changes: 3 additions & 6 deletions tests/unittest_nodes_position.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import textwrap

from astroid import builder, nodes
from astroid.const import IS_PYPY, PY38


class TestNodePosition:
Expand Down Expand Up @@ -65,11 +64,9 @@ class F: #@
assert isinstance(e, nodes.ClassDef)
assert e.position == (13, 0, 13, 7)

if not PY38 or not IS_PYPY:
# The new (2022-12) version of pypy 3.8 broke this
f = ast_nodes[5]
assert isinstance(f, nodes.ClassDef)
assert f.position == (18, 0, 18, 7)
f = ast_nodes[5]
assert isinstance(f, nodes.ClassDef)
assert f.position == (18, 0, 18, 7)

@staticmethod
def test_position_function() -> None:
Expand Down