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

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

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

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

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

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

<?php
/**
 * @psalm-template TKey as array-key
 * @psalm-template TValue
 *
 * @param array<TKey, TValue> $arr
 *
 * @return array<TValue, TKey>
 * @psalm-pure
 */
function array_flip(array $arr)

<?php
/**
 * @psalm-template T
 *
 * @param T[] $arr
 * @param callable(T,T):int $callback
 * @param-out list<T> $arr
 */
function usort(array &$arr, callable $callback): bool

<?php
/**
 * @template T as true|false
 * @param T $getAll
 * @return (T is true ? list<int> : int)
 */
function get(bool $getAll): int|array

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

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

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

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

<?php

if (empty($item_as_ceo['in_pred_name']) || empty($item_as_ceo['in_pred_inn'])) {
    continue;
}
$found_key = null;
foreach ($inCEO as $key => $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 считает типы, указанные там, более приоритетными. Первая версия файла у нас содержала, например, следующее:

<?php

namespace Psr\Http\Message {
   interface ServerRequestInterface {
        // always array in our case
        public function getParsedBody(): array;
   }
}

Во-вторых, для более сложных случаев существуют плагины, их легко найти на Packagist. Например, очень популярны плагины для PHPUnit, Laravel, Symfony и отдельно Doctrine, для PSR-11 контейнеров (он как раз подошёл к PHP-DI, который мы используем) и довольно много других.

Свой плагин для анализа SQL-запросов

Но некоторые случаи довольно специфичны, и плагинов для них не нашлось. Так, у нас в проекте используется довольно много SQL-запросов, отправляемых напрямую через PDO, и они бывают довольно большие. С точки зрения Psalm метод PDOStatement::fetch и его вариации возвращают массив значений неизвестного типа с неизвестными ключами, и это порождало довольно много сообщений об ошибках. Где-то это, конечно, проще явно аннотировать, но что случится, если кто-нибудь поправит запрос, а аннотацию поправить забудет?

Проблема была решена небольшим плагином, который разбирал SQL-запросы с помощью phpmyadmin/sql-parser и сохранённой в xml структуры базы данных, и выводил типы. Это помогло регулярно находить мелкие ошибки. Например, при запросе вида SELECT * FROM ... Psalm уже знает, какие там есть поля, а каких нет, и случайная опечатка будет сразу подчёркнута красненьким в IDE. О создании этого плагина постараюсь рассказать подробнее в формате отдельной статьи.

Phar vs. Composer package

Есть два варианта установки: пакет psalm/phar, который содержит внутри все зависимости, и полноценный пакет vimeo/psalm, который будет разделять зависимости с приложением. Второй вариант даёт возможность нормально писать плагины, но при этом есть риск конфликта зависимостей: Psalm требует довольно много всего, и версии могут быть несовместимы. Один из вариантов решения этой проблемы — плагин composer-bin, который создаёт несколько разных «корзин» (bin; по сути — отдельная директория со своим composer.json и т. п.). При этом, он собирает все исполняемые файлы в общей директории bin. Таких «корзин» кроме глобальной у нас сейчас в проекте две: одна для самого Psalm, другая для phpmyadmin/sql-parser (они в какой-то момент были несовместимы и между собой, и с основным проектом).

Было ещё множество разных мелких проблем, на которые, как правило, есть ответы в issue-трекере Psalm. Но есть ещё два момента, которые хотелось бы осветить.

Интеграция с IDE

Писать код, а потом отдельно проверять его довольно неудобно. К счастью, спустя несколько месяцев появился Psalm-плагин от JetBrains. Ради него я даже какое-то время использовал preview-версию среды. Он неидеален, но подсветка ошибок от статического анализатора прямо в IDE — это чрезвычайно удобно.

На Linux и macOS с запуском Psalm проблем нет, а вот на Windows приходится использовать WSL, что несколько замедляет проверку: чтение из WSL файлов с хост-машины довольно медленное. Эта проблема решается полным переносом (или непрерывной синхронизацией) дерева внутрь файловой системы WSL2, где анализ работает так же быстро, как и на Linux-машине. Но делать это необязательно: Psalm эффективно кеширует результаты сканирования, и перепроверка изменённых файлов происходит практически сразу (примерно в течение 10–15 секунд).

Есть ещё один вариант интеграции: в Psalm встроен Language server, и можно использовать любой поддерживающий LSP плагин к IDE.

Как правильно обновлять Psalm

Psalm активно развивается: появляются новые проверки, уточняются старые, и если просто сделать composer update, то появится какое-то количество свежих сообщений об ошибках. Это, кстати, довольно часто сообщения об избыточных аннотациях, потому что от версии к версии Psalm может выводить типы во всё более сложных случаях. Поэтому его обновление представляет собой чуть более сложную задачу. При этом, для нас важно сохранить ветку master «чистой» (т.е. чтобы все ошибки в ней были внесены в baseline и не отображались). Оно происходит следующим образом:

1. Обновить Psalm

2. Обновить baseline (вычистить оттуда ошибки, которых больше нет, внести новые)

3. Призвать всех влить в свои рабочие ветки обновлённый master и перепроверить их новой версией анализатора

Самая неприятная часть в этой процедуре — синхронизировать все действия с циклами разработки/релизов, чтобы минимально нарушать сложившийся процесс. То есть, например, не возвращать задачу из тестирования для обновления там версии анализатора).

Живи долго и процветай с Psalm

Есть и обратная сторона — в некоторых случаях коллегам приходится, как мы это называем, «бороться с псалмом». Поначалу это могло занимать довольно много времени и вызывало раздражение, но, как правило, эта борьба идёт на пользу простоте и читаемости кода. Эмпирически была обнаружена любопытная закономерность: если код невозможно привести в порядок иначе как с помощью каких-то костылей (типа @psalm-suppress или аннотаций типов переменных по месту использования), то в нём почти наверняка есть какие-то архитектурные проблемы.

В общем, как говорит один из наших разработчиков, «иногда немного бесит, но в целом — очень классная и полезная штука».

* * *

Меня радует, что практика статического анализа кода постепенно становится нормой не только в статически, но и в динамически типизированных языках вроде PHP. Лично для меня это уже практически инструмент по умолчанию, как, например, Composer.

Вы уже используете статический анализ в своих проектах? С какими сложностями приходилось сталкиваться?