Статический анализ и уже выросший проект: внедрять нельзя откладывать03.02.2022 17:02

Зачем нужен статический анализ кода, кажется, никому объяснять сегодня уже не нужно. Но одно дело — поддерживать код «чистым» с первого коммита, и совсем другое — встраивать новый инструмент в проект, который за несколько лет успел разрастись и пережить несколько итераций глобального рефакторинга. Мы работаем с большим количеством плохо документированных источников данных, а статический анализ кода помогает учитывать самые разные граничные случаи.

Не все коллеги поначалу разделяли мой энтузиазм

И ещё один момент: Rusprofile почти целиком написан на PHP, языке со слабой динамической типизацией. Статический анализ кода на PHP уже несколько лет набирает популярность, сказывается здесь и движение самого языка в сторону более строгой типизации. Но мы опасались, что без предварительной подготовки кода пользы от него мало. Аннотировать типами весь код в реальных бизнес-условиях тоже нереально. Сильно медлить с внедрением в рабочий процесс тоже нельзя: чем дальше, тем сложнее что-то кардинально улучшать. Поэтому нужно было оперативно запускаться, чем-то пожертвовав.

Итак, мы выбрали Psalm

На тот момент (март 2020 года) передо мной и командой стоял выбор между Phan, PHPStan и Psalm. Все три инструмента развиваются и сейчас, и даже появились новые (например, NoVerify). Вообще обзор анализаторов довольно подробно расписан в старой, но всё ещё актуальной статье от Badoo на Хабре. Основываясь на этом обзоре и на собственном опыте мы и сделали выбор. Вариант с несколькими инструментами сразу откинули: мы не были уверены, что вообще получится в разумные сроки встроить в процесс статический анализ, поэтому решили — попробуем сначала с одним и посмотрим, что из этого выйдет.

Расширенный синтаксис аннотаций у Psalm сразу привлёк моё внимание. Есть, например, возможность описывать типы-перечисления вида "UL"|"IP" или даже "UL"|"IP"|null, есть метапрограммирование на шаблонах, которое позволяет делать почти чёрную магию, описывая выходные типы в зависимости от значения входных. Ну и прочие приятные вещи, вроде аннотации @psalm-assert-if-true, которая позволяет делать какие-то дополнительные утверждения в том случае, если функция или метод возвращает true.

Немного о расширенных функциях Psalm

 $arr
 *
 * @return array
 * @psalm-pure
 */
function array_flip(array $arr)

 $arr
 */
function usort(array &$arr, callable $callback): bool

 : int)
 */
function get(bool $getAll): int|array

Первый анализ и тысячи ошибок

Мы решили сходу попробовать, а найдётся ли что-нибудь в проекте с минимальными настройками. Сказано — сделано! Я скачал Psalm, автоматически сгенерировал конфигурацию, запустил и получил порядка 25 000 (!) сообщений об ошибках. Многовато.

Буквой E в прогресс-индикаторе обозначены файлы с ошибками. Сейчас, кстати, дефолтный прогресс-бар этого не показывает

Разумеется, просматривать всё это руками глазами было совершенно нереально, и невозможно было бы определить, есть ли там что-то действительно полезное или нет. Но, к счастью, у Psalm есть система уровней ошибок, и чувствительность меняется от 1 (самый строгий) до 8 (самый нестрогий). На нём детектируются только очевидные и почти наверняка опасные ошибки, которые могут привести к Fatal Error и прочим критическим неприятностям. Список проблем, которые всегда считаются ошибками, приведён в начале документа.

Таким образом, было довольно легко устроить быстрый тест. Ставим самый нестрогий уровень 8, запускаем снова… ошибок на два порядка меньше — около пары сотен. Это уже вполне реально просмотреть. Оставалось выяснить, есть ли там что-то, заслуживающее внимания. Кроме случаев, которые и так подсвечивает IDE (например, статический вызов метода у экземпляра класса), нашлось некоторое количество интересного. Например, вот такой код:

 $item_in_ceo) {
    if ($item_in_ceo['inn'] == $item_as_ceo['in_pred_inn']) {
        $found_key = $key;
        break;
    }
    if ($item_in_ceo['name'] == $item_as_ceo['in_pred_name'] && empty($item_as_ceo['in_pred_inn'])) {
        $found_key = $key;
        break;
    }
}

