Статические проверки в PHPStorm: теория и практика.

[pozitronik]

Disclaimer: иногда приходится объяснять разработчикам предмет статьи, заявленный в заголовке. Решил сделать это в письменном виде, чтобы не тратить время на повторение.

Описание не претендует на абсолютную достоверность и может отражать какие-то мои личные предпочтения.

Написанное в общем виде пригодно для использования в среде разработки PHPStorm, и в той или иной мере может быть использовано в других средах разработки JetBrains при разработке web-сервисов на PHP+JS+CSS+HTML+SQL. Многие частности, скорее всего, будут касаться фреймворка Yii2 и его компонентов.

Что такое статический анализ?

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

Зачем он нужен?

Кроме очевидного поиска проблем, инструменты проверки могут предлагать автоматизированные способы их исправления, и улучшать само понимание работы с кодом. По личному многолетнему опыту: использование корректно настроенных и достаточно строгих инспекций полностью избавляет от необходимости написания тестов. По сути - это автоматизированное code review.

Какие проблемы могут возникнуть при использовании статического анализа у неопытного разработчика?

- Информационная перегрузка.

- Депрессия, вызванная большим количеством найденных проблем в, казалось бы, неплохом коде.

- Непонимание причин этих проблем с ещё большим непониманием того, как их исправлять.

- Желание отметить часть "непонятных" срабатываний как ложноположительных.

Первые две проблемы исчезают экспоненциально. Часто исправление одной ошибки приводит к исчезновению целой цепочки проблем. Две последние проблемы уходят по мере набора опыта. В абсолютном большинстве случаев корректно настроенный инспектор не ошибается, и если проблема неясна - это повод начать разбираться, начиная с документации по сработавшей проверке.



---

В PHPStorm уже встроены достаточно неплохие инструменты анализа на самые разные случаи жизни, плюс имеется возможность использовать плагины и сторонние инструменты статанализа. Я буду рассматривать только расширение возможностей через плагины: их достаточно в 100% случаев, они более просты и гибки в настройке. Нам понадобятся следующие плагины:

- Grazie. Это расширенная проверка грамматики и правописания (более полная, нежели встроенная в IDE). Это действительно полезно. Как пример: если написать название переменной с ошибкой, PHP, в большинстве случаев, создаст новую переменную; проверка подсветит такой момент.

- PHP InheritDoc helper. Расширение возможностей автодокументирования IDE. Позволяет создавать блоки PHPDoc, описывать переменные и свойства классов, соблюдая стандарты документирования. Сам по себе плагин, конечно, не относится к проверяющим, но корректная документация позволяет полноценнее отрабатывать другим проверкам.

- Php Inspections (EA Extended). Один из самых мощных и полезных плагинов, содержащий невероятное количество эвристик, вплоть до анализа архитектурных просчётов и проблем производительности. Каждая инспекция подробно документирована, и только одно чтение этой документации способно принести немалую пользу.

- PhpClean. Добавляет ещё одну небольшую пачку проверок.

- Yii2 Inspections. Содержит некоторые проверки и дополнения, полезные при работе с фреймворком Yii2.

- Yii2 Support. Аналогично предыдущему, но набор возможностей отличается, например появится возможность быстрого перехода к View-шаблонам по их имени в render(), проверка актуальности и наличия передаваемых в шаблон атрибутов и прочее такее.

Все плагины ставятся прямо в IDE, в разделе настроек Plugins/Marketplace:



Вопросы с автоустановкой возникли только к Yii2 Support: его пришлось скачать напрямую и установить из архива.

Все настройки, связанные с проверками, находятся в разделе Editor/Inspections, но иногда плагины добавляют собственные разделы настроек.





Я не могу дать конкретных настроек, подходящих конкретно в ваш проект и под принятые у вас стандарты, с этим вы должны определиться сами. Сам я ратую за максимально строгий подход, начиная от включения всех strict-возможностей языка, но, при этом, часто плюю на PSR-соглашения. Так сложилось.

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

1) Определите проверяемую область: в Project=>Edit scopes...



настройте файлы и каталоги, которые должны проверяться. Отсюда нужно исключить, например, каталоги расширений Composer, каталог фреймворка, каталоги тестового кода, рантаймы, и прочее подобное. Здесь работает логика: мой код (код проекта) должен проверяться, а на чистоту остального кода я повлиять не могу, и ресурсы на это тратить не нужно.

2) В Editor/Inspections включите все доступные инспекции для PHP (остальные компоненты потом можно проверить по той же схеме), и через Code=>Inspect code запустите проверку на указанной области.

3) Идите пить чай:



Первая полная проверка достаточно объёмного проекта может занять дикое количество времени



и, скорее всего, количество сообщений будет исчисляться десятками тысяч:



Это нормально. Во-первых, вы наверняка что-то упустили при настройке области проверки. Зацепили какой-то каталог с временными файлами, или нашлось расширение добавленное копипастом в неположенное место, или ещё что-то подобное. Откорректируйте область проверки, это отсеет часть сообщений.

Во-вторых, кроме PHP-проверок по умолчанию включены проверки других компонентов. Это тоже полезно, но на первое время может быть отключено. Конечно, желательно, чтобы область проверки включала в себя только файлы с PHP-кодом, но собственные ассеты и view-шаблоны, включающие HTML/JS/CSS проверять тоже нужно.

