From 84fe72cf5eea14d45dfc6f37191447eb796ead67 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Sat, 10 Oct 2026 00:50:14 +0200 Subject: [PATCH 01/14] Fix PythonCstNode offsets: UTF-8 bytes, end-of-file span, code-only spansThe three CST xfails in test_offset_encoding now pass; spans match the RST backend (decorators included, trailing newline excluded). --- .../integrations/python/ast/cst_node.py | 38 ++++++++++++++----- test/python/ast/test_offset_encoding.py | 15 ++------ 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index b41e93c0..352bf7d7 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -6,15 +6,17 @@ """AI: ASTNode implementation backed by libcst's concrete syntax tree.""" +import io +import itertools +import sys from pathlib import Path from typing import Self import libcst from libcst import BaseCompoundStatement, BaseSmallStatement, ClassDef, CSTNode, FunctionDef, MetadataWrapper -from libcst.metadata import WhitespaceInclusivePositionProvider +from libcst.metadata import CodePosition, PositionProvider from renaissance.integrations.python.ast.kinds import PYTHON_KIND_MAP -from renaissance.integrations.python.ast.util import convert from renaissance.syntax_tree.semantic_kind import SemanticKind from renaissance.utils.ast_utils import next_sibling, preceding_sibling @@ -26,22 +28,40 @@ def __init__(self, content, file_name: str) -> None: """AI: Parse Python source into a libcst tree with position lookups for AST-node wrapping.""" # TODO: see docs/developer/architecture/position-consistency.md - exposes no buffer, so the rewriter derives its own. self.content = content - self.lines = content.splitlines() + self.encoding = sys.getfilesystemencoding() + # newline="" splits on \n, \r\n and \r only, like libcst, and keeps each line ending. + self._lines = io.StringIO(content, newline="").readlines() + self._line_starts = list(itertools.accumulate((len(line.encode(self.encoding)) for line in self._lines), initial=0)) self.file_name = file_name self.references_initialized = False self.wrapper = MetadataWrapper(libcst.parse_module(content)) self.atu = self.wrapper.module - self.spans = self.wrapper.resolve(WhitespaceInclusivePositionProvider) + self.spans = self.wrapper.resolve(PositionProvider) + + def _byte_offset(self, position: CodePosition) -> int: + """Return the offset, in bytes of self.encoding, of a libcst position whose column counts characters. + + A position on the line after the last one (the end of a node that ends with the file) maps to the file's length. + """ + line_index = position.line - 1 + if line_index >= len(self._lines): + return self._line_starts[-1] + return self._line_starts[line_index] + len(self._lines[line_index][: position.column].encode(self.encoding)) def start_of(self, node: CSTNode) -> int: - """AI: Return the character offset where node begins in the source text.""" + """Return the byte offset where node begins in the source text, or 0 if it has no position. + + A decorated function or class begins at its first decorator. + """ + if isinstance(node, (ClassDef, FunctionDef)) and node.decorators: + node = node.decorators[0] span = self.spans.get(node) - return convert(self.lines, span.start.line, span.start.column) if span else 0 + return self._byte_offset(span.start) if span else 0 def end_of(self, node: CSTNode) -> int: - """AI: Return the character offset where node ends in the source text.""" + """Return the byte offset where node's code ends in the source text, or 0 if it has no position.""" span = self.spans.get(node) - return convert(self.lines, span.end.line, span.end.column) if span else 0 + return self._byte_offset(span.end) if span else 0 def signature_of(self, node: CSTNode) -> str: """AI: Return the source code text corresponding to node, or an empty string on failure.""" @@ -108,7 +128,7 @@ def end_offset(self): @property def extended_end_offset(self) -> int: - """AI: Return the end offset; libcst attaches trailing trivia to the node itself, so there is nothing to extend.""" + """Return the end offset; trailing comments are found by the rewriter, as for the RST backend.""" return self.end_offset @property diff --git a/test/python/ast/test_offset_encoding.py b/test/python/ast/test_offset_encoding.py index 269c0a01..83894e37 100644 --- a/test/python/ast/test_offset_encoding.py +++ b/test/python/ast/test_offset_encoding.py @@ -29,15 +29,6 @@ # Each 'e-acute' is one character but two UTF-8 bytes, so character and byte offsets diverge after it. NON_ASCII_SOURCE = 'x = "\u00e9\u00e9\u00e9"\ny = f(1)\n' -CST_ENCODING_REASON = ( - "PythonCstTranslationUnit keeps `content` as str and splits str lines, so convert() returns " - "character offsets, while _RewriteActions slices node.text.encode(...), i.e. UTF-8 bytes. The " - "two units drift apart by one per extra byte, here cutting the slice short mid-codepoint." -) -CST_SPAN_REASON = ( - "WhitespaceInclusivePositionProvider resolves no span for the trailing SimpleStatementLine, so " - "PythonCstTranslationUnit.end_of falls back to 0 and the node reports an end before its start." -) LST_ENCODING_REASON = ( "LSTNode.offset is a byte offset from tree-sitter, but length is len(signature) in characters " "and the signature itself is cut out of the source with byte offsets applied to str, so both " @@ -87,7 +78,7 @@ def test_offsets_address_bytes_for_ascii_source(load: Callable[[str], PythonNode "load", [ pytest.param(_rst_root, id="rst"), - pytest.param(_cst_root, id="cst", marks=pytest.mark.xfail(reason=CST_ENCODING_REASON, strict=True)), + pytest.param(_cst_root, id="cst"), pytest.param(_lst_root, id="lst", marks=pytest.mark.xfail(reason=LST_ENCODING_REASON, strict=True)), ], ) @@ -100,7 +91,7 @@ def test_offsets_address_bytes_for_non_ascii_source(load: Callable[[str], Python "load", [ pytest.param(_rst_root, id="rst"), - pytest.param(_cst_root, id="cst", marks=pytest.mark.xfail(reason=CST_SPAN_REASON, strict=True)), + pytest.param(_cst_root, id="cst"), pytest.param(_lst_root, id="lst"), ], ) @@ -115,7 +106,7 @@ def test_trailing_statement_ends_after_it_starts(load: Callable[[str], PythonNod "load", [ pytest.param(_rst_root, id="rst"), - pytest.param(_cst_root, id="cst", marks=pytest.mark.xfail(reason=CST_ENCODING_REASON, strict=True)), + pytest.param(_cst_root, id="cst"), pytest.param(_lst_root, id="lst", marks=pytest.mark.xfail(reason=LST_ENCODING_REASON, strict=True)), ], ) From 1f4e36aa9c4c1ef69e1734c3a0fb9d8289bf0858 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Sun, 11 Oct 2026 00:15:31 +0200 Subject: [PATCH 02/14] Add libcst scope-based references to PythonCstNode references/referenced_by link each use to its definition (string annotations included), so MatchFinder's reference matching now works on the CST backend. --- .../integrations/python/ast/cst_node.py | 87 ++++++++++++++++-- test/python/ast/test_python_cst_references.py | 90 +++++++++++++++++++ 2 files changed, 170 insertions(+), 7 deletions(-) create mode 100644 test/python/ast/test_python_cst_references.py diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index 352bf7d7..87570397 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -9,18 +9,35 @@ import io import itertools import sys +from collections import defaultdict +from dataclasses import dataclass from pathlib import Path -from typing import Self import libcst from libcst import BaseCompoundStatement, BaseSmallStatement, ClassDef, CSTNode, FunctionDef, MetadataWrapper -from libcst.metadata import CodePosition, PositionProvider +from libcst.metadata import Access, Assignment, CodePosition, PositionProvider, ScopeProvider from renaissance.integrations.python.ast.kinds import PYTHON_KIND_MAP from renaissance.syntax_tree.semantic_kind import SemanticKind from renaissance.utils.ast_utils import next_sibling, preceding_sibling +@dataclass(frozen=True) +class PythonCstReference: + """A link from a node to the definition it uses, or from a definition to one of its uses. + + properties holds the bound "name" and whether the use sits in an annotation ("is_annotation") or another type + hint such as `cast("T", x)` ("is_type_hint"). + """ + + node: PythonCstNode + ref_kind: str + properties: dict[str, str | bool] + + +type _Link = tuple[CSTNode, str, Access] + + class PythonCstTranslationUnit: """AI: Parse Python source into a libcst tree with position lookups for AST-node wrapping.""" @@ -33,10 +50,55 @@ def __init__(self, content, file_name: str) -> None: self._lines = io.StringIO(content, newline="").readlines() self._line_starts = list(itertools.accumulate((len(line.encode(self.encoding)) for line in self._lines), initial=0)) self.file_name = file_name - self.references_initialized = False self.wrapper = MetadataWrapper(libcst.parse_module(content)) self.atu = self.wrapper.module self.spans = self.wrapper.resolve(PositionProvider) + self.wrappers: dict[CSTNode, PythonCstNode] = {} + self._uses_of: dict[CSTNode, list[_Link]] | None = None + self._definitions_of: dict[CSTNode, list[_Link]] = {} + + def _index_references(self) -> dict[CSTNode, list[_Link]]: + """Resolve libcst's scopes once, linking every definition to its uses; return the definition-to-uses map. + + Also fills self._definitions_of, the use-to-definitions map. Builtins, which have no defining node, are skipped. + """ + if self._uses_of is not None: + return self._uses_of + uses_of: defaultdict[CSTNode, list[_Link]] = defaultdict(list) + definitions_of: defaultdict[CSTNode, list[_Link]] = defaultdict(list) + scopes = {scope for scope in self.wrapper.resolve(ScopeProvider).values() if scope is not None} + for scope in scopes: + for assignment in scope.assignments: + if not isinstance(assignment, Assignment): + continue + for access in assignment.references: + uses_of[assignment.node].append((access.node, assignment.name, access)) + definitions_of[access.node].append((assignment.node, assignment.name, access)) + self._uses_of = dict(uses_of) + self._definitions_of = dict(definitions_of) + return self._uses_of + + def _to_references(self, links: list[_Link], ref_kind: str) -> list[PythonCstReference]: + """Wrap links as PythonCstReference objects, ordered by the position of the node each points at.""" + references = [ + PythonCstReference( + self.wrappers[node], + ref_kind, + {"name": name, "is_annotation": access.is_annotation, "is_type_hint": access.is_type_hint}, + ) + for node, name, access in links + if node in self.wrappers + ] + return sorted(references, key=lambda reference: reference.node.offset) + + def uses_of(self, node: CSTNode) -> list[PythonCstReference]: + """Return the uses of the name(s) that node defines; empty if node defines nothing.""" + return self._to_references(self._index_references().get(node, []), "use") + + def definitions_of(self, node: CSTNode) -> list[PythonCstReference]: + """Return the definitions that node, a use of a name, resolves to; empty if node is not a resolved use.""" + self._index_references() + return self._to_references(self._definitions_of.get(node, []), "definition") def _byte_offset(self, position: CodePosition) -> int: """Return the offset, in bytes of self.encoding, of a libcst position whose column counts characters. @@ -74,7 +136,7 @@ def signature_of(self, node: CSTNode) -> str: class PythonCstNode: """AI: ASTNode implementation backed by libcst's concrete syntax tree.""" - def __init__(self, node: CSTNode, translation_unit: PythonCstTranslationUnit, parent=None) -> None: + def __init__(self, node: CSTNode, translation_unit: PythonCstTranslationUnit, parent: PythonCstNode | None = None) -> None: """AI: Wrap a libcst node as an AST node within the given translation unit.""" self.parent = parent if parent and parent.root: @@ -83,12 +145,13 @@ def __init__(self, node: CSTNode, translation_unit: PythonCstTranslationUnit, pa self.root = self self.translation_unit = translation_unit self.node = node + translation_unit.wrappers[node] = self self.is_statement = isinstance(self.node, (BaseSmallStatement, BaseCompoundStatement)) self.parser_kind = type(node).__name__ self.semantic_kind = PYTHON_KIND_MAP.get(self.parser_kind, SemanticKind.NODE) - self.children: list[Self] = [PythonCstNode(node, translation_unit, self) for node in node.children] + self.children: list[PythonCstNode] = [PythonCstNode(node, translation_unit, self) for node in node.children] self.properties = {} # for shower @@ -145,12 +208,22 @@ def name(self): self.name = "" # self._derive_name() @property - def next_sibling(self) -> Self | None: + def references(self) -> list[PythonCstReference]: + """Return the definitions this node uses, resolved with libcst's scope analysis (empty if it uses none).""" + return self.translation_unit.definitions_of(self.node) + + @property + def referenced_by(self) -> list[PythonCstReference]: + """Return the uses of the name(s) this node defines, resolved with libcst's scope analysis (empty if none).""" + return self.translation_unit.uses_of(self.node) + + @property + def next_sibling(self) -> PythonCstNode | None: """AI: Return the sibling node immediately following this one, or None.""" return next_sibling(self) @property - def preceding_sibling(self) -> Self | None: + def preceding_sibling(self) -> PythonCstNode | None: """AI: Return the sibling node immediately preceding this one, or None.""" return preceding_sibling(self) diff --git a/test/python/ast/test_python_cst_references.py b/test/python/ast/test_python_cst_references.py new file mode 100644 index 00000000..0d178d93 --- /dev/null +++ b/test/python/ast/test_python_cst_references.py @@ -0,0 +1,90 @@ +"""Tests for PythonCstNode.references and referenced_by, resolved with libcst's scope analysis.""" + +from typing import TYPE_CHECKING + +import pytest +from hamcrest import assert_that, contains_inanyorder, equal_to + +from renaissance.integrations.python.ast.cst_node import PythonCstNode + +if TYPE_CHECKING: + from collections.abc import Iterator + +HEADER = 'from typing import Generic, Literal, TypeAlias, TypeVar, cast\nT = TypeVar("T")\n' + + +def _load(body: str) -> PythonCstNode: + """Parse HEADER followed by body into a PythonCstNode tree.""" + return PythonCstNode.load_from_text(HEADER + body, "refs.py") + + +def traverse(node: PythonCstNode) -> Iterator[PythonCstNode]: + """Yield node and all its descendants, depth first.""" + yield node + for child in node.children: + yield from traverse(child) + + +def _declared_t(root: PythonCstNode) -> PythonCstNode: + """Return the Name node that `T = TypeVar("T")` assigns.""" + return next( + node + for node in traverse(root) + if node.parser_kind == "Name" and node.signature == "T" and node.parent is not None and node.parent.parser_kind == "AssignTarget" + ) + + +class TestPythonCstReferences: + """References between a module-level name and its uses.""" + + @pytest.mark.parametrize( + ("body", "expected_uses"), + [ + pytest.param("def b(x: T) -> T: ...\n", ["T", "T"], id="plain-names"), + pytest.param('def b(x: "T") -> None: ...\n', ['"T"'], id="whole-string-annotation"), + pytest.param('def b(x: "list[T]") -> None: ...\n', ['"list[T]"'], id="string-inside-annotation"), + pytest.param('def b(x: list["T"]) -> None: ...\n', ['"T"'], id="nested-string"), + pytest.param('def b(x: object) -> None:\n cast("T", x)\n', ['"T"'], id="cast-in-body"), + pytest.param("class Box(Generic[T]): ...\n", ["T"], id="generic-class-base"), + pytest.param( + 'A: TypeAlias = "list[T]"\n', + ['"list[T]"'], + id="type-alias-string", + marks=pytest.mark.xfail(reason="libcst's ScopeProvider doesn't read the string value of a TypeAlias.", strict=True), + ), + ], + ) + def test_definition_and_uses_point_at_each_other(self, body: str, expected_uses: list[str]) -> None: + """Every use of T is in T's referenced_by, and each of those uses references T's definition.""" + declared = _declared_t(_load(body)) + + uses = [reference.node for reference in declared.referenced_by] + + assert_that([use.signature for use in uses], contains_inanyorder(*expected_uses)) + for use in uses: + assert_that([reference.node for reference in use.references], equal_to([declared])) + + @pytest.mark.parametrize( + "body", + [ + pytest.param("def b[T](x: T) -> T: ...\n", id="pep695-parameter"), + pytest.param('def b[T](x: "T") -> "list[T]": ...\n', id="pep695-parameter-in-strings"), + pytest.param("def outer[T](x: T) -> T:\n def inner(y: T) -> T:\n return y\n return inner(x)\n", id="closure"), + pytest.param("def outer() -> None:\n def inner[T](x: T) -> T:\n return x\n", id="nested-pep695-function"), + pytest.param("def b() -> None:\n T = 1\n print(T)\n", id="local-variable"), + pytest.param('d = {}\nd["T"]\n', id="dict-key-string"), + pytest.param('def b(x: Literal["T"]) -> None: ...\n', id="literal-string"), + ], + ) + def test_uses_bound_to_another_name_are_not_references(self, body: str) -> None: + """A T that resolves to a type parameter, a local variable or a plain string is not a use of the module-level T.""" + assert_that(_declared_t(_load(body)).referenced_by, equal_to([])) + + def test_import_reports_which_name_each_use_refers_to(self) -> None: + """An import binding several names lists the uses of each, with the imported name in the reference properties.""" + root = _load('def b(x: object) -> None:\n cast("int", x)\n') + import_from = next(node for node in traverse(root) if node.parser_kind == "ImportFrom") + + names = [(str(reference.properties["name"]), reference.node.signature) for reference in import_from.referenced_by] + + assert_that(sorted(names), equal_to([("TypeVar", "TypeVar"), ("cast", "cast")])) From ae971fb7f2c7193b8d9fa0defcfc2ae566895fbc Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Sun, 11 Oct 2026 14:18:50 +0200 Subject: [PATCH 03/14] Replace workaround with a reference to ASTReference --- .../integrations/python/ast/cst_node.py | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index 87570397..f1ef65c0 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -10,7 +10,6 @@ import itertools import sys from collections import defaultdict -from dataclasses import dataclass from pathlib import Path import libcst @@ -18,23 +17,10 @@ from libcst.metadata import Access, Assignment, CodePosition, PositionProvider, ScopeProvider from renaissance.integrations.python.ast.kinds import PYTHON_KIND_MAP +from renaissance.syntax_tree.ast_node import ASTReference from renaissance.syntax_tree.semantic_kind import SemanticKind from renaissance.utils.ast_utils import next_sibling, preceding_sibling - -@dataclass(frozen=True) -class PythonCstReference: - """A link from a node to the definition it uses, or from a definition to one of its uses. - - properties holds the bound "name" and whether the use sits in an annotation ("is_annotation") or another type - hint such as `cast("T", x)` ("is_type_hint"). - """ - - node: PythonCstNode - ref_kind: str - properties: dict[str, str | bool] - - type _Link = tuple[CSTNode, str, Access] @@ -78,10 +64,14 @@ def _index_references(self) -> dict[CSTNode, list[_Link]]: self._definitions_of = dict(definitions_of) return self._uses_of - def _to_references(self, links: list[_Link], ref_kind: str) -> list[PythonCstReference]: - """Wrap links as PythonCstReference objects, ordered by the position of the node each points at.""" + def _to_references(self, links: list[_Link], ref_kind: str) -> list[ASTReference[PythonCstNode]]: + """Wrap links as references, ordered by the position of the node each points at. + + Each reference's properties hold the bound "name" and whether the use sits in an annotation ("is_annotation") + or another type hint such as `cast("T", x)` ("is_type_hint"). + """ references = [ - PythonCstReference( + ASTReference( self.wrappers[node], ref_kind, {"name": name, "is_annotation": access.is_annotation, "is_type_hint": access.is_type_hint}, @@ -91,11 +81,11 @@ def _to_references(self, links: list[_Link], ref_kind: str) -> list[PythonCstRef ] return sorted(references, key=lambda reference: reference.node.offset) - def uses_of(self, node: CSTNode) -> list[PythonCstReference]: + def uses_of(self, node: CSTNode) -> list[ASTReference[PythonCstNode]]: """Return the uses of the name(s) that node defines; empty if node defines nothing.""" return self._to_references(self._index_references().get(node, []), "use") - def definitions_of(self, node: CSTNode) -> list[PythonCstReference]: + def definitions_of(self, node: CSTNode) -> list[ASTReference[PythonCstNode]]: """Return the definitions that node, a use of a name, resolves to; empty if node is not a resolved use.""" self._index_references() return self._to_references(self._definitions_of.get(node, []), "definition") @@ -208,12 +198,12 @@ def name(self): self.name = "" # self._derive_name() @property - def references(self) -> list[PythonCstReference]: + def references(self) -> list[ASTReference[PythonCstNode]]: """Return the definitions this node uses, resolved with libcst's scope analysis (empty if it uses none).""" return self.translation_unit.definitions_of(self.node) @property - def referenced_by(self) -> list[PythonCstReference]: + def referenced_by(self) -> list[ASTReference[PythonCstNode]]: """Return the uses of the name(s) this node defines, resolved with libcst's scope analysis (empty if none).""" return self.translation_unit.uses_of(self.node) From 611e24bc592d9353256327e2f9c36c6bc687df45 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 13:27:33 +0200 Subject: [PATCH 04/14] Let PythonRefactoring run on a chosen Python backend PythonRefactoring is generic over the node class (RST by default), and body returns the module's top-level statements on both backends. --- src/renaissance/recipes/python_refactoring.py | 19 ++++--- test/recipes/test_python_refactoring.py | 50 +++++++++++++------ 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/renaissance/recipes/python_refactoring.py b/src/renaissance/recipes/python_refactoring.py index 32189d9f..90293c4e 100644 --- a/src/renaissance/recipes/python_refactoring.py +++ b/src/renaissance/recipes/python_refactoring.py @@ -13,6 +13,7 @@ from termcolor import colored +from renaissance.integrations.python.ast.cst_node import PythonCstNode from renaissance.integrations.python.ast.factory import PythonFactory, PythonPatternFactory from renaissance.integrations.python.ast.rst_node import PythonRstNode from renaissance.integrations.python.ast.util import to_str @@ -22,12 +23,12 @@ from renaissance.utils.text_utils import snake_case -class PythonRefactoring(ASTProcessor): +class PythonRefactoring[NodeT: PythonRstNode | PythonCstNode = PythonRstNode](ASTProcessor): """AI: Base processor for Python-specific source refactoring recipes.""" - def __init__(self, file) -> None: - """AI: Prepare a Python-specific refactoring processor for the given source file.""" - factory = PythonFactory(PythonRstNode) + def __init__(self, file: Path, node_class: type[NodeT] = PythonRstNode) -> None: + """Prepare a refactoring processor for file, parsed with node_class (the RST backend by default).""" + factory = PythonFactory(node_class) atu = factory.create(file) super().__init__(atu, factory, False) self.pattern_factory = PythonPatternFactory(self.factory) @@ -111,9 +112,13 @@ def class_base_arguments(self, class_node: PythonRstNode) -> list[str]: return [child.signature for child in bases_implicit.children] @property - def body(self) -> Sequence[PythonRstNode]: - """AI: Return the root node's body statements.""" - return cast("PythonRstNode", cast("object", self.root)).body + def body(self) -> Sequence[NodeT]: + """Return the module's top-level statements; with the CST backend, a simple statement comes as its whole line.""" + root = cast("NodeT", cast("object", self.root)) + if isinstance(root, PythonCstNode): + wrappers = root.translation_unit.wrappers + return cast("Sequence[NodeT]", [wrappers[statement] for statement in root.translation_unit.atu.body]) + return cast("Sequence[NodeT]", root.body) def run(self) -> None: """AI: Run this refactoring recipe. Subclasses override this to perform the refactoring.""" diff --git a/test/recipes/test_python_refactoring.py b/test/recipes/test_python_refactoring.py index 1886b5ca..cb6e84d9 100644 --- a/test/recipes/test_python_refactoring.py +++ b/test/recipes/test_python_refactoring.py @@ -8,13 +8,18 @@ import keyword import textwrap +from pathlib import Path +from typing import cast from unittest.mock import patch -from hamcrest import assert_that, contains_string, is_ +import pytest +from hamcrest import assert_that, contains_string, equal_to, is_ from hypothesis import given, settings from hypothesis import strategies as st from pytest_mock import MockerFixture +from renaissance.integrations.python.ast.cst_node import PythonCstNode +from renaissance.integrations.python.ast.factory import PythonFactory from renaissance.integrations.python.ast.rst_node import PythonRstNode from renaissance.recipes.python_refactoring import PythonRefactoring from renaissance.recipes.unit_to_pytest import UnitToPytest @@ -43,11 +48,18 @@ class TestPythonRefactoring: .filter(lambda name: not keyword.iskeyword(name)) ) - def _patch_factory(self, mocker: MockerFixture, text="pass", filename="test_foo.py") -> None: + def _patch_factory( + self, + mocker: MockerFixture, + text: str = "pass", + filename: str = "test_foo.py", + node_class: type[PythonRstNode | PythonCstNode] = PythonRstNode, + ) -> None: + """Make PythonFactory.create return text parsed by node_class instead of reading filename from disk.""" code = textwrap.dedent(text) mocker.patch( "renaissance.integrations.python.ast.factory.PythonFactory.create", - return_value=PythonRstNode.load_from_text(code, filename), + return_value=node_class.load_from_text(code, filename), ) # ------------------------------------------------------------------ @@ -285,15 +297,23 @@ def test_process_runs_refactor_on_matching_file(self, mocker: MockerFixture, cap # body property # ------------------------------------------------------------------ - def test_body_returns_module_level_statements(self, mocker: MockerFixture) -> None: - """AI: Verify the body property returns the module's top-level statements.""" - self._patch_factory( - mocker, - """ - x = 1 - y = 2 - """, - "test_foo.py", - ) - subject = UnitToPytest("test_foo.py") - assert_that(len(subject.body), is_(2)) + @pytest.mark.parametrize("node_class", [PythonRstNode, PythonCstNode], ids=["rst", "cst"]) + def test_body_returns_module_level_statements(self, mocker: MockerFixture, node_class: type[PythonRstNode | PythonCstNode]) -> None: + """Verify the body property returns the module's top-level statements, whichever backend parsed the file.""" + self._patch_factory(mocker, "import os\nx = 1\ndef f():\n return x\n", "test_foo.py", node_class) + subject = PythonRefactoring(Path("test_foo.py"), node_class=node_class) + + statements: list[str] = [str(statement.signature).strip() for statement in subject.body] + + assert_that(statements, equal_to(["import os", "x = 1", "def f():\n return x"])) + + @pytest.mark.parametrize("node_class", [PythonRstNode, PythonCstNode], ids=["rst", "cst"]) + def test_recipe_rewrites_with_the_chosen_backend(self, mocker: MockerFixture, node_class: type[PythonRstNode | PythonCstNode]) -> None: + """Verify a recipe parses with the node class it is given and rewrites through that backend's nodes.""" + self._patch_factory(mocker, "x = 1\ny = 2\n", "test_foo.py", node_class) + subject = PythonRefactoring(Path("test_foo.py"), node_class=node_class) + + subject.replace("x = 3", subject.body[0], include_whitespace=False, include_comments=False) + + assert_that(cast("PythonFactory", subject.factory).clazz is node_class, is_(True)) + assert_that(subject.apply_to_string(), is_("x = 3\ny = 2\n")) From 68378e979e9eb6495efef5c816eede411eb4209f Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 13:55:40 +0200 Subject: [PATCH 05/14] Clarify existing TODO with extra information, author and related issue. Add extra TODOs regarding AI docstrings. --- .../integrations/python/ast/cst_node.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index f1ef65c0..4beb5293 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -29,7 +29,12 @@ class PythonCstTranslationUnit: def __init__(self, content, file_name: str) -> None: """AI: Parse Python source into a libcst tree with position lookups for AST-node wrapping.""" - # TODO: see docs/developer/architecture/position-consistency.md - exposes no buffer, so the rewriter derives its own. + # TODO(pjljvandelaar): see docs/developer/architecture/position-consistency.md - exposes no buffer, so the rewriter + # derives its own. + # https://github.com/TNO/Renaissance.Py/issues/153 + # TODO(FrancescoPezzella): content is a str while offset/end_offset count bytes of self.encoding, so never slice + # content with them. self.encoding is the filesystem encoding, not the file's (#153, #188). + # https://github.com/TNO/Renaissance.Py/issues/188 self.content = content self.encoding = sys.getfilesystemencoding() # newline="" splits on \n, \r\n and \r only, like libcst, and keeps each line ending. @@ -167,16 +172,25 @@ def signature(self): @property def offset(self): """AI: Return the character offset where this node begins in the source text.""" + # TODO(FrancescoPezzella): the docstring says characters, but is this really counting characters? Since the + # UTF-8 offsets fix it counts bytes of translation_unit.encoding, so shouldn't the docstring say bytes? + # https://github.com/TNO/Renaissance.Py/issues/153 return self.translation_unit.start_of(self.node) @property def length(self): """AI: Return the length in characters of this node's source text.""" + # TODO(FrancescoPezzella): the docstring says characters, but is this really counting characters? Since the + # UTF-8 offsets fix the length is in bytes of translation_unit.encoding, so shouldn't the docstring say bytes? + # https://github.com/TNO/Renaissance.Py/issues/153 return self.end_offset - self.offset @property def end_offset(self): """AI: Return the character offset where this node ends in the source text.""" + # TODO(FrancescoPezzella): the docstring says characters, but is this really counting characters? Since the + # UTF-8 offsets fix it counts bytes of translation_unit.encoding, so shouldn't the docstring say bytes? + # https://github.com/TNO/Renaissance.Py/issues/153 return self.translation_unit.end_of(self.node) @property From 38241974a216e2b78af5558e823e82821ad0e9ad Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 14:06:14 +0200 Subject: [PATCH 06/14] Placeholder Copyright comment, Lint issues and TODO comment for lazy import --- test/python/ast/test_python_cst_node.py | 2 +- test/python/ast/test_python_cst_references.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/test/python/ast/test_python_cst_node.py b/test/python/ast/test_python_cst_node.py index 500e76be..31dd6cdf 100644 --- a/test/python/ast/test_python_cst_node.py +++ b/test/python/ast/test_python_cst_node.py @@ -77,7 +77,7 @@ def next_me(): assert_that(me.name, is_("mememe")) assert_that(me.preceding_sibling.name, is_("previous_me")) assert_that(me.next_sibling.name, is_("next_me")) - assert_that(me.parent.parent.name, is_("Parent")) + assert_that(me.parent and me.parent.parent and me.parent.parent.name, is_("Parent")) # all children are mashed together assert_that(me.children, has_length(8)) diff --git a/test/python/ast/test_python_cst_references.py b/test/python/ast/test_python_cst_references.py index 0d178d93..648546f0 100644 --- a/test/python/ast/test_python_cst_references.py +++ b/test/python/ast/test_python_cst_references.py @@ -1,3 +1,4 @@ +# Copyright (c) 2026 PLACEHOLDER """Tests for PythonCstNode.references and referenced_by, resolved with libcst's scope analysis.""" from typing import TYPE_CHECKING @@ -7,6 +8,8 @@ from renaissance.integrations.python.ast.cst_node import PythonCstNode +# TODO(FrancescoPezzella): Replace with a lazy import in Python 3.15. +# https://github.com/TNO/Renaissance.Py/issues/275 if TYPE_CHECKING: from collections.abc import Iterator From b575c2f3fa1fa231ede91b03567c9e56c6a86616 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Tue, 13 Oct 2026 10:16:15 +0200 Subject: [PATCH 07/14] Link annotations above a definition to it in the CST references. libcst only links a use to definitions above it; annotations and type hints are evaluated lazily, so they now count too. --- .../integrations/python/ast/cst_node.py | 10 ++++++++- test/python/ast/test_python_cst_references.py | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index 4beb5293..cc240b0d 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -52,6 +52,8 @@ def _index_references(self) -> dict[CSTNode, list[_Link]]: """Resolve libcst's scopes once, linking every definition to its uses; return the definition-to-uses map. Also fills self._definitions_of, the use-to-definitions map. Builtins, which have no defining node, are skipped. + An annotation or type hint above a definition in the same scope is linked to it too, since it is evaluated + lazily (PEP 563, PEP 649); a value read there at once is not. """ if self._uses_of is not None: return self._uses_of @@ -62,7 +64,13 @@ def _index_references(self) -> dict[CSTNode, list[_Link]]: for assignment in scope.assignments: if not isinstance(assignment, Assignment): continue - for access in assignment.references: + lazy_accesses_above = { + access + for access in scope.accesses[assignment.name] + if not access.referents and (access.is_annotation or access.is_type_hint) + } + # libcst only links an access to the definitions above it in the same scope. + for access in [*assignment.references, *lazy_accesses_above]: uses_of[assignment.node].append((access.node, assignment.name, access)) definitions_of[access.node].append((assignment.node, assignment.name, access)) self._uses_of = dict(uses_of) diff --git a/test/python/ast/test_python_cst_references.py b/test/python/ast/test_python_cst_references.py index 648546f0..57a3adbd 100644 --- a/test/python/ast/test_python_cst_references.py +++ b/test/python/ast/test_python_cst_references.py @@ -83,6 +83,28 @@ def test_uses_bound_to_another_name_are_not_references(self, body: str) -> None: """A T that resolves to a type parameter, a local variable or a plain string is not a use of the module-level T.""" assert_that(_declared_t(_load(body)).referenced_by, equal_to([])) + @pytest.mark.parametrize( + ("body", "expected_uses"), + [ + pytest.param("def b(x: T) -> T: ...\n", ["T", "T"], id="annotations"), + pytest.param('def b(x: "list[T]") -> None: ...\n', ['"list[T]"'], id="string-annotation"), + pytest.param('y = cast("T", 1)\n', ['"T"'], id="cast"), + pytest.param("w = T\n", [], id="evaluated-at-once"), + ], + ) + def test_lazily_evaluated_use_above_the_declaration_is_a_reference(self, body: str, expected_uses: list[str]) -> None: + """An annotation or type hint above `T = TypeVar("T")` is a use of it; a value read there at once is not.""" + root = PythonCstNode.load_from_text( + f'from __future__ import annotations\nfrom typing import TypeVar, cast\n{body}T = TypeVar("T")\n', "refs.py" + ) + declared = _declared_t(root) + + uses = [reference.node for reference in declared.referenced_by] + + assert_that([use.signature for use in uses], contains_inanyorder(*expected_uses)) + for use in uses: + assert_that([reference.node for reference in use.references], equal_to([declared])) + def test_import_reports_which_name_each_use_refers_to(self) -> None: """An import binding several names lists the uses of each, with the imported name in the reference properties.""" root = _load('def b(x: object) -> None:\n cast("int", x)\n') From e958cb440e64ee9279b5efb548f858403b5d4469 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Fri, 16 Oct 2026 16:15:08 +0200 Subject: [PATCH 08/14] Add Copyright comment --- test/python/ast/test_python_cst_references.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/python/ast/test_python_cst_references.py b/test/python/ast/test_python_cst_references.py index 57a3adbd..2342d2f2 100644 --- a/test/python/ast/test_python_cst_references.py +++ b/test/python/ast/test_python_cst_references.py @@ -1,4 +1,9 @@ -# Copyright (c) 2026 PLACEHOLDER +# 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 PythonCstNode.references and referenced_by, resolved with libcst's scope analysis.""" from typing import TYPE_CHECKING From f35d8dea285378a847831a250e9310bf226caa03 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 2 Nov 2026 21:48:09 +0100 Subject: [PATCH 09/14] Record the decision that AST and regular expression matching use UTF-8 bytes only, and state that the CST adapter's offsets count bytes of the filesystem encoding until that migration. --- .../integrations/python/ast/cst_node.py | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index cc240b0d..8fd09404 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -32,8 +32,9 @@ def __init__(self, content, file_name: str) -> None: # TODO(pjljvandelaar): see docs/developer/architecture/position-consistency.md - exposes no buffer, so the rewriter # derives its own. # https://github.com/TNO/Renaissance.Py/issues/153 - # TODO(FrancescoPezzella): content is a str while offset/end_offset count bytes of self.encoding, so never slice - # content with them. self.encoding is the filesystem encoding, not the file's (#153, #188). + # TODO(FrancescoPezzella): content is to become the file's UTF-8 bytes, with every offset a UTF-8 byte offset into + # them (decided for AST and regular expression matching; no str). Until then content is the str decoded by load + # and offsets count bytes of self.encoding, the filesystem encoding, so never slice content with them (#153). # https://github.com/TNO/Renaissance.Py/issues/188 self.content = content self.encoding = sys.getfilesystemencoding() @@ -179,26 +180,17 @@ def signature(self): @property def offset(self): - """AI: Return the character offset where this node begins in the source text.""" - # TODO(FrancescoPezzella): the docstring says characters, but is this really counting characters? Since the - # UTF-8 offsets fix it counts bytes of translation_unit.encoding, so shouldn't the docstring say bytes? - # https://github.com/TNO/Renaissance.Py/issues/153 + """Return the offset, in bytes of translation_unit.encoding, where this node begins in the source text.""" return self.translation_unit.start_of(self.node) @property def length(self): - """AI: Return the length in characters of this node's source text.""" - # TODO(FrancescoPezzella): the docstring says characters, but is this really counting characters? Since the - # UTF-8 offsets fix the length is in bytes of translation_unit.encoding, so shouldn't the docstring say bytes? - # https://github.com/TNO/Renaissance.Py/issues/153 + """Return the length, in bytes of translation_unit.encoding, of this node's source text.""" return self.end_offset - self.offset @property def end_offset(self): - """AI: Return the character offset where this node ends in the source text.""" - # TODO(FrancescoPezzella): the docstring says characters, but is this really counting characters? Since the - # UTF-8 offsets fix it counts bytes of translation_unit.encoding, so shouldn't the docstring say bytes? - # https://github.com/TNO/Renaissance.Py/issues/153 + """Return the offset, in bytes of translation_unit.encoding, where this node ends in the source text.""" return self.translation_unit.end_of(self.node) @property From e5a98819ec4342f02bfb0daa84642da0925bf070 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 2 Nov 2026 22:24:43 +0100 Subject: [PATCH 10/14] Explain the PythonRefactoring node type parameter in its docstring --- src/renaissance/recipes/python_refactoring.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/renaissance/recipes/python_refactoring.py b/src/renaissance/recipes/python_refactoring.py index 90293c4e..fd2df35d 100644 --- a/src/renaissance/recipes/python_refactoring.py +++ b/src/renaissance/recipes/python_refactoring.py @@ -24,7 +24,13 @@ class PythonRefactoring[NodeT: PythonRstNode | PythonCstNode = PythonRstNode](ASTProcessor): - """AI: Base processor for Python-specific source refactoring recipes.""" + """Base processor for Python-specific source refactoring recipes. + + NodeT is the node class the file is parsed into: PythonRstNode by default, or PythonCstNode. A subclass picks the + CST backend with PythonRefactoring[PythonCstNode] and passes the same class as node_class; NodeT then types body. + See PEP 695 (https://peps.python.org/pep-0695/) for the bound and PEP 696 (https://peps.python.org/pep-0696/) + for the default. + """ def __init__(self, file: Path, node_class: type[NodeT] = PythonRstNode) -> None: """Prepare a refactoring processor for file, parsed with node_class (the RST backend by default).""" From 4672caee5410c81fc69c0791610abe44d16ca113 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 2 Nov 2026 22:40:12 +0100 Subject: [PATCH 11/14] Use Self for the children, parent, root and siblings of PythonCstNode, and build nodes of the subclass in its constructor and loaders so the annotation holds. --- .../integrations/python/ast/cst_node.py | 34 +++++++++---------- test/python/ast/test_python_cst_node.py | 26 ++++++++++++++ 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index 8fd09404..1db11750 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -11,6 +11,7 @@ import sys from collections import defaultdict from pathlib import Path +from typing import Self import libcst from libcst import BaseCompoundStatement, BaseSmallStatement, ClassDef, CSTNode, FunctionDef, MetadataWrapper @@ -140,13 +141,10 @@ def signature_of(self, node: CSTNode) -> str: class PythonCstNode: """AI: ASTNode implementation backed by libcst's concrete syntax tree.""" - def __init__(self, node: CSTNode, translation_unit: PythonCstTranslationUnit, parent: PythonCstNode | None = None) -> None: - """AI: Wrap a libcst node as an AST node within the given translation unit.""" + def __init__(self, node: CSTNode, translation_unit: PythonCstTranslationUnit, parent: Self | None = None) -> None: + """Wrap node and, recursively, its children as nodes of this class within the given translation unit.""" self.parent = parent - if parent and parent.root: - self.root = parent.root - else: - self.root = self + self.root: Self = parent.root if parent and parent.root else self self.translation_unit = translation_unit self.node = node translation_unit.wrappers[node] = self @@ -155,7 +153,7 @@ def __init__(self, node: CSTNode, translation_unit: PythonCstTranslationUnit, pa self.parser_kind = type(node).__name__ self.semantic_kind = PYTHON_KIND_MAP.get(self.parser_kind, SemanticKind.NODE) - self.children: list[PythonCstNode] = [PythonCstNode(node, translation_unit, self) for node in node.children] + self.children: list[Self] = [type(self)(child, translation_unit, self) for child in node.children] self.properties = {} # for shower @@ -222,28 +220,28 @@ def referenced_by(self) -> list[ASTReference[PythonCstNode]]: return self.translation_unit.uses_of(self.node) @property - def next_sibling(self) -> PythonCstNode | None: + def next_sibling(self) -> Self | None: """AI: Return the sibling node immediately following this one, or None.""" return next_sibling(self) @property - def preceding_sibling(self) -> PythonCstNode | None: + def preceding_sibling(self) -> Self | None: """AI: Return the sibling node immediately preceding this one, or None.""" return preceding_sibling(self) - @staticmethod - def load(file_path: Path) -> PythonCstNode: - """AI: Parse the Python source file at file_path into a PythonCstNode tree.""" + @classmethod + def load(cls, file_path: Path) -> Self: + """Parse the Python source file at file_path into a tree of nodes of this class.""" with Path(file_path).open() as file: content = file.read() - return PythonCstNode.load_from_text(content, str(file_path)) + return cls.load_from_text(content, str(file_path)) - @staticmethod + @classmethod def load_from_text( + cls, text: str, file_name: str = "cst_snippet.py", - ) -> PythonCstNode: - """AI: Parse Python source text into a PythonCstNode tree, attributed to file_name.""" + ) -> Self: + """Parse Python source text into a tree of nodes of this class, attributed to file_name.""" translation_unit = PythonCstTranslationUnit(text, file_name=str(file_name)) - root_node = PythonCstNode(translation_unit.atu, translation_unit) - return root_node + return cls(translation_unit.atu, translation_unit) diff --git a/test/python/ast/test_python_cst_node.py b/test/python/ast/test_python_cst_node.py index 31dd6cdf..8a40674d 100644 --- a/test/python/ast/test_python_cst_node.py +++ b/test/python/ast/test_python_cst_node.py @@ -124,3 +124,29 @@ def test(_): """) it = PythonCstNode.load_from_text(ann_fun, "fun.py").children[-1] assert_that(it.signature, contains_string("def test")) + + +class _SubclassNode(PythonCstNode): + """A PythonCstNode subclass that adds nothing.""" + + +@pytest.mark.parametrize( + "source", + [ + "a = 0\n", + "class A:\n def f(self): pass\n def g(self): pass\n", + "@decorator\ndef f(x):\n return x + 1\n", + "items = [n * 2 for n in range(10) if n % 3]\n", + ], +) +def test_load_builds_every_node_from_the_subclass(source: str, tmp_path: Path) -> None: + """Verify a subclass's load yields a tree whose nodes, roots, parents and siblings are all of that subclass.""" + file = tmp_path / "module.py" + file.write_text(source, encoding="utf-8", newline="\n") + pending: list[PythonCstNode] = [_SubclassNode.load(file)] + while pending: + node = pending.pop() + for related in (node, node.root, node.parent, node.next_sibling, node.preceding_sibling): + if related is not None: + assert_that(related, is_(_SubclassNode)) + pending.extend(node.children) From b211152c0cbb72e30898f8b0429258481de491c4 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 2 Nov 2026 22:54:36 +0100 Subject: [PATCH 12/14] Bring over the TODOs and known-gap notes from typing-recipes-2 for the parsers, rewriter, text utilities and tests, with the explicit UTF-8 encoding in the Python loaders. Make Ci/Cd pass with Pyright fix. --- src/rejuvenation/cli.py | 2 +- .../integrations/python/ast/cst_node.py | 4 +++- .../integrations/python/ast/rst_node.py | 4 +++- .../syntax_tree/ast_refactor_actions.py | 2 ++ src/renaissance/syntax_tree/ast_rewriter.py | 19 +++++++++++++++++++ src/renaissance/utils/text_utils.py | 6 +++++- test/examples/test_python_examples.py | 3 +++ test/syntax_tree/test_match_tree.py | 9 +++++++++ 8 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/rejuvenation/cli.py b/src/rejuvenation/cli.py index 59cd2020..212b5fd3 100644 --- a/src/rejuvenation/cli.py +++ b/src/rejuvenation/cli.py @@ -30,7 +30,7 @@ def refactor() -> None: extractor = PythonExtractor() for file in PythonScanner().find_sources(): filename = sys.argv[2] - extractor.process(file) + extractor.process(Path(file)) extractor.save_graph(filename) if sys.argv[1] == "inspect": print(f"inspect {Path().cwd()}") diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index 1db11750..6b253436 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -232,7 +232,9 @@ def preceding_sibling(self) -> Self | None: @classmethod def load(cls, file_path: Path) -> Self: """Parse the Python source file at file_path into a tree of nodes of this class.""" - with Path(file_path).open() as file: + # TODO(FrancescoPezzella): drop the explicit encoding with Python 3.15, where UTF-8 is the default. + # https://github.com/TNO/Renaissance.Py/issues/188 + with Path(file_path).open(encoding="utf-8") as file: content = file.read() return cls.load_from_text(content, str(file_path)) diff --git a/src/renaissance/integrations/python/ast/rst_node.py b/src/renaissance/integrations/python/ast/rst_node.py index 4035ac54..eab7ae4f 100644 --- a/src/renaissance/integrations/python/ast/rst_node.py +++ b/src/renaissance/integrations/python/ast/rst_node.py @@ -356,7 +356,9 @@ def load( # Keep a uniform loader signature across AST node implementations. # Python's AST parser does not need extra arguments or a working dir. _ = extra_args, working_dir - with Path(file_path).open() as file: + # TODO(FrancescoPezzella): drop the explicit encoding with Python 3.15, where UTF-8 is the default. + # https://github.com/TNO/Renaissance.Py/issues/188 + with Path(file_path).open(encoding="utf-8") as file: content = file.read() return PythonRstNode.load_from_text(content, str(file_path)) diff --git a/src/renaissance/syntax_tree/ast_refactor_actions.py b/src/renaissance/syntax_tree/ast_refactor_actions.py index 45a25945..d1277032 100644 --- a/src/renaissance/syntax_tree/ast_refactor_actions.py +++ b/src/renaissance/syntax_tree/ast_refactor_actions.py @@ -16,6 +16,8 @@ from renaissance.syntax_tree.match_finder import MatchFinder, PatternMatch from renaissance.syntax_tree.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 70d8bfc8..396fda7b 100644 --- a/src/renaissance/syntax_tree/ast_rewriter.py +++ b/src/renaissance/syntax_tree/ast_rewriter.py @@ -303,6 +303,16 @@ def __check_for_conflicting_rewrites(self) -> None: "in the same file - applying both would corrupt the output." ) + @staticmethod + def __is_nested(node: Rewritable, maybe_ancestor: Rewritable) -> bool: + """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: + return True + parent = parent.parent + return False + def __is_ancestor_in_nodes(self, node: Rewritable) -> bool: """Check if the given node is a descendant of any nodes in the rewrite list. @@ -327,6 +337,11 @@ def no_conflict(node1, rew) -> bool: result = any(no_conflict(node, rew) for rew in rewrite_nodes) + # TODO(FrancescoPezzella): an outer rewrite should suppress the rewrites nested inside it, but both are applied. + # Dropping `and False` is not enough: no_conflict also matches each node with its own rewrite, so result is almost + # always True and apply() would skip nearly every rewrite. A fix has to leave out the node's own rewrite and use a + # true ancestor check such as __is_nested. + # https://github.com/TNO/Renaissance.Py/issues/286 return result and False # TODO: Why `and False` def __replace( @@ -441,6 +456,8 @@ def __insert( else spaces ) # indent the new content except the first line + # TODO(FrancescoPezzella): unlike __replace, this ignores correct_indent, so the re-indentation can't be turned off. + # https://github.com/TNO/Renaissance.Py/issues/283 new_content = TextUtils.shift_right(new_content, indent, start_line=1) if before: @@ -489,6 +506,8 @@ def __compose_replacement(self, replacement: str, matches: Sequence[PatternMatch if regex_match: raw_signature = "".join(regex_match.groups()) place_holder_length = end_index - index + 1 + # TODO(FrancescoPezzella): this also indents the lines inside a multi-line string in the captured text. + # https://github.com/TNO/Renaissance.Py/issues/283 indent_replacement = raw_signature.replace("\n", "\n" + spaces) if ( placeholder.startswith("$$") diff --git a/src/renaissance/utils/text_utils.py b/src/renaissance/utils/text_utils.py index ec1774a4..93ed6e7b 100644 --- a/src/renaissance/utils/text_utils.py +++ b/src/renaissance/utils/text_utils.py @@ -23,6 +23,8 @@ class TextUtils: @staticmethod def shift_left(text: str, shift: int, start_line: int = 0) -> str: """Shifts each line of the given text to the left by the specified number of spaces. Only spaces are shifted.""" + # TODO(FrancescoPezzella): every line is shifted, also inside a multi-line string literal, changing its value. + # https://github.com/TNO/Renaissance.Py/issues/283 if shift == 0: return text pattern = re.compile(r"\s{0," + str(shift) + "}(.*)") @@ -57,7 +59,9 @@ def strip_indent(text: str, start_line: int = 0) -> str: @staticmethod def shift_right(text: str, shift: int, start_line: int = 0) -> str: - """Shifts each line of the given text to the left by the specified number of spaces. Only spaces are shifted.""" + """Shifts each line of the given text to the right by the specified number of spaces.""" + # TODO(FrancescoPezzella): every line is shifted, also inside a multi-line string literal, changing its value. + # https://github.com/TNO/Renaissance.Py/issues/283 if shift == 0: return text lines = text.split("\n") diff --git a/test/examples/test_python_examples.py b/test/examples/test_python_examples.py index 214d6936..30faac15 100644 --- a/test/examples/test_python_examples.py +++ b/test/examples/test_python_examples.py @@ -12,6 +12,9 @@ from rejuvenation.python_lst_example import python_lst_smoke_test from rejuvenation.python_rst_example import python_rst_smoke_test +# TODO(FrancescoPezzella): `result` is never used and each test compares its own local `result` with itself, so no test +# checks the output of the examples. +# https://github.com/TNO/Renaissance.Py/issues/307 result = ( "\nfrom module import foo, bar, baz, quux\nba(51)\n# changed function f1 to f2\nf2(52\n,123456)\n\n# changed function f1 to f2\n" "f2(53\n,123456)\n\npa(54)\n\n# changed if expr to const\nisAOne=True\nif(isAOne):\n ba()\n\n\npa(54) \n" 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") From b83d4910fead12c3f804e0b4b3616664e96b46d5 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Tue, 3 Nov 2026 22:05:33 +0100 Subject: [PATCH 13/14] Check the CST node family test through hamcrest property matchers, so no assertion accesses an optional parent or sibling. --- test/python/ast/test_python_cst_node.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/python/ast/test_python_cst_node.py b/test/python/ast/test_python_cst_node.py index 8a40674d..97033305 100644 --- a/test/python/ast/test_python_cst_node.py +++ b/test/python/ast/test_python_cst_node.py @@ -14,6 +14,8 @@ assert_that, contains_string, has_length, + has_properties, + has_property, is_, is_not, ) @@ -74,10 +76,15 @@ def next_me(): ) # module class body fun memem me = src.children[-1].children[5].children[2] - assert_that(me.name, is_("mememe")) - assert_that(me.preceding_sibling.name, is_("previous_me")) - assert_that(me.next_sibling.name, is_("next_me")) - assert_that(me.parent and me.parent.parent and me.parent.parent.name, is_("Parent")) + assert_that( + me, + has_properties( + name="mememe", + preceding_sibling=has_property("name", "previous_me"), + next_sibling=has_property("name", "next_me"), + parent=has_property("parent", has_property("name", "Parent")), + ), + ) # all children are mashed together assert_that(me.children, has_length(8)) From 3b2ae6cea768e0d84c2e07f4e584dc6f1dfd1ca3 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Tue, 3 Nov 2026 22:23:45 +0100 Subject: [PATCH 14/14] Check the RST node family test through hamcrest property matchers, matching the CST version of the test. --- test/python/ast/test_python_rst_node.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/python/ast/test_python_rst_node.py b/test/python/ast/test_python_rst_node.py index f104e06b..e43271fb 100644 --- a/test/python/ast/test_python_rst_node.py +++ b/test/python/ast/test_python_rst_node.py @@ -17,6 +17,8 @@ contains_string, empty, has_length, + has_properties, + has_property, is_, ) from hypothesis import HealthCheck, given, settings @@ -144,10 +146,15 @@ def next_me(): ) # module class body fun memem me = src.children[-1].children[2].children[1] - assert_that(me.name, is_("mememe")) - assert_that(me.preceding_sibling.name, is_("previous_me")) - assert_that(me.next_sibling.name, is_("next_me")) - assert_that(me.parent.parent.name, is_("Parent")) + assert_that( + me, + has_properties( + name="mememe", + preceding_sibling=has_property("name", "previous_me"), + next_sibling=has_property("name", "next_me"), + parent=has_property("parent", has_property("name", "Parent")), + ), + ) assert_that(me.children[1].children, has_length(4)) @pytest.mark.skip("don't use ast comment parser")