Зачем нужен статический анализ кода, кажется, никому объяснять сегодня уже не нужно. Но одно дело — поддерживать код «чистым» с первого коммита, и совсем другое — встраивать новый инструмент в проект, который за несколько лет успел разрастись и пережить несколько итераций глобального рефакторинга. Мы работаем с большим количеством плохо документированных источников данных, а статический анализ кода помогает учитывать самые разные граничные случаи.
И ещё один момент: Rusprofile почти целиком написан на PHP, языке со слабой динамической типизацией. Статический анализ кода на PHP уже несколько лет набирает популярность, сказывается здесь и движение самого языка в сторону более строгой типизации. Но мы опасались, что без предварительной подготовки кода пользы от него мало. Аннотировать типами весь код в реальных бизнес-условиях тоже нереально. Сильно медлить с внедрением в рабочий процесс тоже нельзя: чем дальше, тем сложнее что-то кардинально улучшать. Поэтому нужно было оперативно запускаться, чем-то пожертвовав.
Итак, мы выбрали Psalm
На тот момент (март 2020 года) передо мной и командой стоял выбор между Phan, PHPStan и Psalm. Все три инструмента развиваются и сейчас, и даже появились новые (например, NoVerify). Вообще обзор анализаторов довольно подробно расписан в старой, но всё ещё актуальной статье от Badoo на Хабре. Основываясь на этом обзоре и на собственном опыте мы и сделали выбор. Вариант с несколькими инструментами сразу откинули: мы не были уверены, что вообще получится в разумные сроки встроить в процесс статический анализ, поэтому решили — попробуем сначала с одним и посмотрим, что из этого выйдет.
Расширенный синтаксис аннотаций у Psalm сразу привлёк моё внимание. Есть, например, возможность описывать типы-перечисления вида "UL"|"IP"
или даже "UL"|"IP"|null
, есть метапрограммирование на шаблонах, которое позволяет делать почти чёрную магию, описывая выходные типы в зависимости от значения входных. Ну и прочие приятные вещи, вроде аннотации @psalm-assert-if-true
, которая позволяет делать какие-то дополнительные утверждения в том случае, если функция или метод возвращает true
.
Немного о расширенных функциях Psalm
Примеры использования шаблонов есть во встроенных в Psalm аннотациях, например, к функциям работы с массивами:
<?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)
Или даже с callable
:
<?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 (можно навести указатель мыши на переменную или вызов функции и увидеть выведенный тип).
А ещё у Psalm есть любопытная возможность отслеживать безопасность приложения: он пытается найти, в каких случаях неэкранированные данные от пользователя могут попасть туда, куда не следует. Главный разработчик Psalm утверждает, что с помощью этой функции он нашёл 35 потенциальных XSS в коде Vimeo.
Первый анализ и тысячи ошибок
Мы решили сходу попробовать, а найдётся ли что-нибудь в проекте с минимальными настройками. Сказано — сделано! Я скачал 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 сейчас уже тоже находит подобное), и стало понятно, что инструмент однозначно полезный. Осталось разобраться, что же делать с двадцатью тысячами ошибок.
Стратегия подготовки кодовой базы в какой-то момент стала понятна. План выглядел приблизительно так:
Поставить самый низкий уровень чувствительности
Исправить ошибки
Повысить чувствительность, запустить анализатор снова
Если ошибок так много, что исправлять их нереально, тогда записать их в baseline и остановиться, иначе перейти к шагу 2
Поставить самую высокую чувствительность анализатора
Писать новый код без новых сообщений об ошибках
План был единогласно принят. Но, как говорится, гладко было на бумаге, а по факту возникали самые разные проблемы, которые в большинстве случаев удавалось решить.
Плагины, обновления и подводные камни
Сначала пришлось починить сам 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 и не отображались). Оно происходит следующим образом:
Обновить Psalm
Обновить baseline (вычистить оттуда ошибки, которых больше нет, внести новые)
Призвать всех влить в свои рабочие ветки обновлённый master и перепроверить их новой версией анализатора
Самая неприятная часть в этой процедуре — синхронизировать все действия с циклами разработки/релизов, чтобы минимально нарушать сложившийся процесс. То есть, например, не возвращать задачу из тестирования для обновления там версии анализатора).
Живи долго и процветай с Psalm
Есть и обратная сторона — в некоторых случаях коллегам приходится, как мы это называем, «бороться с псалмом». Поначалу это могло занимать довольно много времени и вызывало раздражение, но, как правило, эта борьба идёт на пользу простоте и читаемости кода. Эмпирически была обнаружена любопытная закономерность: если код невозможно привести в порядок иначе как с помощью каких-то костылей (типа @psalm-suppress или аннотаций типов переменных по месту использования), то в нём почти наверняка есть какие-то архитектурные проблемы.
В общем, как говорит один из наших разработчиков, «иногда немного бесит, но в целом — очень классная и полезная штука».
* * *
Меня радует, что практика статического анализа кода постепенно становится нормой не только в статически, но и в динамически типизированных языках вроде PHP. Лично для меня это уже практически инструмент по умолчанию, как, например, Composer.
Вы уже используете статический анализ в своих проектах? С какими сложностями приходилось сталкиваться?
Комментарии (9)
vsh797
04.02.2022 09:31+2Сам подключал psalm на прошлом проекте. Было интересно попробовать. Но в итоге впечатление сложилось двоякое. С одной стороны круто иметь статическую типизацию на php. С другой сама экосистема к этому была не слишком приспособлена. Те же symfony / doctrine тогда не имели нужных аннотаций. Да и вообще куча кода была написана с ориентацией на динамическую природу php.
Я причем эмпирически выставил строгость проверки что-то около 3. И в основном приходилось бороться с "вопросиками". Т.е. избавляться от nullable. И я до сих пор не до конца понимаю, как это нужно правильно делать, например, для сущностей доктрины. Т.е. все не nullable свойства с точки зрения типов нужно инициализировать в конструкторе. Но это сильно выбивается из общей практики. То же самое с автоинкрементным id. При создании новой сущности он должен быть null. Но потом этот вопросик сильно мешается в коде. Приходится добавлять во все сущности getInitializedId() :int с ассертом внутри. И так много в чем. Когда с точки зрения контекста использования ты знаешь лучше анализатора, что за реальный тип у этого значения.
m03r Автор
04.02.2022 16:44+1Я на одном небольшом проекте для себя успешно подружил Psalm с сущностями доктрины. Исходил я примерно из таких принципов:
Если какое-то поле у сущности не может быть
null
, то оно должно быть проинициализировано в конструкторе. То есть логически, если у насUser
не может быть без почты и пароля, тогда конструктор будет их принимать.Я подумал, что для
$id
использовал@psalm-suppress PropertyNotSetInConstructor
, но заглянул в код и, оказывается, нет.$id
честно прописан?int
, и даже это где-то используется для того, чтобы отличать уже записанные в БД сущности от только что сгенерированных. А там, где я точно уверен, что он неnull
, писал(int)$id
.Вероятно, это можно всё как-то сделать на магии шаблонов, чтобы
$em->find()
возвращал специально шаблонизированнуюEntity
, у которой$id
не может бытьnull
, но быстро набросать у меня не получилось.Ну и в любом случае, это немного костыльно решается плагином.
vsh797
04.02.2022 19:39Может и можно через дженерики возвращать сущность из entityManager с не nullable id. Но при инжекте в контроллер с помощью ArgumentResolver уже придется прописывать дженерик вручную. Да и потом везде его по типам пропихивать. Так что трейт с getInitializedId с ассертом внутри кажется меньшей проблемой.
А ведь кроме id у сущности может быть куча автозаполняемых полей. CreatedAt updatedAt, createdBy… Часто они в doctrine event subscriber автоматом заполняются. Но тогда их тоже нужно делать nullable.
Я на одном небольшом проекте для себя успешно подружил Psalm с сущностями доктрины.
На моем текущем проекте у сущности может быть несколько десятков полей. Ну, пусть даже 10 из них не nullable. Как-то сомнительно их все через конструктор инициализировать. Да и формы с таким по дефолту не очень дружат. Не знаю насчет апи платформы и прочих библиотек..
m03r Автор
06.02.2022 01:03На моем текущем проекте у сущности может быть несколько десятков полей. Ну, пусть даже 10 из них не nullable. Как-то сомнительно их все через конструктор инициализировать.
Вот тут как та ситуация, когда код, который никак не хочет укладыватся в правила статического анализа, имеет некоторые архитектурные проблемы. С точки зрения идеальной архитектуры объект с не-nullable свойствами не может существовать, если у них нет значений. Тут просится что-то промежуточное типа builder'а с валидатором внутри. Но это в идеальном мире, конечно... У нас, кстати, часть таких проблем с пользовательским вводом решена с помощью spatie/data-transfer-object, но это довольно специфичное решение.
Но с точки зрения реального, а не идеального кода всё совсем не так :))) Но осознавать неидеальность, конечно, тоже небесползено.
Glomberg
04.02.2022 10:14+2Мы на своём проекте тоже внедрили psalm около года назад. Как боролись с большим количеством ошибок - заигнорили в конфиге псалма все файлы проекта и по одному снимали из игнора, правили, вынимали из игнора следующий файл и т.д. Так постепенно вычистили все замечания псалма. На сегодня псалм выполняется на travis CI, автоматически проверяя запушеный код. Сейчас выставлен level 4, нам хватает. Какой уровень выставлен у вас?
m03r Автор
04.02.2022 16:47Между прочим, весьма успешная альтернативная стратегия!
У нас level 1, писать новый "чистый" код, в общем-то, все довольно быстро привыкли.
mn3m0n1c_3n3m1
Абсолютно согласен с автором, по-поводу важности статического анализа. Для себя могу выделить две сложности:
Наличие false positives;
Огромное количество сообщений об ошибках;
Отсутствие состояние прогресса (не понятно когда завершится проверка)
В качестве решения для JavaScript использую:
ESLint настроенный исправлять, все что он находит;
????Putout, который сообщает только о том, что может исправить сам;
Конечно, это не спасёт от ошибки вроде ParadoxicalCondition, описанной в статье, но с этим не плохо справляются тесты + coverage.
m03r Автор
Конкретно с Psalm у нас довольно мало false-positives, а огромное количество ошибок прячется в baseline, который со временем постепенно худеет. Ведь при модификации кода практически невозможна ситуация, когда новое выражение точно попадёт в baseline. Это связано с форматом: Psalm фиксирует не только количество ошибок определённого типа в файле, но и конкретное выражение, которое их вызывает.
А вот с JS у нас пока ещё не всё хорошо, ESLint в принципе настроен, но не форсируется. Зато новый код всё больше пишется на Typescript, который сам очень помогает с контролем типов.