Ну и в третьих, некоторые проверки взаимоисключающие: их корректность зависит от стандартов, которых придерживаетесь вы у себя. Например, одна проверка сообщает о возможности замены условия if ... else тернарным оператором ?: (в пользу компактности), а другая будет сообщать о том, что тернарный оператор ухудшает читаемость.

В таких случаях вы принимаете решение по тому, какой вариант хотите соблюдать, и отключаете одну из инспекций. Или - если считаете, что оба варианта допустимы, а вам плевать на унификацию, - отключаете обе.

Так, перебирая одну инспекцию за другой, отключая ненужные и настраивая остальные под ваши предпочтения, вы избавитесь от большей части ложно-положительных срабатываний. Оставшиеся актуальными проверки можно сохранить в отдельные конфигурации, чтобы IDE прямо на лету показывала вам наличие проблем в открытых файлах, соответственно используемому конфигу проверки. Также можно настроить несколько отдельных профилей, например один с наиболее частыми и нужными проверками (null-pointers, опечатки, присваивания в сравнении), которые могут выполняться на лету при каждом коммите, и один - с полноценным прогоном всех правил, на случай если опять захочется выпить чаю.

Вот два файла конфигурации проверок, которыми я пользуюсь сам:

«Быстрый»

«Полный»

Не факт, что они подойдут вам на 100%, но как отправная точка - сгодятся.

Перейду к практике, и покажу, как статический анализ помогает выявить проблемы даже в незнакомом коде. Для этого я рассмотрю абсолютно реальный кейс: модель, которую я рефакторил, зная только самые общие принципы того, что ей положено делать. Весь код я выложить не могу, так как это часть рабочего проекта, но показать отдельные проблемные участки, думаю, можно.

Вызов статического метода, как динамического



Метод findOne() в ActiveRecord - статический, пример правильного вызова виден несколькими строчками выше. Здесь же имя класса подставляется из какой-то конфигурации, затем по имени класса создаётся экземпляр объекта, и из него дёргается метод.

Благодаря «мягкости» языка это не приведёт к серьёзным последствиям, но в будущем может себя проявить, к тому же на создание нового экземпляра класса тратятся ресурсы.

Решение - вызывать просто $objModel::findOne(...)

Обращение к нулевому указателю



Хотя в PHPDoc-блоке функции beginTransaction() возвращаемый тип описан, как Transaction, инспектор проводит рекурсивный анализ всей цепочки вызовов, и предполагает, что в некоторых случаях функция может вернуть null (вероятно это будет значить сбой в db-компоненте приложения). В таком случае код непременно упадёт, и всё, что увидит пользователь - HTTP Error 500 без какого-либо намёка на причину ошибки.

Здесь есть два решения. Первое: добавить проверку возвращаемого типа с каким-то обработчиком, например, так:

if (null === $transaction = Yii::$app->db->beginTransaction()) throw new RuntimeException('Не удалось инициализировать транзакцию.');

Второе: если мы железно уверены, что метод всегда будет отдавать транзакцию, и эвристика инспектора ошиблась (такое, например, может быть из-за неверной или недостаточной типизации где-то в глубине стека вызовов), то мы можем инлайново пометить переменную нужным типом:

/** @var Transaction $transaction */
$transaction = Yii::$app->db->beginTransaction();

Это делается из всплывающего меню при выделении переменной.

Доступ к свойству через магический метод



Анализируя класс Transaction, инспектор не обнаружил в нём ни публичной (или хотя бы защищённой) переменной $errors, ни атрибута, получаемого геттером и описанного как @attribute $errors в документации класса. В этом случае делается предположение, что свойство может быть получено через «волшебный» метод __get() в классе или его родителях, что следует проверить и документировать.

И такая проверка выявляет: у класса нет никакого атрибута, а автор строки явно ошибся и хотел вызывать $this->approve->errors

Присвоение в сравнении



Опять же - PHP позволяет делать так, но это не значит, что так делать нужно! Присвоение внутри сравнения часто приводит к возникновению труднообнаружимых ошибок, поскольку условный оператор будет реагировать как на отрицательное условие не только на (bool)false, но и на (int)0, (string)"", [] и null. Решение - посмотреть, что вернёт getForApprove() при неудаче, и переписать условие как.

while(false === $nextOperation = $this->getForApprove()){
$nextOperation->status = ApprovedRequestSteps::STATUS_REJECT;
$nextOperation->save();
}

Обращение к переменной, как к массиву



Проверка выявила сразу две потенциальные ошибки. Первая - более серьёзная: в PHPDoc-блоке параметров $options заявлен, как логическая переменная, и по умолчанию он устанавливается в false. Однако тут же к переменной обращаются, как к массиву; это вызовет немедленное падение исполнения.

Вторая ошибка - в случае, если $options всё-таки передаётся, как массив, но в нём нет ключа 'model' - код тоже упадёт.

Если вы внимательно посмотрели на скриншот, то увидели, что PHPDoc в принципе описан некорректно, как будто разработчик пытался сгенерировать документацию, но не до конца понимал её назначение и пользу.

Поэтому здесь нужны следующие исправления:

Для php 5.x - правильная типизация PHPDoc и корректное задание значений по умолчанию:

/**
* @param int|null $id
* @param array $options array attributes
* @return mixed
*/
static function model($id = null, $options = [])

А для php 7.x - ещё и типизация параметров:

static function model(?int $id = null, array $options = [])

Пожалуй, закончу с примерами, хотя их никогда не бывает много, как и моего свободного времени.

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

Всем добра.

Материал актуален на 20.04.2020, и будет пополняться.