NoVerify: PHP-линтер, который работает быстро

Для PHP есть хорошие утилиты статического анализа: PHPStan, Psalm, Phan, Exakat. Линтеры хорошо выполняют свою работу, но очень медленно, потому что почти все написаны на PHP (или Java). Для личного использования или небольшого проекта это нормально, но для сайта с миллионами пользователей — критический фактор. Медленный линтер замедляет CI pipeline и не даёт возможности использовать его в качестве решения, интегрируемого в текстовый редактор или IDE.



Сайт с миллионами пользователей — это ВКонтакте. Разработка и добавление новых функций, тестирование и починка багов, ревью — все это должно проходить быстро, в условиях жестких дедлайнов. Поэтому хороший и быстрый линтер, который сможет проверять кодовую базу на 5 млн строк за 5−10 секунд, незаменимая вещь. 

Подходящих линтеров на рынке нет, поэтому Юрий Насретдинов (youROCK) из ВКонтакте написал свой в помощь командам разработки — NoVerify. Это линтер для PHP, который написан на Go. Он работает в 10-30 раз быстрее аналогов, может находить то, о чем не предупредит PhpStorm, легко расширяется и хорошо интегрируется в проекты, в которых раньше не слышали о статическом анализе. 

Об этом линтере расскажет Искандер Шарипов. Под катом:как выбирали линтер и предпочли написать свой, почему NoVerify такой быстрый и как устроен изнутри, почему написан на Go, что может находить и как расширяется, на какие компромиссы пришлось пойти ради него и что можно построить на его базе.


Искандер Шарипов (quasilyte) работает в бэкенд-инфраструктуре ВКонтакте и хорошо знаком с NoVerify. В прошлом занимался Go-компилятором в команде Intel. Не пишет на PHP, но это его любимый язык для статического анализа — в нем много вещей, которые могут пойти не так как запланировано.

Примечание. Чтобы понять предысторию, почитайте статью Юрия Насретдинова, автора NoVerify на Хабре с предысторией и сравнением с некоторыми существующими линтерами, которые написаны обычно на PHP.  Все высказывания в сторону PHP (в статье Юрия и здесь) — шутка. Искандер любит PHP, все любят PHP.

Продуктовая разработка

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

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

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

Основные задачи моей команды другие.

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

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

Выбираем линтер

Давайте выберем линтер, который добавим в pipeline. Воспользуемся простым подходом — сформулируем требования.

Линтер должен работать быстро. В нашем pipeline есть несколько шагов: работа линтера не должна занимать много времени и отнимать время разработчика, пока он ждет обратную связь.

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

Поддерживаемость «своих» проверок. Мы можем написать много тестов, но будут ли они хорошо поддерживаться? Например, если напишем на регулярных выражениях, то они будут усложняться, когда нужно учитывать контекст, семантику и синтаксис языка. Поэтому тесты не выход.

Большая часть линтеров, которые мы рассматривали, написаны на PHP. Но они не проходят по первому требованию. Линтеры на PHP (пока нет AOT-компиляции) работают в 10-20 раз медленнее, чем на других языках — наш самый крупный файл могут анализировать десятки секунд. Это очень много и слишком сильно замедляет рабочий процесс —это фатальный недостаток. Что делают разработчики в этом случае? Пишут свое.

Поэтому мы написали свой РНР-линтер NoVerify на Go. Почему на нем? Спойлер: не только потому, что так решил Юра.

NoVerify

Go — это хороший компромисс между скоростью разработки и производительностью.

Первое «доказательство» на картинке с «инфографикой»: хорошая скорость выполнения, легкая поддержка. В скорости разработки проигрываем, но нам важнее первые два пункта.


Цифры взяты из головы, они ничем не подкреплены.

Для второго «доказательства» привел доводы попроще.


PHP медленнее, Go быстрее – ч.т.д. 

Мы выбрали Go по трем причинам.

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

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

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

Сверим NoVerify с нашими требованиями.

• NoVerify работает в несколько раз быстрее альтернатив.

• Для него можно писать расширения, как Open Source, так и свои. Важно, что эти проверки мы можем разделить, а вы — можете писать свои.
• Легко тестировать и разрабатывать. Частично, потому, что в стандартном дистрибутиве Go есть стандартный фреймворк с профилированием и тестированием. В основном он используется для unit-тестирования. Особо ловкие могут использовать и для интеграционного, как мы — у нас интеграционное тестирование написано через тесты Go.

Компромиссы интеграции

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



О мой код! Столько ошибок никто никогда не исправит. Хочется закрыть проект, удалить линтер и больше никогда его не запускать. Что делать, чтобы этого избежать?

Интегрируем

Запускаем в режиме diff. Мы не хотим на каждом CI-шаге запускать все проверки на всем проекте  с миллионом ошибок. Возможно, вы знаете о baseline: в NoVerify это есть из коробки, не нужно встраивать отдельную утилиту. Мы сразу учли, что нужен такой режим.

Добавляем legacy (вендоров) в исключения. Проще не трогать некоторые вещи, оставить в стороне даже с дефектом, чтобы не модифицировать самому и не оставлять след в истории.

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

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

Настройка Git setup

Diff-режим подразумевает, что у вас есть система контроля версий — Git. Если у вас SVN, то инструкция не поможет, переходите на Git.

Устанавливаем pre-push hook с линтером и проверяем до того, как запушим код. Проверяем на локальной машине опцией --no-verify для обхода линтера. Вероятно, удобнее было бы использовать pre-receive hook и отключать линтер на стороне сервера, но во ВКонтакте по историческим причинам много чего исполняется в pre-push hook, поэтому NoVerify встроили туда же.

После пуша запускаются CI-проверки. В NoVerify есть два режима работы: с full-анализом и без него. На CI, скорее всего, захочется (и можно) запускать --git-full-diff — машины в CI можно загрузить сильнее и проверить даже те файлы, которые не менялись. На локальных машинах можем запустить менее строгий, но более быстрый анализ только изменившихся файлов (на 5-15 с быстрее). 

Ложные срабатывания

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

function get_foo($obj) {
    return $obj->foo;
    ^^^
}

Warning:
Property "foo" does not exist

Ложные срабатывания очень мешают. 

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

Часто пушат в обход, когда что-то сработало, но это не ошибка. У многих есть механизм для обхода линтеров — запушить с флагом без проверки линтера. У нас этот флаг назывался no-verify на pre-push hook. Мы часто его использовали и название увековечили в имени линтера.

Придирчивость

Еще одно свойство линтера. Например, многим непонятны алиасы. В PHP sizeof это аналог count: он не вычисляет размер, а возвращает количество элементов. В ментальной модели C-разработчиков у sizeof другое значение. Если в кодовой базе есть sizeof, скорее всего, имели в виду count. Но это придирки.

$len = sizeof($x);
    ^^^^^^

Warning:
use "count" instead of "sizeof"

Что с этим делать?

Быть строгим и заставлять править всё без исключений. Навязать правила, требовать, соблюдать и не давать их обходить — не работает никогда. Чтобы такая жесткость работала, команда должна состоять из одинаковых людей: характером, уровнем культуры, педантичностью и восприятия качества кода. Если это не так — будет бунт. Проще собрать команду из своих клонов, чем заставлять соблюдать все правила. 

Не блокировать push/commit на замечаниях вродеисправления sizeof на count. Скорее всего, это не ошибка, а придирка и не влияет на код. Но тогда 99% срабатываний будут игнорироваться (командой) и в коде всегда будут лишние sizeof.

Разрешать некоторый уровень конфигурации для разных команд и разработчиков. Можно настраивать конфигурации для каждой команды, чтобы те, кто не хочет менять sizeof на count, могли это не делать. Все остальные пусть следуют правилам. Неплохой вариант, но консистентность будет проседать, а в отдельных директориях код будет чуть хуже.

Запускать подобные проверки раз в месяц, на субботниках. Проверки можно запускать не каждый раз на CI или pre-push hook, а в рутинном Cron раз в месяц. Запускаете и правите все, что найдете после активной разработки. Но эта работа требует ресурсов на автоматизацию и проверку.

Не делать ничего. Выключить стилистические проверки тоже вариант.

Компромисс

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

Полезное действие превыше всего.

Технические детали NoVerify

Приватные проверки ВКонтакте. Noverify написан примерно так. В GitHub репозитории NoVerify разделен на две части: фреймворк, который используется для реализации линтера, и отдельно проверки — vklints. Это сделано чтобы линтер подгружал сторонние проверки: можете написать отдельный модуль на Go и они себя регистрируют во фреймворке. После запуска из бинарника NoVerify фреймворк подгружает все регистрирующиеся наборы проверок и они работают как единое целое. 



NoVerify – это и библиотека, и бинарник (линтер).

Наши проверки называются vklints. Они находят то, что не видят PhpStorm и Open Source NoVerify — важные ошибки, которые не подходят для общего использования.

Что такое vklints?

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

Дополнительные проверки стиля. Они не соответствует тому, что принято в PHP-community, не описаны в PHP Standard Recommendation или даже противоречат ему, но для нас — стандарт. В Open Source нет смысла их добавлять, потому что вы не захотите им следовать.

Требования использования строгого сравнения для некоторых типов. Например, у нас есть проверка, которая требует сравнивать строки оператором сравнения ===. В том числе он требует передавать флаг для строгого сравнения функций, чтобы сравнивать строки.

Подозрительные ключи массивов. Еще одна интересная ошибка: иногда, когда разработчики коммитят, перед сохранением файла могут нажимать комбинации клавиш. Эти символы иногда остаются в строке или в части кода. Однажды, в ключе массива была русская буква «Ы». Скорее всего, разработчик нажал «CTRL-S» в русской раскладке, сохранил файл и закоммитил. Иногда такие ключи находим в массивах, но новые ошибки уже не пройдут.

Динамические правила — более простой механизм расширения NoVerify, описываемый на PHP. Об этом написана отдельная статья: как добавить проверки в NoVerify, не написав ни строчки Go-кода.

Как работает NoVerify

Чтобы распарсить PHP нужен парсер. Мы не можем использовать PHP-парсер на PHP: он медленный, из Go его можно использовать только через обертку на C. Поэтому мы используем парсер на Go.

У этого парсера есть несколько проблем. К сожалению, он умеет работать только с UTF-8 и нам нужно различать UTF-8 и не UTF-8. Кроме UTF-8 в российских проектах на PHP часто встречается Windows-1251. Такие файлы у нас тоже есть. Как мы их распознаем? 

В файле encodings.xml перечислены все пути, где лежат файлы с UTF-8. Если мы встречаем файл вне этих путей, то на лету стриминговым потоком конвертируем в UTF-8 (не конвертируя заранее).

Парсинг и анализ

Выполняется за несколько шагов. На первом — загружаем метаданные из phpstorm-stubs. Это данные, которые похожи на PHP-код, но никогда не исполняются и описывают типы входов/выходов из стандартных функций. В PhpStorm metadata есть полезная для линтера директива override… Она позволяет описать, например, что мы принимаем массив типа T[] и возвращаете тип Т (полезно для функций array_pop).


Phpstorm-stubs подгружается в первую очередь. Метаданные мы используем как начальную информацию о типах — фундамент. Этот фундамент поглощается линтером и мы начинаем анализировать источники (source).

Подгружаем актуальный master перед поглощением. Проверяем код в двух режимах:

• локальные изменения: относительно baseline находим новые ошибки в коде;
• указываем диапазон ревизий: первую и последнюю ревизию, а между ними все включительно — это новый код, а все, что «до» — старый.

Дальше идет этап анализа.



Анализ AST. У нас теперь есть метаданные, информация о типах. Берем все PHP-source, парсим и анализируем прямо над AST — у нас пока что нет промежуточного представления. Анализ над сырым AST не очень удобен, особенно если вы зависите от библиотек и типов данных, которые она представляет. 



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

Отчеты и фильтрация. Дальше мы дважды генерируем отчеты или предупреждения: сначала находим предупреждения для старой версии кода (до baseline), потом для новой. Отчеты фильтруем сравнением (diff) — ищем предупреждения, которые появились в новой версии кода и передаем пользователю. В некоторых статических анализаторах это называется «режимом baseline».



Двойной анализ кода (в режиме diff) это очень медленно. Но мы можем себе это позволить — NoVerify все равно в десятки раз быстрее других линтеров на РНР. При этом в нем есть задел для дополнительного ускорения, минимум, на 30 процентов.

Как мы анализируем файлы? В PHP можно вызвать функцию до того, как она определена — нужно знать информацию об этой функции до ее анализа. Поэтому сначала мы проходим весь файл по AST, индексируем, выявляем типы всех функций, регистрируем классы и только после анализируем. 



Анализ – это второй проход по файлу. Большинство интерпретаторов и компиляторов тоже работают с двумя проходами и больше. Чтобы не «сканировать» файл второй раз, у вас должны быть декларации до использования, как в C, например.

Вывод типов

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

Как выглядит модель.


Семантическая модель (демонстрационная).

Виды типов:

• Expected — то, что мы описываем в комментариях.Мы ожидаем какие-то типы в программе, но это не значит, что они действительно в ней используются.
• Actual – реальные, те, что есть в программе. Например, если присваиваем чему-то число, то очевидно, что int или float (если это число с плавающей точкой) будут Actual-типами . 

Кажется, что типы Actual «сильнее» – они же реальные, соответствуют действительности. Но иногда мы можем получить тип только по аннотации.

Аннотации (в типах Expected) можно разделить на две категории: доверительные и недоверительные. Например, phpstorm-stubs относятся к первой категории. Они считаются отмодерированными (без ошибок) до того, как мы их используем. Недоверительные это те, что пишут остальные разработчики, потому что у них могут быть ошибки.

Actual-типы тоже можно разделить на несколько частей: values, assertion, predicates и type hint, который расширяет возможности PHP 7. Но есть проблема, которую type hint не решает.

Expected vs Actual

Допустим, у класса Foo есть наследник. Из класса-наследника можем вызывать методы, которых нет в Foo, потому что наследник расширяет родителя. Но если мы получим наследника Foo из new static() с этой аннотацией возвращаемого типа (из self), то возникнет проблема. Мы сможем вызывать этот метод, но IDE не будет подсказывать — нужно указать static(). Это позднее статическое связывание в PHP, когда может вернуться не Foo, а наследник класса. 

class Foo {
    /** @return static */
    public function newStatic() : self {
        return new static();
    }
}
// actual = Foo
// expected = static

Когда мы пишем new static(), то вернуться может не только класс new Foo. Например, если от Foo наследуется класс bar, то там может быть new bar. Соответственно, нам нужно, как минимум, две информации о типах. Никакая из них не избыточна — обе нужны.

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

class Foo {
    /** @return static */
    public function newStatic() : self {
        return new static();
    }
}
// actual - для PHP интерпретатора
// expected - подсказка для IDE/линтера

Type hint

Статическая типизация и type hint не одно и то же.

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

declare(strict_types=1);
    function f(T $foo) {
        $foo = 10; // Присвоили int
        return $foo;
}

Пример сложнее — возвращаем foo? В начале функции мы определили, что foo это T, и здесь нет никакой информации о возврате. 

declare(strict_types=1);
function f(T $foo) {
    $foo = 10; // Присвоили int
    return $foo;
}
// ? 1. f -> int
// ? 2. f -> T|int
// ? 3. f -> T

Какой из типов правильный? Первые два, разберем разницу между ними. PhpStorm и линтер выводят второй вариант. Несмотря на то, что всегда возвращается int, выводится тип T|int — «объединение» типов. Это тип, которому можно присвоить оба эти значения: сначала у нас была информация о типе T, потом мы присвоили 10, значит тип переменной foo должен быть совместим с этими двумя типами.

Аннотации

Комментарии и аннотации могут лгать.

В примере ниже мы написали, что возвращаем число, но возвращаем строку. Если бы линтер работал только на уровне аннотаций и type hint, то мы бы считали, что всегда возвращается int. Но Actual типы как раз помогают от этого уйти: здесь тип Expected – это int, а Actual — строка. Линтер знает, что возвращается строка и может предупредить, что вы обещали вернуть int. Нам важно это разделение.

/** @return int */
function f() { return "I lied!"; }

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

interface IFoo {
    /** @return int */
    public function foo();
}
class Fooer implements IFoo {
    /** @inheritdoc */
    public function foo() { return "10"; }
}

Что возвращает этот метод? Кажется, возвращается то, что описано в интерфейсе — int, но на деле строка. Это не хорошо: PHP все равно, но нам важна сходимость.

Есть два варианта поправить этот код. Очевидный — возвращать int. Но, возможно, нужно возвращать другой тип. Что делать? Написать, что возвращаем строку. В этом случае явная информация о типах необходима и IDE, и линтеру, чтобы правильно анализировать код.

interface IFoo {
    /** @return int */
    public function foo();
}
class Fooer implements IFoo {
    /** @return string */
    public function foo() { return "10"; }
}

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

В PhpStorm и в линтере есть наборы непересекающихся багов, когда мы используем одни и те же файлы для метаданных (типов). Если мы поправим все, что нам нужно, в phpstorm-stubs из JetBrains-репозитория, то сломается, скорее всего, IDE. Если оставить все по умолчанию — у нас не все правильно будет работать

Поэтому у нас есть небольшой форк — VKCOM/phpstorm-stubs. В него добавлена пара патчей, чтобы исправить то, что не сходится. Не могу рекомендовать его для PhpStorm, но он необходим для работы линтера.

Open Source

Noverify это Open Source проект. Он выложен на GitHub.

Краткая инструкция «если что-то пошло не так».

Если что-то сломалось или не запустилось. Неправильная реакция — негодовать и удалить NoVerify. Правильная реакция: оформить тикет на GitHub и рассказать о своей проблеме. Скорее всего, она будет решена за 1-2 дня.

Вам не хватает какой-то фичи. Неправильнаяреакция: удалить NoVerify и написать свой линтер (хотя написать свой линтер всегда круто). Правильная реакция: оформить тикет на GitHub и, возможно, мы добавим новую функцию. С фичами сложнее чем с ошибками — возникает обсуждение, а видение реализации в команде у каждого разное. Но в итоге они все равно реализуются.

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

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

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