diff --git a/docs/TODO b/docs/TODO index ff4d6680..1437f612 100644 --- a/docs/TODO +++ b/docs/TODO @@ -42,4 +42,4 @@ ### Naming -21. **Type parameter modernization: rename the recipe and update its code references** — The page is `user/features/type-parameter-modernization.md`, but the recipe is still `TypeVarCheck` (`type_var_check.py`, `type_var_domain.py`, `test_type_var_*`). `type_var_domain.py` hard-codes the old page URL (`.../typevar-modernization/`) and 9 old anchors (`feature-typevar-modernization-*`, and `...-declared-typevar-exported` is now `...-declared-type-parameter-exported`), and `test_type_var_domain.py` reads the old file path. Rename the recipe to `TypeParameterCheck` and update those references in one change (ADR 15 resolves recipe names with `snake_case()`), then update the docs that name the recipe. \ No newline at end of file +21. **Type parameter modernization: rename the recipe and update its code references** — The page and rule links use `type-parameter-modernization`, but the recipe is still `TypeVarCheck` (`type_var_check.py`, `type_var_domain.py`, `test_type_var_*`). Rename the recipe to `TypeParameterCheck` and update its implementation and tests in one change (ADR 15 resolves recipe names with `snake_case()`), then update the docs that name the recipe. diff --git a/src/rejuvenation/migration-type-recipes.py b/src/rejuvenation/migration-type-recipes.py new file mode 100644 index 00000000..6837eb9c --- /dev/null +++ b/src/rejuvenation/migration-type-recipes.py @@ -0,0 +1,295 @@ +# 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 TypeVarCheck on a file or directory, and reports which files were modified and which need +manual review. + +Examples: + python src/rejuvenation/migration-type-recipes.py ./some_repo --py 3.12 --report review.md + python src/rejuvenation/migration-type-recipes.py ./some_repo/file.py --py 3.10 + python src/rejuvenation/migration-type-recipes.py ./some_repo --py 3.12 --no-ruff + +""" + +# Printing the report is this CLI's purpose. +# ruff: noqa: T201 + +import argparse +import subprocess +import sys +import textwrap +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_var_check import TypeVarCheck +from renaissance.recipes.type_var_domain import UNSAFE_RULES, UnsafeReason, doc_link +from renaissance.utils.import_resolution import collect_project_imported_names + +_MAJOR_MINOR_PART_COUNT = 2 + + +@dataclass +class FileReport: + """Outcome of running TypeVarCheck against a single file.""" + + path: Path + result: dict[str, dict[str, str]] | None + error: str | None + reasons: dict[str, dict[str, UnsafeReason]] | None = None + + +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. + + """ + 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) + return (int(parts[0]), int(parts[1])) + + +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 left at least one name unsafe to touch.""" + if report.result is None: + return False + return any("unsafe" in phase.values() for phase in report.result.values()) + + +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], +) -> FileReport: + """Run TypeVarCheck'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 = TypeVarCheck(path) + recipe.min_python = min_python + recipe.project_root = project_root + recipe.project_wide_imported_names = project_wide_imported_names + 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) + + +def _run_ruff_unused_import_cleanup(paths: list[Path]) -> None: + """Run `ruff check --fix --select F401` over every path, dropping any now-unused import. + + Best-effort: if ruff can't be invoked (e.g. not installed), prints a warning instead of raising. + The files are already valid without this cleanup. + """ + try: + subprocess.run( # noqa: S603 - fixed argv list (sys.executable + literals + our own discovered paths), no shell + [sys.executable, "-m", "ruff", "check", "--fix", "--select", "F401", *(str(path) for path in paths)], + capture_output=True, + text=True, + check=False, + ) + except OSError as exc: + print(colored(f"warning: could not run ruff for import cleanup: {exc}", "yellow")) + + +def _format_commit_summary(reports: list[FileReport]) -> str: + """Build the short, copy-pasteable commit-message-style summary.""" + modified = sum(1 for report in reports if has_fixed(report)) + needs_review = sum(1 for report in reports if has_unsafe(report)) + clean = sum(1 for report in reports if is_clean(report)) + errors = sum(1 for report in reports if report.error is not None) + return ( + "Modernize TypeVar/ParamSpec/TypeVarTuple usage to PEP 695 syntax\n\n" + f"{modified} files modified, {needs_review} need manual review, {clean} clean, " + f"{errors} errors (of {len(reports)} processed)" + ) + + +def _format_console_report(reports: list[FileReport], *, ruff_ran: bool) -> str: + """Build the full per-file report: MODIFIED / NEEDS MANUAL REVIEW / ERRORS sections. + + Clean files (no type parameter usage at all) are only counted, not listed, so the report shows + what needs attention. Under MODIFIED, a converted name isn't repeated as orphaned. The ruff + import-cleanup line is only included when ruff_ran is True. + """ + 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 TypeVarCheck migration report", + ( + f"Processed {len(reports)} files: {len(modified)} modified, {len(needs_review)} need " + f"manual review, {clean_count} clean, {len(errors)} errors" + ), + ] + if ruff_ran: + lines.append( + "Unused imports across the modified files above were also cleaned up via `ruff check --fix --select F401`.", + ) + lines.extend(["", f"MODIFIED ({len(modified)})"]) + for report in modified: + lines.append(f" {report.path}") + result = report.result or {} + converted = {name for name, status in result.get("converted", {}).items() if status == "fixed"} + for phase, names in result.items(): + # A converted name's declaration is always removed afterwards; listing it under orphaned too would repeat it. + fixed = [name for name, status in names.items() if status == "fixed" and not (phase == "orphaned" and name in converted)] + if fixed: + lines.append(f" {phase}: {', '.join(fixed)}") + + lines.extend(["", f"NEEDS MANUAL REVIEW ({len(needs_review)})"]) + for report in needs_review: + lines.append(f" {report.path}") + for phase, names in (report.result or {}).items(): + phase_reasons = (report.reasons or {}).get(phase, {}) + unsafe = [name for name, status in names.items() if status == "unsafe"] + if unsafe: + lines.append(f" {phase}: {', '.join(unsafe)}") + for name in unsafe: + reason = phase_reasons.get(name) + if reason is not None: + lines.append(f" {name}: {UNSAFE_RULES[reason].message} -> {doc_link(reason)}") + + lines.extend(["", f"ERRORS ({len(errors)})"]) + lines.extend(f" {report.path}: {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 ./some_repo --py 3.12 --report review.md + python src/rejuvenation/migration-type-recipes.py ./some_repo/file.py --py 3.10 + python src/rejuvenation/migration-type-recipes.py ./some_repo --py 3.12 --no-ruff + """), + 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 (not the one running this tool), e.g. 3.12. " + "PEP 695 rewrites need 3.12+, PEP 696 type parameter defaults 3.13+." + ), + ) + 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` pass that drops imports made unused.", + ) + return parser + + +def main(argv: Sequence[str] | None = None) -> int: + """Parse arguments, run TypeVarCheck 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 + # TODO(FrancescoPezzella): computed once upfront, so an origin whose importers all get localized this run is only converted + # on a second run. + # https://github.com/TNO/Renaissance.Py/issues/279 + imported_names_by_file = collect_project_imported_names(files, project_root) + + reports: list[FileReport] = [] + for path in files: + 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, + ) + reports.append(report) + print(f"File {path} checked.") + + 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: + # TODO(FrancescoPezzella): removed statements leave their blank lines behind; ruff's E303 (preview) collapses them, but not + # at file start. + # https://github.com/TNO/Renaissance.Py/issues/279 + _run_ruff_unused_import_cleanup(modified_paths) + + console_report = _format_console_report(reports, ruff_ran=ruff_ran) + print(console_report) + print() + print(colored(_format_commit_summary(reports), "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/project/project_scanner.py b/src/renaissance/project/project_scanner.py index 215001fd..5478a695 100644 --- a/src/renaissance/project/project_scanner.py +++ b/src/renaissance/project/project_scanner.py @@ -69,7 +69,7 @@ def __init__(self, root_dir: str = ".", package_dirs: list[str] | None = None) - self.package_dirs = package_dirs def find_sources(self) -> list[str]: - """Return every .py file under root_dir (or package_dirs, if given), sorted, excluding EXCLUDED_DIRS.""" + """Return every .py file under root_dir (or package_dirs, if given), sorted, skipping EXCLUDED_DIRS subdirectories.""" path = Path(self.root_dir) if not path.exists(): message = f"root_dir does not exist: {self.root_dir}" @@ -78,13 +78,16 @@ def find_sources(self) -> list[str]: message = f"root_dir is not a directory: {self.root_dir}" raise NotADirectoryError(message) - roots = [Path(self.root_dir) / d for d in self.package_dirs] if self.package_dirs else [Path(self.root_dir)] + roots = [path / d for d in self.package_dirs] if self.package_dirs else [path] files: list[Path] = [] for root in roots: if not root.exists(): continue - files.extend(path for path in root.rglob("*.py") if not any(part in self.EXCLUDED_DIRS for part in path.parts)) - return sorted(str(path) for path in files) + for dirpath, dirnames, filenames in root.walk(): + # Pruning in place stops walk() from descending into excluded directories at all. + dirnames[:] = [name for name in dirnames if name not in self.EXCLUDED_DIRS] + files.extend(dirpath / name for name in filenames if (dirpath / name).match("*.py")) + return sorted(str(file) for file in files) class BearCppScanner(CppScanner): diff --git a/src/renaissance/recipes/python_refactoring.py b/src/renaissance/recipes/python_refactoring.py index 90293c4e..238d634d 100644 --- a/src/renaissance/recipes/python_refactoring.py +++ b/src/renaissance/recipes/python_refactoring.py @@ -6,6 +6,7 @@ """AI: Base processor for Python-specific source refactoring recipes.""" +import ast import importlib from collections.abc import Sequence from pathlib import Path @@ -120,5 +121,19 @@ def body(self) -> Sequence[NodeT]: return cast("Sequence[NodeT]", [wrappers[statement] for statement in root.translation_unit.atu.body]) return cast("Sequence[NodeT]", root.body) + def find_rst_node(self, target: ast.AST) -> PythonRstNode: + """Return the PythonRstNode wrapping the raw ast node `target`, e.g. to pass to self.replace().""" + # TODO(FrancescoPezzella): Drop once recipes can navigate wrapper nodes via the unified node protocol? + # decided during 24-09 discussion over future Node Protocol implementation + # https://github.com/TNO/Renaissance.Py/issues/269 + found: list[PythonRstNode] = [] + + def visit(node: PythonRstNode) -> None: + if node.node is target: + found.append(node) + + cast("PythonRstNode", cast("object", self.root)).process(visit) + return found[0] + def run(self) -> None: - """AI: Run this refactoring recipe. Subclasses override this to perform the refactoring.""" + """Perform this recipe's refactoring; a no-op unless a subclass overrides it.""" diff --git a/src/renaissance/recipes/step_runner.py b/src/renaissance/recipes/step_runner.py new file mode 100644 index 00000000..12f2ec46 --- /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.recipes.python_refactoring import PythonRefactoring + + +@dataclass(frozen=True) +class Step: + """One independently-runnable, independently-committable fix action.""" + + label: str + recipe: PythonRefactoring + 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_var_check.py b/src/renaissance/recipes/type_var_check.py new file mode 100644 index 00000000..9cd1dcb9 --- /dev/null +++ b/src/renaissance/recipes/type_var_check.py @@ -0,0 +1,351 @@ +# 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.""" + +import ast +from pathlib import Path +from typing import cast + +from renaissance.integrations.python.ast.rst_node import PythonRstNode +from renaissance.recipes.python_refactoring import PythonRefactoring +from renaissance.recipes.step_runner import Step, run_steps +from renaissance.recipes.type_var_domain import ( + UnsafeReason, + all_refs_shadowed_by_pep695, + build_type_param, + declaration_argument_names, + declared_default, + find_type_param_declarations, + from_import_sources, + functions_in_generic_classes, + functions_using_nodes, + has_unconvertible_argument, + is_safe_to_localize, + is_safe_to_remove, + type_param_constructor_name, + type_param_name, + type_params_defaults_last, +) +from renaissance.utils.import_resolution import rebase_relative_module, resolve_project_module +from renaissance.utils.unparse_utils import unparse_signature_only + +PEP_695_MINIMUM = (3, 12) +PEP_696_MINIMUM = (3, 13) + + +class TypeVarCheck(PythonRefactoring): + """Modernize legacy TypeVar/ParamSpec/TypeVarTuple usage in a Python file to PEP 695 syntax. + + See check() for the three phases this runs, in order. + """ + + # Minimum Python version the target codebase supports; None means unknown. + min_python: tuple[int, int] | None = None + + # Names other files in the target project import directly from this file; never removed. + project_wide_imported_names: frozenset[str] = frozenset() + + # Root that absolute imports resolve from; None falls back to this file's own directory. + project_root: Path | None = None + + def run(self) -> None: + """Entry point called by PythonRefactoring.process(); stores check()'s result.""" + self.result = self.check() + + def _target_supports_pep695(self) -> bool: + """Return True only if min_python is known and is 3.12+, where PEP 695 syntax (`def f[T](...)`) exists.""" + return self.min_python is not None and self.min_python >= PEP_695_MINIMUM + + def _target_supports_pep696(self) -> bool: + """Return True only if min_python is known and is 3.13+, where PEP 696 defaults (`def f[T = int](...)`) exist.""" + return self.min_python is not None and self.min_python >= PEP_696_MINIMUM + + def _conversion_refusal(self, decl_stmt: ast.Assign) -> UnsafeReason | None: + """Return why decl_stmt can't be converted to a PEP 695 type parameter on this target, or None if it can.""" + if not self._target_supports_pep695(): + return UnsafeReason.PEP695_VERSION_GATE + if has_unconvertible_argument(decl_stmt): + return UnsafeReason.NO_PEP695_EQUIVALENT + if declared_default(decl_stmt) is not None and not self._target_supports_pep696(): + return UnsafeReason.PEP696_VERSION_GATE + return None + + 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 a PEP 695 type parameter to every function using a module-level TypeVar/ParamSpec/TypeVarTuple. + + The declaration is left in place. Names whose references are all shadowed by a same-named PEP 695 + type parameter are skipped, and methods of a class generic over the name are never converted. + Bounds, constraints and `default=` are carried over, with defaulted type parameters placed last. + A name only such methods use, or whose declaration _conversion_refusal rejects, is reported + "unsafe", with its UnsafeReason on self.converted_unsafe_reasons. Returns {name: "fixed" | "unsafe"}. + """ + root = cast("PythonRstNode", cast("object", self.root)) + tree = cast("ast.Module", root.node) + declarations = find_type_param_declarations(tree) + usage = functions_using_nodes(tree, declarations.keys()) + + results: dict[str, str] = {} + self.converted_unsafe_reasons: dict[str, UnsafeReason] = {} + # 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. + touched_functions: dict[int, ast.FunctionDef | ast.AsyncFunctionDef] = {} + for name, functions in usage.items(): + decl_stmt = declarations[name] + if all_refs_shadowed_by_pep695(tree, name, decl_stmt): + continue + + generic_class_method_ids = {id(method) for method in functions_in_generic_classes(tree, name)} + candidates = [ + function + for function in functions + if id(function) not in generic_class_method_ids + and not any(type_param_name(existing) == name for existing in function.type_params) + ] + if not candidates: + if any(id(function) in generic_class_method_ids for function in functions): + self._mark_unsafe(results, self.converted_unsafe_reasons, name, UnsafeReason.USED_IN_GENERIC_CLASS) + continue + reason = self._conversion_refusal(decl_stmt) + if reason is not None: + self._mark_unsafe(results, self.converted_unsafe_reasons, name, reason) + continue + + type_param = build_type_param(decl_stmt) + for function in candidates: + function.type_params = [*function.type_params, type_param] + touched_functions[id(function)] = function + results[name] = "fixed" + + for function in touched_functions.values(): + function.type_params = type_params_defaults_last(function.type_params) + rst_node = self.find_rst_node(function) + self.replace(unparse_signature_only(function, rst_node.text), rst_node, include_whitespace=False, include_comments=False) + + return results + + def remove_orphaned_declarations(self) -> dict[str, str]: + """Remove every module-level TypeVar/ParamSpec/TypeVarTuple declaration that nothing uses anymore. + + A declaration is orphaned when no reference to its name remains outside a same-named PEP 695 type + parameter (see all_refs_shadowed_by_pep695). One that is_safe_to_remove rejects is kept and + reported "unsafe", with its UnsafeReason on self.orphaned_unsafe_reasons. Returns + {name: "fixed" | "unsafe"}. + """ + root = cast("PythonRstNode", cast("object", self.root)) + tree = cast("ast.Module", root.node) + + results: dict[str, str] = {} + self.orphaned_unsafe_reasons: dict[str, UnsafeReason] = {} + for name, decl_stmt in find_type_param_declarations(tree).items(): + if not all_refs_shadowed_by_pep695(tree, name, decl_stmt): + continue + + # TODO(FrancescoPezzella): a public TypeVar no file in the project uses (e.g. redis.typing.AnyKeyT) is removed, + # though other projects may import it. + # https://github.com/TNO/Renaissance.Py/issues/279 + reason = is_safe_to_remove(tree, name, self.project_wide_imported_names) + if reason is not None: + self._mark_unsafe(results, self.orphaned_unsafe_reasons, name, reason) + continue + + self._remove_declaration(decl_stmt) + results[name] = "fixed" + + return results + + def _mark_unsafe(self, 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 + + def _remove_declaration(self, decl_stmt: ast.Assign) -> None: + """Remove decl_stmt's statement from the file.""" + for stmt_node in self.body: + if stmt_node.node is decl_stmt: + # TODO(FrancescoPezzella): enable once comment blocks get correctly deleted + # https://github.com/TNO/Renaissance.Py/issues/189 + self.remove(stmt_node, include_comments=False) + break + + 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 becomes a copy of the origin's + declaration, plus imports from the origin for the names its arguments use (e.g. `bound=Shape`). + It is unsafe when is_safe_to_localize rejects the origin or _argument_names_to_import returns None. + Returns {name: "fixed" | "unsafe"}, with each UnsafeReason on self.cross_file_unsafe_reasons. + """ + results: dict[str, str] = {} + self.cross_file_unsafe_reasons: dict[str, UnsafeReason] = {} + importing_tree = cast("ast.Module", cast("PythonRstNode", cast("object", self.root)).node) + + project_root = self.project_root if self.project_root is not None else Path(self.filename).parent + for import_node in self.body: + raw = import_node.node + if not isinstance(raw, ast.ImportFrom): + continue + + origin_path = resolve_project_module(Path(self.filename), project_root, raw.module, raw.level) + if origin_path is None: + continue + + self._localize_names_from(import_node, raw, importing_tree, origin_path, results) + + return results + + def _localize_names_from( + self, + import_node: PythonRstNode, + raw: ast.ImportFrom, + importing_tree: ast.Module, + origin_path: Path, + results: dict[str, str], + ) -> None: + """Localize every safe type parameter name that raw imports from the origin_path file, in one edit of import_node. + + Records each name in results as "fixed" or, via _mark_unsafe, as "unsafe". + """ + origin_tree = ast.parse(origin_path.read_text(encoding="utf-8"), str(origin_path)) + declarations = find_type_param_declarations(origin_tree) + localized: dict[str, ast.Assign] = {} + needed_imports: list[str] = [] + needed_argument_names: set[str] = set() + + for alias in raw.names: + # TODO(FrancescoPezzella): `from x import T as U` is not currently being caught, tbd + # https://github.com/TNO/Renaissance.Py/issues/279 + if alias.asname is not None or alias.name not in declarations: + continue + + decl_stmt = declarations[alias.name] + reason = is_safe_to_localize(origin_tree, alias.name) + if reason is not None: + self._mark_unsafe(results, self.cross_file_unsafe_reasons, alias.name, reason) + continue + argument_names = self._argument_names_to_import(importing_tree, origin_tree, raw, decl_stmt) + if argument_names is None: + self._mark_unsafe(results, self.cross_file_unsafe_reasons, alias.name, UnsafeReason.DECLARATION_NAME_UNAVAILABLE) + continue + + constructor_import = self._missing_constructor_import(origin_tree, origin_path, raw, decl_stmt) + if constructor_import is not None and constructor_import not in needed_imports: + needed_imports.append(constructor_import) + needed_argument_names |= argument_names + localized[alias.name] = decl_stmt + results[alias.name] = "fixed" + + if not localized: + return + if needed_argument_names: + needed_imports.append(f"from {'.' * raw.level}{raw.module or ''} import {', '.join(sorted(needed_argument_names))}") + self._localize_import(import_node, raw, localized, needed_imports) + + def _missing_constructor_import( + self, + origin_tree: ast.Module, + origin_path: Path, + via: ast.ImportFrom, + decl_stmt: ast.Assign, + ) -> str | None: + """Return the "from module import Ctor" line the localized declaration needs, or None if this file has it. + + The module is the one the origin imports the constructor from, as named from this file, which + reaches the origin (origin_path) through via. If this file can't name that module, the + constructor is imported from the origin module itself. + """ + ctor_name = type_param_constructor_name(decl_stmt) + ctor_source = from_import_sources(origin_tree).get(ctor_name) + if ctor_source is None: + # TODO(FrancescoPezzella): an origin using `from typing import *` lands here, + # so a file not importing the constructor itself gets a NameError. + # TBD - needs a fix + # https://github.com/TNO/Renaissance.Py/issues/279 + return None + + via_module = "." * via.level + (via.module or "") + ctor_module = rebase_relative_module(via.module, via.level, origin_path, ctor_source[0] or None, ctor_source[1]) or via_module + + # 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 import_node in self.body: + raw = import_node.node + if ( + isinstance(raw, ast.ImportFrom) + and "." * raw.level + (raw.module or "") == ctor_module + and any((alias.asname or alias.name) == ctor_name for alias in raw.names) + ): + return None + + return f"from {ctor_module} import {ctor_name}" + + def _argument_names_to_import( + self, importing_tree: ast.Module, origin_tree: ast.Module, raw: ast.ImportFrom, decl_stmt: ast.Assign + ) -> set[str] | None: + """Return the names decl_stmt's arguments use that this file must import from the origin module. + + A name this file already imports from the origin module (or from the same absolute module the + origin imports it from) is fine and not returned. Returns None if one of those names can't be + imported from the origin as the same object: the origin doesn't bind it at module level at + runtime (e.g. only under `if TYPE_CHECKING:`), or this file binds it to something else. + """ + origin_sources = from_import_sources(origin_tree) + importing_sources = from_import_sources(importing_tree) + missing: set[str] = set() + for name in declaration_argument_names(decl_stmt): + if name not in origin_sources: + return None + if name not in importing_sources: + missing.add(name) + continue + origin_source = origin_sources[name] + accepted = {(raw.module or "", raw.level)} + 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 missing + + def _localize_import( + self, + import_node: PythonRstNode, + raw: ast.ImportFrom, + declarations: dict[str, ast.Assign], + needed_imports: list[str], + ) -> None: + """Replace import_node with the text of declarations' statements as local declarations, in one edit. + + Narrows or removes the original import for declarations' names, and prepends needed_imports: the + imports the declarations need (their constructors, names their arguments use) that this file + doesn't have yet. + """ + decl_text = "\n".join([*needed_imports, *(ast.unparse(decl_stmt) for decl_stmt in declarations.values())]) + + # TODO(FrancescoPezzella): the narrowed import is rebuilt with ast.unparse, so comments inside it are lost and a trailing + # comment ends up on the last declaration's line. + # https://github.com/TNO/Renaissance.Py/issues/279 + kept: list[ast.alias] = [alias for alias in raw.names if (alias.asname or alias.name) not in declarations] + if kept: + narrowed = ast.unparse(ast.ImportFrom(module=raw.module, names=kept, level=raw.level)) + self.replace(f"{narrowed}\n{decl_text}", import_node, include_whitespace=False, include_comments=False) + else: + self.replace(decl_text, import_node, include_whitespace=False, include_comments=False) diff --git a/src/renaissance/recipes/type_var_domain.py b/src/renaissance/recipes/type_var_domain.py new file mode 100644 index 00000000..1b005ccb --- /dev/null +++ b/src/renaissance/recipes/type_var_domain.py @@ -0,0 +1,358 @@ +# 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 + +"""TypeVar/ParamSpec/TypeVarTuple domain model and safety analysis.""" + +import ast +import builtins +from collections.abc import Iterable +from dataclasses import dataclass +from enum import StrEnum +from typing import cast + +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 and doc_link(). + """ + + PEP695_VERSION_GATE = "pep695_version_gate" + 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" + DECLARATION_NAME_UNAVAILABLE = "declaration_name_unavailable" + 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 plus the docs anchor slug for one UnsafeReason.""" + + message: str + doc_anchor: str + + +UNSAFE_RULES: dict[UnsafeReason, UnsafeRule] = { + UnsafeReason.PEP695_VERSION_GATE: UnsafeRule( + "target's minimum Python version is unknown or below 3.12", + "feature-type-parameter-modernization-pep695-version-gate", + ), + UnsafeReason.PEP696_VERSION_GATE: UnsafeRule( + "declares a default= (PEP 696), which needs the target's minimum Python to be 3.13+", + "feature-type-parameter-modernization-pep696-version-gate", + ), + UnsafeReason.NO_PEP695_EQUIVALENT: UnsafeRule( + "declaration passes an argument with no PEP 695 equivalent, e.g. covariant=True or bound= on a ParamSpec", + "feature-type-parameter-modernization-no-pep695-equivalent", + ), + UnsafeReason.DECLARED_TYPEVAR_EXPORTED: UnsafeRule( + "exported via __all__", + "feature-type-parameter-modernization-declared-type-parameter-exported", + ), + UnsafeReason.USED_IN_GENERIC_CLASS: UnsafeRule( + "only used inside a class that is generic over it", + "feature-type-parameter-modernization-used-in-generic-class", + ), + UnsafeReason.DECLARATION_NAME_UNAVAILABLE: UnsafeRule( + "a name its declaration uses can't be imported here as the same object", + "feature-type-parameter-modernization-declaration-name-unavailable", + ), + UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT: UnsafeRule( + "imported directly by another file in the target project", + "feature-type-parameter-modernization-imported-elsewhere-in-project", + ), + UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY: UnsafeRule( + "origin module imports TypeVar/ParamSpec/TypeVarTuple conditionally, e.g. per Python version", + "feature-type-parameter-modernization-origin-imports-constructor-conditionally", + ), +} + + +def doc_link(reason: UnsafeReason) -> str: + """Return the full URL to the documented rule explaining why `reason` makes a candidate unsafe.""" + return f"{DOCS_BASE_URL}#{UNSAFE_RULES[reason].doc_anchor}" + + +def _is_type_param_call(value: ast.expr) -> bool: + """Return True if `value` is a call to TypeVar/ParamSpec/TypeVarTuple.""" + return isinstance(value, ast.Call) and isinstance(value.func, ast.Name) and value.func.id in ("TypeVar", "ParamSpec", "TypeVarTuple") + + +def find_type_param_declarations(tree: ast.Module) -> dict[str, ast.Assign]: + """Find every module-level "NAME = TypeVar/ParamSpec/TypeVarTuple(...)" declaration.""" + declarations: dict[str, ast.Assign] = {} + for stmt in tree.body: + if isinstance(stmt, ast.Assign) and _is_type_param_call(stmt.value): + for target in stmt.targets: + if isinstance(target, ast.Name): + declarations[target.id] = stmt + return declarations + + +def type_param_name(param: ast.type_param) -> str: + """Return a PEP 695 type parameter's name, narrowing to the subclasses whose stubs declare `.name`.""" + return cast("ast.TypeVar | ast.ParamSpec | ast.TypeVarTuple", param).name + + +def type_param_constructor_name(decl_stmt: ast.Assign) -> str: + """Return the name of the call a declaration uses, e.g. "TypeVar" for `T = TypeVar("T")`.""" + call = cast("ast.Call", decl_stmt.value) + return cast("ast.Name", call.func).id + + +def _find_dunder_all(tree: ast.Module) -> set[str] | None: + """Return the names in this module's plain `__all__ = [...]` assignment, or None if it has none. + + Only a list, tuple or set literal is read; annotated, `+=`, `.extend()` and `.append()` forms are ignored. + """ + for stmt in tree.body: + if ( + isinstance(stmt, ast.Assign) + and any(isinstance(t, ast.Name) and t.id == "__all__" for t in stmt.targets) + and isinstance(stmt.value, ast.List | ast.Tuple | ast.Set) + ): + return {elt.value for elt in stmt.value.elts if isinstance(elt, ast.Constant) and isinstance(elt.value, str)} + return None + + +def _declares_type_param(class_node: ast.ClassDef, name: str) -> bool: + """Return True if the class declares `name` as its own PEP 695 type parameter (`class Box[T]:`).""" + return any(type_param_name(param) == name for param in class_node.type_params) + + +def _inherits_type_param(class_node: ast.ClassDef, name: str) -> bool: + """Return True if the class is generic over the module-level `name` through one of its bases. + + That is, a base mentions `name` (`Generic[T]`, `Protocol[T]`, `Mapping[T]`, `Base[int, T]`) and the + class doesn't declare its own `name`. + """ + return not _declares_type_param(class_node, name) and any( + isinstance(child, ast.Name) and child.id == name for base in class_node.bases for child in ast.walk(base) + ) + + +def declaration_argument_names(decl_stmt: ast.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 = cast("ast.Call", decl_stmt.value) + 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): + try: + expression = ast.parse(argument.value, mode="eval").body + except SyntaxError: + continue + names.update(node.id for node in ast.walk(expression) if isinstance(node, ast.Name)) + return names - set(dir(builtins)) + + +def from_import_sources(tree: ast.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 stmt in tree.body: + if isinstance(stmt, ast.ImportFrom): + for alias in stmt.names: + sources[alias.asname or alias.name] = (stmt.module or "", stmt.level) + elif isinstance(stmt, ast.Import): + for alias in stmt.names: + sources[(alias.asname or alias.name).split(".")[0]] = None + elif isinstance(stmt, ast.ClassDef | ast.FunctionDef | ast.AsyncFunctionDef): + sources[stmt.name] = None + elif isinstance(stmt, ast.Assign | ast.AnnAssign): + sources.update(dict.fromkeys(_assigned_names(stmt))) + return sources + + +def _assigned_names(stmt: ast.Assign | ast.AnnAssign) -> list[str]: + """Return every name an assignment binds, including inside tuple or list targets.""" + targets = stmt.targets if isinstance(stmt, ast.Assign) else [stmt.target] + return [node.id for target in targets for node in ast.walk(target) if isinstance(node, ast.Name)] + + +def _imports_name_conditionally(tree: ast.Module, name: str) -> bool: + """Return True if an import inside a module-level `if`/`try` block binds `name`.""" + for stmt in tree.body: + if not isinstance(stmt, ast.If | ast.Try | ast.TryStar): + continue + for node in ast.walk(stmt): + if isinstance(node, ast.Import | ast.ImportFrom) and any( + (alias.asname or alias.name.split(".")[0]) == name for alias in node.names + ): + return True + return False + + +def is_safe_to_localize(origin_tree: ast.Module, name: str) -> UnsafeReason | None: + """Return None if `name` is safe to duplicate as a local declaration, else the reason it isn't. + + Its constructor (TypeVar/ParamSpec/TypeVarTuple) must not be imported inside a module-level + `if`/`try` block at the origin (ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY), since which + implementation it binds then depends on the runtime, e.g. `typing_extensions` below 3.13. + """ + declaration = find_type_param_declarations(origin_tree).get(name) + if declaration is not None and _imports_name_conditionally(origin_tree, type_param_constructor_name(declaration)): + return UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY + return None + + +def functions_using_nodes(tree: ast.Module, names: Iterable[str]) -> dict[str, list[ast.FunctionDef | ast.AsyncFunctionDef]]: + """Map each of `names`, in the given order, to the outermost function/method node whose signature or body references it. + + A name used in a nested function (a closure) is attributed to the outermost one, never the nested + one: a PEP 695 type parameter on the enclosing function is already visible inside its closures. + """ + usage: dict[str, list[ast.FunctionDef | ast.AsyncFunctionDef]] = {name: [] for name in names} + + def visit(node: ast.AST, enclosing: ast.FunctionDef | ast.AsyncFunctionDef | None) -> None: + current = enclosing + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef) and enclosing is None: + current = node + if isinstance(node, ast.Name) and current is not None and node.id in usage and current not in usage[node.id]: + usage[node.id].append(current) + for child in ast.iter_child_nodes(node): + visit(child, current) + + visit(tree, None) + return usage + + +def functions_in_generic_classes(tree: ast.Module, name: str) -> list[ast.FunctionDef | ast.AsyncFunctionDef]: + """Return every function or method defined inside a class that is generic over `name`. + + A class is generic over `name` when it declares it as a PEP 695 type parameter (`class Box[T]:`) + or inherits it through one of its bases (see _inherits_type_param). + """ + return [ + function + for node in ast.walk(tree) + if isinstance(node, ast.ClassDef) and (_declares_type_param(node, name) or _inherits_type_param(node, name)) + for function in ast.walk(node) + if isinstance(function, ast.FunctionDef | ast.AsyncFunctionDef) + ] + + +def is_safe_to_remove( + tree: ast.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. + + Otherwise returns the reason it may not: DECLARED_TYPEVAR_EXPORTED if exported via a plain `__all__ = [...]`, + or IMPORTED_ELSEWHERE_IN_PROJECT if `name` is in `project_wide_imported_names` (another file in + the target project imports it directly, regardless of `__all__`). + """ + dunder_all = _find_dunder_all(tree) + if dunder_all is not None and name in dunder_all: + return UnsafeReason.DECLARED_TYPEVAR_EXPORTED + if name in project_wide_imported_names: + return UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT + return None + + +def all_refs_shadowed_by_pep695(tree: ast.Module, name: str, decl_stmt: ast.Assign) -> bool: + """Return True if every remaining reference to `name` is shadowed by a PEP 695 type parameter. + + E.g. `def b[T](x: T) -> T:`, where `T` resolves to the function's own parameter rather than + the module-level declaration, making it dead. Also true (vacuously) if `name` isn't + referenced anywhere at all. + """ + found_live_use = False + + def visit(node: ast.AST, *, shadowed: bool) -> None: + nonlocal found_live_use + if node is decl_stmt or found_live_use: + return + if isinstance(node, ast.Name) and node.id == name: + if not shadowed: + found_live_use = True + return + current = shadowed + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef): + current = shadowed or any(type_param_name(param) == name for param in node.type_params) + for child in ast.iter_child_nodes(node): + visit(child, shadowed=current) + + visit(tree, shadowed=False) + return not found_live_use + + +# infer_variance needs no syntax of its own: a PEP 695 type parameter always infers its variance. +_CONVERTIBLE_KEYWORDS: dict[str, frozenset[str]] = { + "TypeVar": frozenset({"bound", "default", "infer_variance"}), + "ParamSpec": frozenset({"default", "infer_variance"}), + "TypeVarTuple": frozenset({"default", "infer_variance"}), +} + + +def declared_default(decl_stmt: ast.Assign) -> ast.expr | None: + """Return the value a legacy declaration passes as `default=` (PEP 696), or None if it passes none.""" + call = cast("ast.Call", decl_stmt.value) + return next((keyword.value for keyword in call.keywords if keyword.arg == "default"), None) + + +def has_unconvertible_argument(decl_stmt: ast.Assign) -> bool: + """Return True if a legacy declaration passes a keyword build_type_param can't carry over. + + That is any keyword outside the constructor's known set, e.g. `covariant=`/`contravariant=`, + `bound=` on a ParamSpec or TypeVarTuple, a keyword added by a later Python version, or a `**mapping`. + """ + call = cast("ast.Call", decl_stmt.value) + convertible = _CONVERTIBLE_KEYWORDS[type_param_constructor_name(decl_stmt)] + return any(keyword.arg is None or keyword.arg not in convertible for keyword in call.keywords) + + +def build_type_param(decl_stmt: ast.Assign) -> ast.type_param: + """Translate a legacy declaration into the equivalent PEP 695 type_param node. + + E.g. `T = TypeVar("T", bound=int, default=int)` becomes `T: int = int`. Bounds, constraints and + `default=` are carried over; `infer_variance=` needs nothing, since a PEP 695 type parameter always + infers its variance. Callers must first reject declarations for which + has_unconvertible_argument() is True. + """ + # TODO(FrancescoPezzella): a default that uses another legacy declaration (`default=T`) still refers to it after conversion. + # https://github.com/TNO/Renaissance.Py/issues/279 + call = cast("ast.Call", decl_stmt.value) + ctor = type_param_constructor_name(decl_stmt) + name = cast("str", cast("ast.Constant", call.args[0]).value) + default = declared_default(decl_stmt) + + if ctor == "ParamSpec": + return ast.ParamSpec(name=name, default_value=default) + if ctor == "TypeVarTuple": + return ast.TypeVarTuple(name=name, default_value=default) + + bound = next((kw.value for kw in call.keywords if kw.arg == "bound"), None) + constraints = call.args[1:] + if bound is None and constraints: + bound = ast.Tuple(elts=list(constraints), ctx=ast.Load()) + return ast.TypeVar(name=name, bound=bound, default_value=default) + + +def type_params_defaults_last(params: list[ast.type_param]) -> list[ast.type_param]: + """Return `params` with every type parameter that has a default moved after those without one. + + The relative order within each group is kept. A non-default type parameter following a defaulted + one is a SyntaxError. + """ + + def has_default(param: ast.type_param) -> bool: + """Return True if `param` declares a default (`T = int`).""" + return isinstance(param, ast.TypeVar | ast.ParamSpec | ast.TypeVarTuple) and param.default_value is not None + + return sorted(params, key=has_default) diff --git a/src/renaissance/syntax_tree/ast_refactor_actions.py b/src/renaissance/syntax_tree/ast_refactor_actions.py index f8ee9adf..298e1449 100644 --- a/src/renaissance/syntax_tree/ast_refactor_actions.py +++ b/src/renaissance/syntax_tree/ast_refactor_actions.py @@ -16,6 +16,8 @@ from .match_finder import MatchFinder, PatternMatch from .semantic_kind import SemanticKind +# TODO(FrancescoPezzella): Replace with a lazy import in Python 3.15. +# https://github.com/TNO/Renaissance.Py/issues/275 if TYPE_CHECKING: from renaissance.integrations.clang.c_pattern_factory import CPPPatternFactory diff --git a/src/renaissance/syntax_tree/ast_rewriter.py b/src/renaissance/syntax_tree/ast_rewriter.py index 3f939ec7..c3576622 100644 --- a/src/renaissance/syntax_tree/ast_rewriter.py +++ b/src/renaissance/syntax_tree/ast_rewriter.py @@ -305,10 +305,7 @@ def __check_for_conflicting_rewrites(self) -> None: @staticmethod def __is_nested(node: Rewritable, maybe_ancestor: Rewritable) -> bool: - """Return True if maybe_ancestor is an ancestor of node, walking .parent. - - Not node.is_ancestor_of() - not every Rewritable implements it (e.g. PythonRstNode). - """ + """Return True if maybe_ancestor is an ancestor of node, walking .parent (works for every Rewritable).""" parent = node.parent while parent: if parent is maybe_ancestor: diff --git a/src/renaissance/utils/import_resolution.py b/src/renaissance/utils/import_resolution.py new file mode 100644 index 00000000..2b6fc664 --- /dev/null +++ b/src/renaissance/utils/import_resolution.py @@ -0,0 +1,169 @@ +# 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 + +"""Resolve project-internal `from X import Y` statements to the .py file they import from, and re-anchor them on other files.""" + +import ast +from collections.abc import Sequence +from pathlib import Path + + +def resolve_project_module(importing_file: Path, project_root: Path, module: str | None, level: int) -> Path | None: + """Resolve one `ast.ImportFrom`'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. + + # TODO: doesn't follow re-exports through an intermediate __init__.py, or handle namespace + # packages (no __init__.py, PEP 420) - out of scope for now. + """ + # 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: + tree = ast.parse(file.read_text(encoding="utf-8")) + except OSError, SyntaxError: + continue + module_bindings: dict[str, Path] = {} + for stmt in ast.walk(tree): + if isinstance(stmt, ast.ImportFrom): + _record_from_import(file, project_root, stmt, imported, module_bindings) + elif isinstance(stmt, ast.Import): + _bind_imported_modules(file, project_root, stmt, module_bindings) + _record_module_attribute_reads(tree, module_bindings, imported) + return {path: frozenset(names) for path, names in imported.items()} + + +def _record_from_import( + file: Path, + project_root: Path, + stmt: ast.ImportFrom, + imported: dict[Path, set[str]], + module_bindings: dict[str, Path], +) -> None: + """Record `stmt`'s imported names against their origin, and bind any alias that is itself a project module.""" + # TODO(FrancescoPezzella): `from pkg.mod import *` isn't expanded to the names it actually pulls in. + # https://github.com/TNO/Renaissance.Py/issues/279 + origin = resolve_project_module(file, project_root, stmt.module, stmt.level) + if origin is not None: + imported.setdefault(origin, set()).update(alias.name for alias in stmt.names) + for alias in stmt.names: + submodule = f"{stmt.module}.{alias.name}" if stmt.module is not None else alias.name + submodule_origin = resolve_project_module(file, project_root, submodule, stmt.level) + if submodule_origin is not None: + module_bindings[alias.asname or alias.name] = submodule_origin + + +def _bind_imported_modules(file: Path, project_root: Path, stmt: ast.Import, module_bindings: dict[str, Path]) -> None: + """Bind the dotted name(s) `stmt` makes available to the project module file each one refers to.""" + for alias in stmt.names: + if alias.asname is not None: + origin = resolve_project_module(file, project_root, alias.name, 0) + if origin is not None: + module_bindings[alias.asname] = origin + continue + # `import a.b.c` binds `a`, and makes `a.b` and `a.b.c` reachable through it. + parts = alias.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(tree: ast.Module, module_bindings: dict[str, Path], imported: dict[Path, set[str]]) -> None: + """Record every `.` in `tree` as a dependency on `attr` in that module's file.""" + if not module_bindings: + return + for node in ast.walk(tree): + if not isinstance(node, ast.Attribute): + continue + dotted = _dotted_name(node.value) + origin = module_bindings.get(dotted) if dotted is not None else None + if origin is not None: + imported.setdefault(origin, set()).add(node.attr) + + +def _dotted_name(expr: ast.expr) -> str | None: + """Return `expr` as a dotted name (`a.b.c`) if it is a plain name/attribute chain, else None.""" + if isinstance(expr, ast.Name): + return expr.id + if isinstance(expr, ast.Attribute): + base = _dotted_name(expr.value) + return f"{base}.{expr.attr}" if base is not None else None + return None diff --git a/src/renaissance/utils/unparse_utils.py b/src/renaissance/utils/unparse_utils.py new file mode 100644 index 00000000..6e55eb23 --- /dev/null +++ b/src/renaissance/utils/unparse_utils.py @@ -0,0 +1,124 @@ +# 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 + +"""Splice a PEP 695 type-params bracket into a function's original source, changing nothing else. + +The function's formatting and comments are kept. +""" + +import ast +import io +import re +import tokenize + + +def _name_end_offset(source: str, name: str) -> int: + """Return the character offset right after `def name`/`async def name` in `source`. + + The `def` line may be indented, e.g. in a decorated function's source. + """ + match = re.search(rf"^[ \t]*(async\s+)?def\s+{re.escape(name)}\b", source, re.MULTILINE) + if match is None: + message = f"no 'def {name}' header found" + raise ValueError(message) + return match.end() + + +def _bracket_end_offset(source: str, open_offset: int) -> int: + """Return the offset right after the `]` matching the `[` at `open_offset` in `source`. + + Tracks bracket depth, so a nested `list[int]` doesn't close it early. A `[` or `]` inside a string + literal is not accounted for. + """ + depth = 0 + for offset in range(open_offset, len(source)): + if source[offset] == "[": + depth += 1 + elif source[offset] == "]": + depth -= 1 + if depth == 0: + return offset + 1 + message = "no closing ']' found" + raise ValueError(message) + + +def _type_params_bracket(node: ast.FunctionDef | ast.AsyncFunctionDef) -> str: + """Return the PEP 695 `[...]` bracket text for node's current type_params, or "" if none. + + E.g. `"[T]"`, `"[T: int, **P]"` - whatever `ast.unparse(node)` would produce. + """ + if not node.type_params: + return "" + unparsed = ast.unparse(node) + start = _name_end_offset(unparsed, node.name) + end = _bracket_end_offset(unparsed, start) + return unparsed[start:end] + + +def _header_end_line(source: str) -> int: + """Return the 1-indexed line where a def header's terminating ':' sits in `source`. + + Tracks `([{`/`)]}` bracket depth (via `tokenize`) so a colon inside a string default, a + lambda default, or an annotation - anything not at the header's own top level - isn't + mistaken for the real one. + """ + depth = 0 + for tok in tokenize.generate_tokens(io.StringIO(source).readline): + if tok.type == tokenize.OP and tok.string in "([{": + depth += 1 + elif tok.type == tokenize.OP and tok.string in ")]}": + depth -= 1 + elif tok.type == tokenize.OP and tok.string == ":" and depth == 0: + return tok.end[0] + message = "no header-terminating ':' found" + raise ValueError(message) + + +def unparse_signature_only(node: ast.FunctionDef | ast.AsyncFunctionDef, original_text: str) -> str: + """Insert node's PEP 695 type-params bracket into `original_text`, changing nothing else. + + `original_text` is node's source from before `node.type_params` was changed. An existing bracket + after the function name (`def f[U](...)`) is replaced by the new one, which already holds both the + old and new type parameters; otherwise a bracket is inserted. + + Everything else is kept, but lines after the first are re-indented relative to a column-0 `def` + (see `_renormalize_indent`), since the rewriter adds the target's own indentation to them again. + """ + new_bracket = _type_params_bracket(node) + insert_at = _name_end_offset(original_text, node.name) + + end = insert_at + if original_text[insert_at : insert_at + 1] == "[": + end = _bracket_end_offset(original_text, insert_at) + + spliced = original_text[:insert_at] + new_bracket + original_text[end:] + lines = spliced.split("\n") + if len(lines) == 1: + return spliced + + header_end_line = _header_end_line(spliced) + header_tail = lines[1:header_end_line] # a multi-line signature's own continuation lines + body = lines[header_end_line:] + # the header's own continuation lines (e.g. a closing ") -> T:") sit at the def's own column, + # so they renormalize to 0; the body sits one Python indentation level deeper, so 4. + return "\n".join([lines[0], *_renormalize_indent(header_tail, 0), *_renormalize_indent(body, 4)]) + + +def _renormalize_indent(lines: list[str], target_indent: int) -> list[str]: + """Shift `lines` so their common leading indent becomes `target_indent`.""" + non_blank = [line for line in lines if line.strip()] + if not non_blank: + return lines + # TODO(FrancescoPezzella): this minimum also counts lines inside multi-line string literals, which can change a literal's value. + # https://github.com/TNO/Renaissance.Py/issues/279 + common = min(len(line) - len(line.lstrip(" ")) for line in non_blank) + shift = common - target_indent + if shift > 0: + return [line[shift:] if line.strip() else line for line in lines] + if shift < 0: + pad = " " * -shift + return [pad + line if line.strip() else line for line in lines] + return lines diff --git a/test/project/test_project_scanner.py b/test/project/test_project_scanner.py index 1086cf9a..cb356755 100644 --- a/test/project/test_project_scanner.py +++ b/test/project/test_project_scanner.py @@ -268,6 +268,20 @@ def test_default_package_dirs_scans_whole_root_dir(self, tmp_path) -> None: assert_that(result, equal_to([str(redis_like / "client.py")])) + @pytest.mark.parametrize("excluded_dir", sorted(PythonScanner.EXCLUDED_DIRS)) + def test_root_dir_inside_excluded_name_is_still_scanned(self, tmp_path: Path, excluded_dir: str) -> None: + """Assert PythonScanner.find_sources only applies EXCLUDED_DIRS below root_dir, never to its ancestors.""" + root = tmp_path / excluded_dir / "project" + noise_dir = root / excluded_dir + noise_dir.mkdir(parents=True) + (noise_dir / "ignored.py").write_text("") + (root / "kept.py").write_text("") + + scanner = PythonScanner(str(root)) + result = [Path(p).name for p in scanner.find_sources()] + + assert_that(result, equal_to(["kept.py"])) + # --------------------------------------------------------------------------- # BearCppScanner diff --git a/test/recipes/conftest.py b/test/recipes/conftest.py new file mode 100644 index 00000000..65f0d2ca --- /dev/null +++ b/test/recipes/conftest.py @@ -0,0 +1,52 @@ +# 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 fixtures for the refactoring recipe test suite.""" + +import textwrap +from collections.abc import Callable +from typing import cast + +import pytest +from pytest_mock import MockerFixture + +from renaissance.integrations.python.ast.rst_node import PythonRstNode +from renaissance.recipes.python_refactoring import PythonRefactoring +from renaissance.recipes.type_var_check import PEP_695_MINIMUM, TypeVarCheck + + +@pytest.fixture +def make_recipe(mocker: MockerFixture) -> Callable[[type[PythonRefactoring], str], PythonRefactoring]: + """Build a `recipe_cls` instance against in-memory, dedented source text. + + `PythonFactory.create` is mocked so nothing touches the filesystem. Shared by every + refactoring recipe's tests instead of each reimplementing this setup - cast the result + to the concrete recipe type if you need attributes/methods beyond PythonRefactoring's own. + """ + + def _make(recipe_cls: type[PythonRefactoring], text: str, filename: str = "x.py") -> PythonRefactoring: + code = textwrap.dedent(text) + mocker.patch( + "renaissance.integrations.python.ast.factory.PythonFactory.create", + return_value=PythonRstNode.load_from_text(code), + ) + subject = recipe_cls(filename) + subject.in_memory = True + return subject + + return _make + + +@pytest.fixture +def create_type_var_check(make_recipe: Callable[[type[PythonRefactoring], str], PythonRefactoring]) -> Callable[[str], TypeVarCheck]: + """Like `make_recipe`, but with min_python set to 3.12 so PEP 695 conversion is enabled.""" + + def _create(text: str) -> TypeVarCheck: + subject = cast("TypeVarCheck", make_recipe(TypeVarCheck, text)) + subject.min_python = PEP_695_MINIMUM + return subject + + return _create diff --git a/test/recipes/test_python_refactoring.py b/test/recipes/test_python_refactoring.py index cb6e84d9..7b18f388 100644 --- a/test/recipes/test_python_refactoring.py +++ b/test/recipes/test_python_refactoring.py @@ -6,6 +6,7 @@ """Tests for the PythonRefactoring recipe base class.""" +import ast import keyword import textwrap from pathlib import Path @@ -317,3 +318,27 @@ def test_recipe_rewrites_with_the_chosen_backend(self, mocker: MockerFixture, no assert_that(cast("PythonFactory", subject.factory).clazz is node_class, is_(True)) assert_that(subject.apply_to_string(), is_("x = 3\ny = 2\n")) + + # ------------------------------------------------------------------ + # find_rst_node + # ------------------------------------------------------------------ + + def test_find_rst_node_returns_wrapper_for_raw_ast_node(self, mocker: MockerFixture) -> None: + """AI: Verify find_rst_node locates the PythonRstNode wrapping a given raw ast.FunctionDef.""" + self._patch_factory( + mocker, + """ + def foo(): + pass + """, + "test_foo.py", + ) + + subject = UnitToPytest("test_foo.py") + root = cast("PythonRstNode", cast("object", subject.root)) + module = cast("ast.Module", root.node) + target = next(node for node in ast.walk(module) if isinstance(node, ast.FunctionDef)) + + found = subject.find_rst_node(target) + + assert_that(found.node, is_(target)) 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_var_check.py b/test/recipes/test_type_var_check.py new file mode 100644 index 00000000..cb33b3e0 --- /dev/null +++ b/test/recipes/test_type_var_check.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 + +"""End-to-end tests for TypeVarCheck.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_var_check import TypeVarCheck + + +class TestTypeVarCheck: + """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_var_check: Callable[[str], TypeVarCheck], + 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_var_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_keeps_declaration_still_used_by_a_generic_class(self, create_type_var_check: Callable[[str], TypeVarCheck]) -> None: + """Verify check() converts a standalone function but keeps the declaration a Generic[...] base still needs.""" + subject = create_type_var_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_var_check_convert.py b/test/recipes/test_type_var_check_convert.py new file mode 100644 index 00000000..4461d4e5 --- /dev/null +++ b/test/recipes/test_type_var_check_convert.py @@ -0,0 +1,587 @@ +# 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 TypeVarCheck.convert_declared_typevars.""" + +import ast +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_var_check import PEP_695_MINIMUM, PEP_696_MINIMUM, TypeVarCheck +from renaissance.recipes.type_var_domain import UnsafeReason, type_param_name + + +class TestTypeVarCheckConvert: + """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_var_check: Callable[[str], TypeVarCheck], 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_var_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( + "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_var_check: Callable[[str], TypeVarCheck], 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_var_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_var_check: Callable[[str], TypeVarCheck], + 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_var_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_var_check: Callable[[str], TypeVarCheck]) -> None: + """AI: Verify converting a signature never reformats or collapses its body's unusual formatting.""" + subject = create_type_var_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_var_check: Callable[[str], TypeVarCheck]) -> 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_var_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]"))) + + def test_merges_into_an_existing_type_params_bracket(self, create_type_var_check: Callable[[str], TypeVarCheck]) -> None: + """AI: Verify converting a second TypeVar merges it into an existing PEP 695 bracket instead of adding a new one.""" + subject = create_type_var_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_var_check: Callable[[str], TypeVarCheck]) -> None: + """AI: Verify converting a decorated @overload signature accounts for its non-zero-column indentation.""" + subject = create_type_var_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_var_check: Callable[[str], TypeVarCheck]) -> None: + """Verify a function using two declared type parameters gets both in one bracket.""" + subject = create_type_var_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_var_check: Callable[[str], TypeVarCheck]) -> 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_var_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: ...")) + + @pytest.mark.xfail( + reason="_renormalize_indent takes the body's minimum indent over every line, including the lines " + "inside a multi-line string literal, so the literal's value changes.", + raises=AssertionError, + strict=True, + ) + def test_converts_function_preserving_multiline_string_literal(self, create_type_var_check: Callable[[str], TypeVarCheck]) -> 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_var_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", + [ + pytest.param("class Box(Generic[T]):", id="generic-base"), + pytest.param("class Box(Protocol[T]):", id="protocol-base"), + pytest.param("class Sub(Base[T]):", 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_var_check: Callable[[str], TypeVarCheck], class_header: str + ) -> 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_var_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"})) + 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:")) + + @pytest.mark.parametrize( + "class_header", + [ + pytest.param("class Box(Generic[T]):", id="generic-base"), + pytest.param("class Box[T]:", id="pep695-class"), + ], + ) + def test_reports_typevar_used_only_in_a_generic_class( + self, create_type_var_check: Callable[[str], TypeVarCheck], class_header: str + ) -> None: + """Verify a TypeVar only a generic class uses is reported unsafe with its reason and left as it is.""" + subject = create_type_var_check(f""" + from typing import Generic, TypeVar + T = TypeVar('T') + + {class_header} + 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:")) + + @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_var_check: Callable[[str], TypeVarCheck], class_header: str + ) -> None: + """Verify a method gets [T] when its class isn't generic over T.""" + subject = create_type_var_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:")) + + def test_converts_function_when_typevar_is_also_used_outside_functions( + self, create_type_var_check: Callable[[str], TypeVarCheck] + ) -> None: + """Verify a function using T is converted even though a module-level alias also uses T.""" + subject = create_type_var_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 TestTypeVarCheckConvertArguments: + """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( + '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_var_check: Callable[[str], TypeVarCheck], + declaration: str, + signature: str, + expected_signature: str, + ) -> None: + """Verify bounds, constraints, defaults and inferred variance are carried over.""" + name = declaration.split(" = ", maxsplit=1)[0] + subject = create_type_var_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")', + _TYPEVAR_USE, + (3, 11), + UnsafeReason.PEP695_VERSION_GATE, + id="typevar-plain-3.11", + ), + pytest.param( + 'T = TypeVar("T", default=int)', + _TYPEVAR_USE, + PEP_695_MINIMUM, + UnsafeReason.PEP696_VERSION_GATE, + id="typevar-default-3.12", + ), + pytest.param( + 'P = ParamSpec("P", default=[int, str])', + _PARAMSPEC_USE, + PEP_695_MINIMUM, + UnsafeReason.PEP696_VERSION_GATE, + id="paramspec-default-3.12", + ), + pytest.param( + 'Ts = TypeVarTuple("Ts", default=Unpack[tuple[int]])', + _TYPEVARTUPLE_USE, + PEP_695_MINIMUM, + UnsafeReason.PEP696_VERSION_GATE, + id="typevartuple-default-3.12", + ), + pytest.param( + 'T = TypeVar("T", covariant=True)', + _TYPEVAR_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="typevar-covariant", + ), + pytest.param( + 'T = TypeVar("T", contravariant=True)', + _TYPEVAR_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="typevar-contravariant", + ), + pytest.param( + 'P = ParamSpec("P", covariant=True)', + _PARAMSPEC_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="paramspec-covariant", + ), + pytest.param( + 'Ts = TypeVarTuple("Ts", bound=int, covariant=True)', + _TYPEVARTUPLE_USE, + PEP_696_MINIMUM, + UnsafeReason.NO_PEP695_EQUIVALENT, + id="typevartuple-bound-covariant", + ), + 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_var_check: Callable[[str], TypeVarCheck], + 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_var_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", "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_var_check: Callable[[str], TypeVarCheck], + 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_var_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)) + + @pytest.mark.xfail( + reason="A default referencing another legacy declaration is carried over as is, so the new type " + "parameter's default refers to the module-level TypeVar.", + raises=AssertionError, + strict=True, + ) + def test_does_not_convert_default_referencing_another_declaration( + self, + create_type_var_check: Callable[[str], TypeVarCheck], + ) -> None: + """Verify a declaration whose default uses another legacy type parameter is reported unsafe and left unconverted.""" + subject = create_type_var_check(_source('T = TypeVar("T")\nU = TypeVar("U", default=T)', "def f(x: U) -> U:")) + subject.min_python = PEP_696_MINIMUM + + result = subject.convert_declared_typevars() + + assert_that(result, has_entry("U", "unsafe")) + assert_that(subject.converted_unsafe_reasons, has_entry("U", not_(UnsafeReason.PEP696_VERSION_GATE))) + assert_that(subject.apply_to_string(), contains_string("def f(x: U) -> U:")) diff --git a/test/recipes/test_type_var_check_localize.py b/test/recipes/test_type_var_check_localize.py new file mode 100644 index 00000000..7d92bf2d --- /dev/null +++ b/test/recipes/test_type_var_check_localize.py @@ -0,0 +1,555 @@ +# 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 TypeVarCheck.localize_imported_typevars.""" + +import ast +import builtins +import textwrap +from pathlib import Path +from typing import TYPE_CHECKING, cast + +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.rst_node import PythonRstNode +from renaissance.recipes.type_var_check import PEP_695_MINIMUM, TypeVarCheck +from renaissance.recipes.type_var_domain 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 hamcrest.core.matcher import Matcher + + +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)) + + +class TestTypeVarCheckLocalize: + """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"), + ) -> TypeVarCheck: + """Write origin_text to the origin file and return an in-memory TypeVarCheck 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 = str(tmp_path / importing_rel_path) + mocker.patch( + "renaissance.integrations.python.ast.factory.PythonFactory.create", + return_value=PythonRstNode.load_from_text(importing_code, importing_file), + ) + subject = TypeVarCheck(importing_file) + subject.in_memory = True + subject.min_python = PEP_695_MINIMUM + subject.project_root = tmp_path + return subject + + @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, mocker: MockerFixture, 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" + subject = self._create_cross_file( + mocker, + tmp_path, + origin, + """ + from file_1 import T + def b(x: T) -> T: + return x + """, + ) + 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( + "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, mocker: MockerFixture, tmp_path: Path, declaration: str) -> None: + """Verify the local copy has the origin's exact arguments and every name they use is defined in the importing file.""" + origin = f"from typing import TypeVar\nclass Shape:\n pass\n{declaration}\n" + subject = self._create_cross_file( + mocker, + tmp_path, + origin, + """ + from file_1 import T + def biggest(items: list[T]) -> T: + return items[0] + """, + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "fixed"})) + module = ast.parse(subject.apply_to_string()) + local_call = next(node.value for node in module.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(module), equal_to(set())) + + @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, mocker: MockerFixture, tmp_path: Path, origin: str, importing_extra: str + ) -> None: + """Verify a declaration using a name that can't be imported from the origin as the same object stays imported.""" + subject = self._create_cross_file( + mocker, + tmp_path, + origin, + f"from file_1 import T\n{importing_extra}\ndef biggest(items: list[T]) -> T:\n return items[0]\n", + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({"T": "unsafe"})) + assert_that(subject.cross_file_unsafe_reasons, equal_to({"T": UnsafeReason.DECLARATION_NAME_UNAVAILABLE})) + assert_that(subject.apply_to_string(), contains_string("from file_1 import T")) + + @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", + ), + ], + ) + @pytest.mark.xfail( + reason="The narrowed import is rebuilt with ast.unparse, which has no comments: comments inside the import are " + "lost and a trailing comment stays after the replacement's last line, the localized declaration.", + raises=AssertionError, + strict=True, + ) + 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, Shape", + "from typing import TypeVar", + "T = TypeVar('T', bound=Shape)", + "U = TypeVar('U', bound=Circle)", + ], + id="bounds-share-one-import", + ), + ], + ) + 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( + ("origin_import", "importing_header"), + [ + pytest.param("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, + mocker: MockerFixture, + tmp_path: Path, + origin_import: str, + importing_header: str, + ) -> None: + """Verify localizing adds "from typing import TypeVar" when missing and never duplicates it.""" + subject = self._create_cross_file( + mocker, + 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", + ) + result = subject.localize_imported_typevars() + + assert_that(result, has_entry("T", "fixed")) + assert_that(subject.apply_to_string().count("from typing import TypeVar"), equal_to(1)) + + @pytest.mark.parametrize( + ("layout", "importing_header", "expected_line"), + [ + 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", + "from ._compat import TypeVar", + 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, + mocker: MockerFixture, + tmp_path: Path, + layout: tuple[str, str], + importing_header: str, + expected_line: str, + ) -> None: + """Verify a constructor the origin imports relatively is imported once, from that same module as seen from here.""" + subject = self._create_cross_file( + mocker, + tmp_path, + 'from ._compat import TypeVar\nT = TypeVar("T")\n', + f"{importing_header}\ndef b(x: T) -> T:\n return x\n", + layout, + ) + result = subject.localize_imported_typevars() + + assert_that(result, has_entry("T", "fixed")) + assert_that(subject.apply_to_string().splitlines().count(expected_line), equal_to(1)) + + def test_does_not_localize_when_origin_imports_constructor_conditionally( + self, + mocker: MockerFixture, + tmp_path: Path, + ) -> None: + """A TypeVar whose origin picks TypeVar per Python version stays imported, marked unsafe.""" + subject = self._create_cross_file( + mocker, + tmp_path, + """ + import sys + if sys.version_info >= (3, 13): + from typing import TypeVar + else: + from typing_extensions import TypeVar + T = TypeVar("T", contravariant=True, default=None) + """, + """ + from typing import TypeVar + from file_1 import T, helper + def b(x: T) -> None: + helper() + """, + ) + result = subject.localize_imported_typevars() + + assert_that(result, has_entry("T", "unsafe")) + assert_that( + subject.cross_file_unsafe_reasons, + has_entry("T", UnsafeReason.ORIGIN_IMPORTS_CONSTRUCTOR_CONDITIONALLY), + ) + output = subject.apply_to_string() + assert_that(output, contains_string("from file_1 import T, helper")) + assert_that(output, not_(contains_string("T = TypeVar"))) + + def test_unparsable_origin_error_names_the_origin_file(self, mocker: MockerFixture, tmp_path: Path) -> None: + """Verify a syntax error in the origin module is raised with the origin file's path, not ``.""" + subject = self._create_cross_file( + mocker, + tmp_path, + """ + from typing import TypeVar + T = TypeVar("T") + def broken(: + """, + """ + from file_1 import T + def b(x: T) -> T: + return x + """, + ) + + with pytest.raises(SyntaxError) as excinfo: + subject.localize_imported_typevars() + + assert_that(excinfo.value.filename, equal_to(str(tmp_path / "file_1.py"))) + + def test_no_typevar_import_found(self, mocker: MockerFixture, tmp_path: Path) -> None: + """AI: Verify localize_imported_typevars reports nothing when the importing file has no cross-file TypeVar.""" + subject = self._create_cross_file( + mocker, + tmp_path, + """ + def helper() -> None: + pass + """, + """ + from file_1 import helper + def b() -> None: + helper() + """, + ) + result = subject.localize_imported_typevars() + + assert_that(result, equal_to({})) + + @pytest.mark.parametrize( + ("min_python", "expected_converted", "output_matcher"), + [ + pytest.param( + PEP_695_MINIMUM, + "fixed", + all_of(contains_string("def b[T](x: T) -> T:"), not_(contains_string("T = TypeVar"))), + id="converts-on-3.12", + ), + pytest.param( + (3, 10), + "unsafe", + all_of(contains_string("def b(x: T) -> T:"), contains_string("T = TypeVar('T')")), + id="only-localizes-below-3.12", + ), + ], + ) + def test_check_localizes_then_converts_when_the_target_allows( + self, + mocker: MockerFixture, + tmp_path: Path, + min_python: tuple[int, int], + expected_converted: str, + output_matcher: Matcher[str], + ) -> None: + """Verify check() localizes a cross-file TypeVar, then converts it only if the target supports PEP 695.""" + 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.min_python = min_python + subject.run() + + assert_that(subject.result["cross_file"], has_entry("T", "fixed")) + assert_that(subject.result["converted"], has_entry("T", expected_converted)) + assert_that(subject.apply_to_string(), output_matcher) diff --git a/test/recipes/test_type_var_check_orphaned.py b/test/recipes/test_type_var_check_orphaned.py new file mode 100644 index 00000000..b363a9f0 --- /dev/null +++ b/test/recipes/test_type_var_check_orphaned.py @@ -0,0 +1,172 @@ +# 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 TypeVarCheck.remove_orphaned_declarations.""" + +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.python_refactoring import PythonRefactoring +from renaissance.recipes.type_var_check import TypeVarCheck +from renaissance.recipes.type_var_domain 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 TestTypeVarCheckOrphaned: + """See module docstring.""" + + @pytest.mark.parametrize( + ("source", "min_python"), + [ + pytest.param(ALREADY_CONVERTED_SOURCE, (3, 12), id="already-converted"), + pytest.param(ALREADY_CONVERTED_SOURCE, (3, 11), id="already-converted-below-pep695"), + pytest.param(UNREFERENCED_SOURCE, (3, 11), id="unreferenced-below-pep695"), + pytest.param(NESTED_ONLY_CONVERTED_SOURCE, (3, 12), id="only-nested-function-converted"), + pytest.param(CLOSURE_IN_CONVERTED_FUNCTION_SOURCE, (3, 12), id="closure-inside-converted-function"), + ], + ) + def test_removes_declaration_whose_references_are_all_shadowed( + self, + make_recipe: Callable[[type[PythonRefactoring], str], PythonRefactoring], + source: str, + min_python: tuple[int, int], + ) -> None: + """Verify a declaration whose references are all shadowed by PEP 695 parameters, or absent, is removed.""" + subject = cast("TypeVarCheck", make_recipe(TypeVarCheck, source)) + subject.min_python = min_python + + 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)) + + def test_keeps_declaration_still_used_without_pep695(self, create_type_var_check: Callable[[str], TypeVarCheck]) -> None: + """Verify a declaration still used by a function without its own type parameter is left alone, unreported.""" + subject = create_type_var_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"), + ], + ) + @pytest.mark.xfail(reason="names inside string annotations aren't seen as references.", strict=True) + def test_keeps_declaration_used_in_a_string_annotation( + self, create_type_var_check: Callable[[str], TypeVarCheck], annotation: str + ) -> None: + """Verify a declaration referenced only from a string annotation is not removed as orphaned.""" + subject = create_type_var_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')")) + + @pytest.mark.parametrize( + ("imported_elsewhere", "expected_reason"), + [ + pytest.param(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_keeps_orphaned_declaration_that_is_unsafe_to_remove( + self, + create_type_var_check: Callable[[str], TypeVarCheck], + imported_elsewhere: frozenset[str], + expected_reason: UnsafeReason, + ) -> None: + """Verify an orphaned declaration is kept and reported unsafe when removing it would break an importer.""" + source = ALREADY_CONVERTED_SOURCE if imported_elsewhere else '__all__ = ["T"]\n' + ALREADY_CONVERTED_SOURCE + subject = create_type_var_check(source) + subject.project_wide_imported_names = imported_elsewhere + + result = subject.remove_orphaned_declarations() + + assert_that(result, equal_to({"T": "unsafe"})) + assert_that(subject.orphaned_unsafe_reasons, equal_to({"T": expected_reason})) + 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_var_check: Callable[[str], TypeVarCheck] + ) -> None: + """Verify removing an orphaned declaration keeps a leading comment that also documents the next declaration.""" + subject = create_type_var_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_var_check_properties.py b/test/recipes/test_type_var_check_properties.py new file mode 100644 index 00000000..98821925 --- /dev/null +++ b/test/recipes/test_type_var_check_properties.py @@ -0,0 +1,37 @@ +# 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 TypeVarCheck.check.""" + +import ast +from unittest.mock import patch + +import hypothesmith +from hypothesis import given, reject, settings + +from renaissance.integrations.python.ast.rst_node import PythonRstNode +from renaissance.recipes.type_var_check import TypeVarCheck + + +class TestTypeVarCheckProperties: + """See module docstring.""" + + @given(source=hypothesmith.from_grammar()) + @settings(max_examples=50, deadline=None) + def test_check_never_crashes(self, source: str) -> None: + """AI: Verify check() never raises on arbitrary hypothesmith-generated valid Python source.""" + try: + ast.parse(source) + except SyntaxError: + reject() + + with patch( + "renaissance.integrations.python.ast.factory.PythonFactory.create", + return_value=PythonRstNode.load_from_text(source), + ): + subject = TypeVarCheck("x.py") + subject.in_memory = True + subject.check() diff --git a/test/recipes/test_type_var_domain.py b/test/recipes/test_type_var_domain.py new file mode 100644 index 00000000..3a25adc8 --- /dev/null +++ b/test/recipes/test_type_var_domain.py @@ -0,0 +1,141 @@ +# 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_var_domain's safety predicates and their UnsafeReason results.""" + +import ast +import re +import textwrap +from pathlib import Path + +import pytest +from hamcrest import assert_that, equal_to, not_ + +from renaissance.recipes.type_var_domain import ( + UNSAFE_RULES, + UnsafeReason, + is_safe_to_localize, + is_safe_to_remove, +) + + +def _parse(source: str) -> ast.Module: + """Parse source after removing its common indentation.""" + return ast.parse(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)) + + +_FEATURE_DOC = Path(__file__).resolve().parents[2] / "docs" / "user" / "features" / "type-parameter-modernization.md" + + +class TestUnsafeRuleDocAnchors: + """Every UnsafeRule's documentation anchor exists where the CLI's report links to it.""" + + @pytest.mark.parametrize("reason", list(UnsafeReason)) + def test_anchor_is_set_on_a_heading(self, reason: UnsafeReason) -> None: + """The rule's anchor is attached to a heading line, the only place attr_list turns it into a link target.""" + anchor = UNSAFE_RULES[reason].doc_anchor + heading = re.compile(rf"^#+ .+ \{{ #{re.escape(anchor)} \}}$", re.MULTILINE) + + assert_that(heading.search(_FEATURE_DOC.read_text(encoding="utf-8")), not_(None)) diff --git a/test/rejuvenation/test_migration_type_recipes.py b/test/rejuvenation/test_migration_type_recipes.py new file mode 100644 index 00000000..bb61884c --- /dev/null +++ b/test/rejuvenation/test_migration_type_recipes.py @@ -0,0 +1,361 @@ +# 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). + +The script's filename is hyphenated (not a legal dotted module path), so it's loaded via +importlib.util.spec_from_file_location instead of a normal import - see _load_script(). +""" + +import importlib.util +import textwrap +from pathlib import Path +from types import ModuleType + +import pytest +from hamcrest import assert_that, contains_string, equal_to, not_ +from hamcrest.core.matcher import Matcher + +from renaissance.recipes.type_var_domain import UnsafeReason, doc_link + +_SCRIPT_PATH = Path(__file__).resolve().parents[2] / "src" / "rejuvenation" / "migration-type-recipes.py" + + +def _load_script() -> ModuleType: + """Import migration-type-recipes.py as a module despite its hyphenated filename.""" + spec = importlib.util.spec_from_file_location("migration_type_recipes", _SCRIPT_PATH) + if spec is None or spec.loader is None: + message = f"could not load {_SCRIPT_PATH} as a module" + raise RuntimeError(message) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +migration = _load_script() + + +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.""" + + @pytest.mark.parametrize( + ("source", "imported_elsewhere", "expected_reason"), + [ + pytest.param(UNSAFE_TYPEVAR_SOURCE, frozenset[str](), UnsafeReason.DECLARED_TYPEVAR_EXPORTED, id="exported-via-dunder-all"), + pytest.param(LEGACY_TYPEVAR_SOURCE, frozenset({"T"}), UnsafeReason.IMPORTED_ELSEWHERE_IN_PROJECT, id="imported-elsewhere"), + ], + ) + def test_converts_function_but_keeps_a_declaration_others_rely_on( + self, tmp_path: Path, source: str, imported_elsewhere: frozenset[str], expected_reason: UnsafeReason + ) -> 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(source, encoding="utf-8") + + report = migration.process_file(target, min_python=(3, 12), project_root=tmp_path, project_wide_imported_names=imported_elsewhere) + + assert_that(migration.has_fixed(report), equal_to(obj=True)) + assert_that(report.reasons, not_(None)) + assert_that((report.reasons or {})["orphaned"], equal_to({"T": expected_reason})) + 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 TestRuffImportCleanup: + """main(): the ruff F401 batch step actually drops now-unused imports end to end.""" + + @pytest.mark.parametrize( + ("extra_args", "import_matcher", "report_matcher"), + [ + pytest.param( + [], + not_(contains_string("TypeVar")), + contains_string("cleaned up via `ruff"), + id="default-drops-import", + ), + pytest.param( + ["--no-ruff"], + contains_string("from typing import TypeVar"), + not_(contains_string("cleaned up via `ruff")), + id="no-ruff-keeps-import", + ), + ], + ) + def test_unused_typevar_import_cleanup( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + extra_args: list[str], + import_matcher: Matcher[str], + report_matcher: Matcher[str], + ) -> None: + """The redundant TypeVar import is dropped, and the report says so, 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) + assert_that(capsys.readouterr().out, report_matcher) + + 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 TestConsoleReportDocLinks: + """main(): each unsafe name printed under NEEDS MANUAL REVIEW links to its documented rule.""" + + def test_needs_manual_review_includes_doc_link_for_the_specific_reason( + self, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """The report links a __all__-exported TypeVar to the DECLARED_TYPEVAR_EXPORTED rule.""" + 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, contains_string(doc_link(UnsafeReason.DECLARED_TYPEVAR_EXPORTED))) + + +class TestConsoleReportModifiedSection: + """main(): what the MODIFIED section lists per file.""" + + def test_converted_name_is_not_listed_again_as_orphaned(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + """A converted name is listed once; only declarations removed without a conversion appear under orphaned.""" + 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(" converted: T\n")) + assert_that(output, contains_string(" orphaned: U\n")) + + +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 TestMainProjectWideImportSafety: + """main(): a TypeVar imported by another file in the batch is localized there and kept at its origin.""" + + @pytest.mark.parametrize( + ("consumer_rel", "import_line"), + [ + pytest.param("pkg/client.py", "from pkg.typing_mod import T", id="absolute-dotted"), + pytest.param("pkg/client.py", "from .typing_mod import T", id="relative"), + pytest.param("other/client.py", "from pkg.typing_mod import T", id="absolute-other-directory"), + ], + ) + def test_consumer_is_localized_and_origin_declaration_survives( + self, + tmp_path: Path, + consumer_rel: str, + import_line: str, + ) -> None: + """The importing file gets a PEP 695 local TypeVar, and the origin declaration is not removed.""" + 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 = tmp_path / consumer_rel + consumer.parent.mkdir(exist_ok=True) + consumer.write_text(f"{import_line}\n\ndef use(x: T) -> T:\n return x\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((pkg / "typing_mod.py").read_text(encoding="utf-8"), contains_string('T = TypeVar("T")')) + + @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:")) + + def test_origin_declaration_survives_module_attribute_access(self, tmp_path: Path) -> None: + """A TypeVar accessed as a module attribute by another file is kept at its origin.""" + 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") + (pkg / "client.py").write_text("import pkg.typing_mod\n\ndef use(x: pkg.typing_mod.T) -> None: ...\n", encoding="utf-8") + + exit_code = migration.main([str(tmp_path), "--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")')) + + +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.11", 'T = TypeVar("T")', "def identity[", id="3.11-gated"), + pytest.param("3.12", "def identity[T]", 'T = TypeVar("T")', id="3.12-converted"), + ], + ) + def test_py_flag_gates_rewrites(self, tmp_path: Path, py_version: str, expected: str, unexpected: str) -> None: + """--py decides whether the PEP 695 rewrite runs.""" + target = tmp_path / "mod.py" + target.write_text(LEGACY_TYPEVAR_SOURCE, 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"], "expected MAJOR.MINOR (e.g. 3.12), got '3'", id="missing-minor"), + 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/syntax_tree/test_match_tree.py b/test/syntax_tree/test_match_tree.py index f7b8a597..15bc176c 100644 --- a/test/syntax_tree/test_match_tree.py +++ b/test/syntax_tree/test_match_tree.py @@ -75,6 +75,15 @@ def test_lists_with_empty_pattern(self) -> None: assert_that(is_match_tree(src, pattern), is_(False)) + # TODO(FrancescoPezzella): passes only after another test has run PythonFactory(ast.AST), which patches parser_kind onto + # every ast class. + # https://github.com/TNO/Renaissance.Py/issues/280 + @pytest.mark.xfail( + reason="pattern_kind() reads parser_kind, which a raw ast.Name only has once PythonFactory(ast.AST) has patched " + "ast.AST, so the result depends on test order.", + raises=AttributeError, + strict=False, + ) def test_is_match_tree_between_list_and_other(self) -> None: """AI: Verify is_match_tree returns False when the pattern list contains a raw non-node object.""" src = self.pattern_factory.create_statements("1") diff --git a/test/utils/__init__.py b/test/utils/__init__.py new file mode 100644 index 00000000..792d2e50 --- /dev/null +++ b/test/utils/__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 shared utility modules in renaissance.utils.""" diff --git a/test/utils/test_import_resolution.py b/test/utils/test_import_resolution.py new file mode 100644 index 00000000..d264d851 --- /dev/null +++ b/test/utils/test_import_resolution.py @@ -0,0 +1,189 @@ +# 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 pytest +from hamcrest import assert_that, equal_to, has_item, has_key, not_ + +from renaissance.utils.import_resolution import collect_project_imported_names, 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.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"))) diff --git a/test/utils/test_unparse_utils.py b/test/utils/test_unparse_utils.py new file mode 100644 index 00000000..bd1b953d --- /dev/null +++ b/test/utils/test_unparse_utils.py @@ -0,0 +1,113 @@ +# 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 signature-only PEP 695 bracket-splice helpers.""" + +import ast +import textwrap +from typing import cast + +import pytest +from hamcrest import assert_that, equal_to + +from renaissance.utils.unparse_utils import ( + _bracket_end_offset, # pyright: ignore[reportPrivateUsage] + _header_end_line, # pyright: ignore[reportPrivateUsage] + _name_end_offset, # pyright: ignore[reportPrivateUsage] + unparse_signature_only, +) + + +class TestNameEndOffset: + """See module docstring.""" + + @pytest.mark.parametrize( + ("source", "name", "expected"), + [ + pytest.param("def f(x: int) -> int:\n return x\n", "f", 5, id="plain-def"), + pytest.param("async def g(x: int) -> int:\n return x\n", "g", 11, id="async-def"), + pytest.param("@overload\n def __call__(self, x: int) -> int: ...\n", "__call__", 26, id="indented-after-decorator"), + ], + ) + def test_finds_the_offset_after_the_name(self, source: str, name: str, expected: int) -> None: + """Verify the offset right after the function name in its "def" line.""" + assert_that(_name_end_offset(source, name), equal_to(expected)) + + def test_raises_when_name_not_found(self) -> None: + """Verify a ValueError is raised when the function name doesn't appear in the source.""" + with pytest.raises(ValueError, match="no 'def f' header found"): + _name_end_offset("x = 1\n", "f") + + +class TestBracketEndOffset: + """See module docstring.""" + + @pytest.mark.parametrize( + ("source", "expected"), + [ + pytest.param("def f[T](x: T) -> T:\n return x\n", 8, id="simple"), + pytest.param("def f[T: list[int]](x: T) -> T:\n return x\n", 19, id="nested-bracket-in-bound"), + ], + ) + def test_finds_the_matching_closing_bracket(self, source: str, expected: int) -> None: + """Verify the offset right after the "]" matching the type-param bracket, across nested brackets.""" + assert_that(_bracket_end_offset(source, 5), equal_to(expected)) + + +class TestHeaderEndLine: + """See module docstring.""" + + @pytest.mark.parametrize( + ("source", "expected"), + [ + pytest.param("def f(x: int) -> int:\n return x\n", 1, id="one-line-signature"), + pytest.param("def f(\n a: int,\n b: str,\n) -> None:\n pass\n", 4, id="multi-line-signature"), + pytest.param('def f(\n b: str = "x:y",\n) -> None:\n pass\n', 3, id="colon-in-string-default"), + pytest.param("def f(cb=lambda: 1) -> int:\n return cb()\n", 1, id="colon-in-lambda-default"), + ], + ) + def test_finds_the_line_of_the_header_terminating_colon(self, source: str, expected: int) -> None: + """Verify the header ends on the line of its own ":", not one inside a default value.""" + assert_that(_header_end_line(source), equal_to(expected)) + + def test_raises_when_no_header_terminating_colon(self) -> None: + """Verify a ValueError is raised when the source has no header-terminating colon at all.""" + with pytest.raises(ValueError, match="no header-terminating ':' found"): + _header_end_line("x = 1\n") + + +class TestUnparseSignatureOnly: + """See module docstring.""" + + @pytest.mark.parametrize( + ("original", "expected"), + [ + pytest.param( + textwrap.dedent("""\ + def f(x): + # explains something + return x + """), + "def f[T](x):\n # explains something\n return x\n", + id="body-comment", + ), + # 8 spaces: one level for the class, one for the method body. + pytest.param("def f(x):\n return x", "def f[T](x):\n return x", id="method-body-absolute-indent"), + pytest.param("def f(x): ...\n", "def f[T](x): ...\n", id="inline-body"), + pytest.param( + "def f(\n x: int,\n y: int = 1,\n) -> int:\n return x\n", + "def f[T](\n x: int,\n y: int = 1,\n) -> int:\n return x\n", + id="multi-line-signature", + ), + pytest.param("def f[U](x: U, y):\n return x\n", "def f[U, T](x: U, y):\n return x\n", id="existing-bracket"), + ], + ) + def test_adds_the_bracket_and_keeps_everything_else(self, original: str, expected: str) -> None: + """Verify adding T inserts or extends the type-param bracket and keeps the rest of the source.""" + node = cast("ast.FunctionDef", ast.parse(original).body[0]) + node.type_params = [*node.type_params, ast.TypeVar(name="T")] + + assert_that(unparse_signature_only(node, original), equal_to(expected))