diff options
author | Shantanu <12621235+hauntsaninja@users.noreply.github.com> | 2022-12-21 03:29:02 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-21 03:29:02 (GMT) |
commit | 79311cbfe718f17c89bab67d7f89da3931bfa2ac (patch) | |
tree | 7135bc6630fb49f9e7cb5c173c39283626acc808 | |
parent | c615286e8576f2555d4380f38a966c300805b1a5 (diff) | |
download | cpython-79311cbfe718f17c89bab67d7f89da3931bfa2ac.zip cpython-79311cbfe718f17c89bab67d7f89da3931bfa2ac.tar.gz cpython-79311cbfe718f17c89bab67d7f89da3931bfa2ac.tar.bz2 |
gh-85267: Improvements to inspect.signature __text_signature__ handling (#98796)
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 #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 #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 #85267), I
add constant folding of a couple binary operations to RewriteSymbolics.
(There's some discussion of making signature expression evaluation
arbitrary powerful in #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 #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`.
-rw-r--r-- | Lib/inspect.py | 25 | ||||
-rw-r--r-- | Lib/test/test_inspect.py | 21 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Library/2022-10-28-07-24-34.gh-issue-85267.xUy_Wm.rst | 6 |
3 files changed, 46 insertions, 6 deletions
diff --git a/Lib/inspect.py b/Lib/inspect.py index 052f0bf..3db7745 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -2122,7 +2122,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) @@ -2221,11 +2221,11 @@ def _signature_fromstr(cls, obj, s, skip_bound_arg=True): 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): @@ -2235,7 +2235,7 @@ def _signature_fromstr(cls, obj, s, skip_bound_arg=True): a.append(n.attr) n = n.value if not isinstance(n, ast.Name): - raise RuntimeError() + raise ValueError a.append(n.id) value = ".".join(reversed(a)) return wrap_value(value) @@ -2245,6 +2245,21 @@ def _signature_fromstr(cls, obj, s, skip_bound_arg=True): raise ValueError() return wrap_value(node.id) + def visit_BinOp(self, node): + # Support constant folding of a couple simple binary operations + # commonly used to define default values in text signatures + 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 default_node and default_node is not _empty: @@ -2252,7 +2267,7 @@ def _signature_fromstr(cls, obj, s, skip_bound_arg=True): default_node = RewriteSymbolics().visit(default_node) default = ast.literal_eval(default_node) except ValueError: - return None + raise ValueError("{!r} builtin has invalid signature".format(obj)) from None parameters.append(Parameter(name, kind, default=default, annotation=empty)) # non-keyword-only parameters diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index 78e6e9e..1b589c8 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -2525,7 +2525,7 @@ class TestSignatureObject(unittest.TestCase): 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) @@ -4323,10 +4323,29 @@ class TestSignatureDefinitions(unittest.TestCase): 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 | 3 | 16)' + sig = inspect.signature(func) + self.assertEqual(str(sig), '(self, a=3, b=1, c=19)') + + 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 diff --git a/Misc/NEWS.d/next/Library/2022-10-28-07-24-34.gh-issue-85267.xUy_Wm.rst b/Misc/NEWS.d/next/Library/2022-10-28-07-24-34.gh-issue-85267.xUy_Wm.rst new file mode 100644 index 0000000..e69fd1c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-28-07-24-34.gh-issue-85267.xUy_Wm.rst @@ -0,0 +1,6 @@ +Several improvements to :func:`inspect.signature`'s handling of ``__text_signature``.
+- Fixes a case where :func:`inspect.signature` dropped parameters
+- Fixes a case where :func:`inspect.signature` raised :exc:`tokenize.TokenError`
+- Allows :func:`inspect.signature` to understand defaults involving binary operations of constants
+- :func:`inspect.signature` is documented as only raising :exc:`TypeError` or :exc:`ValueError`, but sometimes raised :exc:`RuntimeError`. These cases now raise :exc:`ValueError`
+- Removed a dead code path |