From c3c96d7e7cf35b93036f38fed8507f1571a8d7f6 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 5 Aug 2026 10:25:52 +0300 Subject: [PATCH 1/2] gh-155212: List the converters defined in a C file The --converters option now accepts file names and can be used with --make. It parses the specified files, without writing anything, and prints the converters and return converters which they define, instead of the built-in ones. Legacy converters whose format unit does not start with a letter, like those defined in Modules/posixmodule.c, were silently omitted from the list; they are now printed on a separate line. A directory passed as a file name raised IsADirectoryError instead of reporting an error, in every mode. Co-Authored-By: Claude Opus 5 (1M context) --- Lib/test/test_clinic.py | 64 +++++- ...-08-05-12-40-08.gh-issue-155212.Lx8vTm.rst | 4 + Tools/clinic/libclinic/cli.py | 217 ++++++++++-------- 3 files changed, 185 insertions(+), 100 deletions(-) create mode 100644 Misc/NEWS.d/next/Tools-Demos/2026-08-05-12-40-08.gh-issue-155212.Lx8vTm.rst diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 9a8faf777869d4..f35b11cff551a8 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -3364,10 +3364,66 @@ def test_cli_converters(self): with self.subTest(converter=converter): self.assertStartsWith(line, converter) - def test_cli_fail_converters_and_filename(self): - _, err = self.expect_failure("--converters", "test.c") - msg = "can't specify --converters and a filename at the same time" - self.assertIn(msg, err) + def test_cli_converters_file(self): + code = dedent(""" + /*[python input] + class my_type_converter(CConverter): + type = 'my_type' + converter = 'my_type_converter' + + def converter_init(self, *, strict=False): + pass + + class my_result_return_converter(CReturnConverter): + type = 'my_result' + [python start generated code]*/ + """) + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write(code) + out = self.expect_success("--converters", fn) + self.assertIn("Converters:\n my_type(strict=False)\n", out) + self.assertIn("Return converters:\n my_result()\n", out) + # Only the converters defined in the file are listed. + self.assertNotIn("Legacy converters:", out) + self.assertNotIn("bool(", out) + # Listing the converters does not write anything. + with open(fn, encoding="utf-8") as f: + self.assertEqual(f.read(), code) + self.assertEqual(os.listdir(tmp_dir), ["test.c"]) + + def test_cli_converters_make(self): + code = dedent(""" + /*[python input] + class my_type_converter(CConverter): + type = 'my_type' + converter = 'my_type_converter' + [python start generated code]*/ + """) + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write(code) + out = self.expect_success("--converters", "--make", + "--srcdir", tmp_dir) + self.assertIn("Converters:\n my_type()\n", out) + with open(fn, encoding="utf-8") as f: + self.assertEqual(f.read(), code) + + def test_cli_converters_no_converters(self): + with os_helper.temp_dir() as tmp_dir: + fn = os.path.join(tmp_dir, "test.c") + with open(fn, "w", encoding="utf-8") as f: + f.write("/*[clinic input]\n[clinic start generated code]*/\n") + self.assertEqual(self.expect_success("--converters", fn), "") + + def test_cli_fail_directory(self): + with os_helper.temp_dir() as tmp_dir: + subdir = os.path.join(tmp_dir, "test.c") + os.mkdir(subdir) + _, err = self.expect_failure(subdir) + self.assertIn(f"Can't read file {subdir!r}: it is a directory", err) def test_cli_fail_no_filename(self): _, err = self.expect_failure() diff --git a/Misc/NEWS.d/next/Tools-Demos/2026-08-05-12-40-08.gh-issue-155212.Lx8vTm.rst b/Misc/NEWS.d/next/Tools-Demos/2026-08-05-12-40-08.gh-issue-155212.Lx8vTm.rst new file mode 100644 index 00000000000000..a4b52430436f3d --- /dev/null +++ b/Misc/NEWS.d/next/Tools-Demos/2026-08-05-12-40-08.gh-issue-155212.Lx8vTm.rst @@ -0,0 +1,4 @@ +The ``--converters`` option of Argument Clinic now accepts file names and +can be used with ``--make``. +It prints the converters and return converters which the specified files +define, instead of the built-in ones. diff --git a/Tools/clinic/libclinic/cli.py b/Tools/clinic/libclinic/cli.py index 678edc8ca28944..6324532f6ac672 100644 --- a/Tools/clinic/libclinic/cli.py +++ b/Tools/clinic/libclinic/cli.py @@ -6,7 +6,7 @@ import os import re import sys -from collections.abc import Callable +from collections.abc import Callable, Iterable, Iterator, Mapping from typing import NoReturn @@ -69,8 +69,11 @@ def parse_file( except KeyError: raise ClinicError(f"Can't identify file type for file {filename!r}") - with open(filename, encoding="utf-8") as f: - raw = f.read() + try: + with open(filename, encoding="utf-8") as f: + raw = f.read() + except IsADirectoryError: + raise ClinicError(f"Can't read file {filename!r}: it is a directory") # exit quickly if there are no clinic markers in the file find_start_re = BlockParser("", language).find_start_re @@ -115,7 +118,9 @@ def create_cli() -> argparse.ArgumentParser: "of the changes to the standard output")) cmdline.add_argument("--converters", action='store_true', help=("print a list of all supported converters " - "and return converters")) + "and return converters; if files are " + "specified, print only the converters " + "which they define")) cmdline.add_argument("--make", action='store_true', help="walk --srcdir to run over all relevant files") cmdline.add_argument("--srcdir", type=str, default=os.curdir, @@ -154,117 +159,137 @@ def report_changes(writer: libclinic.FileWriter, *, diff: bool) -> None: print(f"would {action} {change.filename}") -def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: - dry_run = ns.dry_run or ns.diff - # The report is written to the standard output, so the progress - # is written to the standard error stream to not mix them. - verbose_file = sys.stderr if dry_run else sys.stdout +AnyConverterType = ConverterType | ReturnConverterType - if ns.converters: - if ns.filename: - parser.error( - "can't specify --converters and a filename at the same time" - ) - if dry_run: - parser.error("can't use --dry-run or --diff with --converters") - AnyConverterType = ConverterType | ReturnConverterType - converter_list: list[tuple[str, AnyConverterType]] = [] - return_converter_list: list[tuple[str, AnyConverterType]] = [] - - for name, converter in converters.items(): - converter_list.append(( - name, - converter, - )) - for name, return_converter in return_converters.items(): - return_converter_list.append(( - name, - return_converter - )) - print() +def defined_in_files( + registry: Mapping[str, AnyConverterType], + builtin: Mapping[str, AnyConverterType], +) -> dict[str, AnyConverterType]: + """Return the converters which the parsed files define or redefine.""" + return {name: cls for name, cls in registry.items() + if builtin.get(name) is not cls} + +def print_converter_list( + title: str, + attribute: str, + registry: Mapping[str, AnyConverterType], +) -> None: + print(title + ":") + for name, cls in sorted(registry.items(), key=lambda item: item[0].lower()): + callable = getattr(cls, attribute, None) + if not callable: + continue + signature = inspect.signature(callable) + parameters = [] + for parameter_name, parameter in signature.parameters.items(): + if parameter.kind == inspect.Parameter.KEYWORD_ONLY: + if parameter.default != inspect.Parameter.empty: + s = f'{parameter_name}={parameter.default!r}' + else: + s = parameter_name + parameters.append(s) + print(' {}({})'.format(name, ', '.join(parameters))) + print() + + +def print_converters( + converters: Mapping[str, AnyConverterType], + legacy_converters: Mapping[str, AnyConverterType], + return_converters: Mapping[str, AnyConverterType], +) -> None: + if not (converters or legacy_converters or return_converters): + return + print() + if legacy_converters: print("Legacy converters:") legacy = sorted(legacy_converters) - print(' ' + ' '.join(c for c in legacy if c[0].isupper())) - print(' ' + ' '.join(c for c in legacy if c[0].islower())) + # A converter defined in a file can use any string, even a C + # expression, as its format unit, not only a letter. + groups = ([c for c in legacy if c[0].isupper()], + [c for c in legacy if c[0].islower()], + [c for c in legacy if not c[0].isalpha()]) + for group in groups: + if group: + print(' ' + ' '.join(group)) print() + if converters: + print_converter_list("Converters", 'converter_init', converters) + if return_converters: + print_converter_list("Return converters", 'return_converter_init', + return_converters) + print("All converters also accept (c_default=None, py_default=None, annotation=None).") + print("All return converters also accept (py_default=None).") + + +def walk_srcdir(srcdir: str, exclude: list[str] | None) -> Iterator[str]: + """Yield the C files in the source directory tree.""" + if exclude: + excludes = [os.path.normpath(os.path.join(srcdir, f)) for f in exclude] + else: + excludes = [] + for root, dirs, files in os.walk(srcdir): + for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'): + if rcs_dir in dirs: + dirs.remove(rcs_dir) + for filename in files: + # handle .c, .cpp and .h files + if not filename.endswith(('.c', '.cpp', '.h')): + continue + path = os.path.normpath(os.path.join(root, filename)) + if path in excludes: + continue + yield path - for title, attribute, ids in ( - ("Converters", 'converter_init', converter_list), - ("Return converters", 'return_converter_init', return_converter_list), - ): - print(title + ":") - - ids.sort(key=lambda item: item[0].lower()) - longest = -1 - for name, _ in ids: - longest = max(longest, len(name)) - - for name, cls in ids: - callable = getattr(cls, attribute, None) - if not callable: - continue - signature = inspect.signature(callable) - parameters = [] - for parameter_name, parameter in signature.parameters.items(): - if parameter.kind == inspect.Parameter.KEYWORD_ONLY: - if parameter.default != inspect.Parameter.empty: - s = f'{parameter_name}={parameter.default!r}' - else: - s = parameter_name - parameters.append(s) - print(' {}({})'.format(name, ', '.join(parameters))) - print() - print("All converters also accept (c_default=None, py_default=None, annotation=None).") - print("All return converters also accept (py_default=None).") - return +def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None: + dry_run = ns.dry_run or ns.diff + # The report is written to the standard output, so the progress + # is written to the standard error stream to not mix them. + verbose_file = sys.stderr if dry_run else sys.stdout + + filenames: Iterable[str] if ns.make: if ns.output or ns.filename: parser.error("can't use -o or filenames with --make") if not ns.srcdir: parser.error("--srcdir must not be empty with --make") - if ns.exclude: - excludes = [os.path.join(ns.srcdir, f) for f in ns.exclude] - excludes = [os.path.normpath(f) for f in excludes] - else: - excludes = [] - writer = libclinic.FileWriter(dry_run=dry_run) - for root, dirs, files in os.walk(ns.srcdir): - for rcs_dir in ('.svn', '.git', '.hg', 'build', 'externals'): - if rcs_dir in dirs: - dirs.remove(rcs_dir) - for filename in files: - # handle .c, .cpp and .h files - if not filename.endswith(('.c', '.cpp', '.h')): - continue - path = os.path.join(root, filename) - path = os.path.normpath(path) - if path in excludes: - continue - if ns.verbose: - print(path, file=verbose_file) - parse_file(path, - verify=not ns.force, limited_capi=ns.limited_capi, - writer=writer) - report_changes(writer, diff=ns.diff) - return - - if not ns.filename: - parser.error("no input files") - - if ns.output and len(ns.filename) > 1: - parser.error("can't use -o with multiple filenames") + filenames = walk_srcdir(ns.srcdir, ns.exclude) + else: + if not ns.filename and not ns.converters: + parser.error("no input files") + if ns.output and len(ns.filename) > 1: + parser.error("can't use -o with multiple filenames") + filenames = ns.filename - writer = libclinic.FileWriter(dry_run=dry_run) - for filename in ns.filename: + if ns.converters: + if dry_run: + parser.error("can't use --dry-run or --diff with --converters") + if not ns.make and not ns.filename: + print_converters(converters, legacy_converters, return_converters) + return + # Converters defined in a file are added to the same registries + # as the built-in ones, so remember the latter to tell them apart. + builtin_converters = dict(converters) + builtin_legacy_converters = dict(legacy_converters) + builtin_return_converters = dict(return_converters) + + writer = libclinic.FileWriter(dry_run=dry_run or ns.converters) + for filename in filenames: if ns.verbose: print(filename, file=verbose_file) parse_file(filename, output=ns.output, verify=not ns.force, limited_capi=ns.limited_capi, writer=writer) - report_changes(writer, diff=ns.diff) + + if ns.converters: + print_converters( + defined_in_files(converters, builtin_converters), + defined_in_files(legacy_converters, builtin_legacy_converters), + defined_in_files(return_converters, builtin_return_converters)) + else: + report_changes(writer, diff=ns.diff) def main(argv: list[str] | None = None) -> NoReturn: From f2685d92da6236669b8ef499ae69aa33bfc1c487 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 5 Aug 2026 11:36:12 +0300 Subject: [PATCH 2/2] gh-155212: Check for a directory before opening the file Opening a directory fails with IsADirectoryError on Unix, but with PermissionError on Windows. Co-Authored-By: Claude Opus 5 (1M context) --- Tools/clinic/libclinic/cli.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tools/clinic/libclinic/cli.py b/Tools/clinic/libclinic/cli.py index 6324532f6ac672..c66084cf314482 100644 --- a/Tools/clinic/libclinic/cli.py +++ b/Tools/clinic/libclinic/cli.py @@ -69,12 +69,12 @@ def parse_file( except KeyError: raise ClinicError(f"Can't identify file type for file {filename!r}") - try: - with open(filename, encoding="utf-8") as f: - raw = f.read() - except IsADirectoryError: + if os.path.isdir(filename): raise ClinicError(f"Can't read file {filename!r}: it is a directory") + with open(filename, encoding="utf-8") as f: + raw = f.read() + # exit quickly if there are no clinic markers in the file find_start_re = BlockParser("", language).find_start_re if not find_start_re.search(raw):