Как внедрить статический анализатор кода в legacy проект и не демотивировать команду20.06.2020 23:36

Попробовать статический анализатор кода легко. А вот, чтобы внедрить его, особенно в разработку большого старого проекта, потребуется умение. При неправильном подходе анализатор может добавить работы, замедлить разработку и демотивировать команду. Давайте кратко поговорим, как правильно подойти к интеграции статического анализа в процесс разработки и начать его использовать как часть CI/CD.

Введение

Недавно моё внимание привлекла публикация «Getting Started With Static Analysis Without Overwhelming the Team». С одной стороны, это хорошая статья, с которой стоит познакомиться. С другой стороны, как мне кажется, в ней так и не прозвучало полного ответа, как безболезненно внедрить статический анализ в проект с большим количеством legacy кода. В статье говорится, что можно смириться с техническим долгом и работать только с новым кодом, но нет ответа на то, что делать с этим техническим долгом потом.

Наша команда PVS-Studio предлагает свой взгляд на эту тему. Давайте рассмотрим, как вообще возникает проблема внедрения статического анализатора кода, как преодолеть эту проблему и как безболезненно постепенно устранить технический долг.

Проблематика

Запустить и посмотреть, как работает статический анализатор, как правило, несложно [1]. Можно увидеть в коде интересные ошибки или даже страшные потенциальные уязвимости. Можно даже что-то поправить, но вот дальше у многих программистов опускаются руки.

Все статические анализаторы выдают ложные срабатывания. Такова особенность методологии статического анализа кода, и с ней ничего нельзя сделать. В общем случае это нерешаемая задача, что подтверждается теоремой Райса [2]. Не помогут и алгоритмы машинного обучения [3]. Если даже человек не всегда может сказать, ошибочен тот или иной код, то не стоит ждать этого от программы :).

Ложные срабатывания не являются проблемой, если статический анализатор уже настроен:

Отключены неактуальные наборы правил;
Отключены отдельные неактуальные диагностики;
Если мы говорим о C или C++, то размечены макросы, содержащие специфические конструкции, из-за которых появляются бесполезные предупреждению в каждом месте, где такие макросы используются;
Размечены собственные функции, выполняющие действия аналогичные системным функциям (свой аналог memcpy или printf) [4];
Точечно с помощью комментариев отключены ложные срабатывания;
И так далее.

В таком случае, можно ожидать низкий уровень ложных срабатываний на уровне порядка 10–15% [5]. Другим словами, 9 из 10 предупреждений анализатора будут указывать на реальную проблему в коде или, по крайней мере, на «код с сильным запахом». Согласитесь, такой сценарий крайне приятен, и анализатор является настоящим другом программиста.

В реальности в большом проекте первоначальная картина будет совсем иная. Анализатор выдаёт сотни или тысячи предупреждений на legacy код. Что из этих предупреждений по делу, а что нет, быстро понять невозможно. Сесть и начать разбираться со всеми этими предупреждениями нерационально, так как основная работа в этом случае остановится на дни или недели. Как правило, команда не может позволить себе такой сценарий. Также возникнет огромное количество diff-ов, которые портят историю изменений. А быстрая массовая правка такого количества фрагментов в коде неизбежно выльется в новые опечатки и ошибки.

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

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

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

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

Здесь та же аналогия, что и с предупреждениями компилятора. Неспроста рекомендуют держать количество предупреждений компилятора на уровне 0. Если предупреждений 1000, то, когда их станет 1001, на это никто не обратит внимание, да и не понятно, где искать это самое новое предупреждение.

Неправильное внедрение статического анализа

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

Внедрение и устранение технического долга

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

CI/CD

Причём анализатор можно сразу сделать частью процесса непрерывной разработки. Например, в дистрибутиве PVS-Studio есть утилиты для удобного просмотра отчёта в нужном вам формате, и уведомления разработчиков, написавших проблемные участки кода. Тем, кто более подробно интересуется запуском PVS-Studio из-под CI/CD систем, рекомендую ознакомиться с соответствующим разделом документации и циклом статей:
Но вернемся к вопросу большого количества ложных срабатываний на первых этапах внедрения инструментов анализа кода.

Фиксация существующего технического долга и работа с новыми предупреждениями

Современные коммерческие статические анализаторы позволяют изучать только новые предупреждения, появляющиеся в новом или изменённом коде. Реализации этого механизма различается, но суть одна. В статическом анализаторе PVS-Studio эта функциональность реализована следующим образом.

Чтобы быстро начать использовать статический анализ, мы предлагаем пользователям PVS-Studio воспользоваться механизмом массового подавления предупреждений [6]. Общая идея в следующем. Пользователь запустил анализатор и получил множество предупреждений. Раз проект, разрабатываемый много лет, жив, развивается и приносит деньги, то, скорее всего, в отчёте не будет много предупреждений, указывающих на критические дефекты. Другими словами, критические баги так или иначе уже поправлены более дорогими способами или благодаря фидбеку от клиентов. Соответственно, всё, что сейчас находит анализатор, можно считать техническим долгом, который непрактично стараться устранить сразу.

Можно указать PVS-Studio считать эти предупреждения пока неактуальными (отложить технический долг на потом), и он больше не будет их показывать. Анализатор создаёт специальный файл, где сохраняет информацию о пока неинтересных ошибках. И теперь PVS-Studio будет выдавать предупреждения только на новый или измененный код. Причем, всё это реализовано умно. Если, например, в начало файла с исходным кодом будет добавлена пустая строка, то анализатор понимает, что, по сути, ничего не изменилось, и по-прежнему будет молчать. Этот файл разметки можно заложить в систему контроля версий. Файл большой, но это не страшно, так как часто его закладывать смысла нет.

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

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

Исправление ошибок и рефакторинг

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

if (a = b)

На подобный код ругается большинство С++ компиляторов и анализаторов, так как высока вероятность, что на самом деле хотели написать (a == b). Но существует негласное соглашение, и это обычно отмечено в документации, что если стоят дополнительные скобки, то считается, что программист сознательно написал такой код, и ругаться не надо. Например, в документации PVS-Studio к диагностике V559 (CWE-481) явно написано, что следующая строчка будет считаться корректной и безопасной:

if ((a = b))

Другой пример. Забыт ли в этом C++ коде break или нет?

case A:
  foo();
case B:
  bar();
  break;

Анализатор PVS-Studio выдаст здесь предупреждение V796 (CWE-484). Возможно, это не ошибка, и тогда следует дать анализатору подсказку, добавив атрибут [[fallthrough]] или, например, __attribute__((fallthrough)):

case A:
  foo();
  [[fallthrough]];
case B:
  bar();
  break;

Можно сказать, что подобные изменения кода не исправляют ошибки. Да, это так, но выполняется два полезных действия. Во-первых, отчёт анализатора избавляется от ложных срабатываний. Во-вторых, код становится более понятным для людей, занимающихся его сопровождением. И это очень важно! Ради одного этого уже стоит проводить мелкий рефакторинг, чтобы код стал понятней и легче в сопровождении. Раз анализатору непонятно, нужен «break» или нет, это будет непонятно и коллегам-программистам.

Помимо исправления ошибок и рефакторинга, можно точечно подавлять явно ложные предупреждения анализатора. Какие-то неактуальные диагностики можно отключить. Например, кто-то считает бессмысленными предупреждения V550 о сравнении значений типа float/double. А кто-то относит их к важным и заслуживающим изучения [7]. Какие предупреждения считать актуальными, а каким нет, решать только команде разработчиков.

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

Также, мы всегда помогаем нашим клиентам настроить PVS-Studio, если возникают какие-то сложности. Более того, были случае, когда мы сами устраняли ложные предупреждения и исправляли ошибки [8]. На всякий случай решил упомянуть, что возможен и такой вариант расширенного сотрудничества :).

Метод храповика

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

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

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

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

У автора статьи есть и доклад на эту тему: «Непрерывный статический анализ».

Заключение

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

Существуют и другие типовые сомнения, сможет ли статический анализатора действительно быть удобен и полезен. Я постарался развеять большинство таких сомнений в публикации «Причины внедрить в процесс разработки статический анализатор кода PVS-Studio» [9].

Спасибо за внимание, и приходите скачать и попробовать анализатор PVS-Studio.

Дополнительные ссылки

Андрей Карпов. Как быстро посмотреть интересные предупреждения, которые выдает анализатор PVS-Studio для C и C++ кода?
Wikipedia. Теорема Райса.
Андрей Карпов, Виктория Ханиева. Использование машинного обучения в статическом анализе исходного кода программ.
PVS-Studio. Документация. Дополнительная настройка диагностик.
Андрей Карпов. Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10–15% ложных срабатываний.
PVS-Studio. Документация. Массовое подавление сообщений анализатора.
Иван Андряшин. О том, как мы опробовали статический анализ на своем проекте учебного симулятора рентгенэндоваскулярной хирургии.
Павел Еремеев, Святослав Размыслов. Как команда PVS-Studio улучшила код Unreal Engine.
Андрей Карпов. Причины внедрить в процесс разработки статический анализатор кода PVS-Studio.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. How to introduce a static code analyzer in a legacy project and not to discourage the team.