И сообщение об ошибке:

ERROR: ParadoxicalCondition: Found a paradox when evaluating $item_as_ceo['in_pred_inn'] of type non-empty-mixed and trying to reconcile it with a empty assertion (see https://psalm.dev/089)

if ($item_in_ceo['name'] == $item_as_ceo['in_pred_name'] && empty($item_as_ceo['in_pred_inn'])) {

Понадобилось какое-то время, чтобы понять, почему это действительно ошибка. Надо заметить, что это часть довольно развесистого метода. Он пытается скомпилировать из нескольких не всегда полных источников информацию для отображения пользователям. При этом важно, чтобы он увидел все имеющиеся данные, но без дублирования. Было неприятно обнаружить эту проблему (кстати, phpStorm сейчас уже тоже находит подобное), и стало понятно, что инструмент однозначно полезный. Осталось разобраться, что же делать с двадцатью тысячами ошибок.

Стратегия подготовки кодовой базы в какой-то момент стала понятна. План выглядел приблизительно так:

1. Поставить самый низкий уровень чувствительности

2. Исправить ошибки

3. Повысить чувствительность, запустить анализатор снова

4. Если ошибок так много, что исправлять их нереально, тогда записать их в baseline и остановиться, иначе перейти к шагу 2

5. Поставить самую высокую чувствительность анализатора

6. Писать новый код без новых сообщений об ошибках

План был единогласно принят. Но, как говорится, гладко было на бумаге, а по факту возникали самые разные проблемы, которые в большинстве случаев удавалось решить.

Плагины, обновления и подводные камни

Сначала пришлось починить сам Psalm…

Ещё до разбора сообщений о проблемах в коде мне пришлось столкнуться со странностями, связанными с многопоточностью работы анализатора. Она, конечно, ускоряет дело, но на тот момент это иногда приводило к вылетам. Кроме того, в зависимости от количества потоков объём ошибок немного различался. Первое мы починили пулл-реквестом — общение с мейнтейнерами Psalm«а оказалось очень приятным, в чём я впоследствии не раз убедился, ну, а вторую проблему решили запуском анализатора в однопоточном режиме. Кстати, такое поведение сохраняется до сих пор (раз, два).

Затем было множество однотипных вопросов: а какой тип у данной переменной в данном фрагменте с точки зрения анализатора? Поиски привели меня к тикету в трекере, оставленному буквально накануне каким-то другим пользователем, и… я сделал очередной пулл-реквест. С аннотацией @psalm-trace стало намного проще понимать, что же всё-таки происходит в дебрях анализатора. Кстати, время от времени там находятся ошибки в логике, и если об этом отрапортовать на гитхабе, то как правило их довольно быстро исправляют — если это действительно ошибки в анализаторе, а не в голове. Такое тоже бывает.

Проблемы с внешними библиотеками

Можно проаннотировать весь код проекта, но с внешними библиотеками такой трюк не выйдет — аннотации типов бывают недостаточно подробными, а кроме того, некоторые вещи в принципе не могут быть аннотированы без использования Psalm-специфичного синтаксиса.

На данный момент, кстати, ситуация значительно лучше, чем пару лет назад. Многие библиотеки теперь специфицируют структуру массивов в совместимом синтаксисе, и сам Psalm теперь активно поддерживается в phpStorm.

Но если библиотека ещё не обзавелась статическим анализом, тогда есть и другие способы. Psalm, кстати, понимает файл .phpstorm.meta.php, но его собственные средства гораздо богаче.

Во-первых, есть опция просто написать стабы — аналог файлов .d.ts из TypeScript. Там описывается только интерфейс, но не реализация, и в процессе анализа Psalm считает типы, указанные там, более приоритетными. Первая версия файла у нас содержала, например, следующее: