FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-106727: Make `inspect.getsource` smarter for class for same name definitions by gaogaotiantian · Pull Request #106815 · python/cpython · GitHub

/ cpython Public

gh-106727: Make inspect.getsource smarter for class for same name definitions #106815

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.

Sign up for GitHub

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

Merged
carljm merged 4 commits into from
Jul 18, 2023
+71 −12
Conversation 14 Commits 4 Checks 0 Files changed 4
Merged

gh-106727: Make inspect.getsource smarter for class for same name definitions #106815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e9d7af
Improve inspect.getsource for class
gaogaotiantian Jul 16, 2023
867730b
📜🤖 Added by blurb_it.
blurb-it[bot] Jul 16, 2023
fc0a563
Add newline at the end of the file
gaogaotiantian Jul 17, 2023
3c880fe
Merge branch 'python:main' into inspect-class
gaogaotiantian Jul 18, 2023
File filter

Filter by extension

Filter by extension .py  (3) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
57 changes: 46 additions & 11 deletions Lib/inspect.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -1034,9 +1034,13 @@ class ClassFoundException(Exception):

class _ClassFinder(ast.NodeVisitor):

def __init__(self, qualname):
def __init__(self, cls, tree, lines, qualname):
self.stack = []
self.cls = cls
self.tree = tree
self.lines = lines
self.qualname = qualname
self.lineno_found = []

def visit_FunctionDef(self, node):
self.stack.append(node.name)
Expand All @@ -1057,11 +1061,48 @@ def visit_ClassDef(self, node):
line_number = node.lineno

# decrement by one since lines starts with indexing by zero
line_number -= 1
raise ClassFoundException(line_number)
self.lineno_found.append((line_number - 1, node.end_lineno))
self.generic_visit(node)
self.stack.pop()

def get_lineno(self):
self.visit(self.tree)
lineno_found_number = len(self.lineno_found)
if lineno_found_number == 0:
raise OSError('could not find class definition')
elif lineno_found_number == 1:
return self.lineno_found[0][0]
else:
# We have multiple candidates for the class definition.
# Now we have to guess.

# First, let's see if there are any method definitions
for member in self.cls.__dict__.values():
if isinstance(member, types.FunctionType):
for lineno, end_lineno in self.lineno_found:
if lineno <= member.__code__.co_firstlineno <= end_lineno:
return lineno
Comment on lines +1081 to +1084

Viicos Jul 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Using the line numbers of the __code__ attribute of the methods is really smart, I haven't thought about that! We can be pretty sure the right class is being detected this way.

As I said in the issue: #106727 (comment), I'm afraid of how it will behave with metaclasses:

class Meta(type):
    def __new__(mcs, name, bases, namespace):
        cls = super().__new__(mcs, name, bases, namespace)
        cls.my_method = lambda a: a
        return cls


class A(metaclass=Meta):
    pass

A.__dict__["my_method"].__code__.co_firstlineno
#> 4

Will it raise false positives if the __code__.co_firstlineno attribute of the method defined in the metaclass happen to falls within the lineno <= ... <= end_lineno range?

carljm Jul 21, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, if the metaclass and the class using it both have the same name, your example will confuse this heuristic. Nice observation!

But since the metaclass must come before the class using it, this just means we effectively maintain the previous first-one-wins behavior in this unusual scenario.

So while there may be scope for further improvement in edge cases, I'm still satisfied this PR was strictly an improvement over the previous behavior.

Viicos Jul 21, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What I meant is if the metaclass is defined in another file, the __code__.co_firstlineno integer of any method of that metaclass could clash with a method defined on the inspected class. I'm greatly satisfied by the improvements made in this PR as well; I'll see if there's a way to handle this metaclass edge case in some way!

gaogaotiantian Jul 21, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actually, we can deal with this. For all the methods defined in the class, they should have __module__ attribute to indicate in which module it was defined. We can simply compare this to the class passed in to rule out all the cases where the metaclass is not defined within the same file. This will also rule out the dynamically added methods.

Of course, there could be other evil cases like assigning the method of one class to another, that's a different story.

But we should get a pretty decent improvement by checking the modules, I'll work on the PR.

class_strings = [(''.join(self.lines[lineno: end_lineno]), lineno)
for lineno, end_lineno in self.lineno_found]

# Maybe the class has a docstring and it's unique?
if self.cls.__doc__:
ret = None
for candidate, lineno in class_strings:
Comment thread
carljm marked this conversation as resolved.
if self.cls.__doc__.strip() in candidate:
if ret is None:
ret = lineno
else:
break
else:
if ret is not None:
return ret

# We are out of ideas, just return the last one found, which is
# slightly better than previous ones
return self.lineno_found[-1][0]


def findsource(object):
"""Return the entire source file and starting line number for an object.
Expand Down Expand Up @@ -1098,14 +1139,8 @@ def findsource(object):
qualname = object.__qualname__
source = ''.join(lines)
tree = ast.parse(source)
class_finder = _ClassFinder(qualname)
try:
class_finder.visit(tree)
except ClassFoundException as e:
line_number = e.args[0]
return lines, line_number
else:
raise OSError('could not find class definition')
class_finder = _ClassFinder(object, tree, lines, qualname)
return lines, class_finder.get_lineno()

if ismethod(object):
object = object.__func__
Expand Down
20 changes: 20 additions & 0 deletions Lib/test/inspect_fodder2.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -290,3 +290,23 @@ def complex_decorated(foo=0, bar=lambda: 0):
nested_lambda = (
lambda right: [].map(
lambda length: ()))

# line 294
if True:
class cls296:
def f():
pass
else:
class cls296:
def g():
pass

# line 304
if False:
class cls310:
def f():
pass
else:
class cls310:
def g():
pass
5 changes: 4 additions & 1 deletion Lib/test/test_inspect.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,6 @@ def test_class_decorator(self):
self.assertSourceEqual(mod2.cls196.cls200, 198, 201)

def test_class_inside_conditional(self):
self.assertSourceEqual(mod2.cls238, 238, 240)
self.assertSourceEqual(mod2.cls238.cls239, 239, 240)

def test_multiple_children_classes(self):
Expand All @@ -975,6 +974,10 @@ def test_nested_class_definition_inside_async_function(self):
self.assertSourceEqual(mod2.cls226, 231, 235)
self.assertSourceEqual(asyncio.run(mod2.cls226().func232()), 233, 234)

def test_class_definition_same_name_diff_methods(self):
self.assertSourceEqual(mod2.cls296, 296, 298)
self.assertSourceEqual(mod2.cls310, 310, 312)

class TestNoEOL(GetSourceBase):
def setUp(self):
self.tempdir = TESTFN + '_dir'
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2023-07-16-23-59-33.gh-issue-106727.bk3uCu.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make :func:`inspect.getsource` smarter for class for same name definitions
Back | FazBrowse Home | New Git URL