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 b41e93c0..6b253436 100644 --- a/src/renaissance/integrations/python/ast/cst_node.py +++ b/src/renaissance/integrations/python/ast/cst_node.py @@ -6,42 +6,129 @@ """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 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.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.""" @@ -54,21 +141,19 @@ 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: - """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 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[Self] = [type(self)(child, translation_unit, self) for child in node.children] self.properties = {} # for shower @@ -93,22 +178,22 @@ def signature(self): @property def offset(self): - """AI: Return the character offset where this node begins in the source text.""" + """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.""" + """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.""" + """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 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 @@ -124,6 +209,16 @@ def name(self): return "" self.name = "" # self._derive_name() + @property + 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) -> Self | None: """AI: Return the sibling node immediately following this one, or None.""" @@ -134,19 +229,21 @@ 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.""" - with Path(file_path).open() as file: + @classmethod + def load(cls, file_path: Path) -> Self: + """Parse the Python source file at file_path into a tree of nodes of this class.""" + # 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 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/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/recipes/python_refactoring.py b/src/renaissance/recipes/python_refactoring.py index 32189d9f..fd2df35d 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,18 @@ from renaissance.utils.text_utils import snake_case -class PythonRefactoring(ASTProcessor): - """AI: Base processor for Python-specific source refactoring recipes.""" +class PythonRefactoring[NodeT: PythonRstNode | PythonCstNode = PythonRstNode](ASTProcessor): + """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) + 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).""" + factory = PythonFactory(node_class) atu = factory.create(file) super().__init__(atu, factory, False) self.pattern_factory = PythonPatternFactory(self.factory) @@ -111,9 +118,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/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/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)), ], ) diff --git a/test/python/ast/test_python_cst_node.py b/test/python/ast/test_python_cst_node.py index 500e76be..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.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)) @@ -124,3 +131,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) 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..2342d2f2 --- /dev/null +++ b/test/python/ast/test_python_cst_references.py @@ -0,0 +1,120 @@ +# 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 + +import pytest +from hamcrest import assert_that, contains_inanyorder, equal_to + +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 + +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([])) + + @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') + 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")])) 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") 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")) 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")