Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ for x in changer.iterate_coordinates():
print(x)
print(changer.apply_coordinate(x))

#> Coordinate(file=None, class_name='SimpleString', start_line=1, start_column=4, end_line=1, end_column=9, converter_id='__main__:change_string:13')
#> Coordinate(file=None, class_name='SimpleString', start_line=1, start_column=4, end_line=1, end_column=16, converter_id='__main__:change_string:10')
#> a = "new string"
```

Expand Down
2 changes: 1 addition & 1 deletion cstvis/changer.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,5 @@ def iterate_coordinates(self) -> Generator[Coordinate, None, None]:
def apply_coordinate(self, coordinate: Coordinate) -> str:
wrapper = metadata.MetadataWrapper(self.module)
node_ranges = wrapper.resolve(metadata.WhitespaceInclusivePositionProvider)
modified = wrapper.visit(SuperTransformer(coordinate, self.converters_by_types, self._comments_by_lines, set(), SourceOffsetResolver(wrapper.module, self.source, node_ranges.values())))
modified = wrapper.visit(SuperTransformer(coordinate, self.converters_by_types, self._comments_by_lines, SourceOffsetResolver(wrapper.module, self.source, node_ranges.values())))
return modified.code
43 changes: 5 additions & 38 deletions cstvis/transformers/super_transformer.py
Original file line number Diff line number Diff line change
@@ -1,40 +1,13 @@
from typing import Any, Callable, Dict, List, Set, Type
from typing import Dict, List, Type

import libcst.matchers as matchers_module
from libcst import CSTNode, metadata
from libcst.matchers import (
BaseMatcherNode,
MatcherDecoratableTransformer,
TypeOf,
leave,
)
from libcst import CSTNode, CSTTransformer, metadata

from cstvis.dto import Context, Coordinate, SourcePosition
from cstvis.source_offsets import SourceOffsetResolver
from cstvis.wrapper import CallableWrapper


def get_all_matcher_nodes() -> List[BaseMatcherNode]:
result = []

for name in dir(matchers_module):
attribute = getattr(matchers_module, name)
try:
if issubclass(attribute, BaseMatcherNode) and attribute is not BaseMatcherNode and attribute is not TypeOf:
result.append(attribute())
except TypeError:
pass

return result

def leave_all(function: Callable[[Any, CSTNode, CSTNode], CSTNode]) -> Callable[[Any, CSTNode, CSTNode], CSTNode]:
for matcher in get_all_matcher_nodes():
function = leave(matcher)(function)

return function


class SuperTransformer(MatcherDecoratableTransformer):
class SuperTransformer(CSTTransformer):
"""
Apply one conversion with positions from the original node.

Expand All @@ -50,23 +23,17 @@ def __init__(
target_coordinate: Coordinate,
nodes_mapping: Dict[Type[CSTNode], List[CallableWrapper[CSTNode]]],
comments: Dict[int, str],
nodes_ids: Set[int],
source_offsets: SourceOffsetResolver,
):
self.target_coordinate = target_coordinate
self.nodes_mapping = nodes_mapping
self.comments = comments
self.nodes_ids = nodes_ids
self.source_offsets = source_offsets

super().__init__()

@leave_all
def leave(self, original_node, updated_node): # type: ignore[no-untyped-def]
if id(original_node) in self.nodes_ids:
return updated_node
self.nodes_ids.add(id(original_node))

# LibCST's generic signature cannot express supported cross-type replacements.
def on_leave(self, original_node: CSTNode, updated_node: CSTNode) -> CSTNode: # type: ignore[override]
converters = self.nodes_mapping.get(type(original_node), []) + self.nodes_mapping.get(CSTNode, []) # type: ignore[type-abstract]
if not converters:
return updated_node
Expand Down
49 changes: 49 additions & 0 deletions docs/plans/2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Переход на `CSTTransformer.on_leave()` по TDD

В проект поступило [issue #10 — Replace matcher-wide dispatch with `CSTTransformer.on_leave()`](https://github.com/mutating/cstvis/issues/10), в котором предложено упростить и оптимизировать `SuperTransformer`: отказаться от регистрации одного callback на всех matcher-классах и последующей дедупликации вызовов в пользу штатного `CSTTransformer.on_leave()`.

Работа не подразумевает изменений публичного API или документированного поведения cstvis и является чистым внутренним рефакторингом. Для защиты от регрессий работа начинается с усиления тестового покрытия: необходимые тесты должны быть добавлены и запущены до любых изменений production-кода.

## Порядок работы

1. Перед началом имплементации сохранить этот план в `docs/plans/2.md`.
2. Создать новый transformer-level тест и усилить существующий catch-all тест, не изменяя production-код.
3. Запустить новые тесты на текущей реализации и зафиксировать исходное поведение.
4. Выполнить полный pytest, чтобы подтвердить корректность тестовых изменений.
5. Только после добавления и проверки тестов заменить matcher-wide dispatch на `CSTTransformer.on_leave()`.
6. Повторно запустить новые тесты, полный набор проверок и compatibility matrix.

Поскольку изменение сохраняет существующее поведение, добавляемые сначала тесты являются characterization-тестами и могут проходить до рефакторинга. Их задача — зафиксировать необходимые гарантии до замены механизма dispatch.

## Детали изменений

- В `cstvis/transformers/super_transformer.py` заменить `MatcherDecoratableTransformer` на `CSTTransformer`.
- Удалить reflection по `libcst.matchers`, matcher-декораторы, `get_all_matcher_nodes()`, `leave_all()` и дедупликацию через `nodes_ids`.
- Сохранить `super().__init__()` и `METADATA_DEPENDENCIES`.
- Перенести существующую conversion-логику в `on_leave(original_node, updated_node)`.
- Получать тип, координаты и metadata только из `original_node`.
- Передавать converter’у только `updated_node`.
- Возвращать `updated_node` для всех нетаргетных узлов, чтобы сохранять изменения их детей.
- Использовать сигнатуру `CSTNode → CSTNode` с точечным `# type: ignore[override]` и кратким пояснением межтиповых замен вроде `Add → Subtract`.
- Не вызывать `super().on_leave()`, поскольку `SuperTransformer` не поддерживает внешний subclass-контракт с `leave_*` или matcher hooks.
- В `cstvis/changer.py` удалить пустой `set()` из аргументов конструктора `SuperTransformer`.
- README, версию, зависимости и публичные экспорты не менять.

