Компания 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


Разработка некоего Ondrej 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 Ondrej 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. :)

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

Комментарии (43)


  1. arturpanteleev
    16.10.2018 18:53

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

    Очень странное желание, обычно при разработке руководствуются правилом, что если происходит какая-то ошибка, то программа должна «упасть» как можно раньше. А здесь вы, по сути, хотели, чтобы функция уже получившая какие-то некорректные данные, во-первых продолжила работу с ними, а во-вторых, делала какие-то неявные преобразования, над уже ошибочными данными, еще больше усугубляя ситуацию


    1. Fedcomp
      16.10.2018 19:04
      -2

      Это прям типичное php. Весь язык построен на попытки сглотнуть ошибку.


      1. arturpanteleev
        16.10.2018 19:39

        Все последние релизы наоборот идёт движение от этого(хоть и не так быстро, как хотелось бы), поэтому меня и удивила подобная идея.


    1. max_m Автор
      16.10.2018 19:21
      +1

      Ну, за желания не судят :) До реальных действий дело не дошло

      Но про патч я всё-таки поясню.
      В типичном PHP-коде, написаном без strict_types, все эти неявные преобразования уже происходят. Такова система типов PHP, по крайней мере до времён strict_types.
      PHP-программисты часто используют например var_export($some_var,1) и это работает так, как программист ожидает, хотя вторым параметром надо передавать boolean. Патч показал бы нам такие места.


    1. MIKEk8
      17.10.2018 11:15
      +1

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


    1. FanatPHP
      17.10.2018 12:18
      +1

      функция, уже получившая какие-то некорректные данные, во-первых продолжила работу с ними, а во-вторых, делала какие-то неявные преобразования

      Учитывая, что такое поведение являлось абсолютно нормальным в течение первых 20 лет существования РНР, а "неявные преобразования" до сих остаются одной из ключевых фич языка, то предложение, чтобы сайт с миллиардной аудиторией падал при срабатывании type juggling выглядит, на мой взгляд, несколько оторванным от реальности.


      1. arturpanteleev
        17.10.2018 12:32

        первых 20 лет существования РНР

        PHP 20 летней давности — это явно не пример для подражания
        чтобы сайт с миллиардной аудиторией падал при срабатывании type juggling

        Прямо таки с милиардной?)) Очень хорошие показатели, учитывая, что по статистике ООН 50% населения планеты не имеют доступа к интернету.
        А если серьезно, то необязательно раскладывать новый функционал на 100% аудитории, можно разложить ветку с изменениями для 1% или еще меньшей доли, и пофиксить баги, если вдруг они появятся + покрывать тестами. Также можно поэтапно вводить strict type, например только для новых/изменненых файлов, но не пытаться делать вид что ничего не произошло и продолжать работу при, например, строке, приведенной к инту.


        1. FanatPHP
          17.10.2018 13:19
          +1

          Срок "первые 20 лет" включает в себя не только код 20-летней давности, но и 5- и 3-летней.


          Миллиарда у них, насколько я знаю, нет, но величины сравнимые.


          1% от 100 миллионов — это миллион. Вам бы хотелось попасть в эту фокус-группу? Пусть даже 10 тысяч — зачем экспериментировать на посетителях, если можно обойтись без этого?


          Лично я воспринял абзац про "пропатчить РНР" не как предложение "сделать вид, что ничего не произошло" и оставить все как есть — в этом случае ваше возмущение было бы оправданным — а как graceful refactoring — залогировать ошибки и исправить потенциально некорректный код.


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


          1. arturpanteleev
            17.10.2018 13:29

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

            Да тут, я соглассен что издержки будут. Но архитектурные ограничения — это стратегические инвестиции в будушее, их смысл уменьшить стоимость поддержки кода через год, три, пять. Допустим, сейчас мы создадим некоторый дискомфорт для малой части пользователей, но это окупится в дальнейшем тем, что у нас не будет фантомных трудноловимых багов, которые будут у 100% юзеров.
            Но про бизнесс процессы это вы верно заметили, в большинстве проектов менеджменту насрать что будет через год и тем более 5 лишь бы сегодня закрыть побольше бизнес задач с как можно меньшими проблемами, а инициативы разрабов по поводу инвестиций в будущее не приветствуются. Так что, может я и не прав.


            1. FanatPHP
              17.10.2018 15:12

              Насколько я знаю ситуацию конрeктно в Баду, это в первую очередь технологическая компания. И учитывая, сколько всего она сделала для РНР сообщества (взять один только php-fpm или фикс проблемы с opcache в 7.0.4), то она не вписывается в нарисованную вами мрачную картину засилья неадекватного менеджмента.


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


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


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

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


  1. mmasiukevich
    16.10.2018 19:05

    Если взять тот же psalm, то у него есть ряд «фич», которые могут вселить ложную уверенность в корректности кода. Ну и оно не очень хорошо дружит с чем-то «нестандартным» по меркам рядового php приложения: генераторы, промисы, variadic и т.д. и т.п.

    Так что анализатор в целом может помочь, но расслабляться ни в коем случае нельзя.


    1. max_m Автор
      16.10.2018 19:31

      А вы пробовали писать автору psalm-а? Все зарепорченные нами баги он довольно быстро исправил. Хотя возможно наши баги не были такими сложными.

      Так что анализатор в целом может помочь, но расслабляться ни в коем случае нельзя.
      Всё верно. Именно поэтому мы используем все 3 анализатора + тесты + ручное тестирование


      1. mmasiukevich
        16.10.2018 19:33

        Пробовали.

        Что касается генераторов и т.д. — ну тут сам язык не очень располагает.
        По поводу «фич» — просто странные допущения, о которых нужно помнить.


  1. zvirusz
    16.10.2018 19:38

    Немного дополню: в PHP есть довольно старый баг (один из сотен, обнаруженных более 10 лет назад и до сих пор не исправленных), связанный с доступом к элементам массива. Точнее не совсем массива. Проблема в том, что при попытке обратиться по какому-либо ключу к любому скалярному типу, отличному от строки, мы не получим ошибку/предупреждение (пруф: https://3v4l.org/D3c2q), т.е. конструкция


    $a = null;
    var_dump($a['one']['two']['three']);

    Выполнится успешно и выдаст в ответе NULL, что не всегда является ожидаемым результатом. Статические анализаторы (при условии корректно заполненного phpdoc, конечно же) позволяют отловить такие участки.


    1. mmasiukevich
      16.10.2018 19:43

      Попробуйте psalm'ом отловить такой (https://getpsalm.org/r/c7a1281897):

      declare(strict_types = 1);
      
      function parse(string $fileContent): array
      {
          return [$fileContent];
      }
      
      parse(file_get_contents('/someWrongPath'));


      1. max_m Автор
        16.10.2018 20:10

        вы считаете psalm должен здесь ругаться на то что file_get_contents() может вернуть false?
        В этом случае будет сгенерирован warning, многие современные фреймворки конвертируют их в Exception-ы. Анализаторы, как мне кажется, считают что эти случаи можно не анализировать.


        1. mmasiukevich
          16.10.2018 20:13
          +1

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


          1. max_m Автор
            16.10.2018 20:47

            Напоминаю, что я не автор psalm-а, претензии лучше высказывать напрямую автору
            На такой пример psalm ругается — getpsalm.org/r/075628070d, так что я думаю, что дело как раз в обработке ошибок.


      1. zvirusz
        16.10.2018 20:22

        Psalm и в fgets(fopen('zzz', 'rt')) не замечает ничего плохого :(


    1. arturpanteleev
      16.10.2018 19:54

      Ага, это жопа ((
      Поэтому всегда пред обращению к элементу массива приходится делать isset/array_key_exists


  1. springimport
    16.10.2018 20:46

    Мне начинает казаться что что-то пошло не так в php. Объясню.
    Вот пример первого попавшегося файла и на который в будущем будут похожи все файлы.

    <?php
    /*
     * This file is part of sebastian/global-state.
     *
     * (c) Sebastian Bergmann <sebastian@phpunit.de>
     *
     * For the full copyright and license information, please view the LICENSE
     * file that was distributed with this source code.
     */
    
    declare(strict_types=1);
    
    namespace SebastianBergmann\GlobalState;
    
    /**
     * Exports parts of a Snapshot as PHP code.
     */
    class CodeExporter


    Это нужно просто взять и убрать.
    <?php
    
    declare(strict_types=1);
    
    For the full copyright and license information, please view the LICENSE file that was distributed with this source code.


    А еще радуют такие вот блоки, которые вполне могли бы заменяться с новым {}, но никто не заменяет.
    use SebastianBergmann\PHPCPD\Detector\Detector;
    use SebastianBergmann\PHPCPD\Detector\Strategy\DefaultStrategy;
    use SebastianBergmann\PHPCPD\Log\PMD;
    use SebastianBergmann\PHPCPD\Log\Text;
    use SebastianBergmann\FinderFacade\FinderFacade;
    use Symfony\Component\Console\Command\Command as AbstractCommand;
    use Symfony\Component\Console\Input\InputArgument;
    use Symfony\Component\Console\Input\InputInterface;
    use Symfony\Component\Console\Input\InputOption;
    use Symfony\Component\Console\Output\OutputInterface;


    Ну а папка vendor все жирнеет и жирнеет: 100, 200 и 300 мб — не редкость.
    Короче, нужно менять это. Наболело.


    1. modestguy
      16.10.2018 21:18

      на гитхабе PHPСPD уже написано, что использовать его не нужно и что код лежит только для истории
      upd: прошу прощения, ошибся, это phpdcd


      1. springimport
        16.10.2018 21:29

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


    1. Sabubu
      16.10.2018 21:48
      +1

      По поводу копирайтов — какое отношение к этому имеет PHP? Он не требует писать копирайты. Я тоже против них, это юристы так перестраховываются.

      Убирать namespace и class не нужно.

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


    1. balamyt92
      17.10.2018 06:48
      +1

      Ну а что плохого то в этих блоках? Визуально «мешают»? Ну так включите автофолдинг для них.


      1. springimport
        17.10.2018 15:54

        Да включен он. Последнее что добило это stric types. Что дальше, very_strict_type? Вот этому не место в коде.


  1. Sabubu
    16.10.2018 21:18

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

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


  1. dumistoklus
    16.10.2018 23:54
    +1

    Расширение для PhpStorm Php Inspections (EA Extended) устанавливается через меню плагинов и запускается через меню Code > Inspect Code. Очень удобно.


  1. Kirill_Dan
    17.10.2018 10:21

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


    А может, все таки, уйти на более подходящую технологию: руби, питон, и т.д.? Сам уже в начале года попрощался с пыхой, проработав с ней более 10 лет, ибо уже в конец заеб… лся (ушел полностью на руби). На огромных проектах пыха — это всегда легаси дичь. Что не внедряй, что не придумывай, все равно это будут тонны костылей и новых граблей. С нестрогой типизацией ничего нельзя сделать. На сколько разработчик не был бы опытным и даже гениальным, все равно произойдет ситуация, когда авто приведение типа будет ломать логику. И эту ошибку не всегда можно заметить и очень сложно отловить. Особенно при работе со всякими разнородными массивами, где не всегда ясно, какого типа данные лежат, могут лежать или должны лежать. Плюс на пыхе разные разработчики используют разные подходы, которые не всегда очевидны для других. Три опытных пыхера хрен когда сойдутся во мнении, как нужно реализовать функционал. И каждый будет отстаивать свое решение. Работал на двух громаднейших порталах, один по финансам, другой по недвижимости. Оба на пыхе. Это незабываемые для меня годы! Через год сами разработчики уже в собственном коде не могут разобраться. Любые улучшения/плюшки/фишки с течением времени легко умножаются на ноль. Какой бы хороший код и логику не писал, все превращается в легаси кашу, которую вынуждены поддерживать и «развивать» команды по 30!!! человек. И как бы мы не работали (канбан/скрам), какие бы методологии и подходы не вырабатывали, все равно выбор ЯП является основополагающим. На новом проекте я ушел полностью на руби. После пыхи, где элементарное действие кодится сотней строк, я просто прозреваю, как тоже самое делается 5-10 строками. Я наконец увидел, что программирование может приносить не боль, а удовольствие. Если что, то на питоне и яве я тоже пишу, но очень редко. Пыху считаю днищем, хотя работал с ней с 2000 года!


    1. lu4e3ar
      17.10.2018 11:32
      +1

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

      Наличие или отсутствие автоматического приведения ничего не меняет — логика всё равно сломается. Например, для упомянутого вами Ruby будет что-то вроде:


      +': no implicit conversion of Integer into String (TypeError) from main.rb:1:in `<main>'


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


      Но не надо отчаиваться, для Ruby тоже есть тулзы для статического анализа https://github.com/mre/awesome-static-analysis#ruby


      1. Kirill_Dan
        17.10.2018 11:42

        Наличие или отсутствие автоматического приведения ничего не меняет — логика всё равно сломается.

        Ну не скажите. Например, '3' + 2 в пыхе выведет 5, а руби словит эксепшн. Значит в руби интерпретатор свалится с указанием проблемы, и вы сможете быстро ее пофиксить. А вот в пыхе это может привести к неправильному расчету в какой-то формуле, что уже реально проблематично. Это такой пример из головы, но суть, думаю ясна.

        Но не надо отчаиваться, для Ruby тоже есть тулзы для статического анализа

        Спасибо за ссылку :)


        1. lu4e3ar
          17.10.2018 12:08

          Значит в руби интерпретатор свалится с указанием проблемы, и вы сможете быстро ее пофиксить. А вот в пыхе это может привести к неправильному расчету в какой-то формуле, что уже реально проблематично.

          Поведение PHP и Ruby будет разным — это правда, но это не меняет сути.
          Ключевой момент в обоих случаях: проблемный код должен кто-то


          • исполнить (человек/тест/автоматика)
          • проверить результат (есть ли ошибка).

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


          1. Kirill_Dan
            17.10.2018 12:26

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

            Вспомнил анекдот: У джуна не проходили тесты, но он смог решить проблему — он их все закомментировал :))


            1. lu4e3ar
              17.10.2018 12:39

              Хотя я и не представляю, как разработчик может не оттестировав свой код лить его на прод.

              Если лить код на прод, не проверяя ничем, уже ничего не поможет. :) Даже при наличии статических анализаторов можно на них забить — это крайности.


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


    1. max_m Автор
      17.10.2018 12:41

      Через год сами разработчики уже в собственном коде не могут разобраться
      У нас 12-летний легаси и мы как-то справляемся :) С этой проблемой очень помогают правильно настроенные процессы. В частности у нас все коммиты привязаны к тикетам в jira. Поэтому сделав git blame и посмотрев описание коммиты, я вижу в каком тикете были сделаны данные изменения и обычно из тикета ясны почти все детали.

      Что касатеся перехода на другие языки, то я в начале статьи писал, что у нас миллионы строк php-кода. Их нельзя по-быстрому переписать на руби или питон. Но у нас есть движение в сторону более активного использования Go.


      1. Kirill_Dan
        17.10.2018 12:57

        Да мы тоже все комиты привязывали к таскам в жире. Но миллион веток, постоянных слияний, разделений, плюс какой-то джун, что-то поломал, где-то пришлось черри пик делать и так далее, превратили гит в какой-то безумный архив несбывшихся надежд. Иногда хотелось взяться за голову и горько заплакать от того пизд… ца, что творился в проекте (этот трэш тянется с 2004 года). Попытались бить на микросервисы. Уперлись в то, что терабайтная база наполовину состоит из виртуальных таблиц. Часть рекламных текстов и баннеров в виде html хранится прямо в базе, а потом инклудится в шаблонах. Часть роутов сделаны через бесконечный свитч кейс с фабриками, а часть со входом прямо на пхп скрипты ($_GET, $_POST). Девопс решил бороться с безопасностью и закрыл доступ к папкам в нжинксе, от чего половина портала перестала работать.

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


  1. rjhdby
    17.10.2018 11:48

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


  1. FreeBa
    17.10.2018 14:26

    И чего только не придумают, только чтобы строгую типизацию (насколько эти слова применимы к пыху) не использовать…


    1. dumistoklus
      17.10.2018 16:08
      +2

      Встроенной типизации недостаточно.

      Например,

      declare(strict_types=1);
      
      class Model {
         /**
           * @var string 
           */
          public $a = ''; // Тут строка и не ожидается ничего кроме строки
      }
      
      function getA(): bool
      {
          return true;
      }
      
      $m = new Model();
      $m->a = getA(); // Тут не будет ошибки для PHP 7.2 и PhpStorm Php Inspections (EA Extended), а вот Psalm найдет ее, при условии что есть phpdoc
      


      В PHP 8 будут введены типы для свойств классов, а там еще немного останется для полной типизации языка


      1. mmasiukevich
        17.10.2018 16:09

        В след. версии подвезут тайпхинты на свойства


      1. FreeBa
        17.10.2018 18:41

        Ну и зачем такая типизация вобще нужна? Она же не выполняет свою главную задачу, а только создает иллюзию порядка.

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


      1. SerafimArts
        18.10.2018 03:18

        Не PHP 8, а PHP 7.4, т.е. уже в мастере висит.


  1. edogs
    17.10.2018 21:28

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