Проверка соблюдения стандартов кодирования РHP через git

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

Для того чтобы наказать говнокодеров облегчить страдания тех, кто делает ревью кода, можно использовать автоматические средства проверки кода, которые всем давно известны. Это PEAR и PHP Code Sniffer.


Большинство проектов компании где я работаю, сделаны на Drupal, поэтому будет рассмотрен пример проверки соблюдения стандартов кодирования Drupal, хотя все описанные ниже средства можно применить и для других стандартов, например Zend.

Сравнительно недавно появился набор правил для проверки стандартов кодирования Drupal через PHP Code Sniffer. Установить и настроить его очень просто, самое сложное начинается дальше. Как же заставить всех проверять свой код? Сказывается влияние человеческого фактора, первый забыл, второй не захотел, а третий просто считает, что его код не может содержать ошибок и вообще все это лишняя трата времени.

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

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

image

Код можно будет закоммитить только тогда, когда все ошибки будут исправлены. Отлично, теперь мы не позволяем добавлять «плохой» код в репозиторий проекта! Но по прежнему остается человеческий фактор. Снова кто-то забывает добавить скрипт проверки в папку .git/hooks своего проекта, кому-то сейчас лень исправлять ошибки и он откладывает на потом и т.д., а «самые умные» просто используют git commit --no-verify, потому что сейчас им не до ошибок. В последнем случае git не вызовет хук pre-commit и измененные файлы попадут в репозиторий без проверки.

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

Отчет отправляемый по почте выглядит так:

image

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

Сильные стороны этого способа:
  • Централизованный контроль;
  • Нельзя полностью избежать проверки кода;
  • Скрипты проверки кода не изменяют исходные файлы и не блокируют добавление кода в случае хотфикса;
  • Скрипты легко установить и настроить;
  • Помимо ошибок стандартов кодирования Drupal, отображаются функциональные ошибки. Например о том, что вы забыли использовать функцию t или check_plain (а это уже дыра в безопасности) при выводе данных на страницу.

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

Ресурсы:


UPD 19.05.2012:

Добавлена возможность игнорирования проверки файлов с использованием файла конфигурации.

Для того чтобы не проверять некоторые файлы и директории, вам нужно создать в корне проекта файл .hooks_ignore. Затем поместить в нем пути к файлам и директориям, которые вы не хотите проверять. Это может быть ядро движка или модули сторонних разработчиков.
Пример содержимого файла .hooks_ignore
includes
sites/all/modules/contrib
sites/all/themes/garland/template.php

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

Исходные коды на Drupal.org и GitHub были обновлены.

Жду ваших пожеланий и замечаний по этому небольшому проекту.
Share post
AdBlock has stolen the banner, but banners are not teeth — they will be back

More
Ads

