fix(private): разбор импортов и вызовов через ast, а не регуляркой #13

Merged

claude-secretary merged 3 commits from fix/ast-private-hooks into main

2026-08-27 12:01:43 +07:00

claude-secretary commented

2026-08-26 20:55:02 +07:00

Owner

Найдено при заведении волн no-private-imports и no-private-method-calls в jamzap/backend (эпик backend#465, задачи #471 и #472).

Третий хук подряд оказался построчной регуляркой с ошибками в обе стороны

Это уже закономерность, и её стоит назвать прямо: хуки писались regex-first и не проверялись на реальном дереве. no-issue-refs-in-comments в #12 был первым, здесь — ещё два. Симптом один и тот же: молчаливый пропуск плюс ложное срабатывание, и обнаруживаются они только когда хук впервые включают на живом коде.

`no-private-imports`

Что	Как ошибалась регулярка	На jamzap/backend
Многострочный импорт в скобках	совпадение искалось в строке с `from ... import`, а имена стоят ниже	30 нарушений из 51 проходили молча — хук показывал вдвое меньше долга, чем есть
Алиас	считался импортируемым символом	7 из 28 срабатываний были ложными: `from app.core.i18n import gettext_lazy as _lazy` падало на публичном имени

Второе особенно неприятно: подчёркивание в алиасе — обычная разметка «не реэкспортируем из этого модуля», и «починка» волной означала бы ломать идиому ради дефекта инструмента.

Теперь проверяется именно импортируемое имя:

from x import _y as y   # нарушение — символ приватный
from x import y as _y   # чисто — приватен только локальный алиас
import pkg._private     # нарушение — приватным делает последний сегмент
from app.core.i18n import _   # чисто — это имя gettext-функции

Голое _ — правка по ревью: ast-версия первой редакции ловила его как приватный символ, хотя старая регулярка требовала букву после подчёркивания и такое пропускала. Ирония в том, что PR мотивирован соседней идиомой из того же i18n-модуля.

`no-private-method-calls`

Регулярка искала ._name( в «коде», отрезая всё после первой решётки:

Class._method(...) в docstring'е считался вызовом. На jamzap/backend так падали 4 файла, у которых весь «вызов» — заголовок теста вида «Тесты GeoConfig._parse_trusted_proxies»;
решётка внутри строкового литерала обрезала строку и прятала настоящий вызов за ней.

ast снимает оба случая по построению: строки и комментарии узлами Call не являются. Заодно ловится вызов, разложенный по строкам, и исключён dunder — obj.__init__() это протокол, а не приватный интерфейс.

Дельта по потребителям

Хук	Дерево	old	new	снято ложняков	найдено пропущенного
`no-private-imports`	backend (`app`+`tests`+`scripts`)	28	51	7	30
`no-private-imports`	dispatcher	81	83	6	8
`no-private-method-calls`	backend (`tests`)	53	49	4	0

Все «снятые» просмотрены поштучно — это алиасы и упоминания в docstring'ах. Все «найденные» — настоящие многострочные импорты приватных символов.

Что изменилось по ревью

Голое _ больше не нарушение (см. выше) — единственный новый ложняк, который ast-версия успела ввести.
Улика указывает на строку самого имени, а не на строку from mypkg.helpers import (: у ast.alias есть собственный lineno, у вызова берётся end_lineno атрибута. Это не косметика — на pre-push вердикт фильтруется по изменённым строкам, и чужой номер там равен молчаливому пропуску. Оба номера теперь пиннятся в smoke через run_case_out.
Фикстура на BOM. Без encoding="utf-8-sig" ast.parse падает на U+FEFF, except SyntaxError глотает это, и весь файл проходит молча. Мутация «убрать encoding=» первую редакцию переживала.
Прежняя фикстура многострочного вызова проходила по случайности — obj._compute_internal( стоял целиком на одной строке, то есть ловился и старой регуляркой. Переписана так, чтобы атрибут был не на строке объекта.
Добавлены кейсы на не-UTF-8 файл, непарсящийся файл, относительный импорт, отложенный импорт (в функции и под if TYPE_CHECKING), from x import *.
Формулировка про кодировки в первой редакции была неверной: «голый open() ронял их на не-UTF-8 стектрейсом» — для этих двух хуков нет, старый код оборачивал чтение в except Exception и возвращал []. Реальные приобретения другие: (а) BOM больше не прячет файл целиком, (б) except Exception сузился до except (OSError, UnicodeDecodeError) — то есть старый код нарушал собственный no-broad-except этого же репозитория.
README дописан там, где хук делал больше, чем сказано: self._helper() внутри тест-класса — тоже FAIL; obj.__mangled() и псевдоприватный stdlib-API (nt._replace()) регулярка пропускала, ast ловит; приватный модуль в пути импорта (from ._private import x) не ловится — записано как известное ограничение и заведено отдельной задачей.

Проверка

Мутации, каждая роняет smoke: алиас вместо импортируемого имени; выпадение ветки ast.Import; снятие исключения dunder (в каждом из двух хуков); проверка только первого имени в списке вместо всех; убрать encoding="utf-8-sig"; вернуть улику на node.lineno (в обоих хуках); снова считать _ приватным.

Мутация «только первое имя» сначала переживала smoke — фикстура многострочного импорта держала приватное имя первым. Заведена вторая, где оно последнее: приватное имя дописывают в конец списка чаще, чем ставят в начало.

По дороге снят собственный неверный комментарий: я написал, что номер строки берётся у Attribute, а не у Call, «чтобы улика указывала на сам ._method». Проверил — у Call и у его Attribute номер строки один и тот же, начало выражения; разница была придумана. Точную строку даёт end_lineno, что и сделано теперь.

Версия

0.14.0 → 0.15.0: no-private-imports начинает ловить существенно больше (многострочные импорты), это поведенческое изменение. Потребителям после бампа стоит ждать новых срабатываний ровно там.

Первая редакция пиновала v0.13.0 — этот номер уже занят grandfather-фиксом (#15), а v0.14.0 — хуком max-filename-words (#7). Отдельно: на remote нет ни одного тега после v0.9.1, из-за чего пин из README у консьюмера не резолвится — заведено #16.

Найдено при заведении волн `no-private-imports` и `no-private-method-calls` в jamzap/backend (эпик backend#465, задачи #471 и #472). ## Третий хук подряд оказался построчной регуляркой с ошибками в обе стороны Это уже закономерность, и её стоит назвать прямо: хуки писались regex-first и не проверялись на реальном дереве. `no-issue-refs-in-comments` в [#12](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/12) был первым, здесь — ещё два. Симптом один и тот же: **молчаливый пропуск** плюс ложное срабатывание, и обнаруживаются они только когда хук впервые включают на живом коде. ### `no-private-imports` | Что | Как ошибалась регулярка | На jamzap/backend | |---|---|---| | Многострочный импорт в скобках | совпадение искалось в строке с `from ... import`, а имена стоят ниже | **30 нарушений из 51 проходили молча** — хук показывал вдвое меньше долга, чем есть | | Алиас | считался импортируемым символом | **7 из 28** срабатываний были ложными: `from app.core.i18n import gettext_lazy as _lazy` падало на публичном имени | Второе особенно неприятно: подчёркивание в алиасе — обычная разметка «не реэкспортируем из этого модуля», и «починка» волной означала бы ломать идиому ради дефекта инструмента. Теперь проверяется именно импортируемое имя: ```python from x import _y as y # нарушение — символ приватный from x import y as _y # чисто — приватен только локальный алиас import pkg._private # нарушение — приватным делает последний сегмент from app.core.i18n import _ # чисто — это имя gettext-функции ``` Голое `_` — правка по ревью: ast-версия первой редакции ловила его как приватный символ, хотя старая регулярка требовала букву после подчёркивания и такое пропускала. Ирония в том, что PR мотивирован соседней идиомой из того же i18n-модуля. ### `no-private-method-calls` Регулярка искала `._name(` в «коде», отрезая всё после первой решётки: - `Class._method(...)` **в docstring'е** считался вызовом. На jamzap/backend так падали 4 файла, у которых весь «вызов» — заголовок теста вида «Тесты `GeoConfig._parse_trusted_proxies`»; - решётка **внутри строкового литерала** обрезала строку и прятала настоящий вызов за ней. `ast` снимает оба случая по построению: строки и комментарии узлами `Call` не являются. Заодно ловится вызов, разложенный по строкам, и исключён dunder — `obj.__init__()` это протокол, а не приватный интерфейс. ## Дельта по потребителям | Хук | Дерево | old | new | снято ложняков | найдено пропущенного | |---|---|---|---|---|---| | `no-private-imports` | backend (`app`+`tests`+`scripts`) | 28 | 51 | 7 | 30 | | `no-private-imports` | dispatcher | 81 | 83 | 6 | 8 | | `no-private-method-calls` | backend (`tests`) | 53 | 49 | 4 | 0 | Все «снятые» просмотрены поштучно — это алиасы и упоминания в docstring'ах. Все «найденные» — настоящие многострочные импорты приватных символов. ## Что изменилось по ревью - **Голое `_` больше не нарушение** (см. выше) — единственный новый ложняк, который ast-версия успела ввести. - **Улика указывает на строку самого имени**, а не на строку `from mypkg.helpers import (`: у `ast.alias` есть собственный `lineno`, у вызова берётся `end_lineno` атрибута. Это не косметика — на pre-push вердикт фильтруется по изменённым строкам, и чужой номер там равен молчаливому пропуску. Оба номера теперь пиннятся в smoke через `run_case_out`. - **Фикстура на BOM.** Без `encoding="utf-8-sig"` `ast.parse` падает на `U+FEFF`, `except SyntaxError` глотает это, и **весь файл проходит молча**. Мутация «убрать `encoding=`» первую редакцию переживала. - Прежняя фикстура многострочного вызова проходила по случайности — `obj._compute_internal(` стоял целиком на одной строке, то есть ловился и старой регуляркой. Переписана так, чтобы атрибут был не на строке объекта. - Добавлены кейсы на не-UTF-8 файл, непарсящийся файл, относительный импорт, отложенный импорт (в функции и под `if TYPE_CHECKING`), `from x import *`. - **Формулировка про кодировки в первой редакции была неверной:** «голый `open()` ронял их на не-UTF-8 стектрейсом» — для этих двух хуков нет, старый код оборачивал чтение в `except Exception` и возвращал `[]`. Реальные приобретения другие: (а) BOM больше не прячет файл целиком, (б) `except Exception` сузился до `except (OSError, UnicodeDecodeError)` — то есть старый код нарушал собственный `no-broad-except` этого же репозитория. - README дописан там, где хук делал больше, чем сказано: `self._helper()` внутри тест-класса — тоже FAIL; `obj.__mangled()` и псевдоприватный stdlib-API (`nt._replace()`) регулярка пропускала, `ast` ловит; приватный **модуль** в пути импорта (`from ._private import x`) не ловится — записано как известное ограничение и заведено отдельной задачей. ## Проверка Мутации, каждая роняет smoke: алиас вместо импортируемого имени; выпадение ветки `ast.Import`; снятие исключения dunder (в каждом из двух хуков); проверка только первого имени в списке вместо всех; убрать `encoding="utf-8-sig"`; вернуть улику на `node.lineno` (в обоих хуках); снова считать `_` приватным. Мутация «только первое имя» сначала переживала smoke — фикстура многострочного импорта держала приватное имя первым. Заведена вторая, где оно последнее: приватное имя дописывают в конец списка чаще, чем ставят в начало. По дороге снят собственный неверный комментарий: я написал, что номер строки берётся у `Attribute`, а не у `Call`, «чтобы улика указывала на сам `._method`». Проверил — у `Call` и у его `Attribute` номер строки один и тот же, начало выражения; разница была придумана. Точную строку даёт `end_lineno`, что и сделано теперь. ## Версия `0.14.0` → **`0.15.0`**: `no-private-imports` начинает ловить существенно больше (многострочные импорты), это поведенческое изменение. Потребителям после бампа стоит ждать новых срабатываний ровно там. Первая редакция пиновала `v0.13.0` — этот номер уже занят grandfather-фиксом ([#15](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/15)), а `v0.14.0` — хуком `max-filename-words` ([#7](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/7)). Отдельно: на remote нет ни одного тега после `v0.9.1`, из-за чего пин из README у консьюмера не резолвится — заведено [#16](https://git.homedevlab.ru/senokosov/pre-commit-hooks/issues/16).

claude-secretary added 1 commit

2026-08-26 20:55:02 +07:00

fix(private): разбор импортов и вызовов через ast, а не регуляркой

ci / smoke (push) Successful in 7s

Details

ci / smoke (pull_request) Successful in 7s

Details

fd39f7de65

Оба хука были построчными регулярками и ошибались в обе стороны. Обе
ошибки нашлись на реальном дереве, при заведении волн jamzap/backend#471
и #472.

no-private-imports:

- многострочный импорт в скобках проходил молча — совпадение искалось в
  строке с `from ... import`, а имена стоят ниже. На jamzap/backend так
  пропускались 30 нарушений из 51, то есть хук показывал вдвое меньше
  долга, чем есть;
- алиас считался импортируемым символом, поэтому
  `from app.core.i18n import gettext_lazy as _lazy` падало на публичном
  имени. Подчёркивание в алиасе — разметка «не реэкспортируем из этого
  модуля», к правилу отношения не имеющая; в jamzap/backend таких мест 7
  из 28, и «починка» означала бы ломать идиому ради дефекта хука.

Теперь проверяется именно импортируемое имя: `from x import _y as y` —
нарушение, `from x import y as _y` — нет. `import pkg._private` тоже
ловится, приватным делает последний сегмент.

no-private-method-calls:

- `Class._method(...)` в docstring'е считался вызовом. На jamzap/backend
  так падали 4 файла, у которых весь «вызов» — заголовок теста вида
  «Тесты GeoConfig._parse_trusted_proxies»;
- решётка внутри строкового литерала обрезала строку и прятала
  настоящий вызов за ней.

ast снимает оба случая по построению: строки и комментарии узлами Call
не являются. Заодно ловится вызов, разложенный по строкам, и исключён
dunder — `obj.__init__()` это протокол, а не приватный интерфейс.

Оба хука читают файл в utf-8-sig: голый open() ронял их на не-UTF-8
стектрейсом вместо вердикта, как и no-issue-refs до v0.12.0.

Мутации: алиас вместо имени, выпадение ast.Import, снятие исключения
dunder в каждом из хуков, проверка только первого имени в списке — все
роняют smoke. По дороге снят собственный неверный комментарий: у Call и
у его Attribute номер строки один и тот же, разница была придумана.