no-issue-refs-in-comments: разбор комментариев построчный — три класса пропусков и ложняков #11

Closed
opened 2026-08-26 14:01:51 +07:00 by claude-secretary · 2 comments

Найдено на ревью #10 (там же закрыты блокеры; эти четыре пункта merge не держали). Общая причина у первых трёх одна: extract_comment в scripts/check_no_issue_refs_in_comments.py разбирает строку изолированно, тогда как соседний check_no_broken_repo_paths.py уже несёт пофайловую машину состояний.

1. Многострочный шаблонный литерал даёт ложное срабатывание

Состояние строкового литерала сбрасывается на каждой строке, поэтому

const q = `
  select 1 // #456
`;

падает как нарушение. README при этом утверждает, что строковые литералы, включая шаблонные, комментарием не считаются — верно только внутри одной строки.

2. В C-подобном режиме разбирается только первый комментарий строки

Возврат происходит на первом найденном комментарии, поэтому не ловятся:

const x = 1; /* ok */ // ссылка #456
/* a */ /* b #456 */

Для хука, чей смысл в том, что молчаливый проход хуже отсутствия хука, это неприятный класс дыры.

3. Апостроф в JSX-тексте съедает остаток строки

const el = <p>don't</p>; // ссылка #456

проходит: апостроф открывает «строку», которая до конца строки не закрывается. .tsx — ровно тот стек, из-за которого заводился #8.

4. Обработка строковых литералов не покрыта тестами

Три мутации выживают со всем зелёным: убрать бэктик из quotes, убрать ветку elif ch in quotes целиком, убрать обработку экранирования. Существующие фикстуры в эту логику не бьют: в slash-режиме # и так не комментарий, а "https://example.com/x#987" не матчится, потому что #987 прижат к букве и падает на lookbehind. Нужна фикстура вида const s = "see // #456 here";.

Как чинить

Не латать построчный разбор по одному случаю, а переиспользовать машину состояний из check_no_broken_repo_paths.py (:274-321): она переживает несколько комментариев на строке и переоткрытие блока. Сейчас логика дублирована в двух скриптах с разной зрелостью, и расхождение будет расти при каждой правке.

Приёмка

  • Все четыре случая выше разобраны корректно
  • Логика разбора комментариев одна на оба хука, а не две копии
  • Мутации по строковым литералам роняют тесты
  • Ограничения, которые останутся, записаны в README явно — как это сделано у no-broken-repo-paths

Смежное, отдельными пунктами

Неизвестные расширения падают в #-режим, и для разметки это неверно: .vue, .html, .svelte — текст <p>Задача #456</p> флагается. README предупреждает только про .md.

Вердикт зависит от неверсионируемых настроек: git check-ignore в no-broken-repo-paths учитывает .git/info/exclude и глобальный core.excludesFile. Добавление пути туда переводит хук с fail на pass, не оставляя следа в PR. Направление отказа безопасное (CI строже локали), но свойство стоит либо записать в README рядом с абзацем про .gitignore, либо прибить окружением на время вызова.

