Pull to refresh
0
Badoo
Big Dating

Статический анализ PHP-кода на примере PHPStan, Phan и Psalm

Reading time20 min
Views63K


Компания Badoo существует уже более 12 лет. У нас очень много PHP-кода (миллионы строк) и наверняка даже сохранились строки, написанные 12 лет назад. У нас есть код, написанный ещё во времена PHP 4 и PHP 5. Мы выкладываем код два раза в день, и каждая выкладка содержит примерно 10—20 задач. Помимо этого, программисты могут выкладывать срочные патчи — небольшие изменения. И в день таких патчей у нас набирается пара десятков. В общем, наш код меняется очень активно.

Мы постоянно ищем возможности как для ускорения разработки, так и для повышения качества кода. И вот однажды мы решили внедрить статический анализ кода. Что из этого получилось, читайте под катом.

Strict types: почему мы пока его не используем


Однажды у нас в корпоративном PHP-чатике развернулась дискуссия. Один из новых сотрудников рассказал, как на предыдущем месте работы они внедрили обязательный strict_types + скалярные type hints для всего кода — и это значительно снизило количество багов на продакшене.

Большинство старожилов чата было против такого нововведения. Основной причиной было то, что у PHP нет компилятора, который бы на этапе компиляции проверял соответствие всех типов в коде, и если у вас не 100%-ное покрытие кода тестами, то всегда есть риск, что ошибки всплывут на продакшене, чего мы не хотим допускать.

Конечно же, strict_types найдёт определённый процент багов, вызванных несоответствием типов и тем, как PHP «молча» конвертирует типы. Но многие опытные PHP-программисты уже знают, как работает система типов в PHP, по каким правилам происходит конвертация типов, и в большинстве случае пишут корректный, работающий код.

Но сама идея иметь некую систему, показывающую, где в коде есть несовпадение типов, нам понравилась. Мы задумались об альтернативах strict_types.

Сначала мы даже хотели пропатчить PHP. Нам хотелось, чтобы если функция принимает какой-то скалярный тип (скажем, int), а на вход пришёл другой скалярный тип (например, float), то не кидался бы TypeError (который по сути своей исключение), а происходила бы конвертация типа, а также логирование этого события в error.log. Это позволило бы нам найти все места, где наши предположения о типах неверные. Но такой патч нам показался делом рискованным, да ещё могли возникнуть проблемы с внешними зависимостями, не готовыми к такому поведению.

Мы отказались от идеи пропатчить PHP, но по времени всё это совпало с первыми релизами статического анализатора Phan, первые коммиты в котором были сделаны самим Расмусом Лердорфом. Так мы пришли к идее попробовать статические анализаторы кода.

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


