From 1a847cff5dd1eb7e370007aa2f1c5e2fdfcbe175 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Sat, 10 Oct 2026 00:50:14 +0200 Subject: [PATCH 1/8] 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 df88fcc0..0d16b7f3 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -1,14 +1,16 @@ """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 @@ -20,22 +22,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.""" @@ -102,7 +122,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 8bfbfd74..efa56579 100644 --- a/test/python/ast/test_offset_encoding.py +++ b/test/python/ast/test_offset_encoding.py @@ -23,15 +23,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 " @@ -81,7 +72,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)), ], ) @@ -94,7 +85,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"), ], ) @@ -109,7 +100,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 72ce7e8c190a3b4bd249a89dcba6676a0899edeb Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Sun, 11 Oct 2026 00:15:31 +0200 Subject: [PATCH 2/8] 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 0d16b7f3..b881441d 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -3,18 +3,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.""" @@ -27,10 +44,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. @@ -68,7 +130,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: @@ -77,12 +139,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 @@ -139,12 +202,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 803c953eb6049fa796d08b4ef52fcdeba154ab6b Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Sun, 11 Oct 2026 14:18:50 +0200 Subject: [PATCH 3/8] 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 b881441d..79ba760b 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -4,7 +4,6 @@ import itertools import sys from collections import defaultdict -from dataclasses import dataclass from pathlib import Path import libcst @@ -12,23 +11,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] @@ -72,10 +58,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}, @@ -85,11 +75,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") @@ -202,12 +192,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 fd6c6ed4c302918741a8c32014e690f8456699ad Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 13:27:33 +0200 Subject: [PATCH 4/8] 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 587d90af..142afdc3 100644 --- a/src/renaissance/recipes/python_refactoring.py +++ b/src/renaissance/recipes/python_refactoring.py @@ -7,6 +7,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 @@ -16,12 +17,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) @@ -105,9 +106,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 a57c2df3..88d6ce2d 100644 --- a/test/recipes/test_python_refactoring.py +++ b/test/recipes/test_python_refactoring.py @@ -2,13 +2,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 @@ -37,11 +42,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), ) # ------------------------------------------------------------------ @@ -279,15 +291,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 da89b35f262bb830ec9c384b2a17069cc3459bca Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 13:55:40 +0200 Subject: [PATCH 5/8] 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 79ba760b..703987fa 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -23,7 +23,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. @@ -161,16 +166,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 cfe132a2445d447a67925853faa1d5d4119cee1b Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 14:06:14 +0200 Subject: [PATCH 6/8] 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 74987161..e776985b 100644 --- a/test/python/ast/test_python_cst_node.py +++ b/test/python/ast/test_python_cst_node.py @@ -71,7 +71,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 13c0fb1d19008cb368d3d4e7c489332ad1ab1b48 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Mon, 12 Oct 2026 14:06:58 +0200 Subject: [PATCH 7/8] Document the CST offsets, references and backend choice --- docs/developer/architecture/position-consistency.md | 7 ++++--- docs/developer/modules/parser-and-ast.md | 6 +++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/developer/architecture/position-consistency.md b/docs/developer/architecture/position-consistency.md index f7bc8b95..5defce30 100644 --- a/docs/developer/architecture/position-consistency.md +++ b/docs/developer/architecture/position-consistency.md @@ -151,9 +151,10 @@ the two sides can silently disagree, producing incorrect or corrupted output. * `src/renaissance/syntax_tree/ast_rewriter.py` - derives the buffer a rewrite batch is applied to. * `src/renaissance/common/rewriter.py` - splices replacement text into that buffer, in bytes. * `src/renaissance/syntax_tree/ast_node.py` - `binary_file_content`, the root-owned buffer accessor. -* `src/renaissance/integrations/python/ast/rst_node.py`, `src/renaissance/integrations/python/ast/cst_node.py` - and `src/renaissance/integrations/python/ast/util.py` - the Python adapters and the line/column to - offset conversion they share. +* `src/renaissance/integrations/python/ast/rst_node.py` and `src/renaissance/integrations/python/ast/util.py` - + the RST adapter and its line/column to offset conversion. +* `src/renaissance/integrations/python/ast/cst_node.py` - the CST adapter, which converts libcst's line/column + positions to byte offsets itself. * `src/renaissance/integrations/tree_sitter/lst.py` - the tree-sitter adapter. * `src/renaissance/integrations/clang/clang_ast_node.py` and `src/renaissance/integrations/clang/clang_json_ast_node.py` - the clang adapters. diff --git a/docs/developer/modules/parser-and-ast.md b/docs/developer/modules/parser-and-ast.md index 809dc514..ad6f78ac 100644 --- a/docs/developer/modules/parser-and-ast.md +++ b/docs/developer/modules/parser-and-ast.md @@ -99,11 +99,15 @@ preserves comments and whitespace, or reparses source. Each adapter supplies those behaviors individually. Reference and data-flow collections (`references`, `referenced_by`) are also parser-specific because not every parser can provide the same analysis. Keep the original parser node available -when callers need parser-specific data. +when callers need parser-specific data. The Python CST adapter resolves them +with libcst's scope analysis, which also finds names used inside string +annotations. The native Python `ast.AST` adapter is a lightweight structural adapter and does not preserve source trivia in the same way as the lossless Python RST/CST adapters. Use the lossless adapter when source-preserving rewriting is needed. +`PythonRefactoring` parses with the RST adapter by default; pass +`node_class=PythonCstNode` to use the CST adapter instead. ## Classify nodes From 5239d9dc5303d8a3872ffe5cb6a0c0cd3f23bbea Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Tue, 13 Oct 2026 10:16:15 +0200 Subject: [PATCH 8/8] 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. --- docs/developer/modules/parser-and-ast.md | 3 ++- .../integrations/python/ast/cst_node.py | 10 ++++++++- test/python/ast/test_python_cst_references.py | 22 +++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/docs/developer/modules/parser-and-ast.md b/docs/developer/modules/parser-and-ast.md index ad6f78ac..fbfca603 100644 --- a/docs/developer/modules/parser-and-ast.md +++ b/docs/developer/modules/parser-and-ast.md @@ -101,7 +101,8 @@ those behaviors individually. Reference and data-flow collections parser can provide the same analysis. Keep the original parser node available when callers need parser-specific data. The Python CST adapter resolves them with libcst's scope analysis, which also finds names used inside string -annotations. +annotations. An annotation or type hint written above a definition in the +same scope still refers to it, since it is evaluated lazily. The native Python `ast.AST` adapter is a lightweight structural adapter and does not preserve source trivia in the same way as the lossless Python RST/CST diff --git a/src/renaissance/integrations/python/ast/cst_node.py b/src/renaissance/integrations/python/ast/cst_node.py index 703987fa..97b2b250 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -46,6 +46,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 @@ -56,7 +58,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')