From 15d4b038a371a3d1b085922e561c759cc38c371d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:23:37 +0200 Subject: [PATCH 01/16] gh-104050: Add more type hints to Argument Clinic DSLParser() Add basic annotations to state_modulename_name() --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 27d3d572868883..b0e5142efa9e51 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4526,7 +4526,7 @@ def state_dsl_start(self, line: str | None) -> None: self.next(self.state_modulename_name, line) - def state_modulename_name(self, line): + def state_modulename_name(self, line: str | None) -> None: # looking for declaration, which establishes the leftmost column # line should be # modulename.fnname [as c_basename] [-> return annotation] @@ -4543,7 +4543,7 @@ def state_modulename_name(self, line): # this line is permitted to start with whitespace. # we'll call this number of spaces F (for "function"). - if not line.strip(): + if not self.valid_line(line): return self.indent.infer(line) From 4744bd922d8e69e3dace97948f7f0a90fc2782f8 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:35:49 +0200 Subject: [PATCH 02/16] Fix shadowing post partition --- Tools/clinic/clinic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index b0e5142efa9e51..c730688fbfcdd0 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4588,9 +4588,9 @@ def state_modulename_name(self, line: str | None) -> None: line, _, returns = line.partition('->') - full_name, _, c_basename = line.partition(' as ') - full_name = full_name.strip() - c_basename = c_basename.strip() or None + left, _, right = line.partition(' as ') + full_name = left.strip() + c_basename = right.strip() or None if not is_legal_py_identifier(full_name): fail("Illegal function name:", full_name) From 7df949b645f544dbd6d777b8e98d41a5fcd20fc0 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:43:51 +0200 Subject: [PATCH 03/16] Tighten up parse_converter() return annotation --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index c730688fbfcdd0..7d69f1145b7b70 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5021,7 +5021,7 @@ def bad_node(self, node): key = f"{parameter_name}_as_{c_name}" if c_name else parameter_name self.function.parameters[key] = p - KwargDict = dict[str | None, Any] + KwargDict = dict[str, Any] @staticmethod def parse_converter(annotation: ast.expr | None) -> tuple[str, bool, KwargDict]: @@ -5031,6 +5031,7 @@ def parse_converter(annotation: ast.expr | None) -> tuple[str, bool, KwargDict]: case ast.Name(name): return name, False, {} case ast.Call(func=ast.Name(name)): + assert isinstance(ast.expr, str) symbols = globals() kwargs = { node.arg: eval_ast_expr(node.value, symbols) From 76677045c5646e61afac183a0f4be8dd25a04436 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:45:39 +0200 Subject: [PATCH 04/16] Loosen up state_parameters_start() --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 7d69f1145b7b70..df51c6062cf626 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4727,7 +4727,7 @@ def state_modulename_name(self, line: str | None) -> None: ps_start, ps_left_square_before, ps_group_before, ps_required, \ ps_optional, ps_group_after, ps_right_square_after = range(7) - def state_parameters_start(self, line: str) -> None: + def state_parameters_start(self, line: str | None) -> None: if not self.valid_line(line): return From 0480ed29566eb907833482b39717138a0ae12166 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:48:09 +0200 Subject: [PATCH 05/16] ast.expr is not str --- Tools/clinic/clinic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index df51c6062cf626..043cac385e6d83 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -5031,7 +5031,6 @@ def parse_converter(annotation: ast.expr | None) -> tuple[str, bool, KwargDict]: case ast.Name(name): return name, False, {} case ast.Call(func=ast.Name(name)): - assert isinstance(ast.expr, str) symbols = globals() kwargs = { node.arg: eval_ast_expr(node.value, symbols) From ec7c1d8e6662508916e04db03fa796a041b32566 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:51:44 +0200 Subject: [PATCH 06/16] Annotate Parameter 'kind' param as inspect._ParameterKind --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 043cac385e6d83..4a2c91d964bcc9 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2531,7 +2531,7 @@ class Parameter: def __init__( self, name: str, - kind: str, + kind: inspect._ParameterKind, *, default = inspect.Parameter.empty, function: Function, From ec38c62ab9fe210dd5fe38a6af615cbb49fe3b05 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 09:53:49 +0200 Subject: [PATCH 07/16] Annotate DSLParser 'function' attr as 'Function | None' --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 4a2c91d964bcc9..42e7c5abfdf8e9 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4311,7 +4311,7 @@ def __init__(self, clinic: Clinic) -> None: self.reset() def reset(self) -> None: - self.function = None + self.function: Function | None = None self.state: StateKeeper = self.state_dsl_start self.parameter_indent = None self.keyword_only = False From b1bd72a230a49b325aa296869ff94a363d81d69d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 10:04:00 +0200 Subject: [PATCH 08/16] Make sure Parameter() gets name as str --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 42e7c5abfdf8e9..f5b090607c59e6 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4652,7 +4652,7 @@ def state_modulename_name(self, line: str | None) -> None: if cls and type == "PyObject *": kwargs['type'] = cls.typedef sc = self.function.self_converter = self_converter(name, name, self.function, **kwargs) - p_self = Parameter(sc.name, inspect.Parameter.POSITIONAL_ONLY, function=self.function, converter=sc) + p_self = Parameter(name, inspect.Parameter.POSITIONAL_ONLY, function=self.function, converter=sc) self.function.parameters[sc.name] = p_self (cls or module).functions.append(self.function) From 3f1a0c016c32b9b042a7778a9748ba96692a53ae Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 10:05:42 +0200 Subject: [PATCH 09/16] Function param self_converter is CConverter or None --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f5b090607c59e6..a4e15e0414a1c8 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2467,7 +2467,7 @@ def __init__( self.docstring = docstring or '' self.kind = kind self.coexist = coexist - self.self_converter = None + self.self_converter: CConverter | None = None # docstring_only means "don't generate a machine-readable # signature, just a normal docstring". it's True for # functions with optional groups because we can't represent From d694f043ae25308dda607fad4ea1d8d85d79982f Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 11:21:36 +0200 Subject: [PATCH 10/16] Make sure str is used as key in parameter dict --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index a4e15e0414a1c8..52b9b9f7124ee8 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4653,7 +4653,7 @@ def state_modulename_name(self, line: str | None) -> None: kwargs['type'] = cls.typedef sc = self.function.self_converter = self_converter(name, name, self.function, **kwargs) p_self = Parameter(name, inspect.Parameter.POSITIONAL_ONLY, function=self.function, converter=sc) - self.function.parameters[sc.name] = p_self + self.function.parameters[name] = p_self (cls or module).functions.append(self.function) self.next(self.state_parameters_start) From e587ba4c25dc659fa99efd79876b49cfd38fb31e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 13:04:50 +0200 Subject: [PATCH 11/16] Annotation for c_basename --- Tools/clinic/clinic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index b7536c718914f3..8049dd95eaaa38 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4564,9 +4564,9 @@ def state_modulename_name(self, line: str | None) -> None: # are we cloning? before, equals, existing = line.rpartition('=') if equals: - full_name, _, c_basename = before.partition(' as ') - full_name = full_name.strip() - c_basename = c_basename.strip() + left, _, right = before.partition(' as ') + full_name = left.strip() + c_basename: str | None = right.strip() existing = existing.strip() if (is_legal_py_identifier(full_name) and (not c_basename or is_legal_c_identifier(c_basename)) and From cd5d675373565347ce828230dd06baa45f62e3c7 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 13:04:14 +0200 Subject: [PATCH 12/16] More accurate type for self.self_converter --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 8049dd95eaaa38..1425f70c49b08c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -38,6 +38,7 @@ TypeGuard, overload, ) +from __future__ import annotations # TODO: # @@ -2467,7 +2468,7 @@ def __init__( self.docstring = docstring or '' self.kind = kind self.coexist = coexist - self.self_converter: CConverter | None = None + self.self_converter: self_converter | None = None # docstring_only means "don't generate a machine-readable # signature, just a normal docstring". it's True for # functions with optional groups because we can't represent From f25dd999d89fe9043f06b668890aee7c35eace2d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 13:17:25 +0200 Subject: [PATCH 13/16] Address review: fix return_converter type --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 1425f70c49b08c..a4c3aa342e56e5 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2449,7 +2449,7 @@ def __init__( cls: Class | None = None, c_basename: str | None = None, full_name: str | None = None, - return_converter: ReturnConverterType, + return_converter: CReturnConverter, return_annotation = inspect.Signature.empty, docstring: str | None = None, kind: str = CALLABLE, From dcd5d32a477c833bcf5c7c16dd4b06b62e62be93 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 13:18:23 +0200 Subject: [PATCH 14/16] from __future__ imports must occur at the beginning of the file! --- Tools/clinic/clinic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index a4c3aa342e56e5..5363e95ea2466c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4,6 +4,7 @@ # Copyright 2012-2013 by Larry Hastings. # Licensed to the PSF under a contributor agreement. # +from __future__ import annotations import abc import ast @@ -38,7 +39,6 @@ TypeGuard, overload, ) -from __future__ import annotations # TODO: # From 140bafcec2f7ad876b05dd404ba3c35a20d45e60 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 15:20:36 +0200 Subject: [PATCH 15/16] Wrap touched lines at 79 chars --- Tools/clinic/clinic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 69d3f966649307..ca631423dc82a7 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4666,7 +4666,8 @@ def state_modulename_name(self, line: str | None) -> None: if cls and type == "PyObject *": kwargs['type'] = cls.typedef sc = self.function.self_converter = self_converter(name, name, self.function, **kwargs) - p_self = Parameter(name, inspect.Parameter.POSITIONAL_ONLY, function=self.function, converter=sc) + p_self = Parameter(name, inspect.Parameter.POSITIONAL_ONLY, + function=self.function, converter=sc) self.function.parameters[name] = p_self (cls or module).functions.append(self.function) From 4a7edf9129eaa0f9da7acba22cd7b3d9633bc51d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 3 Jul 2023 15:36:31 +0200 Subject: [PATCH 16/16] Apply suggestions from code review --- Tools/clinic/clinic.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index ca631423dc82a7..41d4274fc744e0 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4564,10 +4564,11 @@ def state_modulename_name(self, line: str | None) -> None: # are we cloning? before, equals, existing = line.rpartition('=') + c_basename: str | None if equals: - left, _, right = before.partition(' as ') - full_name = left.strip() - c_basename: str | None = right.strip() + full_name, _, c_basename = before.partition(' as ') + full_name = full_name.strip() + c_basename = c_basename.strip() existing = existing.strip() if (is_legal_py_identifier(full_name) and (not c_basename or is_legal_c_identifier(c_basename)) and @@ -4602,9 +4603,9 @@ def state_modulename_name(self, line: str | None) -> None: line, _, returns = line.partition('->') - left, _, right = line.partition(' as ') - full_name = left.strip() - c_basename = right.strip() or None + full_name, _, c_basename = line.partition(' as ') + full_name = full_name.strip() + c_basename = c_basename.strip() or None if not is_legal_py_identifier(full_name): fail("Illegal function name:", full_name)