Статические анализаторы кода просто читают код и пытаются найти в нём ошибки. Они могут выполнять как очень простые и очевидные проверки (например, на существование классов, методов и функций, так и более хитрые (например, искать несоответствие типов, race conditions или уязвимости в коде). Ключевым является то, что анализаторы не выполняют код — они анализируют текст программы и проверяют её на типичные (и не очень) ошибки.

Наиболее очевидным примером статического анализатора PHP-кода являются инспекции в PHPStorm: когда вы пишете код, он подсвечивает неправильные вызовы функций, методов, несоответствие типов параметров и т. п. При этом PHPStorm не запускает ваш PHP-код — он его только анализирует.

Замечу, что в данной статье речь идёт именно об анализаторах, которые ищут ошибки в коде. Есть и другой класс анализаторов — они проверяют стиль написания кода, цикломатическую сложность, размеры методов, длину строк и т. п. Такие анализаторы мы здесь не рассматриваем.

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

Существующие анализаторы PHP-кода


Существует три популярных анализатора PHP-кода:

  1. PHPStan.
  2. Psalm.
  3. Phan.

И есть ещё Exakat, который мы не пробовали.

Со стороны пользователя все три анализатора одинаковы: вы устанавливаете их (скорее всего, через Composer), конфигурируете, после чего можно запустить анализ всего проекта или группы файлов. Как правило, анализатор умеет красиво выводить результаты в консоль. Также можно выводить результаты в формате JSON и использовать их в CI.

Все три проекта сейчас активно развиваются. Их maintainer-ы очень активно отвечают на issues в GitHub. Зачастую в первые сутки после создания тикета на него как минимум реагируют (комментируют или ставят тег типа bug/enhancement). Многие найденные нами баги были исправлены в течение пары дней. Но особенно мне нравится то, что maintainer-ы проектов активно между собой общаются, репортят друг другу баги, отправляют pull requests.

Мы внедрили и используем все три анализатора. У каждого есть свои нюансы, свои баги. Но использование трёх анализаторов одновременно облегчает понимание того, где реальная проблема, а где — ложное срабатывание.

Что умеют анализаторы


У анализаторов много общих возможностей, поэтому сначала рассмотрим, что умеют они все, а затем перейдём к особенностям каждого из них.

Стандартные проверки


Конечно же, анализаторы осуществляют все стандартные проверки кода на предмет того, что:

  • код не содержит синтаксических ошибок;
  • все классы, методы, функции, константы существуют;
  • переменные существуют;
  • в PHPDoc подсказки соответствуют действительности.

Кроме того, анализаторы проверяют код на неиспользуемые аргументы и переменные. Многие из этих ошибок приводят к реальным fatal-ам в коде.

На первый взгляд может показаться, что хорошие программисты таких ошибок не делают, но иногда мы торопимся, иногда копипастим, иногда мы просто невнимательны. И вот в таких случаях эти проверки очень сильно спасают.

Проверки типов данных


Конечно же, статические анализаторы осуществляют и стандартные проверки, касающиеся типов данных. Если в коде написано, что функция принимает, скажем, int, то анализатор проверит, нет ли мест, где бы в эту функцию передавался объект. У большинства анализаторов можно настроить строгость проверки и имитировать strict_types: проверять, что в эту функцию не передаются строки или Boolean.

Кроме стандартных проверок, анализаторы ещё много чего умеют

Union types

Во всех анализаторах поддерживается концепция Union types. Допустим, у вас есть функция типа:

/** 	
 * @var string|int|bool $yes_or_no
 */
function isYes($yes_or_no) :bool 
{
    if (\is_bool($yes_or_no)) {
         return $yes_or_no;
     } elseif (is_numeric($yes_or_no)) {
         return $yes_or_no > 0;
     } else {
         return strtoupper($yes_or_no) == 'YES';
     }
}

Её содержимое не очень важно — важен тип входящего параметра string|int|bool. То есть переменная $yes_or_no — либо строка, либо целое число, либо Boolean.

Средствами PHP такой тип параметра функции описать нельзя. Но в PHPDoc это возможно, и многие редакторы (например, PHPStorm) его понимают.

В статических анализаторах такой тип называется union type, и они очень хорошо умеют проверять такие типы данных. Например, если вышеупомянутую функцию мы написали бы так (без проверки на Boolean):

/** 	
 * @var string|int|bool $yes_or_no
 */
function isYes($yes_or_no) :bool 
{
     if (is_numeric($yes_or_no)) {
         return $yes_or_no > 0;
     } else {
         return strtoupper($yes_or_no) == 'YES';
     }
}

анализаторы бы увидели, что в strtoupper может прийти либо строка, либо Boolean, и вернули бы ошибку — в strtoupper нельзя передавать Boolean.

Этот тип проверок помогает программистам правильно обрабатывать ошибки или ситуации, когда функция не может вернуть данные. Мы ведь часто пишем функции, которые могут вернуть какие-то данные или null:

// load() возвращает null или объект \User
$User = UserLoader::load($user_id);
$User->getName();

В случае такого кода анализатор подскажет, что переменная $User здесь может быть равна null и этот код может привести к fatal-у.

Тип false

В самом языке PHP довольно много функций, которые могут вернуть либо какое-то значение, либо false. Если бы мы писали такую функцию, то как бы мы задокументировали её тип?

          
 /** @return resource|bool */
 function fopen(...) {
        …
 }

Формально здесь вроде бы всё верно: fopen возвращает либо resource, либо значение false (которое имеет тип Boolean). Но когда мы говорим, что функция возвращает какой-то тип данных, это значит, что она может вернуть любое значение из множества, принадлежащего этому типу данных. В нашем примере для анализатора это значит, что fopen() может вернуть и true. И, например, в случае такого кода:

$fp = fopen(‘some.file’,’r’);
if($fp === false) {
      return false;
}
fwrite($fp, "some string");

анализаторы бы жаловались, что fwrite принимает первым параметром resource, а мы ему передаём bool (потому что анализатор видит, что возможен вариант с true). По этой причине все анализаторы понимают такой «искусственный» тип данных как false, и в нашем примере мы можем написать @return false|resource. PHPStorm тоже понимает такое описание типа.

Array shapes

Очень часто массивы в PHP используются как тип record — структуру с чётким списком полей, где каждое поле имеет свой тип. Конечно, многие программисты уже используют для этого классы. Но у нас в Badoo много legacy-кода, и там активно используются массивы. А ещё бывает, что программисты ленятся заводить отдельный класс для какой-то разовой структуры, и в таких местах также часто используют массивы.

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

Анализаторы позволяют заводить описание таким структурам:

/** @param array{scheme:string,host:string,path:string} $parsed_url */
function showUrl(array $parsed_url) { … }

В данном примере мы описали массив с тремя строковыми полями: scheme, host и path. Если внутри функции мы обратимся к другому полю, анализатор покажет ошибку.

Если не описывать типы, то анализаторы будут пытаться «угадать» структуру массива, но, как показывает практика, с нашим кодом у них это не очень получается. :)