## Тесты

| Название теста | Суть: какое именно поведение тестируем | Как происходит подготовка | Что именно проверяем | Детали, если они нужны |
|---|---|---|---|---|
| `test_converter_receives_updated_parent_with_original_context` | Родительский converter вызывается после обработки детей, получает `updated_node`, а координаты и metadata продолжают описывать `original_node`. | В `tests/transformers/test_super_transformer.py` создать `Changer` для `before = 1\n`, зарегистрировать converter для `Assign` и получить его координату. Создать локальный тестовый подкласс `SuperTransformer`, который при выходе из дочернего `Name('before')` возвращает `Name('after')`, а для остальных узлов делегирует production-реализации. Запустить его через настоящий `MetadataWrapper` и `SourceOffsetResolver`. | Итоговый код равен `after = 1\n`. Converter вызван ровно один раз. Полученный converter’ом `Assign` уже содержит `Name('after')`. `Context.position.coordinate` равен исходной координате `Coordinate(None, 'Assign', 1, 0, 1, 10)`. `Context.position.source` остаётся равным `before = 1\n`. | Создать `tests/transformers/__init__.py` для зеркального расположения и Ruff `INP001`. Не использовать structural assertions на MRO или точный базовый класс. Тест должен обнаруживать передачу `original_node` converter’у, metadata lookup по `updated_node`, потерю изменения ребёнка и повторный dispatch. |
| `test_any_converter_dispatches_once_for_every_coordinate` | Каждая координата, найденная для catch-all converter, приводит ровно к одному вызову converter для того же типа узла. | Усилить и переименовать существующий `test_converter_for_any` в `tests/test_changer.py`. Использовать исходник `5 - 5 + 5`, сохранить список всех координат, а identity-converter должен добавлять в список фактически полученный `node`. Применить каждую координату независимо. Сохранить существующую параметризацию `with_context` и `unfold`. | Количество вызовов converter равно количеству координат. Координат по-прежнему больше десяти. Имя класса каждого переданного узла совпадает с `coordinate.class_name` на той же позиции. Каждый результат равен исходному тексту, поскольку converter возвращает узел без изменений. | Исправить текущий `nodes.append(nodes)` на сохранение самого `node`. Тест остаётся в `tests/test_changer.py`, поскольку проверяет публичную оркестрацию `Changer.iterate_coordinates()` и `Changer.apply_coordinate()`. |

## Проверка и критерии приёмки

- План сохранён в `docs/plans/2.md` до начала имплементации.
- Оба тестовых сценария добавлены и запущены до изменения production-кода.
- После рефакторинга оба сценария продолжают проходить без изменения ожидаемого поведения.
- Полный pytest проходит.
- Statement и branch coverage остаются 100%.
- `ruff check cstvis` и `ruff check tests` проходят.
- `mypy --strict cstvis` и `mypy tests` проходят.
- Отдельно проходят Python 3.8 с `libcst==1.1.0` и актуальная поддерживаемая версия Python с LibCST 1.9.0.
- Документированные регистрация converter’ов, координаты, `Context`, комментарии, source fragments и генерируемый код не меняются.
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "cstvis"
version = "0.0.10"
version = "0.0.11"
authors = [{ name = "Evgeniy Blinov", email = "zheni-b@yandex.ru" }]
description = 'Incremental change of CST'
readme = "README.md"
Expand All @@ -30,6 +30,7 @@ classifiers = [
'Programming Language :: Python :: 3.12',
'Programming Language :: Python :: 3.13',
'Programming Language :: Python :: 3.14',
'Programming Language :: Python :: 3.15',
'Programming Language :: Python :: Free Threading',
'Programming Language :: Python :: Free Threading :: 3 - Stable',
'License :: OSI Approved :: MIT License',
Expand Down
38 changes: 25 additions & 13 deletions tests/test_changer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1058,29 +1058,41 @@ def convert_ints(node: int):
assert set(changer.apply_coordinate(coordinate) for coordinate in changer.iterate_coordinates()) == {'6 - 5 + 5', '5 - 6 + 5', '5 - 5 + 6'}


def test_converter_for_any(with_context, unfold):
def test_any_converter_is_invoked_once_per_coordinate(with_context, unfold):
"""
An Any-annotated converter is considered for many CST nodes.
Invoke an `Any` converter exactly once per independently applied coordinate.

Applying every coordinate in `Changer('5 - 5 + 5')` invokes the identity converter more than ten times across node-only/context-aware callbacks and both decorator forms.
The captured node's type must match the coordinate's class name, and identity
conversion must preserve the source. Both callback signatures and decorator
forms are covered.
"""
changer = Changer('5 - 5 + 5')
source = '5 - 5 + 5'
changer = Changer(source)

nodes = []
captured_nodes = []

if with_context:
@unfold(changer.converter)
def do_something(node: Any, context): # noqa: ARG001
nodes.append(nodes)
def capture_node(node: Any, context): # noqa: ARG001
captured_nodes.append(node)
return node
else:
@unfold(changer.converter)
def do_something(node: Any):
nodes.append(nodes)
def capture_node(node: Any):
captured_nodes.append(node)
return node

[changer.apply_coordinate(coordinate) for coordinate in changer.iterate_coordinates()]
assert len(nodes) > 10
coordinates = list(changer.iterate_coordinates())
assert len(coordinates) > 10

for coordinate in coordinates:
call_count_before = len(captured_nodes)
assert changer.apply_coordinate(coordinate) == source

assert len(captured_nodes) == call_count_before + 1
assert captured_nodes[-1].__class__.__name__ == coordinate.class_name

assert len(captured_nodes) == len(coordinates)


def test_if_node_is_not_exist_nothing_changed(with_context, unfold):
Expand Down Expand Up @@ -1122,8 +1134,8 @@ def filter_something(node: float, context): # noqa: ARG001
converter = list(changer.converters_by_types.values())[0][0] # noqa: RUF015
filter = list(changer.filters_by_types.values())[0][0] # noqa: RUF015, A001

assert converter.get_function_id() == 'tests.test_changer:do_something:1114'
assert filter.get_function_id() == 'tests.test_changer:filter_something:1118'
assert converter.get_function_id() == 'tests.test_changer:do_something:1126'
assert filter.get_function_id() == 'tests.test_changer:filter_something:1130'


def test_wrong_converter_and_wrong_filter(unfold):
Expand Down
Empty file added tests/transformers/__init__.py
Empty file.
54 changes: 54 additions & 0 deletions tests/transformers/test_super_transformer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from libcst import Assign, CSTNode, Name, metadata
from libcst.metadata import CodePosition, CodeRange

from cstvis import Changer, Context, Coordinate
from cstvis.source_offsets import SourceOffsetResolver
from cstvis.transformers.super_transformer import SuperTransformer


def test_converter_receives_updated_parent_with_original_context():
"""
Pass a child update to the parent converter with the parent's original context.

The transformer renames the assignment target on child leave, then delegates
parent leave to `SuperTransformer`. The converter receives the updated
`Assign` exactly once, while its `Context` retains the original coordinate,
whitespace-inclusive range, and source.
"""
source = 'before = 1\n'
changer = Changer(source)
converter_calls = []

@changer.converter
def capture_assign(node: Assign, context: Context) -> Assign:
converter_calls.append((node, context))
return node

coordinate = next(changer.iterate_coordinates())

class ChildUpdatingSuperTransformer(SuperTransformer):
def on_leave(self, original_node: CSTNode, updated_node: CSTNode) -> CSTNode: # type: ignore[override]
if isinstance(original_node, Name) and original_node.value == 'before':
return Name('after')
return super().on_leave(original_node, updated_node)

wrapper = metadata.MetadataWrapper(changer.module)
node_ranges = wrapper.resolve(metadata.WhitespaceInclusivePositionProvider)
transformer = ChildUpdatingSuperTransformer(
coordinate,
changer.converters_by_types,
changer._comments_by_lines,
SourceOffsetResolver(wrapper.module, source, node_ranges.values()),
)
transformed_source = wrapper.visit(transformer).code

assert transformed_source == 'after = 1\n'
assert len(converter_calls) == 1
updated_assign, context = converter_calls[0]
updated_target = updated_assign.targets[0].target
assert isinstance(updated_target, Name)
assert updated_target.value == 'after'
position = context.position
assert position.coordinate == Coordinate(None, 'Assign', 1, 0, 1, 10)
assert position.node_range == CodeRange(CodePosition(1, 0), CodePosition(1, 10))
assert position.source == source
Loading