Асимметрия пробелов в todo-needs-issue: TODO (#456) принимается, TODO( #456 ) — нет. Нигде не описано.

Найдено на ревью #10 (там же закрыты блокеры; эти четыре пункта merge не держали). Общая причина у первых трёх одна: `extract_comment` в `scripts/check_no_issue_refs_in_comments.py` разбирает строку изолированно, тогда как соседний `check_no_broken_repo_paths.py` уже несёт пофайловую машину состояний. ## 1. Многострочный шаблонный литерал даёт ложное срабатывание Состояние строкового литерала сбрасывается на каждой строке, поэтому ```ts const q = ` select 1 // #456 `; ``` падает как нарушение. README при этом утверждает, что строковые литералы, включая шаблонные, комментарием не считаются — верно только внутри одной строки. ## 2. В C-подобном режиме разбирается только первый комментарий строки Возврат происходит на первом найденном комментарии, поэтому не ловятся: ```ts const x = 1; /* ok */ // ссылка #456 /* a */ /* b #456 */ ``` Для хука, чей смысл в том, что молчаливый проход хуже отсутствия хука, это неприятный класс дыры. ## 3. Апостроф в JSX-тексте съедает остаток строки ```tsx const el = <p>don't</p>; // ссылка #456 ``` проходит: апостроф открывает «строку», которая до конца строки не закрывается. `.tsx` — ровно тот стек, из-за которого заводился #8. ## 4. Обработка строковых литералов не покрыта тестами Три мутации выживают со всем зелёным: убрать бэктик из `quotes`, убрать ветку `elif ch in quotes` целиком, убрать обработку экранирования. Существующие фикстуры в эту логику не бьют: в slash-режиме `#` и так не комментарий, а `"https://example.com/x#987"` не матчится, потому что `#987` прижат к букве и падает на lookbehind. Нужна фикстура вида `const s = "see // #456 here";`. ## Как чинить Не латать построчный разбор по одному случаю, а переиспользовать машину состояний из `check_no_broken_repo_paths.py` (`:274-321`): она переживает несколько комментариев на строке и переоткрытие блока. Сейчас логика дублирована в двух скриптах с разной зрелостью, и расхождение будет расти при каждой правке. ## Приёмка - [ ] Все четыре случая выше разобраны корректно - [ ] Логика разбора комментариев одна на оба хука, а не две копии - [ ] Мутации по строковым литералам роняют тесты - [ ] Ограничения, которые останутся, записаны в README явно — как это сделано у `no-broken-repo-paths` ## Смежное, отдельными пунктами **Неизвестные расширения падают в `#`-режим, и для разметки это неверно:** `.vue`, `.html`, `.svelte` — текст `<p>Задача #456</p>` флагается. README предупреждает только про `.md`. **Вердикт зависит от неверсионируемых настроек:** `git check-ignore` в `no-broken-repo-paths` учитывает `.git/info/exclude` и глобальный `core.excludesFile`. Добавление пути туда переводит хук с fail на pass, не оставляя следа в PR. Направление отказа безопасное (CI строже локали), но свойство стоит либо записать в README рядом с абзацем про `.gitignore`, либо прибить окружением на время вызова. **Асимметрия пробелов в `todo-needs-issue`:** `TODO (#456)` принимается, `TODO( #456 )` — нет. Нигде не описано.
Author
Owner

Диспозиция после #12 (main, v0.12.0) — четыре основных пункта закрыты, задача остаётся открытой ради смежных.

  • 1. Многострочный шаблонный литерал — закрыт: разбор перенесён в scripts/comment_scan.py, машина состояний пофайловая, бэктик переживает перевод строки (fixtures/issue_refs/template_literal.ts, block_multiline.ts).
  • 2. Только первый комментарий строки — закрыт: c_style_chunks идёт по всей строке и отдаёт все фрагменты (second_comment.ts, block_closes.ts).
  • 3. Апостроф в JSX — закрыт частично и сознательно: <p>don't</p>; // ссылка #456 больше не глушится на своей строке, но признание апострофа литералом остаётся эвристикой, и это записано в README и в шапке comment_scan (полный JS-лексер избыточен).
  • 4. Тесты на строковые литералы — закрыт: фикстуры string_literal.py, escaped_quote.ts, regex_*.ts; мутации по кавычкам и экранированию их роняют.
  • Логика одна на оба хука — да, comment_scan общий с no-broken-repo-paths.

Остаётся в этой задаче — режим для разметки и стилей: .vue/.html/.svelte/.css падают в #-фолбэк, где <p>Задача #456</p> и color: #336699 разбираются как комментарий. README на этот счёт предупреждает и ссылается сюда.

Два смежных пункта из тела не трогались и по-прежнему актуальны: зависимость вердикта no-broken-repo-paths от неверсионируемых .git/info/exclude и core.excludesFile, и асимметрия пробелов в todo-needs-issue (TODO (#456) принимается, TODO( #456 ) — нет).

Диспозиция после [#12](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/12) (`main`, `v0.12.0`) — четыре основных пункта закрыты, задача остаётся открытой ради смежных. - **1. Многострочный шаблонный литерал** — закрыт: разбор перенесён в `scripts/comment_scan.py`, машина состояний пофайловая, бэктик переживает перевод строки (`fixtures/issue_refs/template_literal.ts`, `block_multiline.ts`). - **2. Только первый комментарий строки** — закрыт: `c_style_chunks` идёт по всей строке и отдаёт все фрагменты (`second_comment.ts`, `block_closes.ts`). - **3. Апостроф в JSX** — закрыт частично и сознательно: `<p>don't</p>; // ссылка #456` больше не глушится на своей строке, но признание апострофа литералом остаётся эвристикой, и это записано в README и в шапке `comment_scan` (полный JS-лексер избыточен). - **4. Тесты на строковые литералы** — закрыт: фикстуры `string_literal.py`, `escaped_quote.ts`, `regex_*.ts`; мутации по кавычкам и экранированию их роняют. - **Логика одна на оба хука** — да, `comment_scan` общий с `no-broken-repo-paths`. **Остаётся в этой задаче** — режим для разметки и стилей: `.vue`/`.html`/`.svelte`/`.css` падают в `#`-фолбэк, где `<p>Задача #456</p>` и `color: #336699` разбираются как комментарий. README на этот счёт предупреждает и ссылается сюда. Два смежных пункта из тела не трогались и по-прежнему актуальны: зависимость вердикта `no-broken-repo-paths` от неверсионируемых `.git/info/exclude` и `core.excludesFile`, и асимметрия пробелов в `todo-needs-issue` (`TODO (#456)` принимается, `TODO( #456 )` — нет).
Author
Owner

Закрываю: остаток сделан в #26, выпущено тегом v0.21.0. Четыре основных пункта закрылись раньше, в #12 (диспозиция — комментарием выше).

Режим для разметки и стилей:

  • .css/* */; // там не комментарий, а часть url(//cdn…);
  • .scss/.sass/.less — плюс //, но не внутри url();
  • .html/.htm/.vue/.svelte/.xml/.svg<!-- -->, встроенные <script> и <style> отдаются своим сканерам с пересчётом номеров строк, <style lang="scss"> получает правила препроцессора.

Теперь color: #336699, #main и <p>Задача #456</p> не считаются комментариями, и такие расширения можно скоупить — пример конфига обновлён.

Выбор режима переехал в общую точку chunks_for. У no-broken-repo-paths она зовётся без #-фолбэка: первая редакция молча подключала ему shell и yaml, переворачивая осознанное решение, зафиксированное в коде комментарием. Стили и разметку хук теперь видит — это записано в README как поведенческое изменение.

Ревью поймало дыру, которую стоит назвать отдельно: незакрытый или самозакрытый <script> растягивал диапазон до конца файла, и все комментарии после него пропадали молча — ровно тот класс отказа, ради которого задача и заводилась. Теперь такой блок не сканируется, но и не глушит разбор дальше. Плюс markup_chunks перестал быть квадратичным: 126 КБ с 4000 блоков — 12.7 с → 0.19 с.

Смежные пункты из тела задачи вынесены в #27: зависимость вердикта no-broken-repo-paths от неверсионируемых .git/info/exclude и core.excludesFile, и асимметрия пробелов в todo-needs-issue.

Остаточное ограничение, записанное в README: .sql, .yaml и прочие неопознанные расширения по-прежнему идут построчным #-режимом — там код попадает под правило, и скоупить их не следует.

Закрываю: остаток сделан в [#26](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/26), выпущено тегом `v0.21.0`. Четыре основных пункта закрылись раньше, в [#12](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/12) (диспозиция — комментарием выше). **Режим для разметки и стилей:** - `.css` — `/* */`; `//` там не комментарий, а часть `url(//cdn…)`; - `.scss`/`.sass`/`.less` — плюс `//`, но не внутри `url()`; - `.html`/`.htm`/`.vue`/`.svelte`/`.xml`/`.svg` — `<!-- -->`, встроенные `<script>` и `<style>` отдаются своим сканерам с пересчётом номеров строк, `<style lang="scss">` получает правила препроцессора. Теперь `color: #336699`, `#main` и `<p>Задача #456</p>` не считаются комментариями, и такие расширения можно скоупить — пример конфига обновлён. Выбор режима переехал в общую точку `chunks_for`. У `no-broken-repo-paths` она зовётся без `#`-фолбэка: первая редакция молча подключала ему shell и yaml, переворачивая осознанное решение, зафиксированное в коде комментарием. Стили и разметку хук теперь видит — это записано в README как поведенческое изменение. Ревью поймало дыру, которую стоит назвать отдельно: незакрытый или самозакрытый `<script>` растягивал диапазон до конца файла, и **все комментарии после него пропадали молча** — ровно тот класс отказа, ради которого задача и заводилась. Теперь такой блок не сканируется, но и не глушит разбор дальше. Плюс `markup_chunks` перестал быть квадратичным: 126 КБ с 4000 блоков — 12.7 с → 0.19 с. Смежные пункты из тела задачи вынесены в [#27](https://git.homedevlab.ru/senokosov/pre-commit-hooks/issues/27): зависимость вердикта `no-broken-repo-paths` от неверсионируемых `.git/info/exclude` и `core.excludesFile`, и асимметрия пробелов в `todo-needs-issue`. Остаточное ограничение, записанное в README: `.sql`, `.yaml` и прочие неопознанные расширения по-прежнему идут построчным `#`-режимом — там код попадает под правило, и скоупить их не следует.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
senokosov/pre-commit-hooks#11
No description provided.