У этого подхода есть один недостаток. Допустим, у вас есть структура, которая активно используется в коде. Нельзя в одном месте объявить некоторый псевдотип и потом везде его использовать. Вам придётся везде в коде прописать PHPDoc с описанием массива, что очень неудобно, особенно если в массиве много полей. Также проблематично будет потом редактировать этот тип (добавлять и удалять поля).

Описание типов ключей массивов

В PHP ключами массива могут быть целые числа и строки. Иногда типы могут быть важны для статического анализа (да и для программистов). Статические анализаторы позволяют описывать ключи массива в PHPDoc:

/** @var array<int, \User> $users */
$users = UserLoaders::loadUsers($user_ids);

В данном примере мы с помощью PHPDoc добавили подсказку о том, что в массиве $users ключи — целочисленные int-ы, а значения — объекты класса \User. Мы могли бы описать тип как \User[]. Это сказало бы анализатору, что в массиве объекты класса \User, но ничего не сказало бы нам о типе ключей.

PHPStorm поддерживает такой формат описания массивов начиная с версии 2018.3.

Своё пространство имён в PHPDoc

PHPStorm (да и другие редакторы) и статические анализаторы могут по-разному понимать PHPDoc. Например, анализаторы поддерживают вот такой формат:

/** @param array{scheme:string,host:string,path:string} $parsed_url */	
function showUrl($parsed_url) { … }

А PHPStorm его не понимает. Но мы можем написать так:

/** 
 * @param array $parsed_url
 * @phan-param array{scheme:string,host:string,path:string} $parsed_url
 * @psalm-param array{scheme:string,host:string,path:string} $parsed_url  
*/
function showUrl($parsed_url) { … }

В этом случае будут довольны и анализаторы, и PHPStorm. PHPStorm будет использовать @param, а анализаторы — свои PHPDoc-теги.

Проверки, связанные с особенностями PHP


Этот тип проверок лучше пояснить на примере.

Все ли мы знаем, что может вернуть функция explode()? Если бегло посмотреть документацию, кажется, что она возвращает array. Но если изучить более внимательно, то мы увидим, что она может вернуть ещё и false. На самом деле, она может вернуть и null и ошибку, если передать ей неправильные типы, но передача неправильного значения с неправильным типом данных — это уже ошибка, поэтому этот вариант нас сейчас не интересует.

