Make the libcst (CST) backend usable for recipes

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.

docs/developer/modules/parser-and-ast.md

@@ -99,11 +99,16 @@ 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. 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
 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
 

src/renaissance/integrations/python/ast/cst_node.py

@@ -1,41 +1,126 @@
 """AI: ASTNode implementation backed by libcst's concrete syntax tree."""
 
+import io
+import itertools
+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
-from libcst.metadata import WhitespaceInclusivePositionProvider
+from libcst.metadata import Access, Assignment, CodePosition, PositionProvider, ScopeProvider
 
 from renaissance.integrations.python.ast.kinds import PYTHON_KIND_MAP
-from renaissance.integrations.python.ast.util import convert
+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
 
+type _Link = tuple[CSTNode, str, Access]
+
 
 class PythonCstTranslationUnit:
     """AI: Parse Python source into a libcst tree with position lookups for AST-node wrapping."""
 
     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.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)
+        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.
+        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
+        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
+                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)
+        self._definitions_of = dict(definitions_of)
+        return self._uses_of
+
+    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 = [
+            ASTReference(
+                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[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[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")
+
+    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."""
@@ -48,7 +133,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:
@@ -57,12 +142,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
@@ -88,21 +174,30 @@ 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
     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
@@ -119,12 +214,22 @@ def name(self):
         self.name = ""  # self._derive_name()
 
     @property
-    def next_sibling(self) -> Self | None:
+    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[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)
+
+    @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)
 

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."""

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)),
     ],
 )

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))