diff --git a/docs/developer/feature-test-map/core.md b/docs/developer/feature-test-map/core.md index b5c71d2d..77c55e4b 100644 --- a/docs/developer/feature-test-map/core.md +++ b/docs/developer/feature-test-map/core.md @@ -58,6 +58,7 @@ https://github.com/TNO/Renaissance.Py/blob/main/LICENSE - `test/recipes/test_python_refactoring.py` - `test/utils/test_cst_utils.py` - `test/utils/test_import_resolution.py` + - `test/utils/test_ruff_utils.py` - `test/rejuvenation/test_migration_type_recipes.py` - **Code file(s):** `src/renaissance/recipes/type_parameter_check.py`, `src/renaissance/recipes/type_parameter_rules.py`, `src/renaissance/recipes/type_parameter_declarations.py`, `src/renaissance/recipes/type_parameter_localization.py`, @@ -65,4 +66,5 @@ https://github.com/TNO/Renaissance.Py/blob/main/LICENSE `src/renaissance/recipes/step_runner.py`, `src/renaissance/recipes/python_refactoring.py`, `src/renaissance/utils/import_resolution.py`, `src/renaissance/utils/cst_utils.py`, + `src/renaissance/utils/ruff_utils.py`, `src/rejuvenation/migration_type_recipes.py` diff --git a/docs/developer/modules/recipes.md b/docs/developer/modules/recipes.md index 54c9e99f..28f06ec4 100644 --- a/docs/developer/modules/recipes.md +++ b/docs/developer/modules/recipes.md @@ -34,11 +34,9 @@ safely cover. This page covers `TypeParameterCheck`, the recipe behind names each project file imports from the others. - `src/renaissance/utils/cst_utils.py`: a module's top-level statements, the names they read or bind and the comment trailing a statement's line. - - - +- `src/renaissance/utils/ruff_utils.py`: `run_ruff_import_cleanup` and `run_ruff_format`, which the CLI runs on the + files the recipe changed. The first removes unused imports and sorts the imports, the second formats the files. + ## Public entry points - `TypeParameterCheck.check()` (and `run()`): runs the three phases through `run_steps`, committing to disk between them. diff --git a/docs/user/features/type-parameter-modernization.md b/docs/user/features/type-parameter-modernization.md index 2ffa60c0..387ed870 100644 --- a/docs/user/features/type-parameter-modernization.md +++ b/docs/user/features/type-parameter-modernization.md @@ -404,6 +404,7 @@ run the tool again. - `test/recipes/test_python_refactoring.py` - `test/utils/test_cst_utils.py` - `test/utils/test_import_resolution.py` +- `test/utils/test_ruff_utils.py` - `test/rejuvenation/test_migration_type_recipes.py` ## Implemented by code modules @@ -411,7 +412,7 @@ run the tool again. - [Refactoring recipes](../../developer/modules/recipes.md) - `src/renaissance/recipes/type_parameter_check.py`, `type_parameter_rules.py`, `type_parameter_declarations.py`, `type_parameter_localization.py`, `type_parameter_uses.py`, `step_runner.py` and `python_refactoring.py`. -- `src/renaissance/utils/import_resolution.py` and `cst_utils.py` +- `src/renaissance/utils/import_resolution.py`, `cst_utils.py` and `ruff_utils.py` - `src/rejuvenation/migration_type_recipes.py` (the CLI) ## API entry points @@ -432,12 +433,10 @@ python src/rejuvenation/migration_type_recipes.py --py MAJOR.MINOR \ - `--second-pass`: process again only the files that kept a declaration because another file imported it when the run started, and merge the result into the same report. -After the recipe, the CLI runs `ruff check --fix --select F401,I001,UP049` on the changed files. This removes **every** +After the recipe, the CLI runs `ruff check --fix --select F401,I001` on the changed files. This removes **every** unused import in them, including ones that were unused before (such as `from typing import TypeVar` after the declaration is gone). It also merges and sorts **every** import block in them, including imports that were unsorted -before, using the `ruff` settings of the project. It removes the leading underscores of **every** PEP 695 type -parameter in them, so `def f[_T]` becomes `def f[T]`, unless the new name would shadow another one. Docstrings, -comments and the report keep the old name. The project's `select` and `ignore` lists do not apply to these three +before, using the `ruff` settings of the project. The project's `select` and `ignore` lists do not apply to these two rules. Then it runs `ruff format` on the same files, which reformats **every** line in them with the project's `ruff format` settings, not only the changed ones. `ruff` must be installed in the same environment. Files are changed directly, so run it on a git checkout. diff --git a/src/rejuvenation/migration_type_recipes.py b/src/rejuvenation/migration_type_recipes.py new file mode 100644 index 00000000..d177a3ee --- /dev/null +++ b/src/rejuvenation/migration_type_recipes.py @@ -0,0 +1,589 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""CLI that modernizes type parameters (TypeVar, ParamSpec, TypeVarTuple) to PEP 695 syntax. + +Runs TypeParameterCheck on a file or directory, and reports which files were modified and which need +manual review. + +Example: + python src/rejuvenation/migration_type_recipes.py ./repo --py 3.12 --report review.md --ignore folder/types.py + +""" + +# Printing the report is this CLI's purpose. +# ruff: noqa: T201 + +import argparse +import textwrap +from collections import Counter +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import Path + +from termcolor import colored + +from renaissance.project.project_scanner import PythonScanner +from renaissance.recipes.type_parameter_check import MIN_SUPPORTED_PYTHON, TypeParameterCheck +from renaissance.recipes.type_parameter_rules import DOCS_BASE_URL, UNSAFE_RULES, UnsafeReason +from renaissance.utils.import_resolution import collect_project_imported_names +from renaissance.utils.ruff_utils import run_ruff_format, run_ruff_import_cleanup + +_MAJOR_MINOR_PART_COUNT = 2 +_TYPE_PARAM_KINDS = ("TypeVar", "ParamSpec", "TypeVarTuple") +_FIXED_DESCRIPTIONS = { + "converted": "converted to PEP 695 in functions", + "orphaned": "unused declaration removed", + "cross_file": "import replaced by a local declaration", +} +_MAX_LISTED_NAMES = 10 + + +@dataclass +class FileReport: + """Outcome of running TypeParameterCheck against a single file.""" + + path: Path + result: dict[str, dict[str, str]] | None + error: str | None + reasons: dict[str, dict[str, UnsafeReason]] | None = None + kinds: dict[str, str] | None = None + ignored: frozenset[str] = frozenset() + + +@dataclass +class KindCount: + """How many type parameters of one kind a run reported, and how many were fixed, need manual review or were ignored.""" + + reported: int = 0 + fixed: int = 0 + needs_review: int = 0 + ignored: int = 0 + + +@dataclass(frozen=True) +class LocalizationSettings: + """Whether imported type parameters are copied into the importing file, and the files they are never copied from.""" + + enabled: bool = True + ignored_files: frozenset[Path] = frozenset() + + +_DEFAULT_LOCALIZATION = LocalizationSettings() + + +def resolve_target_files(target: Path) -> list[Path]: + """Return the .py files to process for `target`. + + A single .py file is returned as-is; a directory is scanned recursively with PythonScanner over + the whole tree (no package_dirs allowlist), so any project layout works. + """ + if target.is_file(): + return [target] + return [Path(path) for path in PythonScanner(str(target)).find_sources()] + + +def _parse_py_version(text: str) -> tuple[int, int]: + """Parse a "MAJOR.MINOR" string into a (major, minor) tuple for argparse's type=. + + Raises: + argparse.ArgumentTypeError: If text is not in MAJOR.MINOR form or is below MIN_SUPPORTED_PYTHON. + + """ + parts = text.split(".") + if len(parts) != _MAJOR_MINOR_PART_COUNT or not all(part.isdigit() for part in parts): + message = f"expected MAJOR.MINOR (e.g. 3.12), got {text!r}" + raise argparse.ArgumentTypeError(message) + version = (int(parts[0]), int(parts[1])) + if version < MIN_SUPPORTED_PYTHON: + minimum = ".".join(map(str, MIN_SUPPORTED_PYTHON)) + message = f"expected {minimum} or higher, since PEP 695 syntax does not exist below it, got {text!r}" + raise argparse.ArgumentTypeError(message) + return version + + +def has_fixed(report: FileReport) -> bool: + """Return True if any phase of report.result fixed at least one name.""" + if report.result is None: + return False + return any("fixed" in phase.values() for phase in report.result.values()) + + +def has_unsafe(report: FileReport) -> bool: + """Return True if any phase of report.result gave at least one name a reason to review.""" + return any(report.reasons.values()) if report.reasons else False + + +def is_clean(report: FileReport) -> bool: + """Return True if report.result found no type parameter usage at all.""" + if report.result is None: + return False + return not any(phase for phase in report.result.values()) + + +def process_file( + path: Path, + *, + min_python: tuple[int, int], + project_root: Path, + project_wide_imported_names: frozenset[str], + localization: LocalizationSettings = _DEFAULT_LOCALIZATION, +) -> FileReport: + """Run TypeParameterCheck's three phases on one file and return its FileReport. + + Any exception is caught and stored on FileReport.error, so one bad file never aborts a batch run. + """ + try: + recipe = TypeParameterCheck(path) + recipe.min_python = min_python + recipe.project_root = project_root + recipe.project_wide_imported_names = project_wide_imported_names + recipe.ignore_localize_origins = localization.ignored_files + recipe.localize_imports = localization.enabled + result = recipe.check() + reasons = { + "cross_file": recipe.cross_file_unsafe_reasons, + "converted": recipe.converted_unsafe_reasons, + "orphaned": recipe.orphaned_unsafe_reasons, + } + except Exception as exc: # noqa: BLE001 - isolate one bad file, never abort the whole batch + return FileReport(path=path, result=None, error=f"{type(exc).__name__}: {exc}") + return FileReport( + path=path, + result=result, + error=None, + reasons=reasons, + kinds=recipe.type_param_kinds, + ignored=frozenset(recipe.ignored_names), + ) + + +def _merge_reports(first: FileReport, second: FileReport) -> FileReport: + """Return the report of a file processed twice: second's names replace first's, and first's other names stay. + + A reason from first is dropped when second evaluated the same name again. A second that errored is returned as it is. + """ + if second.error is not None or first.result is None or second.result is None: + return second + first_reasons = first.reasons or {} + second_reasons = second.reasons or {} + return FileReport( + path=first.path, + result={phase: {**names, **second.result.get(phase, {})} for phase, names in first.result.items()}, + error=None, + reasons={ + phase: {name: reason for name, reason in reasons.items() if name not in second.result.get(phase, {})} + | second_reasons.get(phase, {}) + for phase, reasons in first_reasons.items() + }, + kinds={**(first.kinds or {}), **(second.kinds or {})}, + ignored=first.ignored | second.ignored, + ) + + +def _run_second_pass( + reports: list[FileReport], + files: Sequence[Path], + project_root: Path, + min_python: tuple[int, int], + localization: LocalizationSettings, +) -> list[FileReport]: + """Process again the files that kept a declaration because another file imported it, and merge the two reports of each. + + The imported names are collected again, so a declaration whose importers were all localized in the first pass is + no longer protected. Every other report is returned as it was. + """ + candidates = { + report.path + for report in reports + if any(UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT in reasons.values() for reasons in (report.reasons or {}).values()) + } + if not candidates: + return reports + imported_names_by_file = collect_project_imported_names(files, project_root) + merged: list[FileReport] = [] + for report in reports: + if report.path not in candidates: + merged.append(report) + continue + second = process_file( + report.path, + min_python=min_python, + project_root=project_root, + project_wide_imported_names=imported_names_by_file.get(report.path, frozenset()), + localization=localization, + ) + merged.append(_merge_reports(report, second)) + print(f"File {report.path} checked again.") + return merged + + +def _files(count: int) -> str: + """Return count with "file" pluralized, e.g. "1 file" or "2 files".""" + return f"{count} file" if count == 1 else f"{count} files" + + +def _format_names(file_counts: Counter[str]) -> str: + """Return the names, most files first then alphabetical, each with its file count if above one. + + Lists at most _MAX_LISTED_NAMES names and summarises the rest as "and N more". + """ + ranked = sorted(file_counts.items(), key=lambda item: (-item[1], item[0])) + listed = ", ".join(name if count == 1 else f"{name} ({_files(count)})" for name, count in ranked[:_MAX_LISTED_NAMES]) + hidden = len(ranked) - _MAX_LISTED_NAMES + return f"{listed} and {hidden} more" if hidden > 0 else listed + + +def _format_commit_summary(reports: Sequence[FileReport], min_python: tuple[int, int]) -> str: + """Build the copy-pasteable commit message: what was done to which names, and what is left for manual review. + + Names are grouped by action with the number of files they were fixed in. The manual review line, + with the files needing it and their reasons by frequency, is left out when no file needs review. + """ + modified = [report for report in reports if has_fixed(report)] + names_by_action = {action: Counter[str]() for action in _FIXED_DESCRIPTIONS.values()} + for report in modified: + for name, actions in _fixed_descriptions(report).items(): + for action in actions: + names_by_action[action][name] += 1 + version = ".".join(map(str, min_python)) + lines = [ + f"Convert type parameters to PEP 695 syntax in {_files(len(modified))} (Python {version}+).", + "", + *(f"- {action}: {_format_names(file_counts)}" for action, file_counts in names_by_action.items() if file_counts), + ] + needs_review = sum(1 for report in reports if has_unsafe(report)) + if needs_review: + reasons = ", ".join(f"{UNSAFE_RULES[reason].message} ({count})" for reason, count in rank_reasons(reports)) + lines.extend(["", f"{_files(needs_review)} left for manual review: {reasons}."]) + return "\n".join(lines) + + +def _name_statuses(report: FileReport) -> dict[str, str]: + """Return {name: "fixed" | "unsafe"} over all phases of report.result; "unsafe" wins if any phase gave the name a reason.""" + statuses: dict[str, str] = {} + for phase in (report.result or {}).values(): + for name, status in phase.items(): + if statuses.get(name) != "unsafe": + statuses[name] = status + for phase_reasons in (report.reasons or {}).values(): + statuses.update(dict.fromkeys(phase_reasons, "unsafe")) + return statuses + + +def count_by_kind(reports: Sequence[FileReport]) -> dict[str, KindCount]: + """Count the type parameters the reports list, per kind (TypeVar, ParamSpec, TypeVarTuple). + + A name counts once per file, as needing review if any phase left it unsafe and as fixed otherwise. A name + the file imports from an ignored file counts as ignored. Files that errored are skipped. + """ + counts = {kind: KindCount() for kind in _TYPE_PARAM_KINDS} + for report in reports: + kinds = report.kinds or {} + for name in report.ignored: + counts[kinds[name]].reported += 1 + counts[kinds[name]].ignored += 1 + for name, status in _name_statuses(report).items(): + count = counts[kinds[name]] + count.reported += 1 + if status == "unsafe": + count.needs_review += 1 + else: + count.fixed += 1 + return counts + + +def rank_reasons(reports: Sequence[FileReport]) -> list[tuple[UnsafeReason, int]]: + """Return each UnsafeReason the reports hit with its count, most common first, ties in UnsafeReason order. + + Every (phase, name) reason counts, so a name unsafe for two reasons counts under both. + """ + counter = Counter(reason for report in reports for phase in (report.reasons or {}).values() for reason in phase.values()) + order = list(UnsafeReason) + return sorted(counter.items(), key=lambda item: (-item[1], order.index(item[0]))) + + +def _code(text: object) -> str: + """Return text as a Markdown inline code span.""" + return f"`{text}`" + + +def _heading(text: str) -> str: + """Return text as a bold Markdown line, used for the report's section headings.""" + return f"**{text}**" + + +def _table_row(cells: Sequence[str], widths: Sequence[int]) -> str: + """Return one Markdown table row with each cell padded to its column width.""" + return "| " + " | ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)) + " |" + + +def _format_table(headers: Sequence[str], rows: Sequence[Sequence[str]]) -> list[str]: + """Return a Markdown table as lines, with every column padded to its widest cell and the dash row to the same width.""" + widths = [max(len(cell) for cell in column) for column in zip(headers, *rows, strict=True)] + return [_table_row(headers, widths), _table_row(["-" * width for width in widths], widths), *(_table_row(row, widths) for row in rows)] + + +def _summary_tables(reports: Sequence[FileReport], *, show_ignored: bool) -> list[str]: + """Return the report's summary tables: type parameters per kind, then the unsafe reasons by frequency. + + The Ignored column is only there when show_ignored is True. The reasons table is left out when no reason was hit. + """ + headers = ["Kind", "Reported", "Fixed", "Needs review", *(["Ignored"] if show_ignored else [])] + kind_rows = [ + [_code(kind), str(c.reported), str(c.fixed), str(c.needs_review), *([str(c.ignored)] if show_ignored else [])] + for kind, c in count_by_kind(reports).items() + ] + lines = ["", _heading("TYPE PARAMETERS BY KIND"), "", *_format_table(headers, kind_rows)] + ranking = rank_reasons(reports) + if ranking: + reason_rows = [[UNSAFE_RULES[reason].message, str(count)] for reason, count in ranking] + lines.extend(["", _heading("UNSAFE REASONS, MOST COMMON FIRST"), "", *_format_table(["Reason", "Count"], reason_rows)]) + return lines + + +def _join_with_blank_lines(entries: list[list[str]]) -> list[str]: + """Flatten entries into one list of lines, with a blank line between consecutive entries.""" + lines: list[str] = [] + for index, entry in enumerate(entries): + if index: + lines.append("") + lines.extend(entry) + return lines + + +def _fixed_descriptions(report: FileReport) -> dict[str, list[str]]: + """Return {name: [what was done to it]} for the names report.result fixed. + + A converted name isn't described as removed, because its declaration is always removed afterwards, + and a removed name isn't described as localized. + """ + result = report.result or {} + converted = {name for name, status in result.get("converted", {}).items() if status == "fixed"} + removed = {name for name, status in result.get("orphaned", {}).items() if status == "fixed"} - converted + descriptions: dict[str, list[str]] = {} + for phase, names in result.items(): + for name, status in names.items(): + if status == "fixed" and not (phase == "orphaned" and name in converted) and not (phase == "cross_file" and name in removed): + descriptions.setdefault(name, []).append(_FIXED_DESCRIPTIONS[phase]) + return descriptions + + +def _modified_entry(report: FileReport, project_root: Path) -> list[str]: + """Return the MODIFIED lines for one file: its path relative to project_root, then what was done to each name.""" + lines = [f" {_code(report.path.relative_to(project_root))}"] + lines.extend(f" {_code(name)}: {', '.join(texts)}" for name, texts in _fixed_descriptions(report).items()) + return lines + + +def _needs_review_groups(reports: Sequence[FileReport], project_root: Path) -> list[list[str]]: + """Return one block of NEEDS MANUAL REVIEW lines per reason, most common reason first. + + A block is the reason as a heading, then each file it applies to with the names it applies to. + """ + names_by_reason: dict[UnsafeReason, dict[Path, list[str]]] = {} + for report in reports: + for phase_reasons in (report.reasons or {}).values(): + for name, reason in phase_reasons.items(): + names = names_by_reason.setdefault(reason, {}).setdefault(report.path, []) + if name not in names: + names.append(name) + groups: list[list[str]] = [] + for reason, _ in rank_reasons(reports): + message = UNSAFE_RULES[reason].message + files = names_by_reason[reason].items() + groups.append( + [ + _heading(message[:1].upper() + message[1:]), + *(f" {_code(path.relative_to(project_root))}: {', '.join(map(_code, names))}" for path, names in files), + ], + ) + return groups + + +def _format_console_report( + reports: list[FileReport], + *, + project_root: Path, + localization: LocalizationSettings, +) -> str: + """Build the full report as Markdown: MODIFIED / NEEDS MANUAL REVIEW / ERRORS sections. + + Files are listed by their path relative to project_root. MODIFIED lists each file with what was done to + its names, separated by a blank line; NEEDS MANUAL REVIEW groups the files and names by reason. Clean + files (no type parameter usage at all) are only counted, not listed, so the report shows what needs + attention. The IGNORED FILES section is only included when localization.ignored_files is not empty, + and the Ignored column when it is not empty or localization is disabled. + """ + modified = [report for report in reports if has_fixed(report)] + needs_review = [report for report in reports if has_unsafe(report)] + errors = [report for report in reports if report.error is not None] + clean_count = sum(1 for report in reports if is_clean(report)) + + lines = [ + "Renaissance TypeParameterCheck migration report", + ( + f"Processed {len(reports)} files: {len(modified)} modified, {len(needs_review)} need " + f"manual review, {clean_count} clean, {len(errors)} errors" + ), + ] + if localization.ignored_files: + lines.extend(["", _heading(f"IGNORED FILES ({len(localization.ignored_files)})")]) + lines.extend(f" {_code(ignored.relative_to(project_root))}" for ignored in sorted(localization.ignored_files)) + lines.extend(_summary_tables(reports, show_ignored=bool(localization.ignored_files) or not localization.enabled)) + lines.extend(["", _heading(f"MODIFIED ({len(modified)})")]) + lines.extend(_join_with_blank_lines([_modified_entry(report, project_root) for report in modified])) + + lines.extend(["", _heading(f"NEEDS MANUAL REVIEW ({len(needs_review)})")]) + if needs_review: + lines.extend([f" {DOCS_BASE_URL}", ""]) + lines.extend(_join_with_blank_lines(_needs_review_groups(needs_review, project_root))) + + lines.extend(["", _heading(f"ERRORS ({len(errors)})")]) + lines.extend(f" {_code(report.path.relative_to(project_root))}: {report.error}" for report in errors) + + return "\n".join(lines) + + +def build_arg_parser() -> argparse.ArgumentParser: + """Build the argument parser for this CLI's --help/usage text and flags.""" + parser = argparse.ArgumentParser( + prog="migration_type_recipes.py", + description="Modernize legacy TypeVar/ParamSpec/TypeVarTuple usage to PEP 695 syntax.", + epilog=textwrap.dedent("""\ + Examples: + python src/rejuvenation/migration_type_recipes.py ./repo --py 3.12 --report review.md --ignore folder/types.py + """), + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument("path", type=Path, help="A .py file or a directory to scan.") + parser.add_argument( + "--py", + type=_parse_py_version, + required=True, + metavar="MAJOR.MINOR", + help=("Minimum Python version the target project supports is 3.12."), + ) + parser.add_argument("--report", type=Path, metavar="PATH", help="Also write the full report to this file.") + parser.add_argument( + "--no-ruff", + action="store_true", + help=( + "Skip the final `ruff check --fix --select F401,I001` and `ruff format` pass that drops imports made" + " unused, sorts the rest and formats the files." + ), + ) + parser.add_argument( + "--ignore", + action="extend", + nargs="+", + type=Path, + default=[], + metavar="FILE", + help=( + "Files, relative to the scanned directory, that are left untouched; their type parameters are not copied " + "into the files importing them, which keep those imports. Give the path argument first, since this option " + "takes every value after it." + ), + ) + parser.add_argument( + "--no-localization", + action="store_true", + help=( + "Keep the imports of project type parameters instead of copying their declarations into the importing file; " + "the functions using them are not converted." + ), + ) + parser.add_argument( + "--second-pass", + action="store_true", + help=( + "Process again the files that kept a declaration only because another file imported it when the run started, " + "and report both passes together." + ), + ) + return parser + + +def _localization_settings(parser: argparse.ArgumentParser, args: argparse.Namespace, project_root: Path) -> LocalizationSettings: + """Build the LocalizationSettings from --no-localization and --ignore, the latter resolved against project_root. + + Calls parser.error, which exits with code 2, if an --ignore file does not exist. + """ + for ignored in args.ignore: + if not (project_root / ignored).is_file(): + parser.error(f"--ignore: file does not exist: {ignored}") + return LocalizationSettings( + enabled=not args.no_localization, + ignored_files=frozenset((project_root / ignored).resolve() for ignored in args.ignore), + ) + + +def main(argv: Sequence[str] | None = None) -> int: + """Parse arguments, run TypeParameterCheck across the target, print/save the report, return an exit code. + + Exit codes: 0 on normal completion (files needing manual review are informational, not a + failure), 2 on a usage error (bad path/argument), 3 if any file hit an unhandled exception. + """ + parser = build_arg_parser() + args = parser.parse_args(argv) + + target: Path = args.path + if not target.exists(): + parser.error(f"path does not exist: {target}") + if target.is_file() and target.suffix != ".py": + parser.error(f"not a Python file: {target}") + + # Absolute and normalized, so file paths match the keys collect_project_imported_names returns. + target = target.resolve() + files = resolve_target_files(target) + project_root = target if target.is_dir() else target.parent + imported_names_by_file = collect_project_imported_names(files, project_root) + + localization = _localization_settings(parser, args, project_root) + + reports: list[FileReport] = [] + for path in files: + if path in localization.ignored_files: + continue + project_wide_imported_names = imported_names_by_file.get(path, frozenset()) + report = process_file( + path, + min_python=args.py, + project_root=project_root, + project_wide_imported_names=project_wide_imported_names, + localization=localization, + ) + reports.append(report) + print(f"File {path} checked.") + + if args.second_pass: + reports = _run_second_pass(reports, files, project_root, args.py, localization) + + modified_paths = [report.path for report in reports if has_fixed(report)] + ruff_ran = bool(modified_paths) and not args.no_ruff + if ruff_ran: + run_ruff_import_cleanup(modified_paths) + run_ruff_format(modified_paths) + + console_report = _format_console_report( + reports, + project_root=project_root, + localization=localization, + ) + print(console_report) + if modified_paths: + print() + print(colored(_format_commit_summary(reports, args.py), "green", attrs=["bold"])) + + if args.report is not None: + args.report.write_text(console_report, encoding="utf-8") + + return 3 if any(report.error is not None for report in reports) else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/renaissance/recipes/step_runner.py b/src/renaissance/recipes/step_runner.py new file mode 100644 index 00000000..ce7a6d5a --- /dev/null +++ b/src/renaissance/recipes/step_runner.py @@ -0,0 +1,35 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""A small, generic way to sequence independently-committable fix actions across one or more recipes.""" + +from collections.abc import Callable, Sequence +from dataclasses import dataclass + +from renaissance.syntax_tree import ASTProcessor + + +@dataclass(frozen=True) +class Step: + """One independently-runnable, independently-committable fix action.""" + + label: str + recipe: ASTProcessor + action: Callable[[], dict[str, str]] + + +def run_steps(steps: Sequence[Step]) -> dict[str, dict[str, str]]: + """Run each step in order, committing its recipe if the step fixed anything. + + Returns {step.label: {name: "fixed" | "unsafe"}}, one entry per step, in step order. + """ + result: dict[str, dict[str, str]] = {} + for step in steps: + outcome = step.action() + if "fixed" in outcome.values(): + step.recipe.commit() + result[step.label] = outcome + return result diff --git a/src/renaissance/recipes/type_parameter_check.py b/src/renaissance/recipes/type_parameter_check.py new file mode 100644 index 00000000..a34dec09 --- /dev/null +++ b/src/renaissance/recipes/type_parameter_check.py @@ -0,0 +1,372 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Recipe that modernizes legacy TypeVar/ParamSpec/TypeVarTuple usage to PEP 695 syntax.""" + +from collections.abc import Sequence +from pathlib import Path +from typing import cast + +import libcst as cst + +from renaissance.integrations.python.ast.cst_node import PythonCstNode +from renaissance.recipes.python_refactoring import PythonRefactoring +from renaissance.recipes.step_runner import Step, run_steps +from renaissance.recipes.type_parameter_declarations import ( + declared_default, + find_type_param_declarations, + type_param_constructor_name, + type_param_text, + type_params_bracket, +) +from renaissance.recipes.type_parameter_localization import Localization, plan_localizations +from renaissance.recipes.type_parameter_rules import ( + UnsafeReason, + conversion_refusal, + is_safe_to_remove, + mark_unsafe, +) +from renaissance.recipes.type_parameter_uses import ( + generic_classes, + owning_function, + signature_function, + uses_of, +) +from renaissance.utils.cst_utils import names_read_by, trailing_of_sole_statement +from renaissance.utils.import_resolution import is_import_line + +MIN_SUPPORTED_PYTHON = (3, 12) + + +class TypeParameterCheck(PythonRefactoring[PythonCstNode]): + """Modernize legacy TypeVar/ParamSpec/TypeVarTuple usage in a Python file to PEP 695 syntax. + + Attributes: + min_python: Minimum Python version the target codebase supports (default 3.12, where PEP 695 syntax exists). + project_wide_imported_names: Names other project files import from this file; never removed. + project_root: Root that absolute imports resolve from; the file's own directory if None. + ignore_localize_origins: Absolute paths of files whose declarations are never copied into their importers. + localize_imports: When False, no declaration is copied into the files importing it. + type_param_kinds: Each declared name the phases see, mapped to "TypeVar", "ParamSpec" or "TypeVarTuple". + ignored_names: Type parameter names this file imports from an ignored origin, or from any project file when + localize_imports is False; they keep their import. + cross_file_unsafe_reasons: Why each "unsafe" name of the first phase was refused. + converted_unsafe_reasons: The same for the second phase; also holds "fixed" names a generic class still uses. + orphaned_unsafe_reasons: The same for the third phase. + + """ + + min_python: tuple[int, int] = MIN_SUPPORTED_PYTHON + project_wide_imported_names: frozenset[str] = frozenset() + project_root: Path | None = None + ignore_localize_origins: frozenset[Path] = frozenset() + localize_imports: bool = True + + def __init__(self, file: Path) -> None: + """Prepare the recipe for file, parsed with the CST backend.""" + super().__init__(file, PythonCstNode) + self.type_param_kinds: dict[str, str] = {} + self.ignored_names: set[str] = set() + + def run(self) -> None: + """Run check() and store its result on self.result.""" + self.result = self.check() + + def _cst_root(self) -> PythonCstNode: + """Return the root node of the file as it currently is (after the last commit).""" + return cast("PythonCstNode", cast("object", self.root)) + + def check(self) -> dict[str, dict[str, str]]: + """Run the three phases in order, committing each one that fixed something. + + localize_imported_typevars, then convert_declared_typevars, then + remove_orphaned_declarations. Returns {"cross_file": {...}, "converted": {...}, + "orphaned": {...}}, each mapping name -> "fixed" | "unsafe". + """ + return run_steps( + [ + Step("cross_file", self, self.localize_imported_typevars), + Step("converted", self, self.convert_declared_typevars), + Step("orphaned", self, self.remove_orphaned_declarations), + ], + ) + + def convert_declared_typevars(self) -> dict[str, str]: + """Add PEP 695 type parameters to the functions whose signatures use a module-level TypeVar/ParamSpec/TypeVarTuple. + + The declaration stays. Methods of a class generic over the name are left unchanged, and so is a function that + also uses a declaration conversion_refusal rejects, so no function mixes legacy and PEP 695 type parameters. + Bounds, constraints and `default=` are carried over, defaulted ones last. Returns {name: "fixed" | "unsafe"}, + with each UnsafeReason on self.converted_unsafe_reasons; a "fixed" name that a generic class still uses gets + USED_IN_GENERIC_CLASS there too. + """ + root = self._cst_root() + module = root.translation_unit.atu + + results: dict[str, str] = {} + self.converted_unsafe_reasons: dict[str, UnsafeReason] = {} + convertible, generic_classes_by_name, blocked = self._plan_conversions(root, module, results) + + # One self.replace() per function: a function using two converted names would otherwise get two + # rewrites of the same node, which the rewriter rejects as conflicting. + new_params: dict[int, tuple[PythonCstNode, list[tuple[str, bool]]]] = {} + for name, (param, candidates) in convertible.items(): + allowed = [function for function in candidates if id(function) not in blocked] + if not allowed: + mark_unsafe(results, self.converted_unsafe_reasons, name, UnsafeReason.SHARES_FUNCTION_WITH_UNCONVERTIBLE) + continue + for function in allowed: + new_params.setdefault(id(function), (function, []))[1].append(param) + results[name] = "fixed" + + for function, params in new_params.values(): + self._add_type_params(function, params) + + self._report_generic_class_leftovers(results, generic_classes_by_name) + return results + + def _plan_conversions( + self, root: PythonCstNode, module: cst.Module, results: dict[str, str] + ) -> tuple[dict[str, tuple[tuple[str, bool], list[PythonCstNode]]], dict[str, list[PythonCstNode]], set[int]]: + """Return what each declaration that can be converted needs, marking in results the ones that can't. + + Returns {name: ((type parameter text, has default), functions to convert)}, {name: classes generic over it} + and the ids of the functions a refused declaration also uses, which must stay unchanged. + """ + convertible: dict[str, tuple[tuple[str, bool], list[PythonCstNode]]] = {} + generic_classes_by_name: dict[str, list[PythonCstNode]] = {} + blocked: set[int] = set() + for name, decl_stmt in find_type_param_declarations(module).items(): + self.type_param_kinds[name] = type_param_constructor_name(decl_stmt) + uses = uses_of(root, name, decl_stmt) + signature_users = {id(function) for use in uses if (function := signature_function(use)) is not None} + owners = (owning_function(use, signature_users) for use in uses) + functions = list(dict.fromkeys(function for function in owners if function is not None)) + if not functions: + continue + + classes = generic_classes(uses) + # TODO(FrancescoPezzella): convert a class with the name in a base (`class Box(Base[T])`) to + # `class Box[T](Base[T])`, still refusing `covariant=`. + # https://github.com/TNO/Renaissance.Py/issues/279 + # Maybe ruff's UP046 could take over one day, if it handles bases other than `Generic`: + # https://github.com/astral-sh/ruff/issues/29481 + in_generic_class = [function for function in functions if self._is_inside(function, classes)] + candidates = [function for function in functions if function not in in_generic_class] + if not candidates: + if in_generic_class: + mark_unsafe(results, self.converted_unsafe_reasons, name, UnsafeReason.USED_IN_GENERIC_CLASS) + continue + reason = conversion_refusal(decl_stmt, self.min_python) + if reason is not None: + mark_unsafe(results, self.converted_unsafe_reasons, name, reason) + blocked.update(id(function) for function in candidates) + continue + + convertible[name] = ((type_param_text(decl_stmt, module), declared_default(decl_stmt) is not None), candidates) + generic_classes_by_name[name] = classes + return convertible, generic_classes_by_name, blocked + + def _report_generic_class_leftovers(self, results: dict[str, str], generic_classes_by_name: dict[str, list[PythonCstNode]]) -> None: + """Add USED_IN_GENERIC_CLASS to converted_unsafe_reasons for each "fixed" name a generic class still uses.""" + for name, classes in generic_classes_by_name.items(): + if classes and results.get(name) == "fixed": + self.converted_unsafe_reasons[name] = UnsafeReason.USED_IN_GENERIC_CLASS + + @staticmethod + def _is_inside(node: PythonCstNode, containers: Sequence[PythonCstNode]) -> bool: + """Return True if one of containers encloses node.""" + current = node.parent + while current is not None: + if any(current is container for container in containers): + return True + current = current.parent + return False + + def _add_type_params(self, function: PythonCstNode, params: list[tuple[str, bool]]) -> None: + """Add params, given as (text, has_default), to function's PEP 695 type parameters. + + A function without a bracket gets one after its name; an existing bracket is rebuilt with params after its own, + defaulted ones last, which drops its comments and line breaks. + """ + node = cast("cst.FunctionDef", function.node) + unit = function.translation_unit + existing = node.type_parameters + if existing is None: + self.replace( + node.name.value + type_params_bracket(params), unit.wrappers[node.name], include_whitespace=False, include_comments=False + ) + return + # TODO(FrancescoPezzella): rebuilding the existing bracket drops its comments and line breaks. + # https://github.com/TNO/Renaissance.Py/issues/279 + current = [ + (unit.atu.code_for_node(param.with_changes(comma=cst.MaybeSentinel.DEFAULT)), param.default is not None) + for param in existing.params + ] + self.replace(type_params_bracket([*current, *params]), unit.wrappers[existing], include_whitespace=False, include_comments=False) + + def remove_orphaned_declarations(self) -> dict[str, str]: + """Remove every module-level TypeVar/ParamSpec/TypeVarTuple declaration that nothing uses. + + A declaration that is_safe_to_remove rejects is kept and reported "unsafe", with its UnsafeReason on + self.orphaned_unsafe_reasons. Returns {name: "fixed" | "unsafe"}. + """ + root = self._cst_root() + module = root.translation_unit.atu + + results: dict[str, str] = {} + self.orphaned_unsafe_reasons: dict[str, UnsafeReason] = {} + for name, decl_stmt in find_type_param_declarations(module).items(): + self.type_param_kinds[name] = type_param_constructor_name(decl_stmt) + if uses_of(root, name, decl_stmt): + continue + + reason = is_safe_to_remove(module, name, self.project_wide_imported_names) + if reason is not None: + mark_unsafe(results, self.orphaned_unsafe_reasons, name, reason) + continue + + self._remove_declaration(decl_stmt) + results[name] = "fixed" + + return results + + def _remove_declaration(self, decl_stmt: cst.Assign) -> None: + """Remove decl_stmt's statement from the file, with the comment trailing it when it is alone on its line.""" + unit = self._cst_root().translation_unit + targets: list[cst.CSTNode] = [decl_stmt] + trailing = trailing_of_sole_statement(unit.atu, decl_stmt) + if trailing is not None and trailing.comment is not None: + targets.append(trailing.comment) + # TODO(FrancescoPezzella): enable once comment blocks get correctly deleted + # https://github.com/TNO/Renaissance.Py/issues/189 + self.remove([unit.wrappers[target] for target in targets], include_comments=False) + + def localize_imported_typevars(self) -> dict[str, str]: + """Replace imports of TypeVar/ParamSpec/TypeVarTuple names from the target project with local declarations. + + Imports are resolved against project_root. Where safe, the import is dropped and a copy of the origin's + declaration, plus the imports it needs, is added after the file's last import. Names from an + ignore_localize_origins file, or all names when localize_imports is False, keep their import and go to + self.ignored_names. Returns {name: "fixed" | "unsafe"}, with each UnsafeReason on + self.cross_file_unsafe_reasons. + + Raises: + SyntaxError: if an origin file can't be parsed. + + """ + project_root = self.project_root if self.project_root is not None else Path(self.filename).parent + plan = plan_localizations( + self._cst_root().translation_unit.atu, + Path(self.filename), + project_root, + localize_imports=self.localize_imports, + ignore_origins=self.ignore_localize_origins, + ) + self.cross_file_unsafe_reasons: dict[str, UnsafeReason] = plan.unsafe_reasons + self.type_param_kinds.update(plan.kinds) + self.ignored_names |= plan.ignored_names + self._place_localizations(plan.localizations) + return plan.results + + def _place_localizations(self, localizations: list[Localization]) -> None: + """Write the localizations: narrow or drop each import, then add the needed imports and declarations after the last one. + + Each needed import is added once, before the declarations; if every import is dropped, they take the place of the + last one. A localization is written in place of its import instead when that import shares a line with other + statements and would be dropped, when no import remains to go after, or when a statement in between reads one + of its names. + """ + body = self._cst_root().translation_unit.atu.body + wrappers = self._cst_root().translation_unit.wrappers + dropped = {id(item.line) for item in localizations if not item.kept} + anchor, replaces_anchor = self._anchor_index(body, dropped) + if anchor is None: + for item in localizations: + self._write_in_place(item) + return + + imports: list[str] = [] + declarations: list[str] = [] + anchor_raw: cst.ImportFrom | None = None + for item in localizations: + if not self._goes_after_last_import(item, body, anchor): + self._write_in_place(item) + continue + imports.extend(item.imports) + declarations.extend(item.declarations) + if replaces_anchor and item.line is body[anchor]: + anchor_raw = item.raw + elif item.kept: + self._narrow(item) + else: + self._drop(item) + + if not declarations: + return + text = "\n".join([*dict.fromkeys(imports), *declarations]) + if anchor_raw is not None: + self.replace(text, wrappers[anchor_raw], include_whitespace=False, include_comments=False) + else: + self.insert_after(f"\n{text}", wrappers[body[anchor]], include_whitespace=False, include_comments=True) + + def _anchor_index(self, body: Sequence[cst.BaseStatement], dropped: set[int]) -> tuple[int | None, bool]: + """Return the index in body of the statement that localized text goes after, and whether it replaces that statement. + + The anchor is the last import line that stays. If every import is dropped, it is the last dropped line holding + a single statement, whose place the text takes. The index is None when body has neither. + """ + anchor = max( + (index for index, statement in enumerate(body) if is_import_line(statement) and id(statement) not in dropped), + default=None, + ) + if anchor is not None: + return anchor, False + anchor = max( + ( + index + for index, statement in enumerate(body) + if id(statement) in dropped and isinstance(statement, cst.SimpleStatementLine) and len(statement.body) == 1 + ), + default=None, + ) + return anchor, True + + def _write_in_place(self, item: Localization) -> None: + """Replace the names of item where its import is: its needed imports and declarations follow the kept names, if any.""" + wrappers = self._cst_root().translation_unit.wrappers + text = "\n".join([*item.imports, *item.declarations]) + if not item.kept: + self.replace(text, wrappers[item.raw], include_whitespace=False, include_comments=False) + return + self._narrow(item) + self.insert_after(f"\n{text}", wrappers[item.line], include_whitespace=False, include_comments=True) + + def _drop(self, item: Localization) -> None: + """Remove the import of item, with the comment trailing it, from its line.""" + wrappers = self._cst_root().translation_unit.wrappers + targets = [wrappers[item.raw]] + comment = item.line.trailing_whitespace.comment + if comment is not None: + targets.append(wrappers[comment]) + self.remove(targets, include_comments=False) + + def _narrow(self, item: Localization) -> None: + """Replace the import of item with one that only imports its kept names, keeping its comments.""" + module = self._cst_root().translation_unit.atu + narrowed = module.code_for_node(item.raw.with_changes(names=item.kept, semicolon=cst.MaybeSentinel.DEFAULT)) + self.replace(narrowed, self._cst_root().translation_unit.wrappers[item.raw], include_whitespace=False, include_comments=False) + + def _goes_after_last_import(self, item: Localization, body: Sequence[cst.BaseStatement], anchor: int) -> bool: + """Return True if item can be written after body[anchor], the last import that stays. + + It can't if its import would be dropped from a line with other statements, or if a non-import statement between + that import and the anchor reads one of its names. + """ + if not item.kept and len(item.line.body) != 1: + return False + index = next(position for position, statement in enumerate(body) if statement is item.line) + return not any(not is_import_line(statement) and item.names & names_read_by(statement) for statement in body[index + 1 : anchor]) diff --git a/src/renaissance/recipes/type_parameter_declarations.py b/src/renaissance/recipes/type_parameter_declarations.py new file mode 100644 index 00000000..88992444 --- /dev/null +++ b/src/renaissance/recipes/type_parameter_declarations.py @@ -0,0 +1,156 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Read and rewrite the declaration of a TypeVar/ParamSpec/TypeVarTuple: `NAME = TypeVar(...)`.""" + +import builtins +from collections.abc import Iterable +from typing import cast + +import libcst as cst + +from renaissance.utils.cst_utils import module_statements, names_read_by, trailing_of_sole_statement + +_CONSTRUCTORS = frozenset({"TypeVar", "ParamSpec", "TypeVarTuple"}) + + +def _is_type_param_call(value: cst.BaseExpression) -> bool: + """Return True if `value` is a call to TypeVar/ParamSpec/TypeVarTuple.""" + return isinstance(value, cst.Call) and isinstance(value.func, cst.Name) and value.func.value in _CONSTRUCTORS + + +def find_type_param_declarations(module: cst.Module) -> dict[str, cst.Assign]: + """Find every module-level "NAME = TypeVar/ParamSpec/TypeVarTuple(...)" declaration.""" + declarations: dict[str, cst.Assign] = {} + for statement in module_statements(module): + if isinstance(statement, cst.Assign) and _is_type_param_call(statement.value): + for target in statement.targets: + if isinstance(target.target, cst.Name): + declarations[target.target.value] = statement + return declarations + + +def declaration_text(module: cst.Module, decl_stmt: cst.Assign) -> str: + """Return decl_stmt as written in module, without its `;`. + + The comment trailing its line follows it when it is the only statement there, since on a shared + line the comment may belong to another statement. + """ + text = module.code_for_node(decl_stmt.with_changes(semicolon=cst.MaybeSentinel.DEFAULT)) + trailing = trailing_of_sole_statement(module, decl_stmt) + if trailing is not None and trailing.comment is not None: + text += module.code_for_node(trailing.whitespace) + module.code_for_node(trailing.comment) + return text + + +def _call(decl_stmt: cst.Assign) -> cst.Call: + """Return the TypeVar/ParamSpec/TypeVarTuple call a declaration assigns.""" + return cast("cst.Call", decl_stmt.value) + + +def _positional_arguments(call: cst.Call) -> list[cst.Arg]: + """Return the call's positional arguments, `*args` included.""" + return [argument for argument in call.args if argument.keyword is None and argument.star != "**"] + + +def _keyword_value(call: cst.Call, keyword: str) -> cst.BaseExpression | None: + """Return the value the call passes as `keyword=`, or None if it passes none.""" + return next((argument.value for argument in call.args if argument.keyword is not None and argument.keyword.value == keyword), None) + + +def type_param_constructor_name(decl_stmt: cst.Assign) -> str: + """Return the name of the call a declaration uses, e.g. "TypeVar" for `T = TypeVar("T")`.""" + return cast("cst.Name", _call(decl_stmt).func).value + + +def declaration_argument_names(decl_stmt: cst.Assign) -> set[str]: + """Return the non-builtin names a declaration's arguments use, e.g. {"Shape"} for `TypeVar("T", bound=Shape)`. + + The first positional argument (the type parameter's own name) is skipped; a string argument is read as a + forward reference (`bound="Shape"`). A string that isn't a valid expression contributes no names. + """ + call = _call(decl_stmt) + keyword_arguments = [argument for argument in call.args if argument.keyword is not None or argument.star == "**"] + names: set[str] = set() + for argument in [*_positional_arguments(call)[1:], *keyword_arguments]: + expression = argument.value + if isinstance(expression, cst.SimpleString | cst.ConcatenatedString) and isinstance(text := expression.evaluated_value, str): + try: + expression = cst.parse_expression(text) + except cst.ParserSyntaxError: + continue + names |= names_read_by(expression) + return names - set(dir(builtins)) + + +# Variance needs no syntax of its own: a PEP 695 type parameter always infers it, and a function's has none. +_CONVERTIBLE_KEYWORDS: dict[str, frozenset[str]] = { + "TypeVar": frozenset({"bound", "default", "infer_variance", "covariant", "contravariant"}), + "ParamSpec": frozenset({"default", "infer_variance", "covariant", "contravariant"}), + # (Py 3.15) TypeVarTuple now accepts bound, covariant, contravariant, and infer_variance keyword arguments, + # matching the interface of TypeVar and ParamSpec. bound semantics remain undefined in the specification. + "TypeVarTuple": frozenset({"default", "infer_variance"}), +} + + +def declared_default(decl_stmt: cst.Assign) -> cst.BaseExpression | None: + """Return the value a legacy declaration passes as `default=` (PEP 696), or None if it passes none.""" + return _keyword_value(_call(decl_stmt), "default") + + +def has_unconvertible_argument(decl_stmt: cst.Assign) -> bool: + """Return True if a legacy declaration passes a keyword type_param_text can't carry over. + + That is any keyword outside the constructor's known set, e.g. `bound=` on a ParamSpec or TypeVarTuple, + a keyword added by a later Python version, or a `**mapping`. + """ + convertible = _CONVERTIBLE_KEYWORDS[type_param_constructor_name(decl_stmt)] + return any( + argument.star == "**" or (argument.keyword is not None and argument.keyword.value not in convertible) + for argument in _call(decl_stmt).args + ) + + +def type_param_text(decl_stmt: cst.Assign, module: cst.Module) -> str: + """Return the PEP 695 type parameter equivalent to a legacy declaration from `module`, in its original formatting. + + E.g. `T = TypeVar("T", bound=int, default=int)` gives `T: int = int`. Bounds, constraints and + `default=` are carried over; `infer_variance=`, `covariant=` and `contravariant=` are dropped. Callers must + first reject declarations for which has_unconvertible_argument() is True. + """ + call = _call(decl_stmt) + constructor = type_param_constructor_name(decl_stmt) + positional = _positional_arguments(call) + name = cast("str", cast("cst.SimpleString", positional[0].value).evaluated_value) + + if constructor == "ParamSpec": + text = f"**{name}" + elif constructor == "TypeVarTuple": + text = f"*{name}" + else: + text = name + bound = _keyword_value(call, "bound") + constraints = [f"{argument.star}{module.code_for_node(argument.value)}" for argument in positional[1:]] + if bound is not None: + text += f": {module.code_for_node(bound)}" + elif constraints: + # A single constraint keeps a trailing comma, or the parentheses would make it a bound. + text += f": ({', '.join(constraints)}{',' if len(constraints) == 1 else ''})" + + default = declared_default(decl_stmt) + if default is not None: + text += f" = {module.code_for_node(default)}" + return text + + +def type_params_bracket(params: Iterable[tuple[str, bool]]) -> str: + """Return the `[...]` bracket for type parameters given as (text, has_default), with the defaulted ones last. + + The relative order within each group is kept. A non-default type parameter following a defaulted + one is a SyntaxError. + """ + ordered = sorted(params, key=lambda param: param[1]) + return f"[{', '.join(text for text, _ in ordered)}]" diff --git a/src/renaissance/recipes/type_parameter_localization.py b/src/renaissance/recipes/type_parameter_localization.py new file mode 100644 index 00000000..682141bb --- /dev/null +++ b/src/renaissance/recipes/type_parameter_localization.py @@ -0,0 +1,281 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Decide which TypeVar/ParamSpec/TypeVarTuple names a file imports from its project get a local declaration instead.""" + +from collections.abc import Sequence +from dataclasses import dataclass, field +from pathlib import Path +from typing import cast + +import libcst as cst + +from renaissance.recipes.type_parameter_declarations import ( + declaration_argument_names, + declaration_text, + find_type_param_declarations, + type_param_constructor_name, +) +from renaissance.recipes.type_parameter_rules import UnsafeReason, is_safe_to_localize, mark_unsafe +from renaissance.utils.cst_utils import module_statements +from renaissance.utils.import_resolution import ( + from_import_binding, + from_import_sources, + import_from_module, + import_from_text, + imported_name, + rebase_relative_module, + resolve_project_module, +) + + +@dataclass(frozen=True) +class Localization: + """The type parameter names one import statement brings in that are localized, and the text that replaces them.""" + + line: cst.SimpleStatementLine + raw: cst.ImportFrom + names: frozenset[str] + kept: list[cst.ImportAlias] + imports: list[str] + declarations: list[str] + + +@dataclass +class LocalizationPlan: + """What plan_localizations decides for one file. + + Attributes: + localizations: What to write for each import that has names to localize. + results: Each name seen, mapped to "fixed" or "unsafe". + unsafe_reasons: Why each "unsafe" name was refused. + kinds: Each name seen, mapped to "TypeVar", "ParamSpec" or "TypeVarTuple". + ignored_names: The names kept as imports because their origin is ignored or localization is off. + + """ + + localizations: list[Localization] = field(default_factory=list[Localization]) + results: dict[str, str] = field(default_factory=dict[str, str]) + unsafe_reasons: dict[str, UnsafeReason] = field(default_factory=dict[str, UnsafeReason]) + kinds: dict[str, str] = field(default_factory=dict[str, str]) + ignored_names: set[str] = field(default_factory=set[str]) + + +def plan_localizations( + module: cst.Module, + filename: Path, + project_root: Path, + *, + localize_imports: bool, + ignore_origins: frozenset[Path], +) -> LocalizationPlan: + """Return the plan deciding which type parameters imported by module's `from X import ...` statements to localize. + + X is resolved against project_root from filename; only imports that resolve to a project file are considered. + Names from a file in ignore_origins, or from any file when localize_imports is False, keep their import and go to + the plan's ignored_names. `import *` and aliased names are skipped. + + Raises: + SyntaxError: if an origin file can't be parsed, with its path as the filename. + + """ + return _Planner(module, filename, project_root, localize_imports, ignore_origins).run() + + +@dataclass(frozen=True) +class _Planner: + """Decide the localizations of one file, whose parsed module is `module` and path is `filename`.""" + + module: cst.Module + filename: Path + project_root: Path + localize_imports: bool + ignore_origins: frozenset[Path] + + def run(self) -> LocalizationPlan: + """Return the plan for every project import of the module.""" + plan = LocalizationPlan() + for line in self.module.body: + if not isinstance(line, cst.SimpleStatementLine): + continue + for statement in line.body: + if not isinstance(statement, cst.ImportFrom): + continue + origin_path = resolve_project_module( + self.filename, self.project_root, import_from_module(statement), len(statement.relative) + ) + if origin_path is None: + continue + self._localize_names_from(plan, line, statement, origin_path) + return plan + + def _localize_names_from(self, plan: LocalizationPlan, line: cst.SimpleStatementLine, raw: cst.ImportFrom, origin_path: Path) -> None: + """Queue the localization of every safe type parameter name that raw, on line, imports from the origin_path file. + + Records each name in the plan's results as "fixed" or, via mark_unsafe, as "unsafe", its kind in kinds, and + appends what to write for the "fixed" ones to localizations. + """ + if isinstance(raw.names, cst.ImportStar): + return + origin = _parse_origin(origin_path) + declarations = find_type_param_declarations(origin) + localized: dict[str, cst.Assign] = {} + needed_imports: list[str] = [] + + for alias in raw.names: + name = alias.evaluated_name + if alias.asname is not None or name not in declarations: + continue + + decl_stmt = declarations[name] + plan.kinds[name] = type_param_constructor_name(decl_stmt) + if not self.localize_imports or origin_path in self.ignore_origins: + plan.ignored_names.add(name) + continue + reason = is_safe_to_localize(origin, name) + if reason is not None: + mark_unsafe(plan.results, plan.unsafe_reasons, name, reason) + continue + argument_imports = self._argument_imports(origin, origin_path, raw, decl_stmt) + if argument_imports is None: + mark_unsafe(plan.results, plan.unsafe_reasons, name, UnsafeReason.NAME_NOT_AT_MODULE_LEVEL) + continue + + constructor_import = self._missing_constructor_import(origin, origin_path, raw, decl_stmt) + for needed in [constructor_import, *argument_imports]: + if needed is not None and needed not in needed_imports: + needed_imports.append(needed) + localized[name] = decl_stmt + plan.results[name] = "fixed" + + if not localized: + return + plan.localizations.append(_localization(line, raw, origin, localized, needed_imports)) + + def _missing_constructor_import( + self, + origin: cst.Module, + origin_path: Path, + via: cst.ImportFrom, + decl_stmt: cst.Assign, + ) -> str | None: + """Return the "from module import Ctor" line the localized declaration needs, or None if it isn't needed. + + None when this file already imports the constructor from that module, or the origin doesn't import it. The + module is the one the origin imports the constructor from, as named from this file, which reaches origin_path + through via; if this file can't name it, the origin module itself. + """ + ctor_name = type_param_constructor_name(decl_stmt) + ctor_source = from_import_sources(origin).get(ctor_name) + if ctor_source is None: + return None + + via_module = import_from_module(via) + ctor_module = rebase_relative_module(via_module, len(via.relative), origin_path, ctor_source[0] or None, ctor_source[1]) + ctor_module = ctor_module or import_from_text(via) + + # TODO(FrancescoPezzella): the constructor already imported here from another module (e.g. `typing` vs the origin's + # `typing_extensions`) isn't matched, so a second, shadowing import of the same name is added. + # https://github.com/TNO/Renaissance.Py/issues/279 + for statement in module_statements(self.module): + if ( + isinstance(statement, cst.ImportFrom) + and not isinstance(statement.names, cst.ImportStar) + and import_from_text(statement) == ctor_module + and any(imported_name(alias) == ctor_name for alias in statement.names) + ): + return None + + return f"from {ctor_module} import {ctor_name}" + + def _argument_imports( + self, + origin: cst.Module, + origin_path: Path, + raw: cst.ImportFrom, + decl_stmt: cst.Assign, + ) -> list[str] | None: + """Return the import lines, sorted by name, for the names decl_stmt's arguments use that this file doesn't import yet. + + A name already imported from the origin module, or from the module the origin gets it from, needs no line. + Returns None if a name isn't available as the same object at module level: the origin doesn't bind it at + runtime (e.g. only under `if TYPE_CHECKING:`), or this file binds it to something else. + """ + origin_sources = from_import_sources(origin) + importing_sources = from_import_sources(self.module) + imports: list[str] = [] + for name in sorted(declaration_argument_names(decl_stmt)): + if name not in origin_sources: + return None + if name not in importing_sources: + imports.append(_argument_import(origin, origin_path, raw, name)) + continue + origin_source = origin_sources[name] + accepted = {(import_from_module(raw) or "", len(raw.relative))} + if origin_source is not None and origin_source[1] == 0: + accepted.add(origin_source) + if importing_sources[name] not in accepted: + return None + return imports + + +def _parse_origin(origin_path: Path) -> cst.Module: + """Return the parsed origin_path file. + + Raises: + SyntaxError: if the file can't be parsed, with origin_path as its filename. + + """ + try: + return cst.parse_module(origin_path.read_text(encoding="utf-8")) + except cst.ParserSyntaxError as error: + raise SyntaxError(error.message, (str(origin_path), error.raw_line, error.raw_column + 1, None)) from error + + +def _argument_import(origin: cst.Module, origin_path: Path, via: cst.ImportFrom, name: str) -> str: + """Return the import line for name, which a localized declaration uses and the origin binds at module level. + + A name the origin imports is imported from the module it gets it from, as named from this file, which + reaches the origin (origin_path) through via. Any other name, or one this file can't name the module + of, is imported from the origin module itself. + """ + binding = from_import_binding(origin, name) + if binding is not None: + statement, alias = binding + module = rebase_relative_module( + import_from_module(via), + len(via.relative), + origin_path, + import_from_module(statement) or None, + len(statement.relative), + ) + if module is not None: + imported = alias.evaluated_name + return f"from {module} import {name if imported == name else f'{imported} as {name}'}" + return f"from {import_from_text(via)} import {name}" + + +def _localization( + line: cst.SimpleStatementLine, + raw: cst.ImportFrom, + origin: cst.Module, + declarations: dict[str, cst.Assign], + needed_imports: list[str], +) -> Localization: + """Return what replaces the names of declarations that the import raw, on line, brings in. + + The declarations keep their original formatting and the comment trailing them. needed_imports are the + imports they need (their constructors, names their arguments use) that this file doesn't have yet. The + kept aliases are the names raw imports that are not localized. + """ + kept = [alias for alias in cast("Sequence[cst.ImportAlias]", raw.names) if imported_name(alias) not in declarations] + if kept and raw.rpar is None: + # Without parentheses, a trailing comma after the last name is a syntax error. + kept[-1] = kept[-1].with_changes(comma=cst.MaybeSentinel.DEFAULT) + # TODO(FrancescoPezzella): the comment lines above a declaration are not copied: the rewriter only handles a one-line comment. + # https://github.com/TNO/Renaissance.Py/issues/189 + texts = [declaration_text(origin, decl) for decl in declarations.values()] + return Localization(line, raw, frozenset(declarations), kept, needed_imports, texts) diff --git a/src/renaissance/recipes/type_parameter_rules.py b/src/renaissance/recipes/type_parameter_rules.py new file mode 100644 index 00000000..13747029 --- /dev/null +++ b/src/renaissance/recipes/type_parameter_rules.py @@ -0,0 +1,175 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""The reasons a TypeVar/ParamSpec/TypeVarTuple is left unconverted, and the rules that decide them.""" + +from collections.abc import Callable, Iterable +from dataclasses import dataclass +from enum import StrEnum + +import libcst as cst +from libcst.codemod import CodemodContext +from libcst.codemod.visitors import GatherExportsVisitor + +from renaissance.recipes.type_parameter_declarations import ( + declared_default, + find_type_param_declarations, + has_unconvertible_argument, + type_param_constructor_name, +) +from renaissance.utils.import_resolution import imports_name_conditionally + +PEP_696_MINIMUM = (3, 13) +DOCS_BASE_URL = "https://tno.github.io/Renaissance.Py/user/features/type-parameter-modernization/" + + +class UnsafeReason(StrEnum): + """Every distinct, permanent reason a TypeVar/ParamSpec/TypeVarTuple candidate is left unconverted. + + Each member has a matching documented rule under DOCS_BASE_URL - see UNSAFE_RULES. + """ + + PEP696_VERSION_GATE = "pep696_version_gate" + NO_PEP695_EQUIVALENT = "no_pep695_equivalent" + DECLARED_TYPEVAR_EXPORTED = "declared_typevar_exported" + USED_IN_GENERIC_CLASS = "used_in_generic_class" + SHARES_FUNCTION_WITH_UNCONVERTIBLE = "shares_function_with_unconvertible" + NAME_NOT_AT_MODULE_LEVEL = "name_not_at_module_level" + IMPORTED_ELSEWHERE_IN_PROJECT = "imported_elsewhere_in_project" + ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY = "origin_imports_constructor_conditionally" + + +@dataclass(frozen=True) +class UnsafeRule: + """A short human-readable explanation for one UnsafeReason.""" + + message: str + + +UNSAFE_RULES: dict[UnsafeReason, UnsafeRule] = { + UnsafeReason.PEP696_VERSION_GATE: UnsafeRule( + "declares a default= (PEP 696), which needs the target's minimum Python to be 3.13+", + ), + UnsafeReason.NO_PEP695_EQUIVALENT: UnsafeRule( + "declaration passes an argument with no PEP 695 equivalent, e.g. bound= on a ParamSpec or a **mapping", + ), + UnsafeReason.DECLARED_TYPEVAR_EXPORTED: UnsafeRule( + "exported via __all__", + ), + UnsafeReason.USED_IN_GENERIC_CLASS: UnsafeRule( + "only used inside a class that is generic over it", + ), + UnsafeReason.SHARES_FUNCTION_WITH_UNCONVERTIBLE: UnsafeRule( + "every function using it also uses a type variable that can't be converted", + ), + UnsafeReason.NAME_NOT_AT_MODULE_LEVEL: UnsafeRule( + "declaration uses a name that isn't available at module level as the same object", + ), + UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT: UnsafeRule( + "imported directly by another file in the target project", + ), + UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY: UnsafeRule( + "origin module imports TypeVar/ParamSpec/TypeVarTuple inside an if/try block, e.g. under TYPE_CHECKING or per Python version", + ), +} + + +@dataclass(frozen=True) +class RefusalRule[**P]: + """One reason to refuse a type parameter, and the check that decides whether it applies. + + Attributes: + reason: What the report says when the check refuses. + refuses: Return True if the reason applies to the arguments the rule's stage passes. + + """ + + reason: UnsafeReason + refuses: Callable[P, bool] + + +def _first_refusal[**P](rules: Iterable[RefusalRule[P]], *args: P.args, **kwargs: P.kwargs) -> UnsafeReason | None: + """Return the reason of the first rule that refuses, or None if none does.""" + return next((rule.reason for rule in rules if rule.refuses(*args, **kwargs)), None) + + +def _has_no_pep695_equivalent(declaration: cst.Assign, _min_python: tuple[int, int]) -> bool: + """Return True if the declaration passes an argument that PEP 695 syntax can't carry over.""" + return has_unconvertible_argument(declaration) + + +def _needs_newer_python(declaration: cst.Assign, min_python: tuple[int, int]) -> bool: + """Return True if the declaration has a `default=` (PEP 696) and min_python is below the 3.13 that introduced it.""" + return declared_default(declaration) is not None and min_python < PEP_696_MINIMUM + + +def _is_exported(module: cst.Module, name: str, _project_wide_imported_names: frozenset[str]) -> bool: + """Return True if the module lists `name` in `__all__`, assigned, annotated or extended with `+=`. + + `__all__.extend(...)` and `__all__.append(...)` are not read. + """ + exports = GatherExportsVisitor(CodemodContext()) + module.visit(exports) + return name in exports.explicit_exported_objects + + +def _is_imported_elsewhere(_module: cst.Module, name: str, project_wide_imported_names: frozenset[str]) -> bool: + """Return True if another file in the target project imports `name` directly, regardless of `__all__`.""" + return name in project_wide_imported_names + + +def _origin_imports_constructor_conditionally(origin: cst.Module, name: str) -> bool: + """Return True if the origin imports the constructor of `name` inside a module-level `if`/`try` block. + + Which implementation the constructor binds then depends on the runtime or the type checker, e.g. + `typing_extensions` below 3.13 or under `TYPE_CHECKING`. + """ + declaration = find_type_param_declarations(origin).get(name) + return declaration is not None and imports_name_conditionally(origin, type_param_constructor_name(declaration)) + + +# Each stage reports the first rule that refuses, so the order is the order of precedence. Commenting out +# a rule switches it off; the rules a pipeline step enforces by its own structure are not listed here +# (USED_IN_GENERIC_CLASS, SHARES_FUNCTION_WITH_UNCONVERTIBLE, NAME_NOT_AT_MODULE_LEVEL). +CONVERSION_RULES: tuple[RefusalRule[[cst.Assign, tuple[int, int]]], ...] = ( + RefusalRule(UnsafeReason.NO_PEP695_EQUIVALENT, _has_no_pep695_equivalent), + RefusalRule(UnsafeReason.PEP696_VERSION_GATE, _needs_newer_python), +) +REMOVAL_RULES: tuple[RefusalRule[[cst.Module, str, frozenset[str]]], ...] = ( + RefusalRule(UnsafeReason.DECLARED_TYPEVAR_EXPORTED, _is_exported), + RefusalRule(UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT, _is_imported_elsewhere), +) +LOCALIZATION_RULES: tuple[RefusalRule[[cst.Module, str]], ...] = ( + RefusalRule(UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY, _origin_imports_constructor_conditionally), +) + + +def conversion_refusal(declaration: cst.Assign, min_python: tuple[int, int]) -> UnsafeReason | None: + """Return None if `declaration` can become a PEP 695 type parameter on a target with `min_python`, else why not.""" + return _first_refusal(CONVERSION_RULES, declaration, min_python) + + +def is_safe_to_localize(origin: cst.Module, name: str) -> UnsafeReason | None: + """Return None if `name` is safe to duplicate as a local declaration, else the reason it isn't.""" + return _first_refusal(LOCALIZATION_RULES, origin, name) + + +def is_safe_to_remove( + module: cst.Module, + name: str, + project_wide_imported_names: frozenset[str] = frozenset(), +) -> UnsafeReason | None: + """Return None if this file's own binding of `name` may be removed or replaced, else the reason it may not. + + `project_wide_imported_names` holds the names other files of the target project import from this one. + """ + return _first_refusal(REMOVAL_RULES, module, name, project_wide_imported_names) + + +def mark_unsafe(results: dict[str, str], reasons: dict[str, UnsafeReason], name: str, reason: UnsafeReason) -> None: + """Record `name` as unsafe with `reason` in both `results` (status) and `reasons` (why).""" + results[name] = "unsafe" + reasons[name] = reason diff --git a/src/renaissance/recipes/type_parameter_uses.py b/src/renaissance/recipes/type_parameter_uses.py new file mode 100644 index 00000000..7abb2631 --- /dev/null +++ b/src/renaissance/recipes/type_parameter_uses.py @@ -0,0 +1,67 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Find where a module-level TypeVar/ParamSpec/TypeVarTuple name is used, with libcst's scope analysis.""" + +from collections.abc import Iterable + +import libcst as cst + +from renaissance.integrations.python.ast.cst_node import PythonCstNode + + +def uses_of(root: PythonCstNode, name: str, decl_stmt: cst.Assign) -> list[PythonCstNode]: + """Return the nodes that use the module-level `name` that decl_stmt declares, resolved with libcst's scope analysis. + + A use inside a string annotation or `cast("T", x)` counts; a `T` that a PEP 695 type parameter of an + enclosing function or class (`def f[T]`, `class C[T]`) declares does not. + """ + target = next(target.target for target in decl_stmt.targets if isinstance(target.target, cst.Name) and target.target.value == name) + return [reference.node for reference in root.translation_unit.wrappers[target].referenced_by] + + +def signature_function(use: PythonCstNode) -> PythonCstNode | None: + """Return the function whose parameter or return annotation contains use, or None if no signature does. + + A use in a function's body, a parameter default or a decorator is in no signature. + """ + in_annotation = False + child, current = use, use.parent + while current is not None: + if isinstance(current.node, cst.Annotation): + in_annotation = True + elif isinstance(current.node, cst.FunctionDef): + return current if in_annotation and child.node is not current.node.body else None + child, current = current, current.parent + return None + + +def owning_function(node: PythonCstNode, signature_users: set[int]) -> PythonCstNode | None: + """Return the outermost function enclosing node whose id is in signature_users, or None if there is none.""" + owner = None + current = node.parent + while current is not None: + if isinstance(current.node, cst.FunctionDef) and id(current) in signature_users: + owner = current + current = current.parent + return owner + + +def generic_classes(uses: Iterable[PythonCstNode]) -> list[PythonCstNode]: + """Return the classes that have one of `uses` in a base (`Generic[T]`, `Protocol[T]`, `Base[int, T]`). + + Such a class is generic over the used name. + """ + classes: list[PythonCstNode] = [] + for use in uses: + child, current = use, use.parent + while current is not None: + if isinstance(current.node, cst.ClassDef) and any(child.node is base for base in current.node.bases): + if current not in classes: + classes.append(current) + break + child, current = current, current.parent + return classes diff --git a/src/renaissance/recipes/unit_to_pytest.py b/src/renaissance/recipes/unit_to_pytest.py index 134b887f..1d226578 100644 --- a/src/renaissance/recipes/unit_to_pytest.py +++ b/src/renaissance/recipes/unit_to_pytest.py @@ -227,6 +227,8 @@ def restructure_module(self) -> None: # file does not contain any test class, create a new class and add function in class cls = f"class {self.convert_file_to_test_class()}:\n" for fun in funs: + # TODO(FrancescoPezzella): textwrap.indent also indents the lines inside a multi-line string, changing its value. + # https://github.com/TNO/Renaissance.Py/issues/283 cls += textwrap.indent(convert_function(fun), " ") self.remove([fun]) self.insert_before(cls, funs[0]) diff --git a/src/renaissance/utils/import_resolution.py b/src/renaissance/utils/import_resolution.py new file mode 100644 index 00000000..e131d426 --- /dev/null +++ b/src/renaissance/utils/import_resolution.py @@ -0,0 +1,244 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Read the import statements of a module, and resolve project-internal `from X import Y` to the .py file they import from.""" + +from collections.abc import Sequence +from pathlib import Path +from typing import cast + +import libcst as cst +from libcst import matchers +from libcst.helpers import get_full_name_for_node + +from renaissance.utils.cst_utils import assigned_names, module_statements + + +def import_from_module(statement: cst.ImportFrom) -> str | None: + """Return the dotted module an import-from names after its leading dots, or None for a bare `from . import x`.""" + return get_full_name_for_node(statement.module) if statement.module is not None else None + + +def imported_name(alias: cst.ImportAlias) -> str: + """Return the name an import alias binds: its `as` name if it has one, else the imported name.""" + return alias.evaluated_alias or alias.evaluated_name + + +def import_from_text(statement: cst.ImportFrom) -> str: + """Return what follows `from` in an import-from: its leading dots and dotted module, e.g. "..pkg.mod".""" + return "." * len(statement.relative) + (import_from_module(statement) or "") + + +def is_import_line(statement: cst.BaseStatement) -> bool: + """Return True if statement is a line made only of import statements.""" + return isinstance(statement, cst.SimpleStatementLine) and all( + isinstance(small, cst.Import | cst.ImportFrom) for small in statement.body + ) + + +def from_import_sources(module: cst.Module) -> dict[str, tuple[str, int] | None]: + """Map every name bound at module level to its `from` import's (module, level), or None if bound otherwise. + + Names bound by a class, function, assignment or plain `import` map to None. + """ + sources: dict[str, tuple[str, int] | None] = {} + for statement in module_statements(module): + if isinstance(statement, cst.ImportFrom): + source = (import_from_module(statement) or "", len(statement.relative)) + if isinstance(statement.names, cst.ImportStar): + sources["*"] = source + else: + for alias in statement.names: + sources[imported_name(alias)] = source + elif isinstance(statement, cst.Import): + for alias in statement.names: + sources[imported_name(alias).split(".")[0]] = None + elif isinstance(statement, cst.ClassDef | cst.FunctionDef): + sources[statement.name.value] = None + elif isinstance(statement, cst.Assign | cst.AnnAssign): + sources.update(dict.fromkeys(assigned_names(statement))) + return sources + + +def from_import_binding(module: cst.Module, name: str) -> tuple[cst.ImportFrom, cst.ImportAlias] | None: + """Return the module-level `from` import and the alias that bind name, or None if another statement binds it last.""" + if from_import_sources(module).get(name) is None: + return None + binding: tuple[cst.ImportFrom, cst.ImportAlias] | None = None + for statement in module_statements(module): + if isinstance(statement, cst.ImportFrom) and not isinstance(statement.names, cst.ImportStar): + binding = next(((statement, alias) for alias in statement.names if imported_name(alias) == name), binding) + return binding + + +def imports_name_conditionally(module: cst.Module, name: str) -> bool: + """Return True if an import inside a module-level `if`/`try` block binds `name`.""" + for statement in module.body: + if not isinstance(statement, cst.If | cst.Try | cst.TryStar): + continue + for node in matchers.findall(statement, matchers.Import() | matchers.ImportFrom()): + names = cast("cst.Import | cst.ImportFrom", node).names + if not isinstance(names, cst.ImportStar) and any(imported_name(alias).split(".")[0] == name for alias in names): + return True + return False + + +def resolve_project_module(importing_file: Path, project_root: Path, module: str | None, level: int) -> Path | None: + """Resolve one import-from's `(module, level)` to a concrete .py file under `project_root`. + + `level == 0` is an absolute import (`module` is dotted from `project_root`, e.g. + "redis.typing"). `level >= 1` is relative (PEP 328): anchor at `importing_file`'s own + directory for level 1, walking up `level - 1` further parent directories for each extra dot + (`from ..module import x`); `module` is None for a bare `from . import x`, which resolves to + the anchor directory's own `__init__.py`. + + Tries `.py` first, then `/__init__.py`. Returns None if neither exists or if + resolution would walk above `project_root`, as for a stdlib or third-party import. The returned + path is always absolute. + """ + # A relative path can't walk above itself: Path("a.py").parent.parent is still Path("."). + importing_file = importing_file.absolute() + project_root = project_root.absolute() + if level == 0: + anchor = project_root + else: + anchor = importing_file.parent + for _ in range(level - 1): + anchor = anchor.parent + if project_root not in (anchor, *anchor.parents): + return None + + candidate = anchor / (module.replace(".", "/") + ".py") if module is not None else anchor / "__init__.py" + if candidate.is_file(): + return candidate + if module is not None: + candidate_package = anchor / module.replace(".", "/") / "__init__.py" + if candidate_package.is_file(): + return candidate_package + return None + + +def rebase_relative_module(via_module: str | None, via_level: int, origin_file: Path, module: str | None, level: int) -> str | None: + """Rewrite an import target of the origin module so that the importing file names the same module. + + The importing file reaches the origin (`origin_file`) with `from import ...`, where `via` is + `via_level` dots followed by `via_module`. The origin's own `from import ...` with `level` + dots is re-anchored on that same path, e.g. via ".sub.file_1" and "._compat" give ".sub._compat". + An absolute import (`level == 0`) is returned unchanged. + + Returns the text that follows `from`, or None if the importing file can't name the module: an + absolute `via` to a top-level origin leaves no package for the relative import to start from. + """ + if level == 0: + return module + parts = via_module.split(".") if via_module is not None else [] + if origin_file.name != "__init__.py": + parts = parts[:-1] + dots = via_level + for _ in range(level - 1): + if parts: + parts.pop() + elif dots > 0: + dots += 1 + else: + return None + if dots == 0 and not parts: + return None + if module is not None: + parts.append(module) + return "." * dots + ".".join(parts) + + +def collect_project_imported_names(files: Sequence[Path], project_root: Path) -> dict[Path, frozenset[str]]: + """Map each project file to the names any file in `files` imports or reads from it. + + Two kinds of dependency are recorded against the resolved origin file: + + - `from module import name`: `alias.name` (the name as declared in the origin module, not + `alias.asname`) - an aliased import still depends on the original name existing. + - An attribute read through an imported project module (`import pkg.mod` then `pkg.mod.T`, + `import pkg.mod as m` then `m.T`, `from pkg import mod` then `mod.T`): the attribute name. + + Imports that don't resolve inside `project_root` (stdlib/third-party) are skipped, as is any + file that can't be read or parsed. + """ + imported: dict[Path, set[str]] = {} + for file in files: + try: + module = cst.parse_module(file.read_text(encoding="utf-8")) + # TODO(FrancescoPezzella): libcst raises CSTLogicError on an implicit concatenation of t-strings, like `t"a" t"b"`, + # which is valid code. + # https://github.com/Instagram/LibCST/issues/1485 + except OSError, cst.ParserSyntaxError, cst.CSTLogicError: + continue + module_bindings: dict[str, Path] = {} + for statement in matchers.findall(module, matchers.ImportFrom() | matchers.Import()): + if isinstance(statement, cst.ImportFrom): + _record_from_import(file, project_root, statement, imported, module_bindings) + else: + _bind_imported_modules(file, project_root, cast("cst.Import", statement), module_bindings) + _record_module_attribute_reads(module, module_bindings, imported) + return {path: frozenset(names) for path, names in imported.items()} + + +def _record_from_import( + file: Path, + project_root: Path, + statement: cst.ImportFrom, + imported: dict[Path, set[str]], + module_bindings: dict[str, Path], +) -> None: + """Record `statement`'s imported names against their origin, and bind any alias that is itself a project module.""" + module = import_from_module(statement) + level = len(statement.relative) + aliases = [] if isinstance(statement.names, cst.ImportStar) else list(statement.names) + origin = resolve_project_module(file, project_root, module, level) + if origin is not None: + imported.setdefault(origin, set()).update([alias.evaluated_name for alias in aliases] or ["*"]) + for alias in aliases: + submodule = f"{module}.{alias.evaluated_name}" if module is not None else alias.evaluated_name + submodule_origin = resolve_project_module(file, project_root, submodule, level) + if submodule_origin is not None: + module_bindings[imported_name(alias)] = submodule_origin + + +def _bind_imported_modules(file: Path, project_root: Path, statement: cst.Import, module_bindings: dict[str, Path]) -> None: + """Bind the dotted name(s) `statement` makes available to the project module file each one refers to.""" + for alias in statement.names: + if alias.evaluated_alias is not None: + origin = resolve_project_module(file, project_root, alias.evaluated_name, 0) + if origin is not None: + module_bindings[alias.evaluated_alias] = origin + continue + # `import a.b.c` binds `a`, and makes `a.b` and `a.b.c` reachable through it. + parts = alias.evaluated_name.split(".") + for end in range(1, len(parts) + 1): + prefix = ".".join(parts[:end]) + origin = resolve_project_module(file, project_root, prefix, 0) + if origin is not None: + module_bindings[prefix] = origin + + +def _record_module_attribute_reads(module: cst.Module, module_bindings: dict[str, Path], imported: dict[Path, set[str]]) -> None: + """Record every `.` in `module` as a dependency on `attr` in that module's file.""" + if not module_bindings: + return + for node in matchers.findall(module, matchers.Attribute()): + attribute = cast("cst.Attribute", node) + dotted = _dotted_name(attribute.value) + origin = module_bindings.get(dotted) if dotted is not None else None + if origin is not None: + imported.setdefault(origin, set()).add(attribute.attr.value) + + +def _dotted_name(expression: cst.BaseExpression) -> str | None: + """Return `expression` as a dotted name (`a.b.c`) if it is a plain name/attribute chain, else None.""" + if isinstance(expression, cst.Name): + return expression.value + if isinstance(expression, cst.Attribute): + base = _dotted_name(expression.value) + return f"{base}.{expression.attr.value}" if base is not None else None + return None diff --git a/src/renaissance/utils/ruff_utils.py b/src/renaissance/utils/ruff_utils.py index b1914e7d..81ed43b0 100644 --- a/src/renaissance/utils/ruff_utils.py +++ b/src/renaissance/utils/ruff_utils.py @@ -4,7 +4,7 @@ # of this repository, also available at: # https://github.com/TNO/Renaissance.Py/blob/main/LICENSE -"""Ruff fixes for the files a recipe changed.""" +"""Ruff fixes and formatting for the files a recipe changed.""" import subprocess import sys @@ -25,3 +25,17 @@ def run_ruff_import_cleanup(paths: Sequence[Path]) -> None: capture_output=True, check=False, ) + + +def run_ruff_format(paths: Sequence[Path]) -> None: + """Format each whole file in place. + + Runs `ruff format` with the ruff settings that apply to each path. Does nothing when paths is empty. + """ + if not paths: + return + subprocess.run( # noqa: S603 (fixed arguments plus the caller's paths, no shell) + [sys.executable, "-m", "ruff", "format", *(str(path) for path in paths)], + capture_output=True, + check=False, + ) diff --git a/test/python/ast/test_python_cst_node.py b/test/python/ast/test_python_cst_node.py index 97033305..39d5e4cc 100644 --- a/test/python/ast/test_python_cst_node.py +++ b/test/python/ast/test_python_cst_node.py @@ -98,6 +98,23 @@ def test_load_file(self) -> None: atu = PythonCstNode.load(Path(targets.__file__).parent / "demo.py") assert_that(atu, is_not(None)) + @pytest.mark.parametrize( + "source", + [ + pytest.param('x = "“quoted”"\n', id="undecodable-in-cp1252"), + pytest.param('x = "café"\n', id="two-byte-character"), + pytest.param('x = "\U0001f600"\n', id="four-byte-character"), + ], + ) + def test_load_file_decodes_utf8(self, tmp_path: Path, source: str) -> None: + """Verify load reads the file as UTF-8, whatever the locale's default encoding is.""" + path = tmp_path / "utf8.py" + path.write_bytes(source.encode("utf-8")) + + atu = PythonCstNode.load(path) + + assert_that(atu.signature, is_(source)) + def test_load_invalid_file(self) -> None: """AI: Verify loading a syntactically invalid Python file raises a ParserSyntaxError.""" with pytest.raises(ParserSyntaxError, match="Syntax Error"): diff --git a/test/python/ast/test_python_rst_node.py b/test/python/ast/test_python_rst_node.py index e43271fb..3b80bbd0 100644 --- a/test/python/ast/test_python_rst_node.py +++ b/test/python/ast/test_python_rst_node.py @@ -168,6 +168,23 @@ def test_load_file(self) -> None: atu = PythonRstNode.load(Path(targets.__file__).parent / "demo.py") assert_that(atu.translation_unit.atu.type_ignores, is_(empty())) + @pytest.mark.parametrize( + "source", + [ + pytest.param('x = "“quoted”"\n', id="undecodable-in-cp1252"), + pytest.param('x = "café"\n', id="two-byte-character"), + pytest.param('x = "\U0001f600"\n', id="four-byte-character"), + ], + ) + def test_load_file_decodes_utf8(self, tmp_path: Path, source: str) -> None: + """Verify load reads the file as UTF-8, whatever the locale's default encoding is.""" + path = tmp_path / "utf8.py" + path.write_bytes(source.encode("utf-8")) + + atu = PythonRstNode.load(path) + + assert_that(atu.signature, is_(source)) + def test_load_invalid_file(self) -> None: """AI: Verify loading a syntactically invalid Python file raises an IndentationError.""" with pytest.raises(IndentationError, match="unexpected indent"): diff --git a/test/recipes/conftest.py b/test/recipes/conftest.py new file mode 100644 index 00000000..99755473 --- /dev/null +++ b/test/recipes/conftest.py @@ -0,0 +1,36 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Shared fixture for the TypeParameterCheck tests.""" + +import textwrap +from collections.abc import Callable +from pathlib import Path + +import pytest +from pytest_mock import MockerFixture + +from renaissance.integrations.python.ast.cst_node import PythonCstNode +from renaissance.recipes.type_parameter_check import TypeParameterCheck + + +@pytest.fixture +def create_type_parameter_check(mocker: MockerFixture) -> Callable[[str], TypeParameterCheck]: + """Build a TypeParameterCheck for in-memory, dedented source text. + + `PythonFactory.create` is mocked so nothing touches the filesystem. + """ + + def _create(text: str) -> TypeParameterCheck: + mocker.patch( + "renaissance.integrations.python.ast.factory.PythonFactory.create", + return_value=PythonCstNode.load_from_text(textwrap.dedent(text), "x.py"), + ) + subject = TypeParameterCheck(Path("x.py")) + subject.in_memory = True + return subject + + return _create diff --git a/test/recipes/test_step_runner.py b/test/recipes/test_step_runner.py new file mode 100644 index 00000000..5079b417 --- /dev/null +++ b/test/recipes/test_step_runner.py @@ -0,0 +1,56 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for Step/run_steps.""" + +from collections.abc import Callable + +import pytest +from hamcrest import assert_that, equal_to +from pytest_mock import MockerFixture + +from renaissance.recipes.python_refactoring import PythonRefactoring +from renaissance.recipes.step_runner import Step, run_steps + + +class TestRunSteps: + """See module docstring.""" + + def test_collects_each_steps_result_under_its_own_label_in_order(self, mocker: MockerFixture) -> None: + """AI: Verify run_steps returns each step's result keyed by its label, preserving step order.""" + recipe = mocker.Mock(spec=PythonRefactoring) + steps = [ + Step("first", recipe, lambda: {"A": "fixed"}), + Step("second", recipe, lambda: {"B": "unsafe"}), + ] + + result = run_steps(steps) + + assert_that(result, equal_to({"first": {"A": "fixed"}, "second": {"B": "unsafe"}})) + assert_that(list(result.keys()), equal_to(["first", "second"])) + + @pytest.mark.parametrize( + ("action_result", "expect_commit"), + [ + ({"A": "fixed"}, True), + ({"A": "unsafe"}, False), + ({}, False), + ({"A": "fixed", "B": "unsafe"}, True), + ], + ) + def test_commits_only_when_a_step_fixed_something( + self, + mocker: MockerFixture, + action_result: dict[str, str], + expect_commit: bool, # noqa: FBT001 + ) -> None: + """AI: Verify a step's recipe is committed only when its action reports at least one "fixed" result.""" + recipe = mocker.Mock(spec=PythonRefactoring) + action: Callable[[], dict[str, str]] = lambda: action_result # noqa: E731 + + run_steps([Step("only", recipe, action)]) + + assert_that(recipe.commit.called, equal_to(expect_commit)) diff --git a/test/recipes/test_type_parameter_check.py b/test/recipes/test_type_parameter_check.py new file mode 100644 index 00000000..851b2706 --- /dev/null +++ b/test/recipes/test_type_parameter_check.py @@ -0,0 +1,81 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""End-to-end tests for TypeParameterCheck.check() across its phases.""" + +from collections.abc import Callable + +import pytest +from hamcrest import assert_that, contains_string, equal_to, not_ + +from renaissance.recipes.type_parameter_check import TypeParameterCheck + + +class TestTypeParameterCheck: + """See module docstring.""" + + @pytest.mark.parametrize( + ("signature", "expected_converted"), + [ + pytest.param("def b(x: T) -> T:", {"T": "fixed"}, id="legacy-signature"), + pytest.param("def b[T](x: T) -> T:", {}, id="ruff-style-leftover"), + ], + ) + def test_check_converts_then_removes_the_declaration_end_to_end( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + signature: str, + expected_converted: dict[str, str], + ) -> None: + """Verify check() adds the type parameter where missing, then removes the now-orphaned declaration.""" + subject = create_type_parameter_check(f""" + from typing import TypeVar + T = TypeVar('T') + + {signature} + return x + """) + subject.run() + + assert_that(subject.result, equal_to({"cross_file": {}, "converted": expected_converted, "orphaned": {"T": "fixed"}})) + output = subject.apply_to_string() + assert_that(output, contains_string("def b[T](x: T) -> T:")) + assert_that(output, not_(contains_string("T = TypeVar"))) + + def test_check_records_the_kind_of_every_declaration(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """Verify check() exposes each declared name's constructor (TypeVar, ParamSpec or TypeVarTuple) on type_param_kinds.""" + subject = create_type_parameter_check(""" + from typing import ParamSpec, TypeVar, TypeVarTuple + T = TypeVar('T') + P = ParamSpec('P') + Ts = TypeVarTuple('Ts') + """) + subject.run() + + assert_that(subject.type_param_kinds, equal_to({"T": "TypeVar", "P": "ParamSpec", "Ts": "TypeVarTuple"})) + + def test_check_keeps_declaration_still_used_by_a_generic_class( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """Verify check() converts a standalone function but keeps the declaration a Generic[...] base still needs.""" + subject = create_type_parameter_check(""" + from typing import Generic, TypeVar + T = TypeVar('T') + + class Box(Generic[T]): + def get(self, x: T) -> T: + return x + + def first(items: list[T]) -> T: + return items[0] + """) + subject.run() + + assert_that(subject.result, equal_to({"cross_file": {}, "converted": {"T": "fixed"}, "orphaned": {}})) + output = subject.apply_to_string() + assert_that(output, contains_string("def first[T](items: list[T]) -> T:")) + assert_that(output, contains_string("T = TypeVar('T')")) + assert_that(output, contains_string(" def get(self, x: T) -> T:")) diff --git a/test/recipes/test_type_parameter_check_convert.py b/test/recipes/test_type_parameter_check_convert.py new file mode 100644 index 00000000..512534bc --- /dev/null +++ b/test/recipes/test_type_parameter_check_convert.py @@ -0,0 +1,765 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for TypeParameterCheck.convert_declared_typevars.""" + +import ast +import textwrap +from collections.abc import Callable +from typing import cast + +import pytest +from hamcrest import assert_that, contains_string, equal_to, has_entry, not_ + +from renaissance.recipes.type_parameter_check import MIN_SUPPORTED_PYTHON, TypeParameterCheck +from renaissance.recipes.type_parameter_rules import PEP_696_MINIMUM, UnsafeReason + + +def _type_param_name(param: ast.type_param) -> str: + """Return a parsed PEP 695 type parameter's name.""" + return cast("ast.TypeVar | ast.ParamSpec | ast.TypeVarTuple", param).name + + +class TestTypeParameterCheckConvert: + """See module docstring.""" + + @pytest.mark.parametrize( + ("functions", "expected_signatures"), + [ + pytest.param( + "def a(x: T) -> T:\n return x\ndef b(y: T) -> T:\n return y\n", + ["def a[T](x: T) -> T:", "def b[T](y: T) -> T:"], + id="functions", + ), + pytest.param( + "class Foo:\n def a(self, x: T) -> T:\n return x\n def b(self, y: T) -> T:\n return y\n", + ["def a[T](self, x: T) -> T:", "def b[T](self, y: T) -> T:"], + id="methods", + ), + ], + ) + def test_converts_typevar_shared_by_two_functions( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], functions: str, expected_signatures: list[str] + ) -> None: + """Verify a TypeVar shared by two functions, or two methods of one class, gives each its own [T].""" + subject = create_type_parameter_check(f'from typing import TypeVar\n\n{functions}\nT = TypeVar("T")\n') + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + for signature in expected_signatures: + assert_that(output, contains_string(signature)) + + @pytest.mark.parametrize( + ("functions", "converted", "untouched"), + [ + pytest.param( + "def a[T](x: T) -> T:\n return x\ndef b(y: T) -> T:\n return y\n", + "def b[T](y: T) -> T:", + "def a[T](x: T) -> T:", + id="function", + ), + pytest.param( + "class Foo:\n def a[T](self, x: T) -> T:\n return x\n def b(self, y: T) -> T:\n return y\n", + "def b[T](self, y: T) -> T:", + "def a[T](self, x: T) -> T:", + id="method", + ), + ], + ) + def test_leaves_function_with_its_own_type_parameter_alone( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], functions: str, converted: str, untouched: str + ) -> None: + """Verify a function that already declares its own [T] is not rewritten, while another one using the module T is.""" + subject = create_type_parameter_check(f'from typing import TypeVar\n\n{functions}\nT = TypeVar("T")\n') + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, contains_string(converted)) + assert_that(output.count(untouched), equal_to(1)) + + @pytest.mark.parametrize( + "docstring", + [ + pytest.param('"""One liner."""', id="single-line"), + pytest.param('"""First line.\n\nSecond line already indented.\nThird line too.\n"""', id="multi-line"), + pytest.param('"""Produce a cast.\n\n.. seealso::\n\n :ref:`tutorial_casts`\n"""', id="nested-block"), + ], + ) + def test_converts_method_keeping_its_docstring( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], docstring: str + ) -> None: + """Verify converting a method's signature keeps its docstring exactly, including any nested indentation.""" + body = textwrap.indent(f"{docstring}\nreturn x\n", " " * 8) + subject = create_type_parameter_check( + "from typing import TypeVar\n\nclass Foo:\n" + f" def cast(self, x: T) -> T:\n{body}" + " def other(self, y: T) -> T:\n return y\n\n" + 'T = TypeVar("T")\n' + ) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("T", "fixed")) + assert_that(subject.apply_to_string(), contains_string(f" def cast[T](self, x: T) -> T:\n{body}")) + + @pytest.mark.parametrize( + ("extra_line", "imported_elsewhere"), + [ + pytest.param('__all__ = ["T"]', frozenset[str](), id="exported-via-dunder-all"), + pytest.param("", frozenset({"T"}), id="imported-elsewhere"), + ], + ) + def test_converts_functions_using_a_typevar_whose_declaration_must_stay( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + extra_line: str, + imported_elsewhere: frozenset[str], + ) -> None: + """Verify functions get [T] even when T's declaration can't be removed, and the declaration is kept.""" + subject = create_type_parameter_check(f""" + from typing import TypeVar + + {extra_line} + + def a(x: T) -> T: + return x + + T = TypeVar("T") + """) + subject.project_wide_imported_names = imported_elsewhere + + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, contains_string("def a[T](x: T) -> T:")) + assert_that(output, contains_string('T = TypeVar("T")')) + + def test_converts_function_preserving_unusual_body_formatting( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """AI: Verify converting a signature never reformats or collapses its body's unusual formatting.""" + subject = create_type_parameter_check(""" + from typing import TypeVar + + def b(x: T) -> T: + return foo( + x, + extra=1, + ) + + T = TypeVar("T") + """) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("T", "fixed")) + output = subject.apply_to_string() + assert_that(output, contains_string("def b[T](x: T) -> T:")) + assert_that(output, contains_string("return foo(\n x,\n extra=1,\n )")) + + def test_does_not_add_redundant_type_param_to_nested_closure( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """AI: Verify a nested closure referencing an enclosing function's converted type param doesn't get its own copy.""" + # PEP 695 type parameters are already visible in nested scopes. + subject = create_type_parameter_check(""" + from typing import ParamSpec + from collections.abc import Callable + + P = ParamSpec("P") + + def requires(func: Callable[P, int]) -> Callable[P, int]: + def wrapper(*args: P.args, **kwargs: P.kwargs) -> int: + return func(*args, **kwargs) + + return wrapper + """) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("P", "fixed")) + output = subject.apply_to_string() + ast.parse(output) # raises SyntaxError if the nested closure's edit corrupted the output + assert_that(output, contains_string("def requires[**P](func: Callable[P, int]) -> Callable[P, int]:")) + assert_that(output, contains_string("def wrapper(*args: P.args, **kwargs: P.kwargs) -> int:")) + assert_that(output, not_(contains_string("wrapper[**P]"))) + + @pytest.mark.parametrize( + ("declarations", "functions", "expected_lines"), + [ + pytest.param( + 'T = TypeVar("T")', + "def handle(scope: int) -> None:\n def wrap(func: Callable[[], T]) -> T:\n return func()\n", + ["def handle(scope: int) -> None:", " def wrap[T](func: Callable[[], T]) -> T:"], + id="only-nested-signature", + ), + pytest.param( + 'K = TypeVar("K")\nF = TypeVar("F")', + 'def keyword(name: "K | None" = None) -> "K | None":\n def decorator(func: F) -> F:\n return func\n', + ['def keyword[K](name: "K | None" = None) -> "K | None":', " def decorator[F](func: F) -> F:"], + id="outer-and-nested-own-one-each", + ), + pytest.param( + 'T = TypeVar("T")', + ( + "def outer() -> None:\n" + " def middle(x: T) -> T:\n" + " def inner(y: T) -> T:\n" + " return y\n" + " return inner(x)\n" + ), + ["def outer() -> None:", " def middle[T](x: T) -> T:", " def inner(y: T) -> T:"], + id="outermost-signature-of-three-levels", + ), + ], + ) + def test_binds_typevar_to_the_outermost_function_whose_signature_uses_it( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], declarations: str, functions: str, expected_lines: list[str] + ) -> None: + """Verify [T] goes on the outermost function whose own signature uses T, never on an enclosing one that doesn't.""" + subject = create_type_parameter_check( + f"from collections.abc import Callable\nfrom typing import TypeVar\n\n{declarations}\n\n{functions}" + ) + result = subject.convert_declared_typevars() + + names = [declaration.split(" = ", maxsplit=1)[0] for declaration in declarations.splitlines()] + assert_that(result, equal_to(dict.fromkeys(names, "fixed"))) + output = subject.apply_to_string() + for line in expected_lines: + assert_that(output, contains_string(f"{line}\n")) + + @pytest.mark.parametrize( + "body", + [ + pytest.param(" y: list[T] = []\n", id="local-annotation"), + pytest.param(' y = cast("T", x)\n', id="cast"), + ], + ) + def test_leaves_typevar_used_only_in_a_function_body_unattributed( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], body: str + ) -> None: + """Verify a T that no enclosing signature uses gives no function a [T].""" + source = f'from typing import TypeVar, cast\n\nT = TypeVar("T")\n\n\ndef f(x: int) -> None:\n{body}' + subject = create_type_parameter_check(source) + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({})) + assert_that(subject.apply_to_string(), equal_to(source)) + + def test_merges_into_an_existing_type_params_bracket(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """AI: Verify converting a second TypeVar merges it into an existing PEP 695 bracket instead of adding a new one.""" + subject = create_type_parameter_check(""" + from typing import TypeVar + + def f[U](x: U, y: T) -> T: + return y + + T = TypeVar("T") + """) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("T", "fixed")) + output = subject.apply_to_string() + assert_that(output, contains_string("def f[U, T](x: U, y: T) -> T:")) + + def test_converts_a_decorated_overload(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """AI: Verify converting a decorated @overload signature accounts for its non-zero-column indentation.""" + subject = create_type_parameter_check(""" + from typing import TypeVar, overload + + class Config: + @overload + def get(self, key: str, default: T = ...) -> T: ... + def get(self, key: str, default: object = None) -> object: + return default + + T = TypeVar("T") + """) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("T", "fixed")) + output = subject.apply_to_string() + assert_that(output, contains_string("@overload")) + assert_that(output, contains_string("def get[T](self, key: str, default: T = ...) -> T: ...")) + + def test_converts_function_using_two_new_type_params(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """Verify a function using two declared type parameters gets both in one bracket.""" + subject = create_type_parameter_check(""" + from typing import ParamSpec, TypeVar + from collections.abc import Callable + + P = ParamSpec("P") + T = TypeVar("T") + + def run_in_threadpool(func: Callable[P, T]) -> T: + return func() + + def identity(x: T) -> T: + return x + """) + result = subject.check() + + assert_that(result["converted"], has_entry("P", "fixed")) + assert_that(result["converted"], has_entry("T", "fixed")) + assert_that(result["orphaned"], equal_to({"P": "fixed", "T": "fixed"})) + functions = {node.name: node for node in ast.parse(subject.apply_to_string()).body if isinstance(node, ast.FunctionDef)} + assert_that([_type_param_name(param) for param in functions["run_in_threadpool"].type_params], equal_to(["P", "T"])) + assert_that([_type_param_name(param) for param in functions["identity"].type_params], equal_to(["T"])) + + def test_orders_type_params_by_declaration(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """Verify the added type parameters follow the order their declarations appear in the file.""" + names = ["A", "B", "C", "D", "E", "F"] + declarations = "\n".join(f'{name} = TypeVar("{name}")' for name in names) + parameters = ", ".join(f"{name.lower()}: {name}" for name in names) + subject = create_type_parameter_check(f"from typing import TypeVar\n{declarations}\ndef f({parameters}) -> None: ...\n") + + subject.convert_declared_typevars() + + assert_that(subject.apply_to_string(), contains_string(f"def f[{', '.join(names)}]({parameters}) -> None: ...")) + + def test_converts_function_preserving_multiline_string_literal( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """Verify converting a function keeps a multi-line string literal whose continuation line is at column 0.""" + # Built line by line: textwrap.dedent in the fixture would otherwise be blocked by the column-0 line. + source_lines = [ + "from typing import TypeVar", + 'T = TypeVar("T")', + "def f(x: T) -> T:", + ' text = """first', + "second", + ' third"""', + " return x", + ] + subject = create_type_parameter_check("\n".join(source_lines) + "\n") + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("T", "fixed")) + output = subject.apply_to_string() + assert_that(output, contains_string("def f[T](x: T) -> T:")) + text_assign = next( + node for node in ast.walk(ast.parse(output)) if isinstance(node, ast.Assign) and ast.unparse(node.targets[0]) == "text" + ) + assert_that(ast.literal_eval(text_assign.value), equal_to("first\nsecond\n third")) + + @pytest.mark.parametrize( + ("class_header", "expected_reasons"), + [ + pytest.param("class Box(Generic[T]):", {"T": UnsafeReason.USED_IN_GENERIC_CLASS}, id="generic-base"), + pytest.param("class Box(Protocol[T]):", {"T": UnsafeReason.USED_IN_GENERIC_CLASS}, id="protocol-base"), + pytest.param("class Sub(Base[T]):", {"T": UnsafeReason.USED_IN_GENERIC_CLASS}, id="generic-subclass"), + pytest.param("class Box[T]:", {}, id="pep695-class"), + ], + ) + def test_converts_function_but_not_methods_of_a_generic_class( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + class_header: str, + expected_reasons: dict[str, UnsafeReason], + ) -> None: + """Verify a standalone function gets [T] while the methods of a class generic over T keep using the class's T.""" + subject = create_type_parameter_check(f""" + from typing import Generic, Protocol, TypeVar + T = TypeVar('T') + + {class_header} + def get(self, x: T) -> T: + return x + + def first(items: list[T]) -> T: + return items[0] + """) + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + assert_that(subject.converted_unsafe_reasons, equal_to(expected_reasons)) + output = subject.apply_to_string() + assert_that(output, contains_string("def first[T](items: list[T]) -> T:")) + assert_that(output, contains_string(" def get(self, x: T) -> T:")) + + def test_reports_typevar_used_only_in_a_generic_class(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """Verify a TypeVar only a generic class uses is reported unsafe with its reason and left as it is.""" + subject = create_type_parameter_check(""" + from typing import Generic, TypeVar + T = TypeVar('T') + + class Box(Generic[T]): + def get(self, x: T) -> T: + return x + """) + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"T": "unsafe"})) + assert_that(subject.converted_unsafe_reasons, equal_to({"T": UnsafeReason.USED_IN_GENERIC_CLASS})) + assert_that(subject.apply_to_string(), contains_string(" def get(self, x: T) -> T:")) + + def test_ignores_methods_using_their_class_type_parameter( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """Verify a T that is the class's own type parameter (`class Box[T]`) is not a use of the module-level T.""" + subject = create_type_parameter_check(""" + from typing import TypeVar + T = TypeVar('T') + + class Box[T]: + def get(self, x: T) -> T: + return x + """) + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({})) + assert_that(subject.converted_unsafe_reasons, equal_to({})) + assert_that(subject.apply_to_string(), contains_string(" def get(self, x: T) -> T:")) + + @pytest.mark.parametrize( + "class_header", + [ + pytest.param("class Util:", id="plain-class"), + pytest.param("class Box(Generic[U]):", id="generic-over-another-name"), + pytest.param("class Box[U]:", id="pep695-class-over-another-name"), + ], + ) + def test_converts_method_of_a_class_not_generic_over_the_name( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], class_header: str + ) -> None: + """Verify a method gets [T] when its class isn't generic over T.""" + subject = create_type_parameter_check(f""" + from typing import Generic, TypeVar + T = TypeVar('T') + U = TypeVar('U') + + {class_header} + def get(self, x: T) -> T: + return x + """) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("T", "fixed")) + assert_that(subject.apply_to_string(), contains_string(" def get[T](self, x: T) -> T:")) + + @pytest.mark.parametrize( + ("functions", "expected_converted", "expected_unchanged"), + [ + pytest.param( + "def f(x: U) -> U:\n return x\ndef g(x: U, y: Bad) -> Bad:\n return y\n", + "def f[U](x: U) -> U:", + "def g(x: U, y: Bad) -> Bad:", + id="functions", + ), + pytest.param( + "class Util:\n def f(self, x: U) -> U:\n return x\n def g(self, x: U, y: Bad) -> Bad:\n return y\n", + " def f[U](self, x: U) -> U:", + " def g(self, x: U, y: Bad) -> Bad:", + id="methods", + ), + ], + ) + def test_converts_only_functions_free_of_unconvertible_typevars( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + functions: str, + expected_converted: str, + expected_unchanged: str, + ) -> None: + """Verify a function that also uses an unconvertible TypeVar is left whole, while the others using U get [U].""" + subject = create_type_parameter_check( + f'from typing import TypeVar\n\nBad = TypeVar("Bad", future_kw=1)\nU = TypeVar("U")\n\n{functions}' + ) + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"Bad": "unsafe", "U": "fixed"})) + output = subject.apply_to_string() + assert_that(output, contains_string(expected_converted)) + assert_that(output, contains_string(expected_unchanged)) + + @pytest.mark.parametrize( + "class_declaration", + [ + pytest.param('T = TypeVar("T")', id="plain"), + pytest.param('T = TypeVar("T", covariant=True)', id="covariant"), + ], + ) + def test_converts_method_mixing_with_its_class_type_parameter( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], class_declaration: str + ) -> None: + """Verify a method of a class generic over T gets [U] while keeping the class's legacy T.""" + subject = create_type_parameter_check( + f'from typing import Generic, TypeVar\n\n{class_declaration}\nU = TypeVar("U")\n\n' + "class Box(Generic[T]):\n def pair(self, x: T, y: U) -> tuple[T, U]:\n return x, y\n" + ) + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("U", "fixed")) + assert_that(subject.apply_to_string(), contains_string(" def pair[U](self, x: T, y: U) -> tuple[T, U]:")) + + def test_converts_function_when_typevar_is_also_used_outside_functions( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """Verify a function using T is converted even though a module-level alias also uses T.""" + subject = create_type_parameter_check(""" + from typing import TypeVar + T = TypeVar('T') + + Pair = tuple[T, T] + + def first(pair: Pair) -> T: + return pair[0] + + def same(x: T) -> T: + return x + """) + result = subject.convert_declared_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + assert_that(subject.apply_to_string(), contains_string("def same[T](x: T) -> T:")) + + +_HEADER = "from collections.abc import Callable\nfrom typing import ParamSpec, TypeVar, TypeVarTuple, Unpack\n\n" +_TYPEVAR_USE = "def a(x: T) -> T:" +_PARAMSPEC_USE = "def a(f: Callable[P, int]) -> Callable[P, int]:" +_TYPEVARTUPLE_USE = "def a(*args: *Ts) -> tuple[*Ts]:" + + +def _source(declarations: str, signature: str) -> str: + """Build a module with the given legacy declarations and a single function using them.""" + return f"{_HEADER}{declarations}\n\n\n{signature}\n ...\n" + + +class TestTypeParameterCheckConvertArguments: + """Every constructor argument is carried over or refused with a reason, never lost silently.""" + + @pytest.mark.parametrize( + ("declaration", "signature", "expected_signature"), + [ + pytest.param('T = TypeVar("T")', _TYPEVAR_USE, "def a[T](x: T) -> T:", id="typevar-plain"), + pytest.param('T = TypeVar("T", int, str)', _TYPEVAR_USE, "def a[T: (int, str)](x: T) -> T:", id="typevar-constraints"), + pytest.param('T = TypeVar("T", bound=int)', _TYPEVAR_USE, "def a[T: int](x: T) -> T:", id="typevar-bound"), + pytest.param('T = TypeVar("T", default=int)', _TYPEVAR_USE, "def a[T = int](x: T) -> T:", id="typevar-default"), + pytest.param( + 'T = TypeVar("T", bound=str, default=str)', + _TYPEVAR_USE, + "def a[T: str = str](x: T) -> T:", + id="typevar-bound-default", + ), + pytest.param( + 'T = TypeVar("T", int, str, default=int)', + _TYPEVAR_USE, + "def a[T: (int, str) = int](x: T) -> T:", + id="typevar-constraints-default", + ), + pytest.param('T = TypeVar("T", infer_variance=True)', _TYPEVAR_USE, "def a[T](x: T) -> T:", id="typevar-infer-variance"), + pytest.param('T = TypeVar("T", covariant=True)', _TYPEVAR_USE, "def a[T](x: T) -> T:", id="typevar-covariant"), + pytest.param('T = TypeVar("T", contravariant=True)', _TYPEVAR_USE, "def a[T](x: T) -> T:", id="typevar-contravariant"), + pytest.param( + 'T = TypeVar("T", bound=int, covariant=True)', + _TYPEVAR_USE, + "def a[T: int](x: T) -> T:", + id="typevar-bound-covariant", + ), + pytest.param( + 'P = ParamSpec("P", covariant=True)', + _PARAMSPEC_USE, + "def a[**P](f: Callable[P, int]) -> Callable[P, int]:", + id="paramspec-covariant", + ), + pytest.param( + 'P = ParamSpec("P")', + _PARAMSPEC_USE, + "def a[**P](f: Callable[P, int]) -> Callable[P, int]:", + id="paramspec-plain", + ), + pytest.param( + 'P = ParamSpec("P", default=[int, str])', + _PARAMSPEC_USE, + "def a[**P = [int, str]](f: Callable[P, int]) -> Callable[P, int]:", + id="paramspec-default", + ), + pytest.param('Ts = TypeVarTuple("Ts")', _TYPEVARTUPLE_USE, "def a[*Ts](*args: *Ts) -> tuple[*Ts]:", id="typevartuple-plain"), + pytest.param( + 'Ts = TypeVarTuple("Ts", default=Unpack[tuple[int]])', + _TYPEVARTUPLE_USE, + "def a[*Ts = Unpack[tuple[int]]](*args: *Ts) -> tuple[*Ts]:", + id="typevartuple-default", + ), + ], + ) + def test_converts_keeping_every_expressible_argument( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + declaration: str, + signature: str, + expected_signature: str, + ) -> None: + """Verify bounds, constraints and defaults are carried over, and variance arguments are dropped.""" + name = declaration.split(" = ", maxsplit=1)[0] + subject = create_type_parameter_check(_source(declaration, signature)) + subject.min_python = PEP_696_MINIMUM + + result = subject.convert_declared_typevars() + + assert_that(result, has_entry(name, "fixed")) + output = subject.apply_to_string() + ast.parse(output) + assert_that(output, contains_string(expected_signature)) + + @pytest.mark.parametrize( + ("declaration", "signature", "min_python", "expected_reason"), + [ + pytest.param( + 'T = TypeVar("T", default=int)', + _TYPEVAR_USE, + MIN_SUPPORTED_PYTHON, + UnsafeReason.PEP696_VERSION_GATE, + id="typevar-default-3.12", + ), + pytest.param( + 'P = ParamSpec("P", default=[int, str])', + _PARAMSPEC_USE, + MIN_SUPPORTED_PYTHON, + UnsafeReason.PEP696_VERSION_GATE, + id="paramspec-default-3.12", + ), + pytest.param( + 'Ts = TypeVarTuple("Ts", default=Unpack[tuple[int]])', + _TYPEVARTUPLE_USE, + MIN_SUPPORTED_PYTHON, + UnsafeReason.PEP696_VERSION_GATE, + id="typevartuple-default-3.12", + ), + pytest.param( + 'Ts = TypeVarTuple("Ts", bound=int)', + _TYPEVARTUPLE_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="typevartuple-bound", + ), + pytest.param( + 'P = ParamSpec("P", bound=int)', + _PARAMSPEC_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="paramspec-bound", + ), + pytest.param( + 'T = TypeVar("T", future_kw=1)', + _TYPEVAR_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="unknown-keyword", + ), + pytest.param( + 'T = TypeVar("T", **options)', + _TYPEVAR_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="double-star-keywords", + ), + ], + ) + def test_refuses_argument_it_cannot_carry_over( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + declaration: str, + signature: str, + min_python: tuple[int, int], + expected_reason: UnsafeReason, + ) -> None: + """Verify an argument the target can't express is reported unsafe with its reason and the file is left unchanged.""" + name = declaration.split(" = ", maxsplit=1)[0] + source = _source(declaration, signature) + subject = create_type_parameter_check(source) + subject.min_python = min_python + + result = subject.convert_declared_typevars() + + assert_that(result, has_entry(name, "unsafe")) + assert_that(subject.converted_unsafe_reasons, has_entry(name, expected_reason)) + assert_that(subject.apply_to_string(), equal_to(source)) + + @pytest.mark.parametrize( + ("declarations", "signature", "min_python", "expected_reasons"), + [ + pytest.param( + 'Bad = TypeVar("Bad", future_kw=1)\nTs = TypeVarTuple("Ts")', + "def a(func: Callable[[*Ts], Bad], *args: *Ts) -> Bad:", + PEP_696_MINIMUM, + {"Bad": UnsafeReason.NO_PEP695_EQUIVALENT, "Ts": UnsafeReason.SHARES_FUNCTION_WITH_UNCONVERTIBLE}, + id="unconvertible-typevar-with-typevartuple", + ), + pytest.param( + 'T = TypeVar("T")\nD = TypeVar("D", default=int)', + "def a(x: T, y: D) -> T | D:", + MIN_SUPPORTED_PYTHON, + {"D": UnsafeReason.PEP696_VERSION_GATE, "T": UnsafeReason.SHARES_FUNCTION_WITH_UNCONVERTIBLE}, + id="version-gated-default-declared-last", + ), + pytest.param( + 'P = ParamSpec("P", bound=int)\nT = TypeVar("T")', + "def a(f: Callable[P, T]) -> Callable[P, T]:", + PEP_696_MINIMUM, + {"P": UnsafeReason.NO_PEP695_EQUIVALENT, "T": UnsafeReason.SHARES_FUNCTION_WITH_UNCONVERTIBLE}, + id="bound-paramspec-with-typevar", + ), + ], + ) + def test_leaves_function_using_an_unconvertible_typevar_unchanged( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + declarations: str, + signature: str, + min_python: tuple[int, int], + expected_reasons: dict[str, UnsafeReason], + ) -> None: + """Verify a function using one unconvertible TypeVar gets none of its TypeVars converted, so it never mixes syntaxes.""" + source = _source(declarations, signature) + subject = create_type_parameter_check(source) + subject.min_python = min_python + + result = subject.convert_declared_typevars() + + assert_that(result, equal_to(dict.fromkeys(expected_reasons, "unsafe"))) + assert_that(subject.converted_unsafe_reasons, equal_to(expected_reasons)) + assert_that(subject.apply_to_string(), equal_to(source)) + + @pytest.mark.parametrize( + ("declarations", "signature", "expected_signature"), + [ + pytest.param( + 'T = TypeVar("T", default=int)\nU = TypeVar("U")', + "def f(x: T, y: U) -> T:", + "def f[U, T = int](x: T, y: U) -> T:", + id="defaulted-declared-first", + ), + pytest.param( + 'U = TypeVar("U")', + "def f[T = int](x: T, y: U) -> T:", + "def f[U, T = int](x: T, y: U) -> T:", + id="existing-defaulted-param", + ), + pytest.param( + 'T = TypeVar("T", default=int)\nTs = TypeVarTuple("Ts")', + "def f(x: T, *args: *Ts) -> T:", + "def f[*Ts, T = int](x: T, *args: *Ts) -> T:", + id="typevartuple-after-defaulted", + ), + ], + ) + def test_places_defaulted_type_params_last( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + declarations: str, + signature: str, + expected_signature: str, + ) -> None: + """Verify type parameters with a default come after those without one, which Python requires.""" + subject = create_type_parameter_check(_source(declarations, signature)) + subject.min_python = PEP_696_MINIMUM + + subject.convert_declared_typevars() + + output = subject.apply_to_string() + ast.parse(output) + assert_that(output, contains_string(expected_signature)) diff --git a/test/recipes/test_type_parameter_check_localize.py b/test/recipes/test_type_parameter_check_localize.py new file mode 100644 index 00000000..7e64e4a4 --- /dev/null +++ b/test/recipes/test_type_parameter_check_localize.py @@ -0,0 +1,355 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for how TypeParameterCheck.localize_imported_typevars writes the localizations that plan_localizations decides.""" + +import ast +import textwrap +from pathlib import Path + +import pytest +from hamcrest import all_of, assert_that, contains_string, equal_to, has_entry, has_length, not_ +from pytest_mock import MockerFixture + +from renaissance.integrations.python.ast.cst_node import PythonCstNode +from renaissance.recipes.type_parameter_check import TypeParameterCheck + + +def _imports_after_code(module_text: str) -> list[str]: + """Return the module-level imports of module_text that come after its first non-import statement.""" + body = ast.parse(module_text).body + first_code = next((index for index, node in enumerate(body) if not isinstance(node, ast.Import | ast.ImportFrom)), len(body)) + return [ast.unparse(node) for node in body[first_code:] if isinstance(node, ast.Import | ast.ImportFrom)] + + +class TestTypeParameterCheckLocalize: + """See module docstring.""" + + def _create_cross_file( + self, + mocker: MockerFixture, + tmp_path: Path, + origin_text: str, + importing_text: str, + layout: tuple[str, str] = ("file_1.py", "file_2.py"), + ) -> TypeParameterCheck: + """Write origin_text to the origin file and return an in-memory TypeParameterCheck on the importing file holding importing_text. + + layout gives the (origin, importing) file paths, relative to tmp_path, which is the project root. + """ + origin_rel_path, importing_rel_path = layout + origin_file = tmp_path / origin_rel_path + origin_file.parent.mkdir(parents=True, exist_ok=True) + origin_file.write_text(textwrap.dedent(origin_text)) + + importing_code = textwrap.dedent(importing_text) + importing_file = tmp_path / importing_rel_path + mocker.patch( + "renaissance.integrations.python.ast.factory.PythonFactory.create", + return_value=PythonCstNode.load_from_text(importing_code, str(importing_file)), + ) + subject = TypeParameterCheck(importing_file) + subject.in_memory = True + subject.project_root = tmp_path + return subject + + @pytest.mark.parametrize( + ("dunder_all", "imported_elsewhere"), + [ + pytest.param('__all__ = ["T"]', frozenset[str](), id="re-exported-via-dunder-all"), + pytest.param("", frozenset({"T"}), id="imported-from-here-elsewhere"), + ], + ) + def test_localizes_typevar_this_file_re_exports( + self, + mocker: MockerFixture, + tmp_path: Path, + dunder_all: str, + imported_elsewhere: frozenset[str], + ) -> None: + """Verify an imported TypeVar is localized even when this file passes it on to others.""" + subject = self._create_cross_file( + mocker, + tmp_path, + """ + from typing import TypeVar + T = TypeVar("T") + """, + f""" + from file_1 import T + {dunder_all} + def b(x: T) -> T: + return x + """, + ) + subject.project_wide_imported_names = imported_elsewhere + + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, contains_string('T = TypeVar("T")')) + assert_that(output, not_(contains_string("from file_1 import T"))) + + @pytest.mark.parametrize( + ("layout", "importing_import", "expected_line"), + [ + pytest.param(("file_1.py", "file_2.py"), "from file_1 import T, helper", "from file_1 import helper", id="absolute"), + pytest.param(("file_1.py", "file_2.py"), "from .file_1 import T, helper", "from .file_1 import helper", id="relative"), + pytest.param( + ("file_1.py", "sub/use.py"), + "from ..file_1 import T, helper", + "from ..file_1 import helper", + id="relative-parent", + ), + pytest.param(("__init__.py", "file_2.py"), "from . import T, helper", "from . import helper", id="bare-package"), + pytest.param( + ("file_1.py", "file_2.py"), + "from file_1 import T, helper as h", + "from file_1 import helper as h", + id="alias-kept", + ), + ], + ) + def test_keeps_other_names_when_localizing_one_of_several_imports( + self, + mocker: MockerFixture, + tmp_path: Path, + layout: tuple[str, str], + importing_import: str, + expected_line: str, + ) -> None: + """Verify localizing one imported name keeps the other names imported from the same module, alias included.""" + subject = self._create_cross_file( + mocker, + tmp_path, + 'from typing import TypeVar\nT = TypeVar("T")\ndef helper() -> None:\n pass\n', + f"{importing_import}\ndef b(x: T) -> T:\n return x\n", + layout, + ) + result = subject.localize_imported_typevars() + + assert_that(result, has_entry("T", "fixed")) + output = subject.apply_to_string() + assert_that(output.splitlines().count(expected_line), equal_to(1)) + assert_that(output, contains_string('T = TypeVar("T")')) + + @pytest.mark.parametrize( + ("importing_import", "comments"), + [ + pytest.param("from file_1 import T, helper # noqa: F401", ["# noqa: F401"], id="trailing-comment"), + pytest.param( + "from file_1 import ( # header\n T, # the typevar\n helper, # the helper\n)", + ["# header", "# the helper"], + id="multi-line", + ), + ], + ) + def test_narrowing_an_import_keeps_its_comments( + self, + mocker: MockerFixture, + tmp_path: Path, + importing_import: str, + comments: list[str], + ) -> None: + """Verify each comment of a narrowed import is kept once, and never on the localized declaration's line.""" + subject = self._create_cross_file( + mocker, + tmp_path, + 'from typing import TypeVar\nT = TypeVar("T")\ndef helper() -> None:\n pass\n', + f"{importing_import}\ndef b(x: T) -> T:\n helper()\n return x\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, has_entry("T", "fixed")) + lines = subject.apply_to_string().splitlines() + for comment in comments: + holders = [line for line in lines if comment in line] + assert_that(holders, has_length(1), comment) + assert_that(holders[0], not_(contains_string("TypeVar(")), comment) + + @pytest.mark.parametrize( + ("origin_extra", "importing_import", "expected_lines"), + [ + pytest.param( + 'T = TypeVar("T")\nU = TypeVar("U")', + "from file_1 import T, U", + ["from typing import TypeVar", 'T = TypeVar("T")', 'U = TypeVar("U")'], + id="two-names", + ), + pytest.param( + 'T = TypeVar("T")\nU = TypeVar("U")\ndef helper() -> None:\n pass', + "from file_1 import T, helper, U", + ["from file_1 import helper", "from typing import TypeVar", 'T = TypeVar("T")', 'U = TypeVar("U")'], + id="keeps-other-name", + ), + pytest.param( + 'class Shape:\n pass\nclass Circle(Shape):\n pass\nT = TypeVar("T", bound=Shape)\nU = TypeVar("U", bound=Circle)', + "from file_1 import T, U", + [ + "from file_1 import Circle", + "from file_1 import Shape", + "from typing import TypeVar", + 'T = TypeVar("T", bound=Shape)', + 'U = TypeVar("U", bound=Circle)', + ], + id="bounds-each-imported-once", + ), + ], + ) + def test_localizes_two_names_from_one_import_statement( + self, + mocker: MockerFixture, + tmp_path: Path, + origin_extra: str, + importing_import: str, + expected_lines: list[str], + ) -> None: + """Verify one import statement bringing in two localizable names localizes both, adding each needed import once.""" + subject = self._create_cross_file( + mocker, + tmp_path, + f"from typing import TypeVar\n{origin_extra}\n", + f"{importing_import}\ndef b(x: T, y: U) -> T:\n return x\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed", "U": "fixed"})) + output = subject.apply_to_string() + for line in expected_lines: + assert_that(output.splitlines().count(line), equal_to(1), line) + still_imported = { + alias.name + for node in ast.parse(output).body + if isinstance(node, ast.ImportFrom) and node.module == "file_1" + for alias in node.names + } + assert_that(still_imported & {"T", "U"}, equal_to(set())) + + @pytest.mark.parametrize( + "importing_text", + [ + pytest.param("from file_1 import T\nimport os\nfrom collections import OrderedDict\n", id="import-after-removed"), + pytest.param("import os\nfrom file_1 import T\n", id="removed-is-last-import"), + pytest.param("from __future__ import annotations\nfrom file_1 import T\nimport os\n", id="future-import-present"), + pytest.param("from file_1 import T, helper\nimport os\n", id="narrowed-import"), + pytest.param("from file_1 import T\n", id="only-import"), + ], + ) + def test_localized_declaration_comes_after_every_import(self, mocker: MockerFixture, tmp_path: Path, importing_text: str) -> None: + """Verify the declaration is placed after the import block, so no import is left below code (E402).""" + subject = self._create_cross_file( + mocker, + tmp_path, + 'from typing import TypeVar\nT = TypeVar("T")\ndef helper() -> None:\n pass\n', + f"{importing_text}def b(x: T) -> T:\n return x\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, contains_string('T = TypeVar("T")')) + assert_that(_imports_after_code(output), equal_to([])) + assert_that(output.count("from typing import TypeVar"), equal_to(1)) + + @pytest.mark.parametrize( + "dropped_import", + [ + pytest.param("from file_1 import T # noqa: F401", id="trailing-comment"), + pytest.param("from file_1 import (\n T,\n) # noqa: F401", id="multi-line-trailing-comment"), + ], + ) + def test_dropping_an_import_drops_the_comment_on_its_line(self, mocker: MockerFixture, tmp_path: Path, dropped_import: str) -> None: + """Verify a comment trailing a fully localized import goes with it instead of staying behind on a line of its own.""" + subject = self._create_cross_file( + mocker, + tmp_path, + 'from typing import TypeVar\nT = TypeVar("T")\n', + f"import os\n{dropped_import}\nimport sys\ndef b(x: T) -> T:\n return x\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, not_(contains_string("noqa"))) + assert_that(_imports_after_code(output), equal_to([])) + + @pytest.mark.parametrize( + ("origin_extra", "expected_once"), + [ + pytest.param('T = TypeVar("T")\nU = TypeVar("U")', ["from typing import TypeVar"], id="constructor"), + pytest.param( + 'class Shape:\n pass\nT = TypeVar("T", bound=Shape)\nU = TypeVar("U", bound=Shape)', + ["from typing import TypeVar", "from file_1 import Shape"], + id="constructor-and-bound-name", + ), + ], + ) + def test_each_needed_import_is_added_once_per_file( + self, + mocker: MockerFixture, + tmp_path: Path, + origin_extra: str, + expected_once: list[str], + ) -> None: + """Verify names localized from separate import statements share the imports they need, one line each per file.""" + subject = self._create_cross_file( + mocker, + tmp_path, + f"from typing import TypeVar\n{origin_extra}\n", + "from file_1 import T\nfrom file_1 import U\ndef b(x: T, y: U) -> T:\n return x\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed", "U": "fixed"})) + output = subject.apply_to_string() + for line in expected_once: + assert_that(output.splitlines().count(line), equal_to(1), line) + + def test_declaration_stays_before_a_statement_that_uses_it_between_imports( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + """Verify a declaration is not moved below a module-level statement that already needs it.""" + subject = self._create_cross_file( + mocker, + tmp_path, + 'from typing import TypeVar\nT = TypeVar("T")\n', + "from typing import Generic\nfrom file_1 import T\nclass Box(Generic[T]):\n pass\nimport os\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + # Running the module is what proves the declaration is bound before `Box` uses it. + exec(compile(output, "importing.py", "exec"), {}) # noqa: S102 + + def test_check_localizes_then_converts(self, mocker: MockerFixture, tmp_path: Path) -> None: + """Verify check() localizes a cross-file TypeVar, then converts it.""" + subject = self._create_cross_file( + mocker, + tmp_path, + """ + from typing import TypeVar + T = TypeVar("T") + def a(x: T) -> T: + return x + """, + """ + from file_1 import T + def b(x: T) -> T: + return x + """, + ) + subject.run() + + assert_that(subject.result["cross_file"], has_entry("T", "fixed")) + assert_that(subject.result["converted"], has_entry("T", "fixed")) + assert_that( + subject.apply_to_string(), + all_of(contains_string("def b[T](x: T) -> T:"), not_(contains_string("T = TypeVar"))), + ) diff --git a/test/recipes/test_type_parameter_check_orphaned.py b/test/recipes/test_type_parameter_check_orphaned.py new file mode 100644 index 00000000..b83b6222 --- /dev/null +++ b/test/recipes/test_type_parameter_check_orphaned.py @@ -0,0 +1,186 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for TypeParameterCheck.remove_orphaned_declarations.""" + +import textwrap +from collections.abc import Callable + +import pytest +from hamcrest import assert_that, contains_string, equal_to, has_entry, not_ + +from renaissance.recipes.type_parameter_check import TypeParameterCheck +from renaissance.recipes.type_parameter_rules import UnsafeReason + +ALREADY_CONVERTED_SOURCE = """ +from typing import TypeVar +T = TypeVar('T') + +def b[T](x: T) -> T: + return x +""" + +UNREFERENCED_SOURCE = """ +from typing import TypeVar +T = TypeVar('T') + +def b() -> None: + pass +""" + +NESTED_ONLY_CONVERTED_SOURCE = """ +from typing import TypeVar +T = TypeVar('T') + +def outer() -> None: + def inner[T](x: T) -> T: + return x +""" + +CLOSURE_IN_CONVERTED_FUNCTION_SOURCE = """ +from typing import TypeVar +T = TypeVar('T') + +def outer[T](x: T) -> T: + def inner(y: T) -> T: + return y + return inner(x) +""" + +CLASS_WITH_OWN_TYPE_PARAMETER_SOURCE = """ +from typing import TypeVar +T = TypeVar('T') + +class Box[T]: + def get(self, x: T) -> T: + return x +""" + + +class TestTypeParameterCheckOrphaned: + """See module docstring.""" + + @pytest.mark.parametrize( + "source", + [ + pytest.param(ALREADY_CONVERTED_SOURCE, id="already-converted"), + pytest.param(UNREFERENCED_SOURCE, id="unreferenced"), + pytest.param(NESTED_ONLY_CONVERTED_SOURCE, id="only-nested-function-converted"), + pytest.param(CLOSURE_IN_CONVERTED_FUNCTION_SOURCE, id="closure-inside-converted-function"), + pytest.param(CLASS_WITH_OWN_TYPE_PARAMETER_SOURCE, id="class-with-own-type-parameter"), + ], + ) + def test_removes_declaration_whose_references_are_all_shadowed( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + source: str, + ) -> None: + """Verify a declaration whose references are all shadowed by PEP 695 parameters, or absent, is removed.""" + subject = create_type_parameter_check(source) + + result = subject.remove_orphaned_declarations() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, not_(contains_string("T = TypeVar"))) + for def_line in (line.strip() for line in textwrap.dedent(source).splitlines() if line.strip().startswith("def ")): + assert_that(output, contains_string(def_line)) + + @pytest.mark.parametrize( + "declaration", + [ + pytest.param("T = TypeVar('T') # noqa: PLC0105", id="trailing-comment"), + pytest.param("T = TypeVar(\n 'T',\n bound=int,\n) # noqa: PLC0105", id="multi-line-trailing-comment"), + ], + ) + def test_removing_a_declaration_removes_the_comment_trailing_it( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + declaration: str, + ) -> None: + """Verify the comment trailing a removed declaration goes with it instead of staying behind on a line of its own.""" + subject = create_type_parameter_check(f"from typing import TypeVar\n{declaration}\n\ndef b() -> None:\n pass\n") + + result = subject.remove_orphaned_declarations() + + assert_that(result, equal_to({"T": "fixed"})) + output = subject.apply_to_string() + assert_that(output, not_(contains_string("noqa"))) + + def test_keeps_declaration_still_used_without_pep695(self, create_type_parameter_check: Callable[[str], TypeParameterCheck]) -> None: + """Verify a declaration still used by a function without its own type parameter is left alone, unreported.""" + subject = create_type_parameter_check(""" + from typing import TypeVar + T = TypeVar('T') + + def a[T](x: T) -> T: + return x + def b(y: T) -> T: + return y + """) + result = subject.remove_orphaned_declarations() + + assert_that(result, equal_to({})) + assert_that(subject.apply_to_string(), contains_string("T = TypeVar('T')")) + + @pytest.mark.parametrize( + "annotation", + [ + pytest.param('"T"', id="whole-annotation"), + pytest.param('"list[T]"', id="inside-annotation"), + ], + ) + def test_keeps_declaration_used_in_a_string_annotation( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck], annotation: str + ) -> None: + """Verify a declaration referenced only from a string annotation is not removed as orphaned.""" + subject = create_type_parameter_check(f""" + from typing import TypeVar + T = TypeVar('T') + + def b(x: {annotation}) -> None: + pass + """) + result = subject.remove_orphaned_declarations() + + assert_that(result, equal_to({})) + assert_that(subject.apply_to_string(), contains_string("T = TypeVar('T')")) + + def test_keeps_orphaned_declaration_imported_elsewhere( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """Verify an orphaned declaration is kept and reported unsafe when removing it would break an importer.""" + subject = create_type_parameter_check(ALREADY_CONVERTED_SOURCE) + subject.project_wide_imported_names = frozenset({"T"}) + + result = subject.remove_orphaned_declarations() + + assert_that(result, equal_to({"T": "unsafe"})) + assert_that(subject.orphaned_unsafe_reasons, equal_to({"T": UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT})) + assert_that(subject.apply_to_string(), contains_string("T = TypeVar('T')")) + + def test_removing_orphaned_declaration_keeps_comment_shared_with_next_declaration( + self, create_type_parameter_check: Callable[[str], TypeParameterCheck] + ) -> None: + """Verify removing an orphaned declaration keeps a leading comment that also documents the next declaration.""" + subject = create_type_parameter_check(""" + from typing import TypeVar + + # explains both T and U below + T = TypeVar('T') + U = TypeVar('U') + + Pair = tuple[U, U] + + def b[T](x: T) -> T: + return x + """) + result = subject.remove_orphaned_declarations() + + assert_that(result, has_entry("T", "fixed")) + output = subject.apply_to_string() + assert_that(output, contains_string("# explains both T and U below")) + assert_that(output, contains_string("U = TypeVar('U')")) diff --git a/test/recipes/test_type_parameter_check_properties.py b/test/recipes/test_type_parameter_check_properties.py new file mode 100644 index 00000000..49632cad --- /dev/null +++ b/test/recipes/test_type_parameter_check_properties.py @@ -0,0 +1,107 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Property-based tests for the TypeParameterCheck phases on generated modules.""" + +import ast +from pathlib import Path +from typing import TYPE_CHECKING, NamedTuple +from unittest.mock import patch + +import pytest +from hamcrest import assert_that, equal_to +from hypothesis import example, given, settings +from hypothesis import strategies as st + +from renaissance.integrations.python.ast.cst_node import PythonCstNode +from renaissance.recipes.type_parameter_check import TypeParameterCheck + +if TYPE_CHECKING: + from collections.abc import Callable + + +class _Declaration(NamedTuple): + """A module-level type parameter declaration and a parameter that uses it.""" + + constructor: str + statement: str + parameter: str + + +_DECLARATIONS = { + "T": _Declaration("TypeVar", 'T = TypeVar("T")', "x: T"), + "B": _Declaration("TypeVar", 'B = TypeVar("B", bound=int)', "x: B"), + "D": _Declaration("TypeVar", 'D = TypeVar("D", default=int)', "x: D"), + "P": _Declaration("ParamSpec", 'P = ParamSpec("P")', "f: Callable[P, int]"), + "Ts": _Declaration("TypeVarTuple", 'Ts = TypeVarTuple("Ts")', "*args: *Ts"), +} +_PLAIN_PARAMETER = "x: int" + +_PHASES = [ + pytest.param(TypeParameterCheck.check, id="check"), + pytest.param(TypeParameterCheck.convert_declared_typevars, id="convert"), + pytest.param(TypeParameterCheck.remove_orphaned_declarations, id="orphaned"), +] + + +@st.composite +def modules(draw: st.DrawFn, *, with_declarations: bool = True) -> str: + """Draw a module of type parameter declarations, functions using them, and optionally a plain and a generic class.""" + names = draw(st.lists(st.sampled_from(sorted(_DECLARATIONS)), max_size=3, unique=True)) if with_declarations else [] + parameters = [_DECLARATIONS[name].parameter for name in names] or [_PLAIN_PARAMETER] + generic_class = "T" in names and draw(st.booleans()) + + typing_names = {_DECLARATIONS[name].constructor for name in names} | ({"Generic"} if generic_class else set[str]()) + imports = [f"from typing import {', '.join(sorted(typing_names))}"] if typing_names else [] + if "P" in names: + imports.insert(0, "from collections.abc import Callable") + + blocks = ["\n".join(imports)] if imports else [] + if names: + blocks.append("\n".join(_DECLARATIONS[name].statement for name in names)) + blocks.extend( + f"def f{index}({parameter}) -> None:\n pass" + for index, parameter in enumerate(draw(st.lists(st.sampled_from(parameters), max_size=4))) + ) + if draw(st.booleans()): + blocks.append(f"class Plain:\n def method(self, {draw(st.sampled_from(parameters))}) -> None:\n pass") + if generic_class: + blocks.append("class Box(Generic[T]):\n def get(self, x: T) -> T:\n return x") + return "\n\n\n".join(blocks) + "\n" if blocks else "" + + +def _run(source: str, phase: Callable[[TypeParameterCheck], object]) -> str: + """Run one phase of TypeParameterCheck on source in memory and return the rewritten text.""" + with patch( + "renaissance.integrations.python.ast.factory.PythonFactory.create", + return_value=PythonCstNode.load_from_text(source, "x.py"), + ): + subject = TypeParameterCheck(Path("x.py")) + subject.in_memory = True + phase(subject) + return subject.apply_to_string() + + +class TestTypeParameterCheckProperties: + """Properties every phase must have on any module, whatever it converts or removes.""" + + @pytest.mark.parametrize("phase", _PHASES) + @settings(max_examples=50, deadline=None) + @given(source=modules()) + @example(source="") + def test_output_parses_and_a_second_run_changes_nothing(self, phase: Callable[[TypeParameterCheck], object], source: str) -> None: + """The phase leaves valid Python, and running it again on that result changes nothing.""" + once = _run(source, phase) + + ast.parse(once) + assert_that(_run(once, phase), equal_to(once)) + + @pytest.mark.parametrize("phase", _PHASES) + @settings(max_examples=50, deadline=None) + @given(source=modules(with_declarations=False)) + def test_module_without_type_parameters_is_left_unchanged(self, phase: Callable[[TypeParameterCheck], object], source: str) -> None: + """A module that declares no type parameter comes out identical.""" + assert_that(_run(source, phase), equal_to(source)) diff --git a/test/recipes/test_type_parameter_check_variance.py b/test/recipes/test_type_parameter_check_variance.py new file mode 100644 index 00000000..b1416911 --- /dev/null +++ b/test/recipes/test_type_parameter_check_variance.py @@ -0,0 +1,195 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests that dropping `covariant=`/`contravariant=` is safe for functions and unsafe for classes. + +The type checker (pyright) and the interpreter judge the code before and after TypeParameterCheck.convert_declared_typevars. +""" + +import importlib.util +import json +import subprocess +import sys +from typing import TYPE_CHECKING, NamedTuple + +import pytest +from hamcrest import assert_that, contains_string, equal_to, has_item, has_property, not_ + +from renaissance.recipes.type_parameter_rules import UnsafeReason + +# TODO(FrancescoPezzella): Replace with a lazy import in Python 3.15. +# https://github.com/TNO/Renaissance.Py/issues/275 +if TYPE_CHECKING: + from collections.abc import Callable + from pathlib import Path + from types import ModuleType + + from renaissance.recipes.type_parameter_check import TypeParameterCheck + + +class _Diagnostic(NamedTuple): + """One pyright finding, without the file it was found in.""" + + line: int + severity: str + rule: str + message: str + + +def _write(source: str, path: Path) -> Path: + """Write `source` to `path` and return `path`.""" + path.write_text(source, encoding="utf-8") + return path + + +def _type_check(path: Path) -> list[_Diagnostic]: + """Return every diagnostic pyright reports for the file at `path`, `reveal_type` results included.""" + command = [sys.executable, "-m", "pyright", "--pythonpath", sys.executable, "--outputjson", str(path)] + result = subprocess.run(command, capture_output=True, text=True, encoding="utf-8", cwd=path.parent, check=False) # noqa: S603 - fixed argv + report = json.loads(result.stdout[result.stdout.index("{") :]) + return [ + _Diagnostic(item["range"]["start"]["line"] + 1, item["severity"], item.get("rule", ""), item["message"]) + for item in report["generalDiagnostics"] + ] + + +def _import(path: Path) -> ModuleType: + """Import the file at `path` as a module.""" + spec = importlib.util.spec_from_file_location(path.stem, path) + if spec is None or spec.loader is None: + message = f"could not load {path}" + raise RuntimeError(message) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +_FUNCTION_TEMPLATE = """\ +from collections.abc import Callable +from typing import TypeVar, reveal_type + +{declaration} + + +def first(x: T) -> T: + return x + + +reveal_type(first(1)) +reveal_type(first("a")) +reveal_type(first([1])) +accepted: Callable[[bool], int] = first +rejected: Callable[[int], str] = first +""" + +_PROTOCOL = """\ +class Reader(Protocol[T_co]): + def read(self) -> T_co: ... +""" + +_GENERIC_CLASS = """\ +class Reader(Generic[T_co]): + def __init__(self, value: T_co) -> None: + self._value = value + + def read(self) -> T_co: + return self._value +""" + +_CLASS_TEMPLATE = """\ +from typing import Generic, Protocol, TypeVar, reveal_type + +T_co = TypeVar("T_co"{covariant}) + + +{class_definition} + +def read_twice(reader: Reader[T_co]) -> tuple[T_co, T_co]: + return reader.read(), reader.read() + + +def widen(reader: Reader[int]) -> None: + wider: Reader[object] = reader + reveal_type(wider) +""" + + +class TestVarianceOfFunctions: + """A function's TypeVar has no variance, so dropping it must not change anything.""" + + @pytest.mark.parametrize( + "declaration", + [ + pytest.param('T = TypeVar("T", covariant=True)', id="covariant"), + pytest.param('T = TypeVar("T", contravariant=True)', id="contravariant"), + pytest.param('T = TypeVar("T", bound=int, covariant=True)', id="bound-and-covariant"), + ], + ) + def test_function_behaves_the_same_without_the_variance( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + tmp_path: Path, + declaration: str, + ) -> None: + """Verify the type checker accepts, rejects and infers exactly as before, and the function returns the same values.""" + before = _FUNCTION_TEMPLATE.format(declaration=declaration) + subject = create_type_parameter_check(before) + + result = subject.convert_declared_typevars() + + after = subject.apply_to_string() + assert_that(result, equal_to({"T": "fixed"})) + assert_that(after, contains_string("def first[T")) + before_path = _write(before, tmp_path / "before.py") + after_path = _write(after, tmp_path / "after.py") + before_findings = _type_check(before_path) + assert_that(before_findings, has_item(has_property("message", contains_string('Type of "first(1)" is "int"')))) + assert_that(before_findings, has_item(has_property("rule", "reportAssignmentType"))) + assert_that(_type_check(after_path), equal_to(before_findings)) + values = (1, "a", [1]) + assert_that( + [_import(after_path).first(value) for value in values], + equal_to([_import(before_path).first(value) for value in values]), + ) + + +class TestVarianceOfClasses: + """A generic class's variance changes what the type checker accepts, so only functions may lose it.""" + + @pytest.mark.parametrize( + ("class_definition", "class_header"), + [ + pytest.param(_PROTOCOL, "class Reader(Protocol[T_co]):", id="protocol"), + pytest.param(_GENERIC_CLASS, "class Reader(Generic[T_co]):", id="generic-class"), + ], + ) + def test_class_keeps_its_variance_and_dropping_it_changes_the_type_checking( + self, + create_type_parameter_check: Callable[[str], TypeParameterCheck], + tmp_path: Path, + class_definition: str, + class_header: str, + ) -> None: + """Verify the class and its covariant declaration stay, the code still type checks, and dropping the variance does not.""" + covariant = _CLASS_TEMPLATE.format(covariant=", covariant=True", class_definition=class_definition) + subject = create_type_parameter_check(covariant) + + result = subject.convert_declared_typevars() + + converted = subject.apply_to_string() + assert_that(result, equal_to({"T_co": "fixed"})) + assert_that(subject.converted_unsafe_reasons, equal_to({"T_co": UnsafeReason.USED_IN_GENERIC_CLASS})) + assert_that(converted, contains_string("def read_twice[T_co](reader: Reader[T_co])")) + assert_that(converted, contains_string('T_co = TypeVar("T_co", covariant=True)')) + assert_that(converted, contains_string(class_header)) + original_findings = _type_check(_write(covariant, tmp_path / "original.py")) + assert_that(original_findings, has_item(has_property("message", contains_string('Type of "wider" is "Reader[int]"')))) + assert_that(_type_check(_write(converted, tmp_path / "converted.py")), equal_to(original_findings)) + assert_that([finding.rule for finding in original_findings], not_(has_item("reportAssignmentType"))) + + invariant = converted.replace(", covariant=True", "") + invariant_rules = [finding.rule for finding in _type_check(_write(invariant, tmp_path / "invariant.py"))] + assert_that(invariant_rules, has_item("reportAssignmentType")) diff --git a/test/recipes/test_type_parameter_declarations.py b/test/recipes/test_type_parameter_declarations.py new file mode 100644 index 00000000..3728d1dc --- /dev/null +++ b/test/recipes/test_type_parameter_declarations.py @@ -0,0 +1,38 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for renaissance.recipes.type_parameter_declarations.""" + +from typing import cast + +import libcst as cst +import pytest +from hamcrest import assert_that, equal_to + +from renaissance.recipes.type_parameter_declarations import declaration_argument_names + + +class TestDeclarationArgumentNames: + """See module docstring.""" + + @pytest.mark.parametrize( + ("declaration", "expected"), + [ + pytest.param('T = TypeVar("T")', set[str](), id="the-type-parameters-own-name-is-skipped"), + pytest.param('T = TypeVar("T", bound=Shape)', {"Shape"}, id="keyword-argument"), + pytest.param('T = TypeVar("T", Shape, Other)', {"Shape", "Other"}, id="constraints"), + pytest.param('T = TypeVar("T", bound=np.integer)', {"np"}, id="attribute-reads-only-its-root"), + pytest.param('T = TypeVar("T", bound="Shape")', {"Shape"}, id="string-is-a-forward-reference"), + pytest.param('T = TypeVar("T", bound="not valid(")', set[str](), id="string-that-is-not-an-expression"), + pytest.param('T = TypeVar("T", int, str)', set[str](), id="built-ins-are-left-out"), + pytest.param('T = TypeVar("T", **options)', {"options"}, id="mapping-argument"), + ], + ) + def test_returns_the_non_builtin_names_the_arguments_use(self, declaration: str, expected: set[str]) -> None: + """Verify the names the arguments read are returned, with the declared name and the built-ins left out.""" + line = cast("cst.SimpleStatementLine", cst.parse_statement(declaration)) + + assert_that(declaration_argument_names(cast("cst.Assign", line.body[0])), equal_to(expected)) diff --git a/test/recipes/test_type_parameter_localization.py b/test/recipes/test_type_parameter_localization.py new file mode 100644 index 00000000..b1320d1a --- /dev/null +++ b/test/recipes/test_type_parameter_localization.py @@ -0,0 +1,384 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for renaissance.recipes.type_parameter_localization.plan_localizations.""" + +import ast +import builtins +import textwrap +from pathlib import Path +from typing import cast + +import libcst as cst +import pytest +from hamcrest import assert_that, equal_to, has_entry, has_item, has_length, not_ + +from renaissance.recipes.type_parameter_localization import LocalizationPlan, plan_localizations +from renaissance.recipes.type_parameter_rules import UnsafeReason + + +def _module_level_names(module: ast.Module) -> set[str]: + """Return the names bound at module level by imports, classes, functions and assignments.""" + names: set[str] = set() + for node in module.body: + if isinstance(node, ast.Import | ast.ImportFrom): + names.update((alias.asname or alias.name).split(".")[0] for alias in node.names) + elif isinstance(node, ast.ClassDef | ast.FunctionDef | ast.AsyncFunctionDef): + names.add(node.name) + elif isinstance(node, ast.Assign): + names.update(target.id for target in node.targets if isinstance(target, ast.Name)) + return names + + +def _names_used_by(call: ast.Call) -> set[str]: + """Return the non-builtin names a TypeVar call's arguments use, including inside string forward references.""" + names: set[str] = set() + for argument in [*call.args[1:], *(keyword.value for keyword in call.keywords)]: + expression = argument + if isinstance(argument, ast.Constant) and isinstance(argument.value, str): + expression = ast.parse(argument.value, mode="eval").body + names.update(node.id for node in ast.walk(expression) if isinstance(node, ast.Name)) + return names - set(dir(builtins)) + + +def _write_origin(tmp_path: Path, origin_text: str, origin_rel_path: str) -> None: + """Write origin_text to origin_rel_path, relative to tmp_path, which is the project root.""" + origin_file = tmp_path / origin_rel_path + origin_file.parent.mkdir(parents=True, exist_ok=True) + origin_file.write_text(textwrap.dedent(origin_text)) + + +def _plan( + tmp_path: Path, + origin_text: str, + importing_text: str, + layout: tuple[str, str] = ("file_1.py", "file_2.py"), +) -> LocalizationPlan: + """Write origin_text to the origin file and return the plan for importing_text, whose file is not written. + + layout gives the (origin, importing) file paths, relative to tmp_path, which is the project root. + """ + origin_rel_path, importing_rel_path = layout + _write_origin(tmp_path, origin_text, origin_rel_path) + module = cst.parse_module(textwrap.dedent(importing_text)) + return plan_localizations(module, tmp_path / importing_rel_path, tmp_path, localize_imports=True, ignore_origins=frozenset()) + + +class TestPlanLocalizations: + """See module docstring.""" + + @pytest.mark.parametrize( + "origin_extra", + [ + pytest.param("def a(x: T) -> T:\n return x", id="plain-origin"), + pytest.param('__all__ = ["T"]', id="origin-exports-it"), + pytest.param("class Box(Generic[T]):\n pass", id="origin-class-is-generic-over-it"), + ], + ) + def test_localizes_typevar_whatever_its_origin_does_with_it(self, tmp_path: Path, origin_extra: str) -> None: + """Verify an imported TypeVar is localized, also when its origin exports it or has a class generic over it.""" + origin = f"from typing import Generic, TypeVar\nT = TypeVar('T')\n{origin_extra}\n" + plan = _plan(tmp_path, origin, "from file_1 import T\ndef b(x: T) -> T:\n return x\n") + + assert_that(plan.results, equal_to({"T": "fixed"})) + assert_that(plan.localizations, has_length(1)) + assert_that(plan.localizations[0].declarations, equal_to(["T = TypeVar('T')"])) + assert_that(plan.localizations[0].kept, equal_to([])) + + @pytest.mark.parametrize( + ("origin_ignored", "localize_imports", "localized"), + [ + pytest.param(True, True, False, id="ignored-origin"), + pytest.param(False, False, False, id="localization-off"), + pytest.param(False, True, True, id="origin-not-ignored"), + ], + ) + def test_names_are_not_localized_from_an_ignored_origin_or_when_localization_is_off( + self, + tmp_path: Path, + *, + origin_ignored: bool, + localize_imports: bool, + localized: bool, + ) -> None: + """Verify an import from an ignored origin, or any import with localize_imports off, is kept and recorded as ignored.""" + _write_origin(tmp_path, "from typing import TypeVar\nT = TypeVar('T')\n", "file_1.py") + plan = plan_localizations( + cst.parse_module("from file_1 import T\ndef b(x: T) -> T:\n return x\n"), + tmp_path / "file_2.py", + tmp_path, + localize_imports=localize_imports, + ignore_origins=frozenset({tmp_path / "file_1.py"}) if origin_ignored else frozenset(), + ) + + assert_that(plan.results, equal_to({"T": "fixed"} if localized else {})) + assert_that(plan.ignored_names, equal_to(set[str]() if localized else {"T"})) + assert_that(plan.localizations, has_length(1 if localized else 0)) + + @pytest.mark.parametrize( + "declaration", + [ + pytest.param('T = TypeVar("T", bound=Shape)', id="invariant-bound"), + pytest.param('T = TypeVar("T", int, Shape)', id="constraints"), + pytest.param('T = TypeVar("T", bound=Shape, covariant=True)', id="covariant"), + pytest.param('T = TypeVar("T", bound=Shape, contravariant=True)', id="contravariant"), + pytest.param('T = TypeVar("T", bound="Shape")', id="string-bound"), + ], + ) + def test_localized_declaration_keeps_its_meaning(self, tmp_path: Path, declaration: str) -> None: + """Verify the local copy has the origin's exact arguments and every name they use is defined by the imports it brings.""" + origin = f"from typing import TypeVar\nclass Shape:\n pass\n{declaration}\n" + plan = _plan(tmp_path, origin, "from file_1 import T\ndef biggest(items: list[T]) -> T:\n return items[0]\n") + + assert_that(plan.results, equal_to({"T": "fixed"})) + written = ast.parse("\n".join([*plan.localizations[0].imports, *plan.localizations[0].declarations])) + local_call = next(node.value for node in written.body if isinstance(node, ast.Assign)) + origin_call = cast("ast.Assign", ast.parse(origin).body[-1]).value + assert_that(ast.dump(local_call), equal_to(ast.dump(origin_call))) + assert_that(_names_used_by(cast("ast.Call", local_call)) - _module_level_names(written), equal_to(set())) + + @pytest.mark.parametrize( + ("origin_imports", "expected_line"), + [ + pytest.param("from typing import Any", "from typing import Any", id="absolute-module"), + pytest.param("from ._shapes import Shape", "from sub._shapes import Shape", id="relative-module-in-origin"), + pytest.param("from shapes import Base as Shape", "from shapes import Base as Shape", id="aliased-in-origin"), + ], + ) + def test_names_a_declaration_uses_are_imported_from_where_the_origin_gets_them( + self, + tmp_path: Path, + origin_imports: str, + expected_line: str, + ) -> None: + """Verify a name the declaration uses is imported from the module the origin imports it from, not from the origin.""" + name = expected_line.rsplit(" ", maxsplit=1)[-1] + plan = _plan( + tmp_path, + f'from typing import TypeVar\n{origin_imports}\nT = TypeVar("T", bound={name})\n', + "from sub.file_1 import T\ndef b(x: T) -> T:\n return x\n", + ("sub/file_1.py", "use.py"), + ) + + assert_that(plan.results, equal_to({"T": "fixed"})) + imports = plan.localizations[0].imports + assert_that(imports.count(expected_line), equal_to(1)) + assert_that(imports, not_(has_item(f"from sub.file_1 import {name}"))) + + @pytest.mark.parametrize( + ("origin", "importing_extra"), + [ + pytest.param( + 'from typing import TypeVar\nclass Shape:\n pass\nT = TypeVar("T", bound=Shape)\n', + "class Shape:\n pass", + id="name-means-something-else-here", + ), + pytest.param( + "from typing import TYPE_CHECKING, TypeVar\n" + "if TYPE_CHECKING:\n" + " from shapes import Shape\n" + 'T = TypeVar("T", bound="Shape")\n', + "", + id="origin-binds-it-only-for-type-checking", + ), + ], + ) + def test_does_not_localize_when_a_name_the_declaration_uses_cant_be_imported_safely( + self, tmp_path: Path, origin: str, importing_extra: str + ) -> None: + """Verify a declaration using a name that isn't available at module level as the same object stays imported.""" + plan = _plan( + tmp_path, + origin, + f"from file_1 import T\n{importing_extra}\ndef biggest(items: list[T]) -> T:\n return items[0]\n", + ) + + assert_that(plan.results, equal_to({"T": "unsafe"})) + assert_that(plan.unsafe_reasons, equal_to({"T": UnsafeReason.NAME_NOT_AT_MODULE_LEVEL})) + assert_that(plan.localizations, has_length(0)) + + @pytest.mark.parametrize( + ("origin_import", "importing_header", "expected_imports"), + [ + pytest.param("from typing import TypeVar", "", ["from typing import TypeVar"], id="missing-here"), + pytest.param("from typing import TypeVar", 'from typing import TypeVar\nU = TypeVar("U")', [], id="already-here"), + # The origin has no from-import of "TypeVar"; only safe because this file imports it. + pytest.param("from typing import *", "from typing import TypeVar", [], id="origin-wildcard-import"), + ], + ) + def test_localized_file_imports_the_constructor_exactly_once( + self, + tmp_path: Path, + origin_import: str, + importing_header: str, + expected_imports: list[str], + ) -> None: + """Verify localizing adds "from typing import TypeVar" when missing and never when the file already has it.""" + plan = _plan( + tmp_path, + f'{origin_import}\nT = TypeVar("T")\ndef a(x: T) -> T:\n return x\n', + f"{importing_header}\nfrom file_1 import T\ndef b(x: T) -> T:\n return x\n", + ) + + assert_that(plan.results, has_entry("T", "fixed")) + assert_that(plan.localizations[0].imports, equal_to(expected_imports)) + + @pytest.mark.parametrize( + ("origin_declaration", "expected_comment_lines"), + [ + pytest.param('T = TypeVar("T") # noqa: PLC0105', ['T = TypeVar("T") # noqa: PLC0105'], id="noqa"), + pytest.param( + 'T = TypeVar(\n "T",\n bound=int,\n) # noqa: PLC0105', + [") # noqa: PLC0105"], + id="multi-line-declaration", + ), + pytest.param('T = TypeVar("T"); U = 1 # unrelated', [], id="shares-line-with-another-statement"), + ], + ) + def test_localized_declaration_keeps_the_comment_trailing_it( + self, + tmp_path: Path, + origin_declaration: str, + expected_comment_lines: list[str], + ) -> None: + """Verify the comment trailing a declaration's line is copied with it, unless the line holds another statement.""" + plan = _plan( + tmp_path, + f"from typing import TypeVar\n{origin_declaration}\n", + "from file_1 import T\ndef b(x: T) -> T:\n return x\n", + ) + + assert_that(plan.results, equal_to({"T": "fixed"})) + written = "\n".join(plan.localizations[0].declarations) + assert_that([line for line in written.splitlines() if "#" in line], equal_to(expected_comment_lines)) + + @pytest.mark.parametrize( + ("layout", "importing_header", "expected_imports"), + [ + pytest.param(("file_1.py", "file_2.py"), "from .file_1 import T", ["from ._compat import TypeVar"], id="same-dir"), + pytest.param( + ("sub/file_1.py", "use.py"), + "from .sub.file_1 import T", + ["from .sub._compat import TypeVar"], + id="origin-in-subpackage", + ), + pytest.param( + ("file_1.py", "sub/use.py"), + "from ..file_1 import T", + ["from .._compat import TypeVar"], + id="importing-in-subpackage", + ), + pytest.param(("sub/file_1.py", "use.py"), "from sub.file_1 import T", ["from sub._compat import TypeVar"], id="absolute-via"), + pytest.param( + ("file_1.py", "file_2.py"), + "from ._compat import TypeVar\nfrom .file_1 import T", + [], + id="already-here", + ), + # A top-level origin's `._compat` names no package from here, so the origin itself provides TypeVar. + pytest.param(("file_1.py", "use.py"), "from file_1 import T", ["from file_1 import TypeVar"], id="not-rebasable"), + ], + ) + def test_localized_file_imports_a_relatively_imported_constructor_from_the_same_module( + self, + tmp_path: Path, + layout: tuple[str, str], + importing_header: str, + expected_imports: list[str], + ) -> None: + """Verify a constructor the origin imports relatively is imported once, from that same module as seen from here.""" + plan = _plan( + tmp_path, + 'from ._compat import TypeVar\nT = TypeVar("T")\n', + f"{importing_header}\ndef b(x: T) -> T:\n return x\n", + layout, + ) + + assert_that(plan.results, has_entry("T", "fixed")) + assert_that(plan.localizations[0].imports, equal_to(expected_imports)) + + @pytest.mark.parametrize( + "origin_imports", + [ + pytest.param( + """ + import sys + if sys.version_info >= (3, 13): + from typing import TypeVar + else: + from typing_extensions import TypeVar + """, + id="python-version", + ), + pytest.param( + """ + from typing import TYPE_CHECKING + if TYPE_CHECKING: + from typing_extensions import TypeVar + else: + from typing import TypeVar + """, + id="type-checking", + ), + ], + ) + def test_does_not_localize_when_origin_imports_constructor_conditionally( + self, + tmp_path: Path, + origin_imports: str, + ) -> None: + """Verify a TypeVar whose origin imports TypeVar inside an `if` block stays imported, marked unsafe.""" + plan = _plan( + tmp_path, + textwrap.dedent(origin_imports) + 'T = TypeVar("T", contravariant=True, default=None)\n', + """ + from typing import TypeVar + from file_1 import T, helper + def b(x: T) -> None: + helper() + """, + ) + + assert_that(plan.results, has_entry("T", "unsafe")) + assert_that(plan.unsafe_reasons, has_entry("T", UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY)) + assert_that(plan.localizations, has_length(0)) + + def test_unparsable_origin_error_names_the_origin_file(self, tmp_path: Path) -> None: + """Verify a syntax error in the origin module is raised with the origin file's path, not ``.""" + with pytest.raises(SyntaxError) as excinfo: + _plan( + tmp_path, + """ + from typing import TypeVar + T = TypeVar("T") + def broken(: + """, + """ + from file_1 import T + def b(x: T) -> T: + return x + """, + ) + + assert_that(excinfo.value.filename, equal_to(str(tmp_path / "file_1.py"))) + + def test_no_typevar_import_found(self, tmp_path: Path) -> None: + """Verify nothing is planned when the importing file has no cross-file TypeVar.""" + plan = _plan( + tmp_path, + """ + def helper() -> None: + pass + """, + """ + from file_1 import helper + def b() -> None: + helper() + """, + ) + + assert_that(plan.results, equal_to({})) + assert_that(plan.localizations, has_length(0)) diff --git a/test/recipes/test_type_parameter_rules.py b/test/recipes/test_type_parameter_rules.py new file mode 100644 index 00000000..4ae03299 --- /dev/null +++ b/test/recipes/test_type_parameter_rules.py @@ -0,0 +1,170 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for type_parameter_rules's safety predicates and their UnsafeReason results.""" + +import textwrap +from collections import Counter +from typing import cast + +import libcst as cst +import pytest +from hamcrest import assert_that, equal_to + +from renaissance.recipes.type_parameter_rules import ( + CONVERSION_RULES, + LOCALIZATION_RULES, + PEP_696_MINIMUM, + REMOVAL_RULES, + UnsafeReason, + conversion_refusal, + is_safe_to_localize, + is_safe_to_remove, +) + + +def _parse(source: str) -> cst.Module: + """Parse source after removing its common indentation.""" + return cst.parse_module(textwrap.dedent(source)) + + +class TestIsSafeToRemove: + """is_safe_to_remove: None when safe, the specific UnsafeReason otherwise.""" + + def test_returns_none_when_safe(self) -> None: + """A TypeVar not exported and not imported by another project file is safe to remove, whatever else is.""" + tree = _parse(""" + from typing import TypeVar + + def a(x: T) -> T: + return x + + T = TypeVar("T") + """) + + assert_that(is_safe_to_remove(tree, "T", frozenset({"U"})), equal_to(None)) + + @pytest.mark.parametrize( + ("dunder_all", "imported_elsewhere", "expected_reason"), + [ + pytest.param('__all__ = ["T"]', frozenset[str](), UnsafeReason.DECLARED_TYPEVAR_EXPORTED, id="exported-via-dunder-all"), + pytest.param("", frozenset({"T"}), UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT, id="imported-elsewhere"), + ], + ) + def test_returns_the_specific_reason_when_unsafe( + self, dunder_all: str, imported_elsewhere: frozenset[str], expected_reason: UnsafeReason + ) -> None: + """Each unsafe condition is distinguishable, not collapsed into one generic reason.""" + tree = _parse(f""" + from typing import TypeVar + + {dunder_all} + + def a(x: T) -> T: + return x + + T = TypeVar("T") + """) + + assert_that(is_safe_to_remove(tree, "T", imported_elsewhere), equal_to(expected_reason)) + + +class TestIsSafeToLocalize: + """is_safe_to_localize: None when safe, the specific UnsafeReason otherwise.""" + + @pytest.mark.parametrize( + "source", + [ + pytest.param('from typing import TypeVar\nT = TypeVar("T")\n', id="unconditional-import"), + pytest.param( + "from typing import TYPE_CHECKING, TypeVar\n" + "if TYPE_CHECKING:\n" + " from collections.abc import Sequence\n" + 'T = TypeVar("T")\n', + id="conditional-import-of-another-name", + ), + ], + ) + def test_returns_none_when_safe(self, source: str) -> None: + """A TypeVar whose constructor its origin imports unconditionally is safe to localize, whatever else is conditional.""" + assert_that(is_safe_to_localize(_parse(source), "T"), equal_to(None)) + + @pytest.mark.parametrize( + ("source", "expected_reason"), + [ + pytest.param( + """ + import sys + + if sys.version_info >= (3, 13): + from typing import TypeVar + else: + from typing_extensions import TypeVar + + T = TypeVar("T", default=None) + """, + UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY, + id="if-version-check", + ), + pytest.param( + """ + try: + from typing import ParamSpec + except ImportError: + from typing_extensions import ParamSpec + + T = ParamSpec("T") + """, + UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY, + id="try-except-import", + ), + ], + ) + def test_returns_the_specific_reason_when_unsafe(self, source: str, expected_reason: UnsafeReason) -> None: + """Each unsafe condition is distinguishable, not collapsed into one generic reason.""" + tree = _parse(source) + + assert_that(is_safe_to_localize(tree, "T"), equal_to(expected_reason)) + + +class TestConversionRefusal: + """See module docstring.""" + + @pytest.mark.parametrize( + ("declaration", "min_python", "expected"), + [ + pytest.param('T = TypeVar("T")', (3, 12), None, id="plain-declaration"), + pytest.param('T = TypeVar("T", default=int)', (3, 12), UnsafeReason.PEP696_VERSION_GATE, id="default-below-3.13"), + pytest.param('T = TypeVar("T", default=int)', PEP_696_MINIMUM, None, id="default-from-3.13"), + pytest.param('P = ParamSpec("P", bound=int)', (3, 12), UnsafeReason.NO_PEP695_EQUIVALENT, id="bound-on-a-param-spec"), + pytest.param( + 'T = TypeVar("T", default=int, **options)', + (3, 12), + UnsafeReason.NO_PEP695_EQUIVALENT, + id="the-first-registered-rule-wins", + ), + ], + ) + def test_returns_none_when_convertible_else_the_first_reason( + self, + declaration: str, + min_python: tuple[int, int], + expected: UnsafeReason | None, + ) -> None: + """Verify a declaration is refused only for a reason that applies to it, and the first registered reason wins.""" + statement = cast("cst.SimpleStatementLine", cst.parse_statement(declaration)) + + assert_that(conversion_refusal(cast("cst.Assign", statement.body[0]), min_python), equal_to(expected)) + + +class TestRefusalRuleRegistry: + """See module docstring.""" + + def test_no_reason_is_registered_twice(self) -> None: + """Verify each UnsafeReason is enforced by at most one rule, so a report never has two rules for one reason.""" + registered = [rule.reason for rules in (CONVERSION_RULES, REMOVAL_RULES, LOCALIZATION_RULES) for rule in rules] + + assert_that(max(Counter(registered).values()), equal_to(1)) diff --git a/test/rejuvenation/__init__.py b/test/rejuvenation/__init__.py new file mode 100644 index 00000000..fab4784b --- /dev/null +++ b/test/rejuvenation/__init__.py @@ -0,0 +1,7 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for the command-line scripts in src/rejuvenation.""" diff --git a/test/rejuvenation/test_migration_type_recipes.py b/test/rejuvenation/test_migration_type_recipes.py new file mode 100644 index 00000000..6375f982 --- /dev/null +++ b/test/rejuvenation/test_migration_type_recipes.py @@ -0,0 +1,727 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for the migration_type_recipes.py CLI script (src/rejuvenation).""" + +import ast +import codecs +import textwrap +from pathlib import Path + +import pytest +from hamcrest import assert_that, contains_string, equal_to, not_ +from hamcrest.core.matcher import Matcher + +from rejuvenation import migration_type_recipes as migration +from renaissance.recipes.type_parameter_rules import DOCS_BASE_URL, UNSAFE_RULES, UnsafeReason + +LEGACY_TYPEVAR_SOURCE = textwrap.dedent("""\ + from typing import TypeVar + + T = TypeVar("T") + + + def identity(x: T) -> T: + return x + """) + +UNSAFE_TYPEVAR_SOURCE = textwrap.dedent("""\ + from typing import TypeVar + + T = TypeVar("T") + + __all__ = ["T"] + + + def identity(x: T) -> T: + return x + """) + + +class TestProcessFile: + """process_file: writes changes for real, and isolates per-file errors.""" + + def test_converts_function_but_keeps_a_declaration_imported_elsewhere(self, tmp_path: Path) -> None: + """The function is converted on disk, the declaration stays, and the report says why it stayed.""" + target = tmp_path / "mod.py" + target.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + + report = migration.process_file(target, min_python=(3, 12), project_root=tmp_path, project_wide_imported_names=frozenset({"T"})) + + assert_that(migration.has_fixed(report), equal_to(obj=True)) + reasons = report.reasons or {} + assert_that(reasons.get("orphaned"), equal_to({"T": UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT})) + written = target.read_text(encoding="utf-8") + assert_that(written, contains_string("def identity[T](x: T) -> T:")) + assert_that(written, contains_string('T = TypeVar("T")')) + + def test_syntax_error_reported_as_error_not_raised(self, tmp_path: Path) -> None: + """A file that fails to parse is reported on FileReport.error, not raised.""" + target = tmp_path / "broken.py" + target.write_text("def broken(:\n", encoding="utf-8") + + report = migration.process_file(target, min_python=(3, 12), project_root=tmp_path, project_wide_imported_names=frozenset[str]()) + + assert_that(report.error, not_(None)) + assert_that(report.result, equal_to(None)) + + +class TestRuffCleanup: + """main(): the final ruff pass cleans up the imports and type parameter names of the modified files.""" + + @pytest.mark.parametrize( + ("extra_args", "import_matcher"), + [ + pytest.param([], not_(contains_string("TypeVar")), id="default-drops-import"), + pytest.param(["--no-ruff"], contains_string("from typing import TypeVar"), id="no-ruff-keeps-import"), + ], + ) + def test_unused_typevar_import_cleanup( + self, + tmp_path: Path, + extra_args: list[str], + import_matcher: Matcher[str], + ) -> None: + """The redundant TypeVar import is dropped unless --no-ruff; conversion runs either way.""" + target = tmp_path / "mod.py" + target.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + + exit_code = migration.main([str(target), "--py", "3.12", *extra_args]) + + assert_that(exit_code, equal_to(0)) + written = target.read_text(encoding="utf-8") + assert_that(written, contains_string("def identity[T]")) + assert_that(written, import_matcher) + + @pytest.mark.parametrize( + ("source", "signature"), + [ + pytest.param( + textwrap.dedent("""\ + from __future__ import annotations + + from typing import Generic, TypeVar + + _T = TypeVar("_T") + + + def first(items: list[_T]) -> _T: + return items[0] + + + class Box(Generic[_T]): + pass + """), + "def first[_T](items: list[_T]) -> _T:", + id="declaration-kept-by-generic-class", + ), + ], + ) + def test_converted_signature_keeps_its_names(self, tmp_path: Path, source: str, signature: str) -> None: + """After the ruff pass, a declared type parameter and its uses in the signature have the same name.""" + target = tmp_path / "mod.py" + target.write_text(source, encoding="utf-8") + + exit_code = migration.main([str(target), "--py", "3.12"]) + + assert_that(exit_code, equal_to(0)) + assert_that(target.read_text(encoding="utf-8"), contains_string(signature)) + + def test_localized_imports_are_merged_with_existing_ones(self, tmp_path: Path) -> None: + """The localized TypeVar's import joins the file's own `typing` import.""" + (tmp_path / "xarray_types.py").write_text( + 'from typing import TypeVar\n\nJoinOptions = str\nAlignable = int\nT = TypeVar("T")\n', + encoding="utf-8", + ) + target = tmp_path / "use.py" + target.write_text( + textwrap.dedent("""\ + from typing import Any + from xarray_types import JoinOptions + from xarray_types import T + from xarray_types import Alignable + + + def pick(x: T, how: JoinOptions, other: Alignable) -> Any: + return x + + + class Items(list[T]): + pass + """), + encoding="utf-8", + ) + + exit_code = migration.main([str(target), "--py", "3.12"]) + + assert_that(exit_code, equal_to(0)) + lines = target.read_text(encoding="utf-8").splitlines() + for import_line in ["from typing import Any, TypeVar", "from xarray_types import Alignable, JoinOptions"]: + assert_that(lines.count(import_line), equal_to(1), import_line) + + def test_unmodified_sibling_file_is_left_untouched(self, tmp_path: Path) -> None: + """A sibling file with no TypeVar usage - and its own genuinely-unused import - survives main() byte-for-byte.""" + (tmp_path / "mod.py").write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + sibling = tmp_path / "sibling.py" + sibling_source = textwrap.dedent("""\ + import os + + + def greet() -> str: + return "hi" + """) + sibling.write_text(sibling_source, encoding="utf-8") + + exit_code = migration.main([str(tmp_path), "--py", "3.12"]) + + assert_that(exit_code, equal_to(0)) + assert_that(sibling.read_text(encoding="utf-8"), equal_to(sibling_source)) + + +class TestConsoleReportNeedsReviewSection: + """main(): what the NEEDS MANUAL REVIEW section lists per reason.""" + + def test_names_are_listed_per_file_under_their_reason_and_the_documentation_is_linked_once( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """Each reason is a heading with its files and names below it; the documentation is linked once, without per-name anchors.""" + target = tmp_path / "mod.py" + target.write_text(UNSAFE_TYPEVAR_SOURCE, encoding="utf-8") + + migration.main([str(target), "--py", "3.12"]) + + output = capsys.readouterr().out + assert_that(output.count(DOCS_BASE_URL), equal_to(1)) + assert_that(output, not_(contains_string("#feature-"))) + assert_that(output, contains_string("**Exported via __all__**\n `mod.py`: `T`\n")) + assert_that(output, not_(contains_string(" orphaned:"))) + + def test_files_and_names_sharing_a_reason_are_listed_under_one_heading( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """A reason is printed once, with every file it applies to, and a file lists all its names on one line.""" + (tmp_path / "one.py").write_text(UNSAFE_TYPEVAR_SOURCE, encoding="utf-8") + (tmp_path / "two.py").write_text( + textwrap.dedent("""\ + from typing import TypeVar + + U = TypeVar("U") + + __all__ = ["U"] + + + def identity(x: U) -> U: + return x + """), + encoding="utf-8", + ) + (tmp_path / "three.py").write_text( + textwrap.dedent("""\ + from typing import TypeVar + + A = TypeVar("A") + B = TypeVar("B") + + __all__ = ["A", "B"] + + + def pair(a: A, b: B) -> tuple[A, B]: + return a, b + """), + encoding="utf-8", + ) + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + output = capsys.readouterr().out + assert_that(output.count("**Exported via __all__**"), equal_to(1)) + for line in [" `one.py`: `T`\n", " `three.py`: `A`, `B`\n", " `two.py`: `U`\n"]: + assert_that(output, contains_string(line)) + + def test_name_converted_in_functions_but_still_used_by_a_generic_class_is_in_both_sections( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """A name converted in some functions that a generic class still uses is listed as modified and as needing review.""" + target = tmp_path / "mod.py" + target.write_text( + textwrap.dedent("""\ + from typing import Generic, TypeVar + + T = TypeVar("T") + + + def first(items: list[T]) -> T: + return items[0] + + + class Box(Generic[T]): + def get(self, x: T) -> T: + return x + """), + encoding="utf-8", + ) + + migration.main([str(target), "--py", "3.12", "--no-ruff"]) + + output = capsys.readouterr().out + modified, needs_review = output.split("**NEEDS MANUAL REVIEW (") + assert_that(output, contains_string("1 modified, 1 need manual review")) + assert_that(modified, contains_string(" `T`: converted to PEP 695 in functions\n")) + assert_that(needs_review, contains_string("**Only used inside a class that is generic over it**\n `mod.py`: `T`\n")) + + +class TestConsoleReportModifiedSection: + """main(): what the MODIFIED section lists per file.""" + + def test_converted_name_is_not_listed_again_as_removed(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """A converted name is listed once; only declarations removed without a conversion are listed as removed.""" + target = tmp_path / "mod.py" + target.write_text( + textwrap.dedent("""\ + from typing import TypeVar + + T = TypeVar("T") + U = TypeVar("U") + + + def f(x: T) -> T: + return x + + + def g[U](y: U) -> U: + return y + """), + encoding="utf-8", + ) + + migration.main([str(target), "--py", "3.12", "--no-ruff"]) + + output = capsys.readouterr().out + assert_that(output, contains_string(" `T`: converted to PEP 695 in functions\n")) + assert_that(output, not_(contains_string(" `T`: unused declaration removed"))) + assert_that(output, contains_string(" `U`: unused declaration removed\n")) + + +def _table_rows(output: str, first_header: str) -> list[list[str]]: + """Return the data rows, as cells without padding or backticks, of the printed Markdown table whose header starts with first_header.""" + rows = [[cell.strip().strip("`") for cell in line.strip("|").split("|")] for line in output.splitlines() if line.startswith("|")] + start = next(index for index, row in enumerate(rows) if row[0] == first_header) + table: list[list[str]] = [] + for row in rows[start + 2 :]: + if row[0] in {"Kind", "Reason"}: + break + table.append(row) + return table + + +class TestSummaryTables: + """main(): the summary tables at the top of the printed report.""" + + def test_counts_per_kind_and_reasons_by_frequency(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """A name counts once per file, needs review if it has a reason, errored files are skipped, reasons are ranked.""" + (tmp_path / "fixed.py").write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + (tmp_path / "exported_1.py").write_text(UNSAFE_TYPEVAR_SOURCE, encoding="utf-8") + (tmp_path / "exported_2.py").write_text(UNSAFE_TYPEVAR_SOURCE, encoding="utf-8") + (tmp_path / "bound_paramspec.py").write_text( + textwrap.dedent("""\ + from collections.abc import Callable + from typing import ParamSpec + + P = ParamSpec("P", bound=int) + + + def wrap(f: Callable[P, int]) -> Callable[P, int]: + return f + """), + encoding="utf-8", + ) + (tmp_path / "broken.py").write_text("def broken(:\n", encoding="utf-8") + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + output = capsys.readouterr().out + assert_that( + _table_rows(output, "Kind"), + equal_to([["TypeVar", "3", "1", "2"], ["ParamSpec", "1", "0", "1"], ["TypeVarTuple", "0", "0", "0"]]), + ) + assert_that( + _table_rows(output, "Reason"), + equal_to( + [ + [UNSAFE_RULES[UnsafeReason.DECLARED_TYPEVAR_EXPORTED].message, "2"], + [UNSAFE_RULES[UnsafeReason.NO_PEP695_EQUIVALENT].message, "1"], + ], + ), + ) + + +_COMMIT_SUBJECT = "Modernize TypeVar/ParamSpec/TypeVarTuple usage to PEP 695 syntax" +_DEAD_TYPEVAR_SOURCE = 'from typing import TypeVar\n\nT_FreqStr = TypeVar("T_FreqStr")\n' +_TWELVE_TYPEVARS_SOURCE = "from typing import TypeVar\n\n" + "".join( + f'T{n:02} = TypeVar("T{n:02}")\n\n\ndef f{n:02}(x: T{n:02}) -> T{n:02}:\n return x\n\n\n' for n in range(1, 13) +) + + +class TestCommitSummary: + """main(): the commit-style message printed after the report.""" + + @pytest.mark.parametrize( + ("files", "expected"), + [ + pytest.param( + {"a.py": LEGACY_TYPEVAR_SOURCE}, + "Convert type parameters to PEP 695 syntax in 1 file (Python 3.12+).\n\n- converted to PEP 695 in functions: T", + id="converted-only", + ), + pytest.param( + {"a.py": LEGACY_TYPEVAR_SOURCE, "b.py": LEGACY_TYPEVAR_SOURCE}, + "Convert type parameters to PEP 695 syntax in 2 files (Python 3.12+).\n\n- converted to PEP 695 in functions: T (2 files)", + id="name-counted-per-file", + ), + pytest.param( + {"a.py": UNSAFE_TYPEVAR_SOURCE, "b.py": _DEAD_TYPEVAR_SOURCE}, + "Convert type parameters to PEP 695 syntax in 2 files (Python 3.12+).\n\n" + "- converted to PEP 695 in functions: T\n" + "- unused declaration removed: T_FreqStr\n\n" + f"1 file left for manual review: {UNSAFE_RULES[UnsafeReason.DECLARED_TYPEVAR_EXPORTED].message} (1).", + id="removed-and-left-for-review", + ), + pytest.param( + {"a.py": _TWELVE_TYPEVARS_SOURCE}, + "Convert type parameters to PEP 695 syntax in 1 file (Python 3.12+).\n\n" + "- converted to PEP 695 in functions: T01, T02, T03, T04, T05, T06, T07, T08, T09, T10 and 2 more", + id="long-list-is-capped", + ), + ], + ) + def test_lists_what_changed_per_action_and_what_is_left_for_review( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + files: dict[str, str], + expected: str, + ) -> None: + """Names are grouped by what was done to them, with the files they were fixed in; leftover review is summarised by reason.""" + for name, source in files.items(): + (tmp_path / name).write_text(source, encoding="utf-8") + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + summary = capsys.readouterr().out.split(_COMMIT_SUBJECT, 1)[1].strip() + assert_that(summary, equal_to(expected)) + + def test_no_message_when_no_file_was_modified(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """A run that changes nothing prints no commit message.""" + (tmp_path / "a.py").write_text("x = 1\n", encoding="utf-8") + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + assert_that(capsys.readouterr().out, not_(contains_string(_COMMIT_SUBJECT))) + + +class TestIgnore: + """main(): --ignore leaves a shared declaration file and the imports of its names untouched, and reports them.""" + + def test_ignored_file_and_its_importer_are_left_untouched_and_the_report_lists_it( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """Neither file changes, the ignored one is listed but not checked, and its name counts as reported and ignored.""" + shared_source = 'from typing import TypeVar\n\nT = TypeVar("T")\n\n\ndef first(items: list[T]) -> T:\n return items[0]\n' + user_source = "from shared import T\n\n\ndef identity(x: T) -> T:\n return x\n" + (tmp_path / "shared.py").write_text(shared_source, encoding="utf-8") + (tmp_path / "user.py").write_text(user_source, encoding="utf-8") + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff", "--ignore", "shared.py"]) + + output = capsys.readouterr().out + assert_that((tmp_path / "shared.py").read_text(encoding="utf-8"), equal_to(shared_source)) + assert_that((tmp_path / "user.py").read_text(encoding="utf-8"), equal_to(user_source)) + assert_that(output, contains_string("**IGNORED FILES (1)**\n `shared.py`\n")) + assert_that(output, not_(contains_string("shared.py checked"))) + assert_that( + _table_rows(output, "Kind"), + equal_to([["TypeVar", "1", "0", "0", "1"], ["ParamSpec", "0", "0", "0", "0"], ["TypeVarTuple", "0", "0", "0", "0"]]), + ) + + +class TestSecondPass: + """main(): --second-pass removes a declaration whose importers were all localized, and reports it in the same report.""" + + @pytest.mark.parametrize( + ("extra_args", "declaration_kept", "expected_review"), + [ + pytest.param([], True, "NEEDS MANUAL REVIEW (1)", id="single-pass"), + pytest.param(["--second-pass"], False, "NEEDS MANUAL REVIEW (0)", id="second-pass"), + ], + ) + def test_declaration_left_behind_by_localizing_is_removed_in_the_same_run( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + extra_args: list[str], + *, + declaration_kept: bool, + expected_review: str, + ) -> None: + """Only the second pass removes the origin's declaration, and the one report counts both files once.""" + (tmp_path / "origin.py").write_text('from typing import TypeVar\n\nT = TypeVar("T")\n', encoding="utf-8") + (tmp_path / "user.py").write_text("from origin import T\n\n\ndef identity(x: T) -> T:\n return x\n", encoding="utf-8") + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff", *extra_args]) + + output = capsys.readouterr().out + assert_that('T = TypeVar("T")' in (tmp_path / "origin.py").read_text(encoding="utf-8"), equal_to(declaration_kept)) + assert_that(output, contains_string(expected_review)) + assert_that(output, contains_string("Processed 2 files")) + + +class TestNoLocalization: + """main(): --no-localization keeps every project import of a type parameter and reports the names left.""" + + def test_importer_is_left_untouched_and_the_report_counts_the_name_as_ignored( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """The importer is unchanged, no file is listed as ignored, and the imported name counts as reported and ignored.""" + (tmp_path / "shared.py").write_text('from typing import TypeVar\n\nT = TypeVar("T")\n', encoding="utf-8") + user_source = "from shared import T\n\n\ndef identity(x: T) -> T:\n return x\n" + (tmp_path / "user.py").write_text(user_source, encoding="utf-8") + + migration.main([str(tmp_path), "--py", "3.12", "--no-ruff", "--no-localization"]) + + output = capsys.readouterr().out + assert_that((tmp_path / "user.py").read_text(encoding="utf-8"), equal_to(user_source)) + assert_that(output, not_(contains_string("IGNORED FILES"))) + assert_that(_table_rows(output, "Kind")[0], equal_to(["TypeVar", "2", "0", "1", "1"])) + + +class TestPerFileProgressFeedback: + """main(): prints a per-file progress line as each file is checked.""" + + def test_progress_line_path_has_no_parent_segments(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """A target containing `..` is normalized before its files are reported.""" + good = tmp_path / "good.py" + good.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + (tmp_path / "sub").mkdir() + + migration.main([str(tmp_path / "sub" / ".."), "--py", "3.12"]) + + output = capsys.readouterr().out + assert_that(output, contains_string(f"File {good} checked.")) + assert_that(output, not_(contains_string(".."))) + + +class TestMainBatchErrorIsolation: + """main(): one bad file in a batch must not abort processing of the rest.""" + + def test_one_bad_file_does_not_abort_the_batch( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """A batch with one broken file still checks and converts the good file, and exits with code 3.""" + good = tmp_path / "good.py" + good.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + broken = tmp_path / "broken.py" + broken.write_text("def broken(:\n", encoding="utf-8") + + exit_code = migration.main([str(tmp_path), "--py", "3.12"]) + + assert_that(exit_code, equal_to(3)) + output = capsys.readouterr().out + assert_that(output, contains_string(f"File {good} checked.")) + assert_that(output, contains_string(f"File {broken} checked.")) + assert_that(good.read_text(encoding="utf-8"), contains_string("def identity[T]")) + + +class TestMainFileEncoding: + """main(): a file's encoding is never changed, and a file that is not UTF-8 is reported instead of aborting the run.""" + + @pytest.mark.xfail( + strict=True, + reason="A UTF-8 byte order mark corrupts the rewritten file: https://github.com/TNO/Renaissance.Py/issues/153", + ) + @pytest.mark.parametrize( + "source", + [ + pytest.param(LEGACY_TYPEVAR_SOURCE, id="lf"), + pytest.param(LEGACY_TYPEVAR_SOURCE.replace("\n", "\r\n"), id="crlf"), + ], + ) + def test_modified_file_keeps_its_utf8_bom_and_still_parses(self, tmp_path: Path, source: str) -> None: + """A file that starts with a UTF-8 byte order mark is converted, keeps the mark, and is valid Python afterwards.""" + target = tmp_path / "mod.py" + target.write_bytes(codecs.BOM_UTF8 + source.encode("utf-8")) + + exit_code = migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + assert_that(exit_code, equal_to(0)) + written = target.read_bytes() + assert_that(written.startswith(codecs.BOM_UTF8), equal_to(obj=True)) + text = written.decode("utf-8-sig") + ast.parse(text) + assert_that(text, contains_string("def identity[T](x: T) -> T:")) + + @pytest.mark.xfail( + strict=True, + raises=UnicodeDecodeError, + reason="A file that is not UTF-8 aborts the whole run: https://github.com/TNO/Renaissance.Py/issues/188", + ) + @pytest.mark.parametrize( + "content", + [ + pytest.param("# café\nx = 1\n".encode("latin-1"), id="latin-1"), + pytest.param("# café\nx = 1\n".encode("utf-16"), id="utf-16"), + ], + ) + def test_file_that_is_not_utf8_is_reported_and_the_rest_of_the_batch_is_converted( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + content: bytes, + ) -> None: + """The undecodable file is listed under ERRORS and left byte-identical, and the other file is converted.""" + good = tmp_path / "good.py" + good.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + undecodable = tmp_path / "undecodable.py" + undecodable.write_bytes(content) + + exit_code = migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + assert_that(exit_code, equal_to(3)) + assert_that(capsys.readouterr().out, contains_string("**ERRORS (1)**\n `undecodable.py`")) + assert_that(undecodable.read_bytes(), equal_to(content)) + assert_that(good.read_text(encoding="utf-8"), contains_string("def identity[T]")) + + +class TestMainProjectWideImportSafety: + """main(): a TypeVar imported by another file in the batch is localized there and kept at its origin.""" + + def test_consumer_is_localized_and_origin_declaration_survives(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """The importing file gets a PEP 695 local TypeVar, and the origin declaration is not removed.""" + import_line = "from pkg.typing_mod import T" + pkg = tmp_path / "pkg" + pkg.mkdir() + (pkg / "__init__.py").write_text("", encoding="utf-8") + (pkg / "typing_mod.py").write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + consumer = pkg / "client.py" + consumer.write_text(f"{import_line}\n\ndef use(x: T) -> T:\n return x\n", encoding="utf-8") + (pkg / "unused.py").write_text(f"{import_line}\n", encoding="utf-8") + + exit_code = migration.main([str(tmp_path), "--py", "3.12"]) + + assert_that(exit_code, equal_to(0)) + consumer_text = consumer.read_text(encoding="utf-8") + assert_that(consumer_text, contains_string("def use[T](x: T) -> T:")) + assert_that(consumer_text, not_(contains_string(import_line))) + assert_that(capsys.readouterr().out, contains_string(" `T`: unused declaration removed\n")) + assert_that((pkg / "typing_mod.py").read_text(encoding="utf-8"), contains_string('T = TypeVar("T")')) + + @pytest.mark.xfail( + strict=True, + reason="`import *` is not checked when deciding whether a declaration is imported elsewhere in the project: " + "see 'imported by another project file' in docs/user/features/type-parameter-modernization.md", + ) + @pytest.mark.parametrize( + ("package", "import_line"), + [ + pytest.param("", "from origin import *", id="absolute"), + pytest.param("pkg", "from .origin import *", id="relative"), + ], + ) + def test_origin_declaration_survives_a_star_import_of_its_module(self, tmp_path: Path, package: str, import_line: str) -> None: + """A declaration that another file reaches through `import *` is kept, and that file is left untouched.""" + directory = tmp_path / package + directory.mkdir(exist_ok=True) + if package: + (directory / "__init__.py").write_text("", encoding="utf-8") + origin = directory / "origin.py" + origin.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + consumer = directory / "consumer.py" + consumer_source = f"{import_line}\n\n\ndef use(x: T) -> T:\n return x\n" + consumer.write_text(consumer_source, encoding="utf-8") + + exit_code = migration.main([str(tmp_path), "--py", "3.12", "--no-ruff"]) + + assert_that(exit_code, equal_to(0)) + assert_that(origin.read_text(encoding="utf-8"), contains_string('T = TypeVar("T")')) + assert_that(consumer.read_text(encoding="utf-8"), equal_to(consumer_source)) + + @pytest.mark.parametrize("target_arg", [pytest.param(".", id="dot"), pytest.param("pkg", id="subdirectory")]) + def test_origin_declaration_survives_with_relative_target( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + target_arg: str, + ) -> None: + """A relative target path still protects a declaration imported by another file in the batch.""" + pkg = tmp_path / "pkg" + pkg.mkdir() + (pkg / "typing_mod.py").write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + (pkg / "client.py").write_text("from .typing_mod import T\n\ndef use(x: T) -> T:\n return x\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + + exit_code = migration.main([target_arg, "--py", "3.12"]) + + assert_that(exit_code, equal_to(0)) + assert_that((pkg / "typing_mod.py").read_text(encoding="utf-8"), contains_string('T = TypeVar("T")')) + assert_that((pkg / "client.py").read_text(encoding="utf-8"), contains_string("def use[T](x: T) -> T:")) + + +class TestPyVersionFlag: + """main(): the required --py flag alone sets the target's minimum Python version.""" + + @pytest.mark.parametrize( + ("py_version", "expected", "unexpected"), + [ + pytest.param("3.12", 'T = TypeVar("T", default=int)', "def identity[", id="3.12-default-kept"), + pytest.param("3.13", "def identity[T = int]", 'T = TypeVar("T", default=int)', id="3.13-default-converted"), + ], + ) + def test_py_flag_gates_rewrites(self, tmp_path: Path, py_version: str, expected: str, unexpected: str) -> None: + """--py decides whether a type parameter default (PEP 696) is converted.""" + target = tmp_path / "mod.py" + target.write_text(LEGACY_TYPEVAR_SOURCE.replace('TypeVar("T")', 'TypeVar("T", default=int)'), encoding="utf-8") + + exit_code = migration.main([str(target), "--py", py_version]) + + assert_that(exit_code, equal_to(0)) + written = target.read_text(encoding="utf-8") + assert_that(written, contains_string(expected)) + assert_that(written, not_(contains_string(unexpected))) + + @pytest.mark.parametrize( + ("bad_args", "expected_error"), + [ + pytest.param([], "the following arguments are required: --py", id="flag-missing"), + pytest.param(["--py", "3.x"], "expected MAJOR.MINOR (e.g. 3.12), got '3.x'", id="non-numeric"), + ], + ) + def test_bad_version_arguments_are_usage_errors( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + bad_args: list[str], + expected_error: str, + ) -> None: + """A missing or malformed --py flag exits with code 2 and an error saying what is wrong with it.""" + target = tmp_path / "mod.py" + target.write_text(LEGACY_TYPEVAR_SOURCE, encoding="utf-8") + + with pytest.raises(SystemExit) as excinfo: + migration.main([str(target), *bad_args]) + + assert_that(excinfo.value.code, equal_to(2)) + assert_that(capsys.readouterr().err, contains_string(expected_error)) diff --git a/test/utils/test_import_resolution.py b/test/utils/test_import_resolution.py new file mode 100644 index 00000000..517d9245 --- /dev/null +++ b/test/utils/test_import_resolution.py @@ -0,0 +1,282 @@ +# Copyright (C) 2024-2026 TNO +# +# Licensed under the terms specified in the LICENSE file in the root directory +# of this repository, also available at: +# https://github.com/TNO/Renaissance.Py/blob/main/LICENSE + +"""Tests for renaissance.utils.import_resolution.""" + +import ast +from pathlib import Path +from typing import cast + +import libcst as cst +import pytest +from hamcrest import assert_that, equal_to, has_item, has_key, is_, none, not_, not_none + +from renaissance.utils.import_resolution import ( + collect_project_imported_names, + from_import_binding, + from_import_sources, + import_from_module, + imports_name_conditionally, + rebase_relative_module, + resolve_project_module, +) + + +@pytest.fixture +def project_tree(tmp_path: Path) -> Path: + """Build a small redis-py-shaped tree: redis/typing.py, redis/commands/{__init__,sibling,core}.py.""" + (tmp_path / "redis" / "commands").mkdir(parents=True) + (tmp_path / "redis" / "typing.py").write_text("AnyKeyT = 1\n") + (tmp_path / "redis" / "commands" / "__init__.py").write_text("Y = 1\n") + (tmp_path / "redis" / "commands" / "sibling.py").write_text("Z = 1\n") + (tmp_path / "redis" / "commands" / "core.py").write_text("from ..typing import AnyKeyT\n") + return tmp_path + + +class TestResolveProjectModule: + """See module docstring.""" + + @pytest.mark.parametrize( + ("importing_file_rel", "module", "level", "expected_rel"), + [ + ("redis/commands/core.py", "redis.typing", 0, "redis/typing.py"), + ("redis/commands/core.py", "redis.commands", 0, "redis/commands/__init__.py"), + ("redis/commands/core.py", "sibling", 1, "redis/commands/sibling.py"), + ("redis/commands/core.py", "typing", 2, "redis/typing.py"), + ("redis/commands/core.py", None, 1, "redis/commands/__init__.py"), + ("redis/commands/core.py", "typing", 0, None), + ("redis/commands/core.py", "nonexistent.module", 0, None), + ], + ) + def test_resolve( + self, + project_tree: Path, + importing_file_rel: str, + module: str | None, + level: int, + expected_rel: str | None, + ) -> None: + """Absolute, relative and package imports resolve to their project file, or None outside the project.""" + importing_file = project_tree / importing_file_rel + expected = project_tree / expected_rel if expected_rel is not None else None + assert_that(resolve_project_module(importing_file, project_tree, module, level), equal_to(expected)) + + @pytest.mark.parametrize( + ("module", "level"), + [ + pytest.param("sibling", 2, id="parent-module"), + pytest.param(None, 2, id="parent-package"), + ], + ) + def test_relative_import_above_relative_root_is_none( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + module: str | None, + level: int, + ) -> None: + """A relative import walking above a relative project root (".") resolves to None.""" + (tmp_path / "sibling.py").write_text("X = 1\n") + (tmp_path / "__init__.py").write_text("") + monkeypatch.chdir(tmp_path) + + assert_that(resolve_project_module(Path("top.py"), Path(), module, level), equal_to(None)) + + +class TestRebaseRelativeModule: + """Tests for rebase_relative_module.""" + + @pytest.mark.parametrize( + ("via", "origin_file", "origin_import", "expected"), + [ + pytest.param("from .file_1 import T", "file_1.py", "from ._compat import X", "._compat", id="same-dir"), + pytest.param("from .sub.file_1 import T", "sub/file_1.py", "from ._compat import X", ".sub._compat", id="origin-in-subpackage"), + pytest.param("from .sub.file_1 import T", "sub/file_1.py", "from .._compat import X", "._compat", id="origin-climbs-back"), + pytest.param("from .file_1 import T", "file_1.py", "from .._compat import X", ".._compat", id="origin-climbs-above-via"), + pytest.param("from .sub import T", "sub/__init__.py", "from ._compat import X", ".sub._compat", id="origin-is-package"), + pytest.param("from . import T", "__init__.py", "from ._compat import X", "._compat", id="origin-is-current-package"), + pytest.param("from pkg.file_1 import T", "pkg/file_1.py", "from ._compat import X", "pkg._compat", id="absolute-via"), + pytest.param("from .file_1 import T", "file_1.py", "from typing import X", "typing", id="absolute-import"), + pytest.param("from .file_1 import T", "file_1.py", "from . import X", ".", id="origin-imports-its-own-package"), + pytest.param("from file_1 import T", "file_1.py", "from ._compat import X", None, id="origin-at-top-level"), + ], + ) + def test_rebase(self, via: str, origin_file: str, origin_import: str, expected: str | None) -> None: + """The origin's `from import` is rewritten to name the same module from the importing file, or None.""" + via_stmt = cast("ast.ImportFrom", ast.parse(via).body[0]) + origin_stmt = cast("ast.ImportFrom", ast.parse(origin_import).body[0]) + + rebased = rebase_relative_module(via_stmt.module, via_stmt.level, Path(origin_file), origin_stmt.module, origin_stmt.level) + + assert_that(rebased, equal_to(expected)) + + +class TestCollectProjectImportedNames: + """See module docstring.""" + + @pytest.mark.parametrize( + ("consumer_source", "expected"), + [ + pytest.param("from origin import X\n", {"origin.py": frozenset({"X"})}, id="absolute-import"), + pytest.param("from .origin import X\n", {"origin.py": frozenset({"X"})}, id="relative-import"), + pytest.param("from origin import X as Z\n", {"origin.py": frozenset({"X"})}, id="aliased-import-records-original-name"), + pytest.param("from typing import TypeVar\n", {}, id="stdlib-import-not-recorded"), + ], + ) + def test_records_imported_names_against_their_origin_file( + self, + tmp_path: Path, + consumer_source: str, + expected: dict[str, frozenset[str]], + ) -> None: + """A name imported from a project file is recorded against that file, under its declared name.""" + (tmp_path / "origin.py").write_text("X = 1\n") + (tmp_path / "consumer.py").write_text(consumer_source) + + result = collect_project_imported_names([tmp_path / "origin.py", tmp_path / "consumer.py"], tmp_path) + + assert_that(result, equal_to({tmp_path / name: names for name, names in expected.items()})) + + @pytest.mark.parametrize( + "unparsable_source", + [ + pytest.param("def (:\n", id="syntax-error"), + pytest.param('x = "a" t"b"\n', id="invalid-string-then-t-string"), + pytest.param('x = t"a" t"b"\n', id="valid-t-string-concatenation"), + ], + ) + def test_skips_a_file_libcst_cannot_parse(self, tmp_path: Path, unparsable_source: str) -> None: + """A file libcst cannot parse is skipped, and the imports of the other files are still recorded.""" + (tmp_path / "unparsable.py").write_text(unparsable_source, encoding="utf-8") + (tmp_path / "origin.py").write_text("X = 1\n") + (tmp_path / "consumer.py").write_text("from origin import X\n") + + files = [tmp_path / "unparsable.py", tmp_path / "origin.py", tmp_path / "consumer.py"] + result = collect_project_imported_names(files, tmp_path) + + assert_that(result, equal_to({tmp_path / "origin.py": frozenset({"X"})})) + + +@pytest.fixture +def module_tree(tmp_path: Path) -> Path: + """Build pkg/__init__.py and pkg/mod.py (declaring T) under tmp_path.""" + (tmp_path / "pkg").mkdir() + (tmp_path / "pkg" / "__init__.py").write_text("X = 1\n") + (tmp_path / "pkg" / "mod.py").write_text("T = 1\n") + return tmp_path + + +class TestCollectModuleAttributeAccess: + """collect_project_imported_names: names reached as attributes of an imported project module.""" + + @pytest.mark.parametrize( + ("consumer_source", "origin_rel", "expected"), + [ + pytest.param("import pkg.mod\nx = pkg.mod.T\n", "pkg/mod.py", "T", id="import-dotted"), + pytest.param("import pkg.mod as m\nx = m.T\n", "pkg/mod.py", "T", id="import-as"), + pytest.param("from pkg import mod\nx = mod.T\n", "pkg/mod.py", "T", id="from-package-import-module"), + pytest.param("from . import mod\nx = mod.T\n", "pkg/mod.py", "T", id="from-dot-import-module"), + pytest.param("from pkg import mod as m\nx = m.T\n", "pkg/mod.py", "T", id="from-import-module-as"), + pytest.param("import pkg.mod\nx = pkg.X\n", "pkg/__init__.py", "X", id="import-dotted-parent-package"), + ], + ) + def test_records_attribute_accessed_through_module_import( + self, + module_tree: Path, + consumer_source: str, + origin_rel: str, + expected: str, + ) -> None: + """An attribute read through an imported project module is recorded against that module's file.""" + consumer = module_tree / "pkg" / "consumer.py" + consumer.write_text(consumer_source) + + result = collect_project_imported_names([consumer], module_tree) + + assert_that(sorted(result.get(module_tree / origin_rel, frozenset[str]())), has_item(expected)) + + @pytest.mark.parametrize( + "consumer_source", + [ + pytest.param("import pkg.mod\n", id="module-imported-but-unused"), + pytest.param("import pkg.mod\nx = other.T\n", id="attribute-on-unimported-name"), + pytest.param("import typing\nx = typing.TypeVar\n", id="stdlib-module"), + pytest.param("mod = object()\nx = mod.T\n", id="local-name-shadowing-module-name"), + ], + ) + def test_does_not_record_unrelated_attribute_access(self, module_tree: Path, consumer_source: str) -> None: + """Attribute reads not made through an imported project module record nothing against it.""" + consumer = module_tree / "pkg" / "consumer.py" + consumer.write_text(consumer_source) + + result = collect_project_imported_names([consumer], module_tree) + + assert_that(result, not_(has_key(module_tree / "pkg" / "mod.py"))) + + +class TestFromImportSources: + """See module docstring.""" + + @pytest.mark.parametrize( + ("source", "expected"), + [ + pytest.param("from a.b import X\n", {"X": ("a.b", 0)}, id="absolute"), + pytest.param("from ..pkg import X as Y\n", {"Y": ("pkg", 2)}, id="relative-alias-keys-the-bound-name"), + pytest.param("from . import x\n", {"x": ("", 1)}, id="bare-relative"), + pytest.param("import os.path\n", {"os": None}, id="plain-import-binds-the-top-package"), + pytest.param("class C: ...\n", {"C": None}, id="class"), + pytest.param("from a import X\nX = 1\n", {"X": None}, id="later-assignment-rebinds"), + ], + ) + def test_maps_each_bound_name_to_its_from_import(self, source: str, expected: dict[str, tuple[str, int] | None]) -> None: + """Verify a name maps to its `from` import's (module, level), or to None when anything else binds it last.""" + assert_that(from_import_sources(cst.parse_module(source)), equal_to(expected)) + + +class TestFromImportBinding: + """See module docstring.""" + + def test_returns_the_import_and_alias_binding_the_name(self) -> None: + """Verify the alias that binds an aliased name is returned with its import statement.""" + binding = from_import_binding(cst.parse_module("from a import X as Y\n"), "Y") + + assert_that(binding, not_none()) + statement, alias = cast("tuple[cst.ImportFrom, cst.ImportAlias]", binding) + assert_that(alias.evaluated_name, equal_to("X")) + assert_that(import_from_module(statement), equal_to("a")) + + @pytest.mark.parametrize( + "source", + [ + pytest.param("from a import X\nX = 1\n", id="rebound-by-assignment"), + pytest.param("import X\n", id="plain-import"), + pytest.param("A = 1\n", id="not-imported"), + ], + ) + def test_is_none_when_no_from_import_binds_the_name_last(self, source: str) -> None: + """Verify None is returned when another statement binds the name, or nothing does.""" + assert_that(from_import_binding(cst.parse_module(source), "X"), none()) + + +class TestImportsNameConditionally: + """See module docstring.""" + + @pytest.mark.parametrize( + ("source", "expected"), + [ + pytest.param("if TYPE_CHECKING:\n from typing import TypeVar\n", True, id="under-if"), + pytest.param( + "try:\n from typing import TypeVar\nexcept ImportError:\n from typing_extensions import TypeVar\n", + True, + id="under-try", + ), + pytest.param("from typing import TypeVar\n", False, id="unconditional"), + pytest.param("if TYPE_CHECKING:\n from typing import ParamSpec\n", False, id="other-name"), + ], + ) + def test_detects_an_import_of_the_name_inside_a_module_level_if_or_try(self, source: str, *, expected: bool) -> None: + """Verify only an import of the name inside a module-level `if` or `try` counts.""" + assert_that(imports_name_conditionally(cst.parse_module(source), "TypeVar"), is_(expected)) diff --git a/test/utils/test_ruff_utils.py b/test/utils/test_ruff_utils.py index ee7f725a..416fc622 100644 --- a/test/utils/test_ruff_utils.py +++ b/test/utils/test_ruff_utils.py @@ -4,18 +4,25 @@ # of this repository, also available at: # https://github.com/TNO/Renaissance.Py/blob/main/LICENSE -"""Tests for the ruff import cleanup helper.""" +"""Tests for the ruff helpers that clean up and format the files a recipe changed.""" +from collections.abc import Callable, Sequence from pathlib import Path import pytest from hamcrest import assert_that, equal_to -from renaissance.utils.ruff_utils import run_ruff_import_cleanup +from renaissance.utils.ruff_utils import run_ruff_format, run_ruff_import_cleanup UNUSED_IMPORT = ("import os\nimport sys\n\nprint(sys.argv)\n", "import sys\n\nprint(sys.argv)\n") UNSORTED_IMPORTS = ("import sys\nimport os\n\nprint(os, sys)\n", "import os\nimport sys\n\nprint(os, sys)\n") NOTHING_TO_FIX = ("import os\n\nprint(os)\n", "import os\n\nprint(os)\n") +UNFORMATTED = ("x = { 'a':1 }\n", 'x = {"a": 1}\n') +LONG_SIGNATURE = ( + "def f[T](first: list[T], second: list[T]) -> T: ...\n", + "def f[T](\n first: list[T], second: list[T]\n) -> T: ...\n", +) +ALREADY_FORMATTED = ('x = {"a": 1}\n', 'x = {"a": 1}\n') @pytest.mark.parametrize( @@ -38,13 +45,45 @@ def test_cleans_up_the_imports_of_each_file(tmp_path: Path, cases: list[tuple[st assert_that([path.read_text(encoding="utf-8") for path in paths], equal_to([expected for _, expected in cases])) -def test_no_paths_changes_nothing(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize( + "cases", + [ + pytest.param([UNFORMATTED], id="unformatted"), + pytest.param([LONG_SIGNATURE], id="long_signature"), + pytest.param([ALREADY_FORMATTED], id="already_formatted"), + pytest.param([UNFORMATTED, LONG_SIGNATURE], id="several_files"), + ], +) +def test_formats_each_file(tmp_path: Path, cases: list[tuple[str, str]]) -> None: + """Each file is rewritten in the ruff format style, with the line length of the ruff settings next to it.""" + (tmp_path / "ruff.toml").write_text("line-length = 40\n", encoding="utf-8") + paths = [tmp_path / f"module_{index}.py" for index in range(len(cases))] + for path, (source, _) in zip(paths, cases, strict=True): + path.write_text(source, encoding="utf-8") + + run_ruff_format(paths) + + assert_that([path.read_text(encoding="utf-8") for path in paths], equal_to([expected for _, expected in cases])) + + +@pytest.mark.parametrize( + ("ruff_helper", "source"), + [ + pytest.param(run_ruff_import_cleanup, UNUSED_IMPORT[0], id="import_cleanup"), + pytest.param(run_ruff_format, UNFORMATTED[0], id="format"), + ], +) +def test_no_paths_changes_nothing( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ruff_helper: Callable[[Sequence[Path]], None], + source: str, +) -> None: """An empty list leaves the files in the working directory untouched.""" - source, _ = UNUSED_IMPORT path = tmp_path / "module.py" path.write_text(source, encoding="utf-8") monkeypatch.chdir(tmp_path) - run_ruff_import_cleanup([]) + ruff_helper([]) assert_that(path.read_text(encoding="utf-8"), equal_to(source))