Pull to refresh

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

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

Тема регулярных выражений для 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), большое ему спасибо за это.
Tags:
Hubs:
+16
Comments21

Articles

Change theme settings