-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
bpo-35113: Fix inspect.getsource to return correct source for inner classes #10307
Changes from all commits
51aa209
341933d
dbbb38e
22b9c9c
2655b1d
bd51f49
558895f
7d99c19
5f84344
8a63f36
4c7a61b
30865f6
054c317
26d1c96
d7ae710
aef4228
797c48d
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 |
---|---|---|
|
@@ -32,6 +32,7 @@ | |
'Yury Selivanov <[email protected]>') | ||
|
||
import abc | ||
import ast | ||
import dis | ||
import collections.abc | ||
import enum | ||
|
@@ -770,6 +771,42 @@ def getmodule(object, _filename=None): | |
if builtinobject is object: | ||
return builtin | ||
|
||
|
||
class ClassFoundException(Exception): | ||
pass | ||
|
||
|
||
class _ClassFinder(ast.NodeVisitor): | ||
|
||
def __init__(self, qualname): | ||
self.stack = [] | ||
self.qualname = qualname | ||
|
||
def visit_FunctionDef(self, node): | ||
self.stack.append(node.name) | ||
self.stack.append('<locals>') | ||
self.generic_visit(node) | ||
self.stack.pop() | ||
self.stack.pop() | ||
|
||
visit_AsyncFunctionDef = visit_FunctionDef | ||
|
||
def visit_ClassDef(self, node): | ||
self.stack.append(node.name) | ||
if self.qualname == '.'.join(self.stack): | ||
# Return the decorator for the class if present | ||
if node.decorator_list: | ||
line_number = node.decorator_list[0].lineno | ||
else: | ||
line_number = node.lineno | ||
|
||
# decrement by one since lines starts with indexing by zero | ||
line_number -= 1 | ||
raise ClassFoundException(line_number) | ||
self.generic_visit(node) | ||
self.stack.pop() | ||
|
||
|
||
def findsource(object): | ||
"""Return the entire source file and starting line number for an object. | ||
|
||
|
@@ -802,25 +839,15 @@ def findsource(object): | |
return lines, 0 | ||
|
||
if isclass(object): | ||
name = object.__name__ | ||
pat = re.compile(r'^(\s*)class\s*' + name + r'\b') | ||
# make some effort to find the best matching class definition: | ||
# use the one with the least indentation, which is the one | ||
# that's most probably not inside a function definition. | ||
candidates = [] | ||
for i in range(len(lines)): | ||
match = pat.match(lines[i]) | ||
if match: | ||
# if it's at toplevel, it's already the best one | ||
if lines[i][0] == 'c': | ||
return lines, i | ||
# else add whitespace to candidate list | ||
candidates.append((match.group(1), i)) | ||
if candidates: | ||
# this will sort by whitespace, and by line number, | ||
# less whitespace first | ||
candidates.sort() | ||
return lines, candidates[0][1] | ||
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') | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,18 +138,124 @@ def func137(): | |
never_reached1 | ||
never_reached2 | ||
|
||
#line 141 | ||
# line 141 | ||
class cls142: | ||
a = """ | ||
class cls149: | ||
... | ||
""" | ||
|
||
# line 148 | ||
class cls149: | ||
|
||
def func151(self): | ||
pass | ||
|
||
''' | ||
class cls160: | ||
pass | ||
''' | ||
|
||
# line 159 | ||
class cls160: | ||
|
||
def func162(self): | ||
pass | ||
|
||
# line 165 | ||
class cls166: | ||
a = ''' | ||
class cls175: | ||
... | ||
''' | ||
|
||
# line 172 | ||
class cls173: | ||
|
||
class cls175: | ||
pass | ||
|
||
# line 178 | ||
class cls179: | ||
pass | ||
|
||
# line 182 | ||
class cls183: | ||
|
||
class cls185: | ||
|
||
def func186(self): | ||
pass | ||
|
||
def class_decorator(cls): | ||
return cls | ||
|
||
# line 193 | ||
@class_decorator | ||
@class_decorator | ||
class cls196: | ||
|
||
@class_decorator | ||
@class_decorator | ||
class cls200: | ||
pass | ||
|
||
class cls203: | ||
class cls204: | ||
class cls205: | ||
pass | ||
class cls207: | ||
class cls205: | ||
pass | ||
|
||
# line 211 | ||
def func212(): | ||
class cls213: | ||
pass | ||
return cls213 | ||
|
||
# line 217 | ||
class cls213: | ||
def func219(self): | ||
class cls220: | ||
pass | ||
return cls220 | ||
|
||
# line 224 | ||
async def func225(): | ||
class cls226: | ||
pass | ||
return cls226 | ||
|
||
# line 230 | ||
class cls226: | ||
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. should be 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. Same as my above comment but this is to check for This to check that the inner class which has the same name |
||
async def func232(self): | ||
class cls233: | ||
pass | ||
return cls233 | ||
|
||
if True: | ||
class cls238: | ||
class cls239: | ||
'''if clause cls239''' | ||
else: | ||
class cls238: | ||
class cls239: | ||
'''else clause 239''' | ||
pass | ||
|
||
#line 247 | ||
def positional_only_arg(a, /): | ||
pass | ||
|
||
#line 145 | ||
#line 251 | ||
def all_markers(a, b, /, c, d, *, e, f): | ||
pass | ||
|
||
# line 149 | ||
# line 255 | ||
def all_markers_with_args_and_kwargs(a, b, /, c, d, *args, e, f, **kwargs): | ||
pass | ||
|
||
#line 153 | ||
#line 259 | ||
def all_markers_with_defaults(a, b=1, /, c=2, d=3, *, e=4, f=5): | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
:meth:`inspect.getsource` now returns correct source code for inner class | ||
with same name as module level class. Decorators are also returned as part | ||
of source of the class. Patch by Karthikeyan Singaravelan. |
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.
should be
cls218
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 to check that the inner class which has the same name
cls213
insidefunc212
doesn't conflict with this classcls213
. Previously regex was used and hence there were cases where two classes with same name defined under different scopes might conflict with each other returning wrong results.