Comments 55

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

    Во-вторых, все эти стандарты, не более чем условность, а в случае с phpcs это в двойне условность, потому что никто не гарантирует соответствие его самого каким-то стандартам. К примеру, пользовался им в плоть до версии 1.3 после обновления, на которую код, который был валидным в 1.2 вдруг стал не валидным.

    >«самые умные» просто используют git commit --no-verify, потому что сейчас им не до ошибок
    Самые умные создают алиас для этой команды и вообще забывают об этом недоразумении, и имхо правильно делают.

    По-моему лучше, периодически выделять время на чистку кода, чем задалбывать всех каждый раз, при комите.
      +10
      >По-моему лучше, периодически выделять время на чистку кода, чем задалбывать всех каждый раз, при комите.

      К сожалению, такой подход обычно порождает «синдром разбитых окон», о чем хорошо написано в книге «Программист-прагматик» Соблюдать конвенцию — не слишком сложно, и стоит просто приучится делать это на автомате.
        +5
        > Соблюдать конвенцию — не слишком сложно, и стоит просто приучится делать это на автомате.

        Мы уже используем это средство в нашей компании. И то что я регулярно вижу ошибки и тут же исправляю их привело к тому что я стал очень редко допускать их ) Остальные сотрудники отмечают примерно тоже самое.
        +2
        >в плоть до версии 1.3 после обновления, на которую код, который был валидным в 1.2 вдруг стал не валидным.
        Ну это нормально произошло ужесточение стандарта в следующей версии.
        стандартизация идет шаг за шагом
        мы в свое время вводили стандарты кодирования в крупном проекте там тоже все было более сурово с каждым нововведением.

        Стандарты нужны и соблюдать и надо перманентно для того чтоб была высокая читаемость кода.
        Сколько у вас людей работает одновременно над одним проектом?
          +1
          Я использовал стандарт Zend. Там ситуация немного другая, автора снифера и зенды не поделили лицензии и послали друг друга на 3 буквы, в итоге поддержка этого стандарта, является номинальной. В одном проекте использовал анонимные функции, так и не нашел способа их объявления чтоб снивер не ругался, потому что стандарт написан под первый ZF1, а он под PHP 5.2.

          Кстати что произойдет, если кто-то добавит в код, к примеру, трит из PHP5.4 или еще что-нибудь новенькое? Он наверное долго будет пытается его закомитить с проверкой на стандарты.
            +1
            Что вам мешает написать свой снифер (правила для проверки)? Это не сложно.
              0
              Зачем мне писать снивер под себя? У меня, как и у любого опытного программиста вполне сформировавшийся стиль оформления кода. Идея снифера в том чтобы уйти от персональных стандартов к общепринятым.
                +1
                Зачем мне писать снивер под себя?
                Если существующие чем-то не устраивают.
          +13
          Во-первых, если человек использует способ оформления кода отличный от вашего, то не стоит его называть говнокодером.

          В проекте может быть только один способ форматирования, так что если разработчик в команде использует своё форматирование, отличное от принятого в проекте, то да — он говнокодер.
            0
            что вообще за манера лепить слово говнокодер где не попади?!

            В моем понимании это человек, который:
            — пишет код, который работает в зависимости от фазы луны;
            — не обрабатывает ошибки;
            — пишет не семантичный код ( когда метод getObject возвращает к примеру число).

            Если вы будете за каждый пробел, человека называть говнокодером, то ваша команда долго не протянет, либо вам устроят темную.
              +4
              Если человек плюет на разумное наставление своего лида и продолжает форматировать код в зависимости от фазы луны — то он говнокодер. Потому что ему насрать на тех, кто после него будет разгребать это.
                +1
                Возьмем, к примеру, меня, я предпочитаю зендовый стандарт, при этом я не могу сказать что человек использующий, к примеру пировский стандарт как вы выразились «срет на меня». У меня есть проекты, написанные в разных стилях, и если мне приходиться там что-то разгребать то не, потому что они оформлены в не моем стиле, а потому что код кривой.

                Имхо хороший не зависит от какогото стандарта, количества коментариев, это код который интуетивно понятен и логичен. Ну нравится человеку открывать фигурную скобочку на новой/тойже строке, помоему это не большая проблема, и уж точно неповод для открытых конфликтов
                  +1
                  Код в разных стилях в рамкох одного проекта просто мозолит глаза, и лично я всегда использую стиль проекта в котором учавствую. Но есть и более существуенные проблемы. Я, на самом деле, вообще не слежу за тем сколько пробелов я поставил и где — ИДЕЯ за меня все и так автоматически реформатит. Так вот, когда я начну пользоваться этим в чужом файле — она отреформатит весь файл, в результате маленькие изменения првевратятися в большие, что совершенно не удобно.
            0
            Лучше делать code review глазами, чем проверять на несущественные ворнинги роботами.
              0
              Это совсем не условность, а острая необходимость, если над проектом работает больше 1 человека и хоть кого-то заботит будущая поддержка продукта. Ваше отношение подходит только к работе в одиночестве. Тем более, что если уж лень тратить время на соблюдение стандарта, то на «периодическую чистку кода» его не будет совсем.
              0
              > Во-первых, если человек использует способ оформления кода отличный от вашего, то не стоит его называть говнокодером.

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

                0
                причем тут стиль оформления кода к говнокодерству?

                Нельзя человека, который пишет натюрморты называть плохим художником, только по тому, что вы предпочитаете пейзажи.
                  0
                  Особенно весело, когда каждый разработчик генерирует 100500 изменений, потому что его стиль кодирования ему больше нравится. Т.е. вместо правки 1 строки трогает либо сам либо своим IDE весь файл, что конечно же не облегчит code review и будет загрязнять проект лишними данными
                    0
                    При том, что данный человек пишет в команде, которая применяет определенный стиль.
                    И тут находится умник, которому положить на всю команду. Которой придется ДЕЛАТЬ ЭТО ЗА НЕГО.
                    А вы любите выполнять чужую работу?

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

                  Правильно, нельзя. Но я напрямую никого не обвиняю в своем посте. Просто это наболело и те кому приходилось столкнуться с этим наверняка поймут меня.
                    0
                    > Просто это наболело и те кому приходилось столкнуться с этим наверняка поймут меня.

                    Из личного опыта могу сказать, что с годами приходит понимание и терпимость. И неправильный перенос строки уже не вызывает такого желания размозжить человеку голову.
                      0
                      неправильный перенос — возможно. А вот множество однострочных конструкций, отсутствие коментариев ко всему что только можно, именование методов и классов, табы вперемешку с пробелами, 10-20 ретурнов в одном методе и тому подобные красоты, вызывает множество свойких желаний…

                      Так что CodeSniffer — это TRUE
                    –1
                    Для того чтобы наказать говнокодеров

                    Думаю говнокодеров не стоит брать на работу вообще…
                    А вот новичкам проекта полезно будет чтобы запомнить code-style проекта
                      0
                      Все замечательно, только хук надо вешать не на pre-commit, а на update в мастере.
                        0
                        не совсем понял? в локальном репозитории или удаленном? и почему именно в мастере?
                          0
                          Под мастером я подразумевал не ветку, а удаленный репозиторий в который отправляют свои комиты (куда делается git push).
                            0
                            У меня две проверки, при коммите на локальном и при добавлении на удаленный. На удаленном отправляется email старшему разработчику.
                              0
                              На локальном, ИМХО, хук вообще не нужен:
                              1. можно проскипать при помощи --no-verify
                              2. можно его вообще снести или временно отключить
                              3. мешает быстренько закомитить локально (например, в конце рабочего дня, а завтра причесать)
                              4. сложнее поддерживать, если пишете свои правила для проверки
                                0
                                так идея не в том чтобы отловить ошибки а чтобы их не было. Каждый за собой следит локально, а для нарушителей есть проверка удаленно.

                                >сложнее поддерживать, если пишете свои правила для проверки

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

                                Вроде есть какая то возможность настроить PEAR чтобы он искал правила удаленно — это может понадобиться для распределенной команды. Но я пока с этим не разбирался.
                                  0
                                  1 и 3 пункты немного противоречат друг другу :)
                                    0
                                    Про ключ --no-verify знает не каждый пользователь git.
                                      0
                                      Теперь — каждый)
                                        0
                                        Сомневаюсь.
                          +1
                          А ещё можно этот шаг добавить в пайплайн Continuous Integration сервера
                            0
                            Да, интересная тема.
                            Что то видел об этом в PHPUnderControl. Есть примеры готовых связок сервера CI и скриптов проверки?
                              0
                              jenkins-php.org/ — я такой вот конфиг юзаю в качестве базового
                            0
                            btw, вот ещё интересный набор снифоф для юнит тестов: github.com/elblinkin/PHPUnit-CodeSniffer
                            руки пока не дошли, но вообще по статьям — вполне интересные
                              0
                              На маджентовском проекте подобное хотели заимплементить, только в связке с гиториусом. Так как проект уже не новый (~25к комитов в истории). Идея была в том, чтобы запускать checkstyle только для того кода, который пушается, но потом сбились на проверку только тех файлов, которые затрагивает пуш. Когда были ошибки — не давали сделать пуш, но это как-то неправильно оказалось. Идея с письмом намного интереснее, спасибо вам.
                                –1
                                >>Сравнительно недавно появился набор правил для проверки стандартов кодирования Drupal

                                пфф лучше б эти идиоты ядро нормальное запилили.
                                  0
                                  Пфф, вот возьмите и запилите, если «идиоты» не могут.
                                • UFO just landed and posted this here
                                    0
                                    Интересно, как быть если в проекте с одним стилем, понадобилось подключить библиотеку из проекта с другим стилем.

                                    Например в Drupal сделан модуль с кусками Zend. Получается код модуля должен быть оформлен в одном стиле, Zend классы при этом оформлены в другом, все это идет скопом в коммит, на что естественно в ответ будут матюки от hook'ов.

                                    Интересно если подключить сторонник классы как модуль, git вызовет для них эти hook'и и если не вызовет (то есть все ок и так и надо делать) то как быть если сторонник классы хостятся не на git, а к примеру на svn
                                      0
                                      для этого, хук должен быть настроен так, что сторонние либы не проверялись. Так же как и устаревшие, архивные проекты например.
                                        0
                                        Для этого я хочу в следующем обновлении сделать конфиг, который возможно стоит добавлять в контроль версий, чтобы он был доступен для локального и удаленного скриптов проверки и был общим для каждого из разработчиков.
                                        В этом конфиге, можно будет указывать файлы и папки которые не нужно проверять, например ядро Drupal, сторонние модули и features.
                                          0
                                          Хук вызовется при любом комите.
                                          Надо PHP Code Sniffer скармливать только то, что нужно проверять, а не все, что есть в комите.
                                          Насколько просто это сделать зависит от архитектуры.
                                          0
                                          А лучше расположить на сервере и не post-receive, а pre-receive, чтобы вообще не давать коммитить нестандартный код.
                                          Мы в наших проектах успешно используем подобный хук, но с настройками для игнора некоторых файлов и каталогов. Планирую его доработать, чтобы можно было настраивать уровень «фашизма» при проверке стандарта, т.к. иногда нужно быстро запушить код для фикса ошибки, а причесать его можно и потом.
                                            0
                                            Я выше об этом как раз и писал.
                                            0
                                            Так это замечательная вещь!!! Ведь соблюдения единого стиля кода на проекте очень важно! У меня раз человек на проекте 10 раз переписывал свой код, потому что мне по стилю не нравилось, но для этого мне приходилось проверять, что он коммитил. Так намного легче! Спасибо!
                                              –1
                                              Или т.н. «стандарты кодирования» описаны их автором в виде настроек для IDE которой пользуется вся команда или это потеря времени, а т.н. «лиду» рановато пока быть «лидом».
                                                0
                                                1. Заставлять людей работать в конкретной IDE – это бред.
                                                2. Не каждая IDE позволяет настроить все нюансы форматирования.
                                                0
                                                Статья обновлена. Добавлена возможность исключить файлы и директории из проверки используя конфигурационный файл.
                                                  0
                                                  Ссылки в статье не кликабельные. Печаль.
                                                    0
                                                    Извиняюсь что так получилось, я добавил рабочую ссылку на скрипты. Надеюсь вам это будет полезно.
                                                      0

                                                      Only users with full accounts can post comments. Log in, please.