Формально с точки зрения анализатора, если функция может вернуть false или массив, то, скорее всего, потом в коде должна быть проверка на false. Но функция explode() возвращает false, только если разделитель (первый параметр) равен пустой строке. Зачастую он явно прописан в коде, и анализаторы могут проверить, что он не пуст, а значит, в данном месте функция explode() точно возвращает массив и проверка на false не нужна.

Таких особенностей у PHP не так уж мало. Анализаторы постепенно добавляют соответствующие проверки или совершенствуют их, и нам, программистам, уже не надо запоминать все эти особенности.

Переходим к описанию конкретных анализаторов.

PHPStan


Разработка некоего Ondřej Mirtes из Чехии. Активно разрабатывается с конца 2016 года.

Чтобы начать использовать PHPStan, нужно:

  1. Установить его (проще всего это сделать через Composer).
  2. (опционально) Сконфигурировать.
  3. В простейшем случае достаточно запустить:

vendor/bin/phpstan analyse ./src

(вместо src может быть список конкретных файлов, которые вы хотите проверить).

PHPStan прочитает PHP-код из переданных файлов. Если ему встретятся неизвестные классы, он попробует подгрузить их автолоадом и через reflection понять их интерфейс. Вы можете также передать путь к Bootstrap-файлу, через который вы настроите автолоад, и подключить какие-то дополнительные файлы, чтобы упростить PHPStan анализ.

Ключевые особенности:

  1. Можно анализировать не всю кодовую базу, а только часть — неизвестные классы PHPStan попытается подгрузить автолоадом.
  2. Если по какой-то причине какие-то ваши классы не в автолоаде, PHPStan не сможет их найти и выдаст ошибку.
  3. Если у вас активно используется магические методы через __call / __get / __set, то вы можете написать плагин для PHPStan. Уже существуют плагины для Symfony, Doctrine, Laravel, Mockery  и др.
  4. На самом деле, PHPStan выполняет автолоад не только для неизвестных классов, а вообще для всех. У нас много старого кода, написанного до появления анонимных классов, когда мы в одном файле создаём некий класс, а потом сразу его инстанцируем и, возможно, даже вызываем какие-то методы. Автолоад (include) таких файлов приводит к ошибкам, потому что код выполняется не в обычном окружении.
  5. Конфиги в формате neon (никогда не слышал, чтобы где-нибудь ещё использовался такой формат).
  6. Нет поддержки своих PHPDoc-тегов типа @phpstan-var, @phpstan-return и т. п.

Ещё одной особенностью является то, что у ошибок есть текст, но нет никакого типа. То есть вам возвращается текст ошибки, например:

  • Method \SomeClass::getAge() should return int but returns int|null
  • Method \SomeOtherClass::getName() should return string but returns string|null

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

Для сравнения, в других анализаторах у ошибок есть тип. Например, в Phan такая ошибка имеет тип PhanPossiblyNullTypeReturn, и можно в конфиге указать, что не требуется проверка на такие ошибки. Также, имея тип ошибки, можно, например легко собрать статистику по ошибкам.

