From 16ae02e3be811e1c54fd253473774066520bcb70 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Thu, 27 Oct 2022 23:45:40 -0700 Subject: [PATCH 1/6] gh-85267: Improvements to inspect.signature __text_signature__ 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 #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`. --- Lib/inspect.py | 29 ++++++++++++++++++----------- Lib/test/test_inspect.py | 22 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index f6750c3b211fbd..e52fe0a94093ab 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -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) @@ -2185,7 +2185,6 @@ def _signature_fromstr(cls, obj, s, skip_bound_arg=True): parameters = [] empty = Parameter.empty - invalid = object() module = None module_dict = {} @@ -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): @@ -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 diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index cfc6e411ea680d..64395e10881cfd 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -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) @@ -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 From b60fb02c779130bea1bad2146ca5b30da82c9870 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 28 Oct 2022 07:24:36 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Library/2022-10-28-07-24-34.gh-issue-85267.xUy_Wm.rst | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-10-28-07-24-34.gh-issue-85267.xUy_Wm.rst 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 00000000000000..e69fd1ca1c2f3b --- /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 From dd2e08fc85ce0c78e483a0ceab8f31e814a5c658 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Tue, 8 Nov 2022 22:08:10 -0800 Subject: [PATCH 3/6] Update Lib/test/test_inspect.py Co-authored-by: Jelle Zijlstra --- Lib/test/test_inspect.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index 64395e10881cfd..5b32cdcd042543 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -4240,9 +4240,9 @@ def func(*args, **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)' + 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=21)') + 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) From ea41f4002e1215eb7d8f87f1ab622389e54fdaa5 Mon Sep 17 00:00:00 2001 From: Shantanu <12621235+hauntsaninja@users.noreply.github.com> Date: Tue, 8 Nov 2022 22:08:16 -0800 Subject: [PATCH 4/6] Update Lib/test/test_inspect.py Co-authored-by: Jelle Zijlstra --- Lib/test/test_inspect.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index 5b32cdcd042543..5310e23612bd89 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -4258,7 +4258,6 @@ def func(*args, **kwargs): with self.assertRaises(ValueError): inspect.signature(func) - def test_base_class_have_text_signature(self): # see issue 43118 from test.ann_module7 import BufferedReader From 7f9f2572503ebf9ba230f79d2cf67da54518d389 Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Tue, 8 Nov 2022 22:09:14 -0800 Subject: [PATCH 5/6] another runtime error --- Lib/inspect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index e52fe0a94093ab..cdb4858469c531 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -2222,7 +2222,7 @@ def visit_Attribute(self, node): 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) From 126b8c3fc674346b0ee6d773ec30a28579505aaf Mon Sep 17 00:00:00 2001 From: hauntsaninja Date: Tue, 8 Nov 2022 22:15:40 -0800 Subject: [PATCH 6/6] add a comment --- Lib/inspect.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/inspect.py b/Lib/inspect.py index cdb4858469c531..3fb10c2d150b9c 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -2233,6 +2233,8 @@ def visit_Name(self, node): 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):