Свести чтение файла хуками в один хелпер; молчаливый skip недекодируемого файла #23

Closed
opened 2026-08-27 12:31:37 +07:00 by claude-secretary · 1 comment

Хвост #17 плюс две находки ревью #21 и #22.

1. Шестая копия одной и той же строки

open(path, encoding="utf-8-sig") + except (OSError, UnicodeDecodeError) живёт в check_no_broken_repo_paths.py, check_no_private_imports.py, check_no_private_method_calls.py, check_no_issue_refs_in_comments.py, check_no_session_marks_in_code.py, check_no_broad_except.py, check_no_plan_stage_refs.py. Каждый из них чинился отдельным PR по мере того, как дыру находили — v0.12.0, v0.15.0, v0.17.0, v0.18.0. Седьмой хук напишут с той же ошибкой, если не свести чтение в один хелпер рядом с comment_scan.

Заодно расплетается расхождение стиля: у no-broad-except чтение и ast.parse стоят в одном try с except SyntaxError первым, у остальных — в двух.

2. Недекодируемый файл пропускается молча

Все хуки на нечитаемом файле возвращают [] и exit 0 — ни строки в stderr. Файл был в скоупе files: потребителя, проверен не был, и никто об этом не узнает. Политике fail-loud, которую репозиторий декларирует у себя же (no-broad-except, шапка check_file_length.sh), это противоречит.

Для no-broad-except особенно: у него types: [python], значит UnicodeDecodeError означает буквально «python-файл, который мы не смогли прочитать». Проверено на живых файлах: BOM в середине, исходник в cp1251 с PEP-263 cookie, utf-16 — все три содержат except Exception и все три дают тихий ноль.

Предложение: печатать строку в stderr («файл пропущен: не читается в utf-8») без изменения exit-кода. Это не ломает прогон, но перестаёт быть невидимым.

3. migrations/versions/… не исключается check-file-length

alembic init migrations — не менее типовая раскладка, чем alembic/, а аргумент #17 был именно про типовые раскладки. Сейчас такие миграции проверяются на длину. Решить, ловить ли по имени versions/ с любым родителем (риск переширения) или перечислить известные каталоги.

Приёмка

  • Чтение файла — один хелпер, все хуки зовут его
  • Недекодируемый файл оставляет след в stderr; кейс в smoke
  • migrations/versions/… решён явно (исключён или сознательно нет — с записью в README)
  • Мутация «убрать encoding в хелпере» роняет smoke каждого хука, а не одного
