Статический анализ кода в PHP: регулярные выражения

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

    Тема регулярных выражений для PHP довольно щекотлива (примерно как манипулирование массивами), поэтому я вкратце напомню, с чем мы имеем дело.

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

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

    Инструменты


    В качестве инструментов были использованы PhpStorm и статический анализатор кода Php Inspections (EA Extended), который ставится как расширение (плагин).

    Php Inspections (EA Extended) содержит большой набор правил, разделенных по группам. Если вы ещё не пользовались этим плагином, то первое, что стоит сделать — это проанализировать весь проект. Второе — это настроить под себя Code Style инспекции (Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended)), чтобы сфокусироваться на других сообщениях.

    И, конечно, не стоит слепо верить анализаторам: семантика проектов — вещь нетривиальная. Обязательно убедитесь, что код покрыт тестами до внесения изменений — работа со статическими анализаторами требует определенной техники безопасности.

    Примеры дефектов


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

    ZF2


    Предупреждение: 'str_replace("-", ...)' can be used instead

        return preg_replace('#-#', $this->separator, $value);
        /*
         * Эффективнее будет использовать следующую конструкцию, 
         * т.к. не нужно разбирать и сопоставлять регулярное выражение.
         *
         * return str_replace('-', $this->separator, $value);
         */
    

    Предупреждение: '0 === strpos("...", «get»)' can be used instead

        if (preg_match('/^get/', $method)) {
            ...
        }
        /*
         * Случай, похожий на предыдущий, когда
         * можно использовать базовые функции для работы со строками.
         *
         * if (0 === strpos($method, 'get')) {
         *     ...
         * }
         */
    

    Предупреждение: '[0-9]' can be replaced with '\d' (safe in non-unicode mode)

        if (!preg_match('/^([0-9]{1,3}\.){3}[0-9]{1,3}$/', $host)) {
            ...
        }
        /*
         * Замена не меняет поведения выражения до тех пор пока мы не добавим /u
         * С /u шаблон сможет работать с юникод-числами, например, с арабскими.
         *
         * if (!preg_match('/^(\d{1,3}\.){3}\d{1,3}$/', $host)) {
         *     ...
         * }
         */
    

    Предупреждение: '[^\s]' can be replaced with '\S' (как варианты: \D, \W)

        if ($property->isPublic() && preg_match_all('/@var\s+([^\s]+)/m', $property->getDocComment(), $matches)) {
            ...
        }
        /*
         * Замена на \S не меняет поведения, и это, скорее, приведение к каноническому виду.
         */
    

    Предупреждение: 'i' modifier is ambiguous here (no a-z in given pattern)

        if (preg_match('/([^.]{2,10})$/iu', end($domainParts), $matches)
                            || (array_key_exists(end($domainParts), $this->validIdns))) {
            ...
        }
        /*
         * /i бесполезен, т.к. в шаблоне нет буквенных символов.
         */
    


    Symfony2


    Предупреждение: 'false !== strpos("...", «file»)' can be used instead

        return preg_match('/file/', $this->getFilename());
        /*
         * Эффективнее будет использовать следующую конструкцию, 
         * т.к. не нужно разбирать и сопоставлять регулярное выражение.
         *
         * return false !== strpos($this->getFilename(), 'file');
         */
    

    Предупреждение: '[^A-Za-z0-9_]' can be replaced with '\W' (safe in non-unicode mode)

        $gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
        /*
         * Замена на \W не меняет поведения, и это, скорее, приведение к каноническому виду.
         */
    

    Предупреждение: '[a-zA-Z0-9_]' can be replaced with '\w' (safe in non-unicode mode)

        return '' === $name || null === $name || preg_match('/^[a-zA-Z0-9_][a-zA-Z0-9_\-:]*$/D', $name);
        /*
         * Замена на \w не меняет поведения, и это, скорее, приведение к каноническому виду.
         */
    

    Предупреждение: Probably /s modifier is missing (nested tags are not recognized)

        $content = preg_replace('#<esi\:remove>.*?</esi\:remove>#', '', $content);
        /*
         * А вот это уже ошибка: переносы строки во вложенных тегах не учитываются.
         * Необходимо указать модификатор /s
         */
    


    Вместо заключения


    Во-первых, я хочу подчеркнуть, что статические анализаторы — это такой же инструмент, как, скажем, XDebug. Например, с Php Inspections (EA Extended) анализ возможных ошибок идёт в процессе написания кода. Стоимость такого «внедрения» нулевая, а трудоёмкость рецензирования и обучения коллег заметно снижается.

    Во-вторых, хочу поблагодарить Denis Ryabov, который предложил идею и проработал спецификацию для анализа регулярных выражений в Php Inspections (EA Extended), большое ему спасибо за это.
    Share post

    Comments 21

      0
      В последнем случае вместо инвертирования жадности *? эффективнее использовать инвертированный символьный класс [^<]*. Тогда и модификатор s не потребуется:
      $content = preg_replace('#<esi\:remove>[^<]*</esi\:remove>#', '', $content);
        0
        Этот вариант меняет поведение, по спецификации возможны вложенные теги (во всяком случае мой пулл-реквест с этими правками завернули):

        <esi:include src="http://www.example.com/ad.html"/> 
        <esi:remove> 
          <a href="http://www.example.com">www.example.com</a>
        </esi:remove>
        
          +1
          Согласен.
        0
        Пришёл на роботу, прочитал статью, поставил плагин.

        И к сожалению, он мне не понравился, так как зачастую «ругается» на вполне адекватные вещи.
        Например:

        protected $testArray = [
            'testKey' => 'testValue',
        ]
        


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

        P.S. Хотел найти в настройках плагина «отключение» некоторых фич. Не нашёл…
          0
          Перемещаете курсор на запятую, нажимаете alt+enter или появившуюся лампочку, выбираете «Inspection options->edit inspection profile setting», убираете ненужную галочку. По крайней мере так в PhpStorm 7
            0
            В 8 сработало. Спасибо!
            0
            Второе — это настроить под себя Code Style инспекции, чтобы сфокусироваться на других сообщениях.


            Это как раз о таких вещах: запятые, двойные кавычки, вложенные условия.

            Как вариант, Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended): просмотрите всё, что Code Style и настройте под ваши реалии.
              0
              В Probable Bugs что-нибудь интересное нашли?
              0
              Спасибо, поставил. Будем пробовать.

              А подскажите еще, пожалуйста, где найти настройки плагина?
                0
                Пожалуйста. Настроить можно здесь: Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended)

                Посмотрите что из Code Style вам подходит, остальные группы можно не трогать — по ним нареканий ещё не было.
                  0
                  Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended)
                  Спасибо, именно то, что искал.
                0
                Когда внезапно начинают пролезать арабские числа, это может быть неприятно. Так что лучше уж [0-9].
                  0
                  На проверенных проектах надо вычищать либо в одну сторону, либо в другую: \d и [0-9] сосуществуют.

                  Но [0-9] гораздо реже используется, поэтому, выбор — решение команды, зависящие от конкретного проекта.
                    0
                    Да, но команда должна знать о таком нюансе, что \d это не только 0-9
                  +1
                  0 === strpos($method, "get")
                  и
                  preg_match('/^get/', $method)

                  вообще-то не эквиваленты. regex будет работать быстрей на больших строках.
                  Хотя и эту оптимизацию можно провести через substr.
                    –1
                    Официальная документация говорит в пользу моего варианта:
                    Do not use preg_match() if you only want to check if one string is contained in another string. Use strpos() or strstr() instead as they will be faster.
                      0
                      Вы понимаете разницу между /get/ и /^get/?
                        0
                        Да, я прекрасно понимаю разницу и вам следует таки опираться на факты и публичные матералы, а не переходить на личности.

                        Вот ещё один пруф, что strpos будет бытрее.
                          0
                          Про большие строки (например блоки XML), там же говорится что preg_match будет быстрее, про эти случаи вы правы.
                            0
                            Вот про эти случаи я и говорил. Извините, я хотел обратить внимание на тонкости того, что регулярка привязана к концу строки.
                            Кстати, я как-то видел, что регулярка с ^ работала быстро, а с $ — медленно (обе на больших строках). Правда, это было в каком-то javascript-движке (вроде, V8, но давно), но тем не менее такое возможно и в php, так что конечно проверять надо.
                              0
                              На самом деле спасибо за комментарий, я как-то не за то зацепился =)

                              Думаю, что надо добавить ремарку про этот нюанс в текст анализатора.

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