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


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


В этой публикации мы расскажем о том, как пришли к пониманию необходимости единого code style, и выработали методы его постепенного внедрения в масштабный проект. Этот опыт может быть интересен тем, кто пока не использует стандартизацию, но уже ощущает в этом потребность.


Что мы имеем


  • База кода (PHP, MySQL, HTML, SCSS, JavaScript), формировавшаяся на протяжении 10 лет;
  • Негласное понимание общего формата, к которому можно отнести:
    — snake_case для названий переменных и функций в бэкенде;
    — базовые правила форматирования (отступы, пробелы, переносы);
    — регламент написания sql запросов;
    — наименования таблиц и колонок;
    — венгерскую нотацию для переменных, содержащих вводные данные.
    Это лишь несколько примеров тех нюансов, которых существует огромное множество в каждом проекте.

Чего у нас нет?


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


Осознание проблемы


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


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


for ($i=0; $i<$num; $i++)
{
    if (in_array($items[$i]['value'],$filtered))
    continue;

    array_push($valid, $items[$i]);

    if(!$items[$i]['in_menu']) continue;

    $in_menu[] = $items[$i];
}

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


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


Несогласованный выбор методологии в вёрстке и последующее редактирование иногда приводили к подобному:


<div class="i-article__item__item__content">...</div>

или


<div class="i-article__about-article-block_dropdown-handler">...</div>

Если в первом случае прослеживается использование БЭМ, то во втором случае понять «что происходит» вообще невозможно. Кроме того, использование БЭМ в наименовании классов в HTML иногда вовсе не влекло за собой преимуществ в CSS. Класс из первого варианта, содержащий в себе описание вложенности элемента, всё равно описывался с помощью вложенных стилей в CSS:


.i-article__item .i-article__item__item .i-article__item__item__content {
...
}

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


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


Выработка решения


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


Далее следовало определиться с самим форматом. Выяснилось, что у участников команды были существенные различия во взглядах на «правильный стиль»: одни предпочитали camelCase, другие настаивали на использовании snake_case во всём, даже в клиентском JavaScript.



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


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


За довольно короткий период мы сформировали собственные style guide по php, sql и частично JavaScript коду. На очереди были CSS и SCSS.


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


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


Мы использовали Sass, CSS, а ознакомление и согласование поправок заняло у команды не более 3 часов.


Контроль за исполнением


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


Выбор пал на SonarQube – платформу статического анализа кода, поддерживающую разные языки программирования и предлагающую несколько форматов использования, в том числе selfhosted, наряду с облачной версией (SonarCloud). К тому же, есть интеграция с нашей платформой CI (Teamcity) и возможность предварительной проверки в редакторе кода (SonarLint).


Процесс установки системы вместе с пакетом плагинов занял не более получаса, интеграция с Teamcity – чуть более часа. Нам предстояло сформировать так называемые «профили качества» – набор правил для каждого языка, используемых во время проверки. Иначе результат анализа не отражал бы реальную картину.


На настройку профилей под наши предпочтения потребовалось около трёх часов. Интересно, что для PHP по умолчанию предлагается 3 варианта: «PSR2», «Drupal» и фирменный «Sonar way» – многие, вероятно, смогут использовать систему «из коробки».


После установки и настройки наступил «момент истины» – тест проверки качества кода на соответствие сформулированным правилам. Запускаем команду sonar-scanner и ждем. На индексацию файлов и проверку более 500 тыс. строк кода ушло около 4 минут. Результат анализа доступен в довольно симпатичном и удобном интерфейсе:



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


Что дальше?


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


Если у вас был подобный опыт внедрения code style – будет интересно обсудить это в комментариях!

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


  1. T-362
    20.09.2017 17:09

    Немного странный подход, правильный конечно, но ИМХО лучше с другой стороны. Наш пример — были взяты стандартные PSR темплейты от phpcs, phpmd и phpstorm, слегка дополнены персональными предпочтениями и были розданы разработчикам. Собственно и все, IDE автоматически форматирует и грязно ругается на попытки писать не по стандартам. Никто не накоммитит не по стандартам, если не может писать не по стандартам.


    А вот рефакторинг старого кода это отдельная история.


    1. dvalid Автор
      20.09.2017 17:45
      +1

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


      1. Vorchun
        20.09.2017 20:51

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


        1. kloppspb
          20.09.2017 21:04
          +2

          Мы думали проверять при заливке в SVN. Но это стопорит.

          Чем стопорит?
          Например, pre-commit хук в гите, который форматирует исходник сам (если, конечно, правила формализуются, и могут быть применены автоматически), ничуть не стопорит. Ну, при условии, что за один коммит не заливается хрелион файлов мегабайтных размеров :)


          1. Dreyk
            20.09.2017 22:11

            pre-commit хук точно также можно выключить


            для проверки нужен pre-receive server-side hook. когда-то делал так с ПХП, работало нормально. проблемой было то, что стоит только тронуть один из старых файлов, и приходилось целиком его чистить


            1. kloppspb
              20.09.2017 22:39

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


          1. Vorchun
            21.09.2017 10:54

            да, все правильно говорите )


    1. Cromathaar
      21.09.2017 11:55
      +1

      Наличие конвенции важнее самой конвенции. Разруха — она, как известно, в головах.