Хвост [#17](https://git.homedevlab.ru/senokosov/pre-commit-hooks/issues/17) плюс две находки ревью [#21](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/21) и [#22](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/22). ## 1. Шестая копия одной и той же строки `open(path, encoding="utf-8-sig")` + `except (OSError, UnicodeDecodeError)` живёт в `check_no_broken_repo_paths.py`, `check_no_private_imports.py`, `check_no_private_method_calls.py`, `check_no_issue_refs_in_comments.py`, `check_no_session_marks_in_code.py`, `check_no_broad_except.py`, `check_no_plan_stage_refs.py`. Каждый из них чинился отдельным PR по мере того, как дыру находили — `v0.12.0`, `v0.15.0`, `v0.17.0`, `v0.18.0`. Седьмой хук напишут с той же ошибкой, если не свести чтение в один хелпер рядом с `comment_scan`. Заодно расплетается расхождение стиля: у `no-broad-except` чтение и `ast.parse` стоят в одном `try` с `except SyntaxError` первым, у остальных — в двух. ## 2. Недекодируемый файл пропускается молча Все хуки на нечитаемом файле возвращают `[]` и exit 0 — ни строки в stderr. Файл был в скоупе `files:` потребителя, проверен не был, и никто об этом не узнает. Политике fail-loud, которую репозиторий декларирует у себя же (`no-broad-except`, шапка `check_file_length.sh`), это противоречит. Для `no-broad-except` особенно: у него `types: [python]`, значит `UnicodeDecodeError` означает буквально «python-файл, который мы не смогли прочитать». Проверено на живых файлах: BOM в середине, исходник в cp1251 с PEP-263 cookie, utf-16 — все три содержат `except Exception` и все три дают тихий ноль. Предложение: печатать строку в stderr («файл пропущен: не читается в utf-8») без изменения exit-кода. Это не ломает прогон, но перестаёт быть невидимым. ## 3. `migrations/versions/…` не исключается `check-file-length` `alembic init migrations` — не менее типовая раскладка, чем `alembic/`, а аргумент [#17](https://git.homedevlab.ru/senokosov/pre-commit-hooks/issues/17) был именно про типовые раскладки. Сейчас такие миграции проверяются на длину. Решить, ловить ли по имени `versions/` с любым родителем (риск переширения) или перечислить известные каталоги. ## Приёмка - [ ] Чтение файла — один хелпер, все хуки зовут его - [ ] Недекодируемый файл оставляет след в stderr; кейс в smoke - [ ] `migrations/versions/…` решён явно (исключён или сознательно нет — с записью в README) - [ ] Мутация «убрать encoding в хелпере» роняет smoke каждого хука, а не одного
Author
Owner

Закрываю: сделано в #24, выпущено тегом v0.19.0.

По пунктам приёмки:

  • Чтение — один хелпер, все хуки зовут его. scripts/read_source.py; переведены все восемь читателей. Восьмой (check_model_comments) обнаружился на ревью — он читал голым read_text и падал трейсбеком на не-UTF-8, а на файле с BOM давал ложный FAIL с SyntaxError.
  • Недекодируемый файл оставляет след в stderr, кейсы есть у всех восьми — с пином id хука, иначе неверная метка в сообщении проходила незамеченной.
  • migrations/versions/ исключён; Django-раскладка <app>/migrations/0001_x.py и похожее имя schema_migrations/versions/ намеренно не исключены — обе границы закреплены негативными кейсами.
  • Мутация «убрать encoding в хелпере» роняет smoke каждого хука — четыре BOM-кейса разом вместо одного.

Важная оговорка, которую стоит знать: pre-commit печатает вывод хука только когда тот упал или у него verbose: true, поэтому в зелёном прогоне сообщения о пропуске не видно. Проверено на pre-commit 4.6.1. В README это записано, а инстансу с широким скоупом в примере конфига проставлен verbose: true. Тем же ограничением давно страдают WARN-строки check-file-length.

Из побочного: в run_smoke.sh появился command_not_found_handle. Дважды за эту работу кейс оказывался вызван выше объявления своего хелпера — bash отдавал 127 и шёл дальше, кейсы молча не исполнялись, а сюита оставалась зелёной. Теперь любая неизвестная команда валит прогон.

Закрываю: сделано в [#24](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/24), выпущено тегом `v0.19.0`. По пунктам приёмки: - **Чтение — один хелпер, все хуки зовут его.** `scripts/read_source.py`; переведены все восемь читателей. Восьмой (`check_model_comments`) обнаружился на ревью — он читал голым `read_text` и падал трейсбеком на не-UTF-8, а на файле с BOM давал ложный FAIL с `SyntaxError`. - **Недекодируемый файл оставляет след в stderr**, кейсы есть у всех восьми — с пином id хука, иначе неверная метка в сообщении проходила незамеченной. - **`migrations/versions/`** исключён; Django-раскладка `<app>/migrations/0001_x.py` и похожее имя `schema_migrations/versions/` намеренно не исключены — обе границы закреплены негативными кейсами. - **Мутация «убрать encoding в хелпере» роняет smoke каждого хука** — четыре BOM-кейса разом вместо одного. Важная оговорка, которую стоит знать: pre-commit печатает вывод хука только когда тот упал или у него `verbose: true`, поэтому в зелёном прогоне сообщения о пропуске не видно. Проверено на `pre-commit 4.6.1`. В README это записано, а инстансу с широким скоупом в примере конфига проставлен `verbose: true`. Тем же ограничением давно страдают `WARN`-строки `check-file-length`. Из побочного: в `run_smoke.sh` появился `command_not_found_handle`. Дважды за эту работу кейс оказывался вызван выше объявления своего хелпера — bash отдавал 127 и шёл дальше, кейсы молча не исполнялись, а сюита оставалась зелёной. Теперь любая неизвестная команда валит прогон.
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#23
No description provided.