Поскольку мы не используем Laravel, Symfony, Doctrine и подобные решения и у нас в коде довольно редко используются магические методы, основная фича PHPStan у нас оказалась невостребованной. ;( К тому же из-за того, что PHPStan include-ит все проверяемые классы, иногда его анализ просто не работает на нашей кодовой базе.

Тем не менее для нас PHPStan остаётся полезным:

  • Если надо проверить несколько файлов, PHPStan заметно быстрее Phan и немного (на 20—50%) быстрее Psalm.
  • Отчёты PHPStan позволяют нам проще находить false-positive в других анализаторах. Обычно, если в коде есть какой-то явный fatal, он показывается всеми анализаторами (или как минимум двумя из трёх).


Update:
Автор PHPStan Ondřej Mirtes тоже прочитал нашу статью и подсказал нам, что PhpStan, также как и Psalm, имеет сайт с «песочницей»: https://phpstan.org/. Это очень удобно для баг-репортов: воспроизводишь ошибку в и даёшь ссылку в GitHub.

Phan


Разработка компании Etsy. Первые коммиты от Расмуса Лердорфа.

Из рассматриваемой тройки Phan — единственный настоящий статический анализатор (в том плане, что он не исполняет никакие ваши файлы — он парсит всю вашу кодовую базу, а затем анализирует то, что вы скажете). Даже для анализа нескольких файлов в нашей кодовой базе ему требуется порядка 6 Гб оперативной памяти, и занимает этот процесс четыре—пять минут. Но зато полный анализ всей кодовой базы занимает примерно шесть—семь минут. Для сравнения, Psalm анализирует её за несколько десятков минут. А от PHPStan мы вообще не смогли добиться полного анализа всей кодовой базы из-за того, что он include-ит классы.

Впечатление от Phan двоякое. С одной стороны, это наиболее качественный и стабильный анализатор, он многое находит и с ним меньше всего проблем, когда надо проанализировать всю кодовую базу. С другой стороны, у него есть две неприятные особенности.

Под капотом Phan использует расширение php-ast. По-видимому, это одна из причин того, что анализ всей кодовой базы проходит относительно быстро. Но php-ast показывает внутреннее представление AST-дерева так, как оно отображается в самом PHP. А в самом PHP AST-дерево не содержит информации о комментариях, которые расположены внутри функции. То есть, если вы написали что-то вроде:

/**
 * @param int $type
 */
function doSomething($type) {
    /** @var \My\Object $obj **/
    $obj = MyFactory::createObjectByType($type);
    …
}

то внутри AST-дерева есть информация о внешнем PHPDoc для функции doSomething(), но нет информации PHPDoc-подсказки, которая внутри функции. И, соответственно, Phan тоже о ней ничего не знает. Это наиболее частая причина false-positive в Phan. Есть некие рекомендации по тому, как вставлять подсказки (через строки или assert-ы), но, к сожалению, они сильно отличаются от того, к чему привыкли наши программисты. Частично мы решили эту проблемы написанием плагина для Phan. Но о плагинах речь пойдёт ниже.

Вторая неприятная особенность состоит в том, что Phan плохо анализирует свойства объектов. Вот пример:

class A {
    /**
     * @var string|null
     */
     private $a;
	
     public function __construct(string $a = null)
     {
           $this->a = $a;
     }
	
     public function doSomething()
     {
           if ($this->a && strpos($this->a, 'a') === 0) {
                var_dump("test1");
           }
     }
}

В этом примере Phan скажет вам, что в strpos вы можете передать null. Подробнее об этой проблеме можно узнать здесь: https://github.com/phan/phan/issues/204.

Резюме. Несмотря на некоторые сложности, Phan — очень крутая и полезная разработка. Кроме этих двух типов false-positive, он почти не ошибается, либо ошибается, но на каком-то действительно сложном коде. Нам также понравилось, что конфиг находится в PHP-файле — это даёт определённую гибкость. Ещё Phan умеет работать как language server, но мы не использовали эту возможность, так как нам хватает PHPStorm.

Плагины


У Phan хорошо проработанный API для разработки плагинов. Можно добавлять свои проверки, улучшать выведение типов для вашего кода. У этого API есть документация, но особенно классно, что внутри уже есть готовые рабочие плагины, которые можно использовать как примеры.

Мы успели написать два плагина. Первый был предназначен для разовой проверки. Мы захотели оценить, насколько наш код готов к PHP 7.3 (в частности, узнать, нет ли в нём case-insensitive-констант). Мы практически были уверены в том, что таких констант нет, но за 12 лет могло произойти всякое — следовало проверить. И мы написали плагин для Phan, который бы ругался, если бы в define() использовался третий параметр.

Плагин получился очень простым
<?php declare(strict_types=1);

use Phan\AST\ContextNode;
use Phan\CodeBase;
use Phan\Language\Context;
use Phan\Language\Element\Func;
use Phan\PluginV2;
use Phan\PluginV2\AnalyzeFunctionCallCapability;
use ast\Node;

class DefineThirdParamTrue extends PluginV2 implements AnalyzeFunctionCallCapability
{
	
    public function getAnalyzeFunctionCallClosures(CodeBase $code_base) : array
   { 	
        $define_callback = function (	
                   CodeBase $code_base, 
                   Context $context, 
                   Func $function, 
                   array $args
         ) {
             if (\count($args) < 3) {   	
                 return;
              }	
              $this->emitIssue(  	
                   $code_base,
                   $context,
                   'PhanDefineCaseInsensitiv',
                   'Define with 3 arguments',
                   []
             );
         };
         return [  	
             'define'  => $define_callback,
         ];
     }
}

return new DefineThirdParamTrue();



В Phan разные плагины можно повесить на разные события. В частности, плагины с интерфейсом AnalyzeFunctionCallCapability срабатывают, когда анализируется вызов функции. В этом плагине мы сделали так, чтобы при вызове функции define() вызывалась наша анонимная функция, которая проверяет, что у define() не более двух аргументов. Потом мы просто запустили Phan, нашли все места, где define() вызывалась с тремя аргументами, и убедились, что у нас нет case-insensitive-констант.

С помощью плагина мы также частично решили проблему false-positive, когда Phan не видит PHPDoc-подсказок внутри кода.

У нас часто используются фабричные методы, которые на вход принимают константу и по ней создают некоторый объект. Зачастую код выглядит примерно так:

/** @var \Objects\Controllers\My $Object */
$Object = \Objects\Factory::create(\Objects\Config::MY_CONTROLLER);

Phan не понимает такие PHPDoc-подсказки, но в этом коде класс объекта можно получить из названия константы, переданной в метод create(). Phan позволяет написать плагин, который срабатывает в момент анализа возвращаемого значения функции. И с помощью этого плагина можно подсказать анализатору, какой конкретно тип возвращает функция в данном вызове.

Пример этого плагина более сложный. Но в коде Phan есть хороший пример в vendor/phan/phan/src/Phan/Plugin/Internal/DependentReturnTypeOverridePlugin.php.

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

Psalm


Psalm — разработка компании Vimeo. Честно говоря, я даже не знал, что в Vimeo используется PHP, пока не увидел Psalm.

Этот анализатор — самый молодой из нашей тройки. Когда я прочитал новость о том, что Vimeo выпустила Psalm, был в недоумении: «Зачем вкладывать ресурсы в Psalm, если уже есть Phan и PHPStan?» Но выяснилось, что у Psalm есть свои полезные особенности.

Psalm пошёл по стопам PHPStan: ему тоже можно дать список файлов для анализа, и он проанализирует их, а ненайденные классы подключит автолоадом. При этом он подключает только ненайденные классы, а файлы, которые мы попросили проанализировать, не будут include-иться (в этом отличие от PHPStan). Конфиг хранится в XML-файле (для нас это скорее минус, но не очень критично).

У Psalm есть сайт с «песочницей», где можно написать код на PHP и проанализировать его. Это очень удобно для баг-репортов: воспроизводишь ошибку на сайте и даёшь ссылку в GitHub. И, кстати, на сайте описаны все возможные типы ошибок. Для сравнения: в PHPStan у ошибок нет типов, а в Phan они есть, но нет единого списка, с которым можно было бы ознакомиться.

Ещё нам понравилось, что при выводе ошибок Psalm сразу показывает строки кода, где они были найдены. Это сильно упрощает чтение отчётов.

Но, пожалуй, самой интересной особенностью Psalm являются его кастомные PHPDoc-теги, которые позволяют улучшить анализ (особенно определение типов). Перечислим наиболее интересные из них.

@psalm-ignore-nullable-return


Бывает так, что формально метод может возвращать null, но код уже организован таким образом, что этого никогда не происходит. В этом случае очень удобно, что можно добавить вот такую   PHPDoc-подсказку к методу/функции — и Psalm будет считать, что null не возвращается.

Аналогичная подсказка существует и для false: @psalm-ignore-falsable-return.

Типы для closure


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

function my_filter(array $ar, \Closure $func) { … }

Как программисту понять, какой интерфейс у функции во втором параметре? Какие параметры она должна принимать? Что она должна возвращать?

В Psalm поддерживается синтаксис для описания функций в PHPDoc:

/**
  * @param array $ar
  * @psalm-param Closure(int):bool $func
  */
function my_filter(array $ar, \Closure $func) { … }

С таким описанием уже понятно, что в my_filter нужно передать анонимную функцию, которая на вход примет int и вернёт bool. И, конечно же, Psalm будет проверять, что у вас в коде сюда передаётся именно такая функция.

Enums


Допустим, у вас есть функция, принимающая строковый параметр, и туда можно передавать только определённые строки:

function isYes(string $yes_or_no) : bool
{
      $yes_or_no = strtolower($yes_or_no)
      switch($yes_or_no)  {
            case ‘yes’:
                  return true;
            case ‘no’:
                  return false;
            default:
                  throw new \InvalidArgumentException(…);
      }
}

Psalm позволяет описать параметр этой функции вот так:

/** @psalm-param ‘Yes’|’No’ $yes_or_no **/
function isYes(string $yes_or_no) : bool { … }

В этом случае Psalm будет пытаться понять, какие конкретно значения передают в эту функцию, и выдавать ошибки, если там будут значения, отличные от Yes и No.

Подробнее об enum-ах здесь.

Type aliases


Выше при описании array shapes я упоминал, что, хотя анализаторы и позволяют описывать структуру массивов, пользоваться этим не очень удобно, так как описание массива приходится копировать в разных местах. Правильным решением, конечно же, является использование классов вместо массивов. Но в случае с многолетним legacy это не всегда возможно.

На самом деле, проблема возникает не только с массивами, а с любым типом, который не является классом:

  • массив;
  • closure;
  • union-тип (например, несколько классов или класс и другие типы);
  • enum.

Любой такой тип, если он используется в нескольких местах, нужно дублировать в PHPDoc и при его изменении, соответственно, везде исправлять. Поэтому у Psalm есть небольшое улучшение в этом плане. Вы можете объявить alias для типа и потом в PHPDoc использовать этот alias. К сожалению, есть ограничение: это работает в рамках одного PHP-файла. Но это уже упрощает описание типов. Правда, только для Psalm.

Generics aka templates


Рассмотрим эту возможность на примере. Допустим, у вас есть такая функция:

function identity($x) { return $x; }

Как описать тип этой функции? Какой тип она принимает на вход? Что она возвращает?

Наверное, первое, что приходит на ум, — mixed, то есть она может принимать на вход любое значение и возвращать любое значение.

Для статического анализатора встретить mixed — это катастрофа. Это значит, что нет абсолютно никакой информации о типе и нельзя делать никакие предположения. Но на самом деле, хотя функция identity() и принимает/возвращает любые типы, у неё есть логика: она возвращает тот же тип, который она приняла. А для статического анализатора это уже что-то. Это значит, что в коде:

$i = 5; // int
$y = identity($i);

анализатор может определить тип входящего аргумента (int), а значит, может определить и тип переменной $y (тоже int).

Но как нам передать эту информацию анализатору? В Psalm для этого есть специальные PHPDoc-теги:

/**
  * @template T
  * @psalm-param T $x
  * @psalm-return T
  */
 function identity($x) { $return $x; }

То есть templates позволяют передать Psalm информацию о типе, если класс/метод может работать с любым типом.

Внутри Psalm есть хорошие примеры работы с templates:

vendor/vimeo/psalm/src/Psalm/Stubs/CoreGenericFunctions.php;
vendor/vimeo/psalm/src/Psalm/Stubs/CoreGenericClasses.php.

Подобный функционал есть и в Phan, но он работает только с классами: https://github.com/phan/phan/wiki/Generic-Types.

В целом, нам Psalm очень понравился. Похоже, что автор пытается «сбоку» прикрутить более умную систему типов и более умные и практически полезные подсказки для анализатора. Нам также понравилось, что Psalm сразу показывает строки кода, в которых найдены ошибки, и мы даже реализовали такое для Phan и PHPStan. Но об этом чуть ниже.

Инспекции кода в PHPStorm


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

Для программиста было бы удобнее получать информацию об ошибках в процессе редактирования кода. В этом направлении двигается Phan, который развивает свой language server. Но нам в PHPStorm, увы, неудобно его использовать.

Но, к счастью, у PHPStorm есть свой отличный анализатор (инспекции кода), по качеству соизмеримый с описанными выше решениями. А в дополнение к нему есть крутой плагин — Php Inspections (EA Extended). Главное отличие от анализаторов — удобство для программиста, заключающееся в том, что ошибки видны в редакторе во время написания кода. Кроме того, эти инспекции можно очень тонко настроить. Например, можно в проекте выделить разные scopes файлов и настроить инспекции по-разному для разных scopes.

Ещё отмечу такой полезный плагин, как deep-assoc-completion. Он хорошо понимает структуру массивов и упрощает автокомплит ключей.

Использование анализаторов в Badoo


Как это работает у нас?

Сегодня статический анализ кода используется несколькими командами, но в наших планах внедрить эту практику во всех.

Мы анализируем только изменённые файлы, вплоть до строк. То есть, когда девелопер завершает свою задачу, мы берём его git diff и запускаем анализаторы только для изменённых/добавленных файлов, а из полученного списка ошибок убираем те, которые относятся к старым (неизменённым) строкам. Таким образом мы прячем от девелопера ошибки, которые были сделаны ранее.

Конечно, этот подход не совсем правильный: программист может своим кодом сломать что-то за пределами своего git diff. Здесь мы пошли на компромисс. Даже в таком виде использование статического анализа даёт свои плоды в виде ошибок, найденных в новом коде. И мы не хотим заставлять программиста исправлять старые ошибки. Но, конечно, в будущем, когда наш код станет более чистым с точки зрения анализаторов, мы пересмотрим это решение.

Получив отчёты от трёх анализаторов, мы объединяем их в один, где ошибки группируются по файлам и строкам:



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

Место анализаторов в нашем QA


Мы всячески стараемся снизить количество багов на продакшене:



Статические анализаторы — это, по сути, ещё один инструмент в этом списке, и он хорошо его дополняет. У статических анализаторов есть ряд преимуществ:

  • они могут покрыть 100% кода (в отличие от тестов, которые для каждого участка кода надо писать отдельно);
  • они часто отлавливают такие ошибки, которые сложно заметить в процессе code review;
  • они способны анализировать даже тот код, который сложно или невозможно запустить при ручном тестировании.

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

  • анализаторы работают для всего кода, а чтобы увидеть ошибки strict types, нужно исполнить код;
  • ошибку анализатора можно исправить позже, если она не критична (возможно, это не лучшая практика, но в некоторых случаях это может быть полезно);
  • система типов в статических анализаторах даже более гибкая, чем в самом PHP (например, они поддерживают union types, которых нет в PHP);
  • статические анализаторы приближают нас к внедрению strict types, поскольку они умеют эмулировать такие же строгие проверки.

Отчёты анализаторов: мнение программистов


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

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

Во-вторых, как уже было сказано выше, многое в отчётах анализаторов — просто неточности, например, неправильно указанные типы в PHPDoc. Некоторые программисты пренебрежительно относятся к таким ошибкам — код ведь работает.

В-третьих, у некоторых программистов завышенные ожидания. Они думали, что анализаторы будут находить какие-то хитрые баги, а вместо этого они вынуждают их добавлять проверки типов и исправлять PHPDoc. :)

Однако польза, принесённая анализаторами, перекрывает все эти незначительные недовольства. И как ни крути, это хорошая инвестиция в будущий код.
Tags:
Hubs:
+75
Comments51

Articles

Information

Website
badoo.com
Registered
Founded
2006
Employees
501–1,000 employees
Location
Россия
Representative
Yuliya Telezhkina