fix(issue-refs): разбор через tokenize, общий с no-broken-repo-paths #12
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/issue-refs-tokenize"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Найдено при заведении волны
no-issue-refs-in-commentsв jamzap/backend (эпик backend#465). Refs #11.Хук флагал формат, который сам предписывает
Построчный разбор «первый
#открывает комментарий» на строке docstring'авидел комментарий
#286).— исключениеTODO(...)в этот фрагмент уже не попадало, и легальнейший из возможных форматов падал как нарушение.Тот же разбор давал вторую, противоположную ошибку
Обрезка строки по первому
#ставила номер в начало фрагмента, и lookbehind(?<!\w)его пропускал. Из-за этого префиксные ссылки —backend#451,jamzap/backend#386,issue#32— ловились только в docstring'ах и только по случайности:# комментарии#451backend#451Поведение зависело от того, комментарий это или docstring, — при том что в описании хука они равноправны.
Что сделано
Разбор вынесен в
scripts/comment_scan.pyи стал общим сno-broken-repo-paths— как просит #11 («не латать построчный разбор по одному случаю, а переиспользовать машину состояний»). Три режима:python_chunks(COMMENT-токены черезtokenize+ docstring'и черезast),c_style_chunks(//и/* */, пофайловая машина),hash_chunks(#построчно — для shell/yaml/toml, где пофайловый трекинг кавычек опаснее). Дублирование машины устранено:check_no_broken_repo_paths.pyпохудел на 94 строки.Правило про префикс записано прямо, а не как побочный эффект обрезки: ссылкой считается
#NNNнезависимо от того, что стоит перед решёткой.Якорь на путь или домен ссылкой не считается. Признак — точка в токене перед решёткой: у номеров задач её не бывает, у доменов и файлов есть всегда. Так отсекаются
https://x/y#123,example.com/spec#456иdocs/adr/0001.md#456.Regex-литералы пропускаются как литералы. Кавычка, бэктик или
/*внутри/[^"]+/больше не открывают мнимую строку или мнимый блочный комментарий — раньше это уводило разбор до конца файла, молчаливым пропуском в одну сторону и ложными срабатываниями на строковых литералах в другую. Распознавание — эвристика JS-лексеров, а не парсер: literal начинается после оператора, скобки или ключевого слова (return /re/,=> /re/) и обязан закрыться на своей строке.Чего эвристика намеренно не считает regex'ом, и почему это важно:
total / count,i++ / 2prevформально разрешает, поэтому literal, чей «закрывающий» слэш сам оказался началом//, отвергается — принять его значило бы съесть комментарий<Foo bar={1} /></p><убран из набора: перед слэшем он стоит только в закрывающем тегеОба механизма для JSX нужны и оба под тестом: гард смотрит только на
//, а с<в наборе мнимый literal после</p>дотягивался до слэша в/*, блочный комментарий не открывался вовсе и ссылка в нём терялась молча.Файлы читаются в
utf-8-sig. BOM ронялast.parseна U+FEFF, классификация docstring'ов отключалась целиком, и ссылка в docstring'е проходила молча — в обоих хуках.Гейт против устаревшего артефакта сборки —
fixtures/check_wheel_matches_sources.py. Хуки сlanguage: pythonприезжают потребителю установленным пакетом; колесо собирается in-tree, иsetuptoolsкопирует исходник только если он новее артефакта. Залежавшийсяbuild/libподменял бы код молча, а smoke этого не видит — он гоняетscripts/*.pyнапрямую. Гейт собирает колесо и сверяет содержимое в обе стороны; чужойbuild/не удаляет — это улика.Побочный эффект:
no-broken-repo-pathsстал строжеПропуск regex-литералов чинит то же ограничение и у него. На реальных деревьях это ничего не меняет — сверил вывод и коды возврата old vs new на backend (974
.py), frontend (226), mobile (103), dispatcher (26): байт-в-байт идентично. Улучшение подтверждено фикстурой, не деревом.Проверка
Мутационная. Каждая фикстура привязана к мутации; все роняют smoke. По разбору python: docstring'и не сканируются, убрать
ast.Module/ClassDef/FunctionDef/AsyncFunctionDef, убрать.pyi,utf-8вместоutf-8-sig. По ссылкам: якорь не вырезается, вернуть lookbehind, отключить исключениеTODO. По C-подобной машине: regex не пропускается, класс[...]игнорируется, regex через перевод строки, экранирование внутри regex, символ перед слэшем не отслеживается, ключевые слова не учитываются, вернуть<в набор, снять гард по//, не сбрасывать строку на\n, бэктик не считается кавычкой, блок не закрывается, экранирование в строке,#-режим без трекинга кавычек, номер строки на продолжении через\.Фикстура
docstring_ref.pyразбита по видам узла: пока модуль, класс и функция нарушали в одном файле, мутации «убратьClassDef» и «убратьFunctionDef» переживали smoke — одного падения хватало на exit 1.Smoke получил помощник
run_case_outс проверкой подстроки в выводе: сдвиг номера строки иначе не ловится вовсе, код возврата у него правильный.Дельта по потребителям. Хук подключён в backend и dispatcher; для frontend/mobile — прикидка на будущее.
app+scripts, 500)dispatcher/, 26)Новая версия не супермножество старой, и обе просадки объяснимы: у backend единственная потеря —
app/admin/core/layout.py:440, тот самыйTODO(#286)в docstring'е, ради которого PR и заведён; у dispatcher 28 потерь вdashboard.py— все на HTML-мнемониках (🟢) внутри строкового литерала с HTML-шаблоном, то есть снятые ложняки. Файл там и так подexclude. Прирост просмотрен: настоящиеmobile#144,jamzap/backend#83,issue#155.Версия
0.11.0→0.12.0: расширение того, что считается нарушением, — поведенческое изменение, не патч.Что #11 остаётся должен
Апостроф в тексте JSX (
<p>don't</p>глушит комментарий своей строки — зафиксировано фикстурой и README), отдельные режимы для.vue/.html/.svelte/.css, зависимостьno-broken-repo-pathsот неверсионируемыхgit check-ignore-настроек, асимметрия пробелов вtodo-needs-issue. ПоэтомуRefs #11, а неCloses.Ревью нашло два блокера. 1. В ветку был закоммичен build/lib — и колесо, которое реально едет потребителю, собиралось из него, а не из scripts/. Колесо собирается in-tree, а setuptools копирует исходник только если он новее артефакта; в свежем клоне mtime совпадают, поэтому у потребителя отсутствовал бы весь второй коммит. Smoke этого не видел вовсе — он гоняет scripts/*.py напрямую и никогда установленный пакет. build/ и dist/ убраны из индекса и добавлены в .gitignore. Заведён гейт fixtures/check_wheel_matches_sources.py: собирает колесо и сверяет каждый упакованный scripts/*.py с деревом. Проверено, что он ловит ровно этот сценарий — подложенный build/lib с совпадающим mtime даёт exit 1. Чужой build/ гейт не удаляет: это улика. 2. Заявление «радиус рассинхрона ограничен строкой» было неверным. Сброс состояния на переводе строки сделан для ' и ", но бэктик из него исключён законно (шаблонный литерал перевод строки переживает), и одиночный бэктик в regex-литерале — /[`'"]/ — уводил разбор до конца файла. Симметрично /* внутри regex открывал мнимый блочный комментарий, и это уже не пропуск, а ложные срабатывания на обычных строковых литералах. Вместо расширения сброса машина теперь пропускает regex-литерал целиком. Regex распознаётся по символу перед слэшем (после значения слэш — деление) и обязан закрываться на своей строке, поэтому `<Foo bar={1} />` и `total / count` под него не подпадают, а `/` внутри символьного класса литерал не закрывает. Это закрывает и ограничение no-broken-repo-paths: битый путь в комментарии на строке regex-литерала теперь ловится. Единственным остаточным каналом остаётся апостроф в тексте JSX, и он ограничен своей строкой — фикстура limitation.ts переписана под него. Побочно: продолжение строки через `\` в конце строки съедало перевод строки вместе с экранированием, не инкрементируя номер. Сдвиг копился до конца файла, нарушение печаталось с чужим номером, а на pre-push такое молча выпадает из фильтра по изменённым строкам. Smoke расширен помощником run_case_out — с проверкой подстроки в выводе, иначе сдвиг номера строки не ловится вовсе: код возврата у него правильный. Пять новых мутаций (regex не пропускается, класс игнорится, regex через перевод строки, номер строки на продолжении, символ перед слэшем не отслеживается) роняют smoke. Refs #11Четвёртая итерация ревью нашла, что `}` в наборе символов открывает ту же дыру, что закрывало удаление `<`: слэш самозакрытия JSX с атрибутом- выражением (`<Foo bar={1} />`) запускал попытку regex-скипа, «закрывающим» слэшем оказывался слэш в `/*`, и гард его пропускал — он смотрел только на `//`. Блочный комментарий не открывался вовсе. Пять входов, каждый — регрессия против v0.11.0 в обоих хуках: <Foo bar={1} />; {/* ссылка */} — идиоматичный React items.map((i) => <Row key={i} />); {/* */} count! / total; /* ссылка */ — TS non-null x.in / 2; /* ссылка */ — ключевое слово как имя свойства <Foo a={1} />; const p = `a/b`; // ... — уезжало до конца файла Последний — худший: мнимый literal глотал открывающий бэктик, закрывающий открывал шаблонный литерал, тот переживает переводы строк, и терялся весь остаток файла. Ровно тот failure mode, который PR объявлял закрытым. Гард расширен на `*`: у настоящего literal'а за закрывающим слэшем стоят пробел, `;`, `)` или флаг из dgimsuvy — ни `/`, ни `*` там не бывает. `}` убран из набора. Слово, начавшееся сразу после точки, ключевым не считается — иначе `x.in`, `x.of`, `x.do` открывали бы regex. Greenwash: мутация «regex через перевод строки» была заявлена покрытой, но переживала весь smoke — названная фикстура ловила соседнюю мутацию. Заведена regex_unterminated.ts: намеренно битый синтаксис, где поиск «закрывающего» слэша уходит по файлу и находит его внутри шаблонного литерала. Мутации `<` и `}` в наборе тоже переживали exit-code-проверку — им нужны фикстуры формы derail, где теряется не одна строка, а хвост файла, поэтому проверяются через run_case_out по последней строке. Батарея по C-подобной машине — 15 мутаций, все убиты. Ниты: расширение python-файла теперь нечувствительно к регистру (`A.PY` молча уходил в `#`-режим, где docstring'и не разбираются, а решётка в строке даёт ложняк); лог setuptools в гейте колеса заглушён, иначе настоящий FAIL тонет в сорока строках. Refs #11Ревью — четыре итерации, все блокеры закрыты
Регламент проекта — не более трёх итераций независимого ревью на один PR. Здесь их четыре, и это стоит объяснить: каждая находила новые настоящие дефекты, а не повторяла прошлые. Схождение есть, но цена тоже — привожу историю целиком, чтобы решение о мерже принималось с открытыми глазами.
build/lib— колесо у потребителя собиралось бы из устаревшего кода, а smoke этого не видит вовсе. Плюс бэктик и/*внутри regex по-прежнему уводили разбор.</p>; // ссылкаиi++ / 2; // ссылкаv0.11.0 ловил, ветка молчала. Regex после ключевого слова не распознавался. Инвариант «радиус — строка» перестал проверяться чем-либо.}в наборе открывал ту же дыру для<Foo bar={1} />; {/* ссылка */}— идиоматичного React. Худший случай уезжал до конца файла через проглоченный бэктик. Мутация «regex через перевод строки» переживала весь smoke.Три из четырёх итераций нашли дефекты в моих же исправлениях предыдущей. Это свойство задачи: эвристика JS-лексера без парсера — та область, где каждое сужение открывает соседний случай. Отсюда и то, как выглядит финальное состояние: не «работает на примерах», а 15 мутаций по C-подобной машине, каждая убита своей фикстурой.
Отдельно стоит назвать два места, где exit-кода недостаточно и проверка идёт по выводу (
run_case_out):\— нарушение находится, но печатается с чужим номером, и наpre-pushмолча выпадает из фильтра по изменённым строкам;jsx_close_derail.tsx,keyword_as_property.ts,jsx_multiline_derail.tsx) — там теряется не одна строка, а хвост файла, и одного оставшегося срабатывания хватило бы на зелёный exit.Что проверено на реальном коде
no-broken-repo-paths— вывод и коды возврата байт-в-байт совпадают с v0.11.0 на четырёх деревьях: backend (974.py), frontend (226), mobile (103), dispatcher (26). Ни одного расхождения.no-issue-refs-in-comments— прирост просмотрен, ложных срабатываний нет; обе просадки объяснимы и обе желательны (снятый ложнякTODO(#286)в docstring'е backend и 28 HTML-мнемоник🟢внутри строкового литерала dispatcher).Пять входов-репро из четвёртой итерации проверены на финальном состоянии — все ловятся.
Что остаётся честным ограничением
Апостроф в тексте JSX (
<p>don't</p>) глушит комментарий своей строки. Зафиксировано фикстуройbroken_paths/limitation.ts, записано в README и в docstring'ах обоих модулей. Эвристика не полна и на полноту не претендует: она гарантирует, что промах не выходит за пределы строки, и все известные пути уехать дальше — через проглоченный открывающий бэктик — закрыты фикстурами.Пункты #11 про
.vue/.html/.svelte/.css,git check-ignoreот неверсионируемых настроек и асимметрию пробелов вtodo-needs-issueне тронуты — поэтомуRefs, а неCloses./>на своей строке; не-UTF-8 файл не роняет хукРевью — итерация 5, блокеров нет
Ревьюеру был задан не только вопрос «есть ли блокеры», но и прямой: сошлась ли эвристика или её стоит выбросить целиком. Ответ — сошлась, и обоснование стоит привести, потому что оно опирается на измерение, а не на впечатление.
Ключевая проверка: четвёртая правка впервые не открыла соседнего случая. Предыдущие три каждая закрывала один и открывала другой — это и было основанием сомневаться в подходе. Прогон HEAD против предыдущего коммита одной батареей: инлайновое самозакрытие JSX с шаблонным литералом — было runaway, стало ok; восемь настоящих regex'ов через гард — ни одного ложного отказа; восемь кейсов слова после точки — все верны.
Эмпирика по 365 C-подобным файлам frontend и mobile: в ветку regex-скипа машина входит 1509 раз, принимает 724, из них ложных принятий 0, проглоченных бэктиков 0, файлов, заканчивающихся не в состоянии
code, — 0. Два подозрительных принятия на/>оказались настоящими/>/gв.replace(/>/g, …).Цена альтернативы — вернуть молчаливый пропуск до конца файла на ~20 живых местах фронта и мобилки, то есть ровно тот режим отказа, который #11 называет хуже отсутствия хука.
Что поправлено по этой итерации
/>на своей строке. Prettier переносит самозакрытие тега на отдельную строку, и многострочная форма переоткрывала дыру, закрытую для инлайновой: перед слэшем оказывается перевод строки, он в наборе. Достижимость на реальных деревьях нулевая (строк вида^\s*/>.+— ноль), но инвариант «промах не выходит за пределы строки» PR продаёт как главный, и обещание должно быть правдой.Не-UTF-8 файл ронял хук стектрейсом.
except OSErrorне ловитUnicodeDecodeError.types:у хука намеренно не задан — скоуп задаёт потребитель, — так что достаточно чуть более широкогоfiles:и одного застейдженного PNG. Наmainто же падение было уno-broken-repo-paths: унификация протянула более слабое поведение вместо более сильного. Исправлено в обоих.Обоснование исключения
.cssбыло перевёрнуто. Комментарий утверждал, что исключение снимает проблему hex-цветов; на деле оно её усугубляет — файл попадает в#-режим, где флагается каждый цвет уже в самом коде. Переписан.README обещал больше, чем делает код — «все известные пути к проглатыванию бэктика закрыты». Один остаётся, экзотический: regex, оканчивающийся бэктиком вплотную перед комментарием. Назван прямо.
Замечание про мутации, которое стоит зафиксировать
Условие на
/>пересеклось с удалением}из набора — обе правки закрывают JSX-самозакрытие, и мутация «вернуть}» перестала быть покрытой. Это повторилось в третий раз за PR: новая защита делает старую ненаблюдаемой. Каждый раз ответ один — не оставлять непроверяемую страховку, а найти вход, где работает только она. Здесь это деление объекта ({ a: 1 } / Number(\a/b`)) — валидный, хоть и бессмысленный, JS, где условие на/>` неприменимо.Итог: 17 мутаций по C-подобной машине, все убиты; greenwash ревьюер искал прицельно и не нашёл — каждая падает на фикстуре, чьё имя совпадает с заявленной причиной.
Уточнения к описанию PR
no-broken-repo-pathsбайт-в-байт на всех четырёх» — верно для.py-скоупа backend. На полном дереве с.mdновая версия добавляет два истинных срабатывания вdocs/architecture/ADMIN_PLAN.md(оба файла действительно удалены). Улучшение, не регрессия.scripts/*.pyне попал в колесо» приpackages.find include = ["scripts*"]недостижима — любой новый файл включается автоматически. Защита на будущее, тестом не покрыта по построению.