Skip to content

Commit

Permalink
pythongh-85267: Improvements to inspect.signature __text_signature__ …
Browse files Browse the repository at this point in the history
…handling

This makes a couple related changes to inspect.signature's behaviour
when parsing a signature from `__text_signature__`.

First, `inspect.signature` is documented as only raising ValueError or
TypeError. However, in some cases, we could raise RuntimeError.  This PR
changes that, thereby fixing python#83685.

(Note that the new ValueErrors in RewriteSymbolics are caught and then
reraised with a message)

Second, `inspect.signature` could randomly drop parameters that it
didn't understand (corresponding to `return None` in the `p` function).
This is the core issue in python#85267. I think this is very surprising
behaviour and it seems better to fail outright.

Third, adding this new failure broke a couple tests. To fix them (and to
e.g. allow `inspect.signature(select.epoll.register)` as in python#85267), I
add constant folding of a couple binary operations to RewriteSymbolics.

(There's some discussion of making signature expression evaluation
arbitrary powerful in python#68155. I think that's out of scope. The
additional constant folding here is pretty straightforward, useful, and
not much of a slippery slope)

Fourth, while python#85267 is incorrect about the cause of the issue, it turns
out if you had consecutive newlines in __text_signature__, you'd get
`tokenize.TokenError`.

Finally, the `if name is invalid:` code path was dead, since
`parse_name` never returned `invalid`.
  • Loading branch information
hauntsaninja committed Oct 28, 2022
1 parent 75a6fad commit 16ae02e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 12 deletions.
29 changes: 18 additions & 11 deletions Lib/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -2109,7 +2109,7 @@ def _signature_strip_non_python_syntax(signature):
self_parameter = None
last_positional_only = None

lines = [l.encode('ascii') for l in signature.split('\n')]
lines = [l.encode('ascii') for l in signature.split('\n') if l]
generator = iter(lines).__next__
token_stream = tokenize.tokenize(generator)

Expand Down Expand Up @@ -2185,7 +2185,6 @@ def _signature_fromstr(cls, obj, s, skip_bound_arg=True):

parameters = []
empty = Parameter.empty
invalid = object()

module = None
module_dict = {}
Expand All @@ -2209,11 +2208,11 @@ def wrap_value(s):
try:
value = eval(s, sys_module_dict)
except NameError:
raise RuntimeError()
raise ValueError

if isinstance(value, (str, int, float, bytes, bool, type(None))):
return ast.Constant(value)
raise RuntimeError()
raise ValueError

class RewriteSymbolics(ast.NodeTransformer):
def visit_Attribute(self, node):
Expand All @@ -2233,19 +2232,27 @@ def visit_Name(self, node):
raise ValueError()
return wrap_value(node.id)

def visit_BinOp(self, node):
left = self.visit(node.left)
right = self.visit(node.right)
if not isinstance(left, ast.Constant) or not isinstance(right, ast.Constant):
raise ValueError
if isinstance(node.op, ast.Add):
return ast.Constant(left.value + right.value)
elif isinstance(node.op, ast.Sub):
return ast.Constant(left.value - right.value)
elif isinstance(node.op, ast.BitOr):
return ast.Constant(left.value | right.value)
raise ValueError

def p(name_node, default_node, default=empty):
name = parse_name(name_node)
if name is invalid:
return None
if default_node and default_node is not _empty:
try:
default_node = RewriteSymbolics().visit(default_node)
o = ast.literal_eval(default_node)
default = ast.literal_eval(default_node)
except ValueError:
o = invalid
if o is invalid:
return None
default = o if o is not invalid else default
raise ValueError("{!r} builtin has invalid signature".format(obj)) from None
parameters.append(Parameter(name, kind, default=default, annotation=empty))

# non-keyword-only parameters
Expand Down
22 changes: 21 additions & 1 deletion Lib/test/test_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -2474,7 +2474,7 @@ def p(name): return signature.parameters[name].default
self.assertEqual(p('f'), False)
self.assertEqual(p('local'), 3)
self.assertEqual(p('sys'), sys.maxsize)
self.assertNotIn('exp', signature.parameters)
self.assertEqual(p('exp'), sys.maxsize - 1)

test_callable(object)

Expand Down Expand Up @@ -4235,10 +4235,30 @@ def func(*args, **kwargs):
sig = inspect.signature(func)
self.assertIsNotNone(sig)
self.assertEqual(str(sig), '(self, /, a, b=1, *args, c, d=2, **kwargs)')

func.__text_signature__ = '($self, a, b=1, /, *args, c, d=2, **kwargs)'
sig = inspect.signature(func)
self.assertEqual(str(sig), '(self, a, b=1, /, *args, c, d=2, **kwargs)')

func.__text_signature__ = '(self, a=1+2, b=4-3, c=1 | 4 | 16)'
sig = inspect.signature(func)
self.assertEqual(str(sig), '(self, a=3, b=1, c=21)')

func.__text_signature__ = '(self, a=1,\nb=2,\n\n\n c=3)'
sig = inspect.signature(func)
self.assertEqual(str(sig), '(self, a=1, b=2, c=3)')

func.__text_signature__ = '(self, x=does_not_exist)'
with self.assertRaises(ValueError):
inspect.signature(func)
func.__text_signature__ = '(self, x=sys, y=inspect)'
with self.assertRaises(ValueError):
inspect.signature(func)
func.__text_signature__ = '(self, 123)'
with self.assertRaises(ValueError):
inspect.signature(func)


def test_base_class_have_text_signature(self):
# see issue 43118
from test.ann_module7 import BufferedReader
Expand Down

0 comments on commit 16ae02e

Please sign in to comment.