Посмотреть содержание

Вызывает интерес ваш технический прогресс.
Как у вас там сеют брюкву, с кожурою, али без?

© «Сказка про Федота-стрельца, удалого молодца»,
Леонид Филатов —

Программист — творческая профессия. Мы создаем что-то новое, руководствуясь своими знаниями, внутренним пониманием качества и поставленными дедлайнами. Дедлайны и знания пока оставим в стороне и сосредоточимся на качестве.

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

«Что такое плохой/хороший код» — вопрос риторический. В нашей статье под качественным кодом мы будем понимать:

• корректный с точки зрения платформы (без запросов к БД в шаблонах компонентов, запросы к БД с использованием индексов),

• совместимый с актуальными версиями PHP (5.6, 7.0),

• соответствующий единым стандартам кодирования, принятым в команде,

• без ляпов (запросы к БД в цикле, запросы к внешним системам на хитах),

• без уязвимостей (XSS, CSRF и т.д.).

С написанием хорошего кода отлично справляются самые опытные из нас: тимлиды. Но их мало (в штуках), а их время стоит дорого. У менее опытных разработчиков бывают проблемы со всем вышеперечисленным.

Как обеспечить качество кода с точки зрения методологии известно. Нужно развивать в команде процессы Continuous Integration: версионирование, тестирование, code review. Мы решили разобраться: какие инструменты подойдут для автоматизации code review при промышленной веб-разработке на PHP. А заодно проверим, как найденные инструменты оценят код нашего собственного сайта и нескольких решений из Marketplace 1С-Битрикс.

Инструменты для автоматизации code review PHP

Методика поиска инструментов

Мы нашли на github.com и packagist.org самые оцениваемые и самые скачиваемые проекты, связанные с анализом кода PHP. В обзор мы включили только надежные (созданы не вчера), поддерживаемые (есть сообщество и контрибьюторы) и популярные (количество звезд-«лайков»).

Выделили три категории инструментов:

• проводящие анализ кода с целью поиска проблемных мест;

• проверяющие совместимость версий PHP 5-7;

• проверяющие уязвимости.

Инструменты для анализа PHP-кода

PHP Code Sniffer анализирует PHP, CSS и JavaScript-файлы на соответствие стандартам кодирования, находит и исправляет ошибки. Стандарт представляет собой совокупность sniff-файлов, задающих правила. Количество установок анализатора с 2016 года превысило ~330 тысяч. Использование зафиксировано в ~7500 проектах.

PHP CS Fixer — инструмент, разработанный автором фрэймворка Symfony. Обнаруживает и исправляет ошибки в коде. Помимо следования стандартам кодирования (PSR-1, PSR-2 и др.). Позволяет писать собственные правила. Поддерживает PHP 7. Проект стремительно развивается и приобретает популярность как среди пользователей (~325 тысяч установок с начала 2016 года), так и у разработчиков (с 2017 года используется в ~1500 проектах).

PHP Mess Detector определяет ошибки кода, неоптимальные и усложненные места, неиспользуемые переменные, методы, свойства. Позволяет создавать пользовательские правила. Поддерживает PHP 7. Сохраняет отчеты в трех форматах: текстовый, html, xml. С 2016 года пользователи установили ~315 тысяч раз, используется в ~2000 проектах.

PHP Metrics — инструмент статического анализа для PHP. Выдает информацию о проекте и используемых классах в виде сгенерированного сайта. Поддерживает PHP 7. С 2016 года зарегистрировано ~260 тысяч скачиваний.

PHP Static Analysis Tool анализирует количество и типы параметров, передаваемых конструкторам, методам и функциям; типы, возвращаемые методами и функциями и т.д. Инструмент включает поддержку PHP 7 и предоставляет написание собственных правил. Количество скачиваний с 2016 года ~150 тысяч раз, использован разработчиками в ~200 проектах.

PHP Copy Paste Detector выдает информацию о дублированных участках кода. Проект набирает популярность (количество установок с 2016 года составляет ~100 тысяч раз, используется в ~1000 проектах).

Phan — статический анализатор для PHP. Проверяет определение и доступность методов, функций, классов, traits, интерфейсов, констант, свойств и переменных, обнаруживает неиспользуемый код, проверяет код на обратную совместимость php7 к php5. С 2016 года зафиксировано ~80 тысяч раз скачиваний.

PHP Dead Code Detector сообщает пользователю о функциях и методах, которые не используются в коде. Развитие проекта было остановлено в 2015 году. Начиная с 2016 года, количество установок пошло на спад.

Qafoo Quality Analyzer — инструмент, предназначенный для визуализации метрик исходного кода. Начиная с 2015 года, инструмент установлен пользователями ~4800 раз.

Инструменты для контроля совместимости кода с разными версиями PHP

PHP Compatibility — набор правил для PHP Code Sniffer, которые проверяют совместимость текущего кода с другими версиями PHP, включая PHP 7. Начиная с 2016 года, инструмент установлен пользователями ~22 тысячи раз.

PHP 7 Compatibility Checker — инструмент для проверки кода PHP 5.3 — 5.6 на совместимость с PHP 7. Находит потенциальные проблемы в коде и генерирует отчеты, содержащие имена файлов, номера строк и краткое описание проблемы. При этом проблемы двух типов: ошибки, вызывающие серьезные проблемы (фатальная ошибка, ошибка синтаксиса и т.д.) и предупреждения, приводящие к логическим ошибкам. Начиная с 2016 года, инструмент установлен пользователями ~15 тысяч раз.

Инструменты для поиска уязвимостей в PHP-коде

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

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

Что на выходе

PHP CS Fixer

Результат запуска PHP CS Fixer — единый патч-файл, в котором указываются:

• файлы, в которых обнаружены ошибки;

• правила, которые сработали;

• изменения, затронувшие строки файла: удаленные, оставшиеся без изменений, добавленные строки.

Пример:

PHP Code Sniffer

Итог работы PHP Code Sniffer — список файлов и сопутствующих им сообщений о том, сколько ошибок может быть автоматически исправлено, а также таблиц с описаниями нарушений в виде: строка, вид (ошибка, предупреждение), возможность исправления, описание.

Пример:

PHP Mess Detector

Отчет в PHP Mess Detector генерируется в одном из трех форматов: текстовый, html, xml. Полученный файл содержит информацию об обнаруженных проблемах в файлах. Проблема описывается начальной и конечной строкой, сработавшим правилом и поясняющим сообщением.

Пример:

PHP Dead Code Detector

Отчет в PHP Dead Code Detector — это список неиспользуемых функций и методов с указанием количества занимаемых строк, файла и номера строки в файле.

Пример:

PHP Copy Paste Detector

В отчет по PHP Copy Paste Detector заносятся файлы, в которых дублируются строки.

Пример:

PHP Static Analysis Tool

В отчете PHP Static Analysis Tool для каждого файла, в котором обнаружены ошибки, указывается номер строки в файле и описание ошибки. В конце отчета приводится общее количество найденных ошибок.

Пример:

Инструмент показал себя не лучшим образом и был исключён из анализа. Почему — читайте в конце статьи.

Phan

Результат работы в Phan может быть выведен в следующих режимах: ’text’, ’json’, ’csv’, ’codeclimate’, ’checkstyle’, и ’pylint’. В режиме ’csv’ выводятся следующие столбцы: файл, строка, категория ошибки, phan-тип ошибки, сообщение.

Пример:

PHP Compatibility

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

Пример:

PHP 7 Compatibility Checker

При проверке на совместимость PHP 7 Compatibility Checker генерирует список нарушений в коде: указывается файл, строка, поясняющее сообщение и ошибочный код.

Пример:

Qafoo Quality Analyzer

Qafoo Quality Analyzer генерирует отчет в формате xml, где для каждого файла приводится список обнаруженных ошибок с описанием.

Пример:

PHP Metrics

PHP Metrics по окончании своего выполнения генерирует количественный отчет по метрикам и визуализирует полученные данные в виде созданного сайта.

Пример:

Итоговая таблица сравнения инструментов

Апробация инструментов на 1С-Битрикс

Каждый из инструментов был опробован на коде:

• тиражных решений из Marketplace 1С-Битрикс (3 платных решения, 5 бесплатных),

• современного сайта ИНТЕРВОЛГИ (на 1С-Битрикс),

• устаревшего сайта ИНТЕРВОЛГИ (на самописной CMS).

Прежде чем подвести итоги, сообщаем что инструмент PHP Static Analysis Tool был дисквалифицирован и исключён из соревнования. Причина — 90% его ошибок это «неизвестная константа», «неизвестный метод», «неизвестный класс» ядра Битрикса (мы проверяли только папку с решением). То есть, чем больше решение использовало API Bitrix, тем больше в нём было «ошибок». Попытка добавить ядро Битрикса в анализ привела к аварийному завершению работы инструмента с ошибкой «Класс Bitrix\Main\SystemException объявлен дважды». И действительно, в ядре множество классов объявленных в разных местах. Решив «а давайте временно уберём этот класс», получили другую ошибку. Убрали её — получили третью и т.д. Попытки прекратили после 13-ой итерации.

Теперь к итогам сравнения решений. Мы считали не общее количество ошибок, а количество ошибок на 1000 строк кода.

ВАЖНО! Чтобы получить максимально показательную картину, мы суммировали ВСЕ замечания ВСЕХ 15 инструментов. Не удивлятесь что число ошибок часто больше числа строк.

Места распределились таким образом:

1. Старый сайт ИВ (703 ошибки на 1000 строк)

2. 1С-Битрикс: Современный интернет-магазин (1457 ошибки на 1000 строк)

3. Новый сайт ИВ (1881 ошибки на 1000 строк)

4. 1С-Битрикс: Корпоративный сайт (1998 ошибки на 1000 строк)

5. Платный ИМ (2506 ошибки на 1000 строк)

6. Платный ИМ (2525 ошибки на 1000 строк)

7. Платный ИМ (2682 ошибки на 1000 строк)

8. Бесплатный ИМ (3542 ошибки на 1000 строк)

9. 1С-Битрикс: Информационный портал (3578 ошибки на 1000 строк)

10. Бесплатный ИМ (7361 ошибки на 1000 строк)

ВАЖНО! Чтобы получить максимально показательную картину, мы суммировали ВСЕ замечания ВСЕХ 15 инструментов. Не удивлятесь что число ошибок часто больше числа строк.

Подробные результаты приведены в таблице.

То, что лучшим стал старый сайт ИВ нас искренне удивило — мы ожидали прямо противоположного результата. У этого проекта 2 рекордных показателя — количество ошибок и количество строк кода. Так и вышло, что на 1000 строк кода проблем меньше всего. Но старый сайт — «почётный» участник, посмотрим на реальные результаты.

Настоящий победитель нашего исследования — 1С-Битрикс: Современный интернет-магазин. Что можно о нём сказать по результатам исследования:

1. Из 1000 строк в этом решении — в среднем 17 строк «копипасты».

2. В 2 строках есть вызовы устаревшего API.

3. Есть 6 нарушений при работе с методами и полями.

4. 284 строки кода можно отформатировать автоматически с помощью PHP CS Fixer.

5. Почти нет проблем с совместимостью PHP.

Второе место получает новый сайт ИНТЕРВОЛГИ. Разрабатывавшийся силами всей компании в разные годы, он как-то смог не уронить высокую планку качества, заданную компанией 1С-Битрикс.

Третье место — 1С-Битрикс: Корпоративный сайт. Снова 1С-Битрикс показал всем, как надо делать сайты.

Худшим решением по мнению «жюри» признан бесплатный ИМ из ТОПа маркетплейса.

Добавим ложку дёгтя в бочку мёда компании 1С-Битрикс — вторым с конца оказалось тоже их решение, на которой выросло не одно поколение программистов.

Предпоследнее место — 1С-Битрикс: Информационный портал.

Что касается инструментов... По-настоящему полезным, почти без оговорок, показал себя PHP Copy Paste Detector — на нашем сайте, например, он находил блоки кода по 50-100 строк, повторяющиеся в 2-3 местах. Единственная оговорка — часто он ругается на вызовы вложенных компонентов (например, catalog.element внутри catalog).

Отличить настоящие ошибки от ложного срабатывания было сложно в PHP Dead Code Detector — тут оказывались обработчики событий, агенты, конструкторы и инсталляторы, которые явным образом нигде не вызываются в коде или вызываются извне модулей.

Насчёт PHP Static Analysis Tool уже было сказано, что почти все его ошибки были «ложными срабатываниями».

Хорошо показал себя PHP Mess Detector: отследил мелкую, но неприятную ошибку статического вызова динамических методов и наоборот. В то же время он ругался на каждый else, так как «else is never necessary and you can simplify the code to work without else».

Главный «хлеб» таких инструментов, как Qafoo Quality Analyzer, PHP Code Sniffer, PHP CS Fixer: превышение длины строк, переводы строк в конце файла, короткие-длинные открывающие php-теги, пробелы после ключевых слов if, for, while и т.п. Из предлагаемого инструментами набора правил мы выбрали наиболее похожие на стандарт оформления кода 1С-Битрикс, но всё равно получили около 50% ложных срабатываний.

Полезную статистику предоставляет PHP Metrics, если в вашей команде есть понимание, сколько операторов для класса уже «много», а сколько — «ещё нормально». Аналогично с цикломатической сложностью.

Вывод

Смысл затеи с инструментами проверки качества кода был в том, чтобы выяснить какой инструмент стоит выбрать для автоматизации Code Review. Эту цель мы достигли и свой выбор остановили на PHP CS Fixer и PHP Code Sniffer. Они адекватны задаче, популярны, развиваются, их можно расширять, и есть масса уже готовых тестов. Осталось только адаптировать их к реалиям разработки сайтов на 1С-Битрикс: Управление Сайтом.

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

• Инструменты для автоматизации code review PHP

  • Методика поиска инструментов

  • Инструменты для анализа PHP-кода

  • Инструменты для контроля совместимости кода с разными версиями PHP

  • Инструменты для поиска уязвимостей в PHP-коде

• Что на выходе

  • PHP CS Fixer

  • PHP Code Sniffer

  • PHP Mess Detector

  • PHP Dead Code Detector

  • PHP Copy Paste Detector

  • PHP Static Analysis Tool

  • Phan

  • PHP Compatibility

  • PHP 7 Compatibility Checker

  • Qafoo Quality Analyzer

  • PHP Metrics

  • Итоговая таблица сравнения инструментов

• Апробация инструментов на 1С-Битрикс

• Вывод

Василий Вишняков
Генеральный директор интернет-агентства Bquadro

На этапе Code Review нельзя полагаться исключительно на автоматическое тестирование, так как это не позволит достичь желаемого результата. Да, оно может закрыть пробелы в знаниях, если часть команды чего-то не умеет, и поможет избежать случайных ошибок у профессионалов. Однако подобные сервисы не позволят прийти к единым стандартам проектирования. В этом плане лучше всего пользоваться уже готовыми паттернами, такими как БЭМ и MVC. При этом нужно заранее договориться об определённом типе проектирования информационной системы.

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

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

Николай Апурин
Генеральный директор ООО «АРТВЕЛЛ»

На российском рынке за качеством разработки никто не следит. Ни один проект за последние 5 лет мы не приняли в адекватном виде.

Тем не менее — мы документируем код и работаем по ГОСТу в рамках предоставления отчетной документации. Наши разработчики работают слаженными рабочими группами через git/redmine/svn/jira и т.д. и ведут продукт открыто и понятно для любого стороннего разработчика.

Статья носит хороший теоретический смысл, но, как показывают исследования, есть лишь единицы компаний, у которых в штате есть хотя бы 50 разработчиков. Как правило — их 0.

Максим Ляпцев
Ведущий разработчик digital-агентства webit

В своей работе мы используем систему контроля версий. А банальные мердж реквесты позволяют ведущему программисту проверять код. Иногда достаточно специалиста, который был вовлечен в проект дольше всех: он знает, что уже было сделано и не позволит велосипедам проникнуть в код. Такой подход особенно актуален при развитии сайта и его технической поддержки. Новые программисты, незнакомые с проектом, или с фреймворком, по всей вероятности, пойдут по пути меньшего сопротивления и напишут свои, удобные им методы, не найдя или считая плохо написанными уже существующие. Говоря о Битриксе, можно просто не знать некоторых способов, которыми можно обойти его многочисленные проблемы и сложности. Когда в игру вступит code reviewer, последует комментарий с рекомендациями по исправлению. Не все специалисты правильно понимают эту задачу: если код не содержит архитектурных или логических ошибок, если он не создает лишнюю нагрузку и работает правильно, но написан не так, как написал бы проверяющий, не стоит считать, что код плохой — у всех стиль разный и решений одной задачи может быть множество.

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

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

Важно понимать code review не равен code style review. Касаясь стиля написания кода — проще использовать IDE, которые позволяют легко форматировать все написанное, используя стандарты кодирования PSR-2. Существуют также продвинутые текстовые редакторы и расширения для них. Например, мы используем phpStorm и его вполне достаточно для анализа и автоматического форматирования кода при комитах. Можно использовать и автоматические анализаторы, которые будут делать то же самое. Тут больше дело вкуса.

Кирилл Чистобородов
Генеральный директор MINISOL

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

Для нас главный принцип во всех бизнес-процессах: адекватность. Критерии адекватности описаны для каждого этапа работ, в т.ч. и разработки. Основной критерий — стоимость затрачиваемых ресурсов должна приносить пользу, соотносимую со стоимостью затрат. Например, мы принципиально не делаем верстку с кроссбраузерным pixel-perfect (pp). Соблюдение базовых расстояний, отступов и соотношений гораздо важнее, чем расхождение в несколько пикселей между браузерами. Пользователи сайтов никогда не будут сравнивать сайт в 2-х браузерах, это удел исключительно тестировщиков. В случае с разработкой похожая ситуация — есть вещи, целесообразность которых под вопросом, при выполнении и соблюдении базовых принципов.

Мы в разработке поддерживаем качество «пост-фактум». Например, для 1С-Битрикс есть встроенные инструменты тестирования, а также есть внешние инструменты оценки скорости загрузки страниц, нагрузочного тестирования и обеспечения безопасности кода. Если проект не проходит регламентное тестирование, то мы начинаем изучать проблему и разбираться, но такое бывает редко на проектах, разработанных в MINISOL.

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

Оригинал: https://www.intervolga.ru/blog/projects/bad-code-go-away/

Иллюстрация: https://img1.goodfon.ru/original/1366x768/0/e3/programmist-noutbuk-kompyuter.jpg