Продолжая развивать тему статического анализа, который в общем случае занимается поиском любых дефектов в исходных кодах программ, давайте коснёмся проверки правильности регулярных выражений.
Тема регулярных выражений для PHP довольно щекотлива (примерно как манипулирование массивами), поэтому я вкратце напомню, с чем мы имеем дело.
Регулярные выражения используются для анализа и обработки текста, например, проверки правильности электронного адреса или номера телефона. Несмотря на их гибкость, человеку трудно воспринимать регулярные выражения — можно легко ошибиться в описании шаблона и выборе модификаторов. Нередко встречается употребление регулярных выражений там, где можно обойтись простыми строковыми функциями.
Итак, давайте посмотрим, на примере многим знакомых ZF2 и Symfony2, какие из упомянутых проблем можно найти с помощью статического анализа.
В качестве инструментов были использованы PhpStorm и статический анализатор кода Php Inspections (EA Extended), который ставится как расширение (плагин).
Php Inspections (EA Extended) содержит большой набор правил, разделенных по группам. Если вы ещё не пользовались этим плагином, то первое, что стоит сделать — это проанализировать весь проект. Второе — это настроить под себя Code Style инспекции (Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended)), чтобы сфокусироваться на других сообщениях.
И, конечно, не стоит слепо верить анализаторам: семантика проектов — вещь нетривиальная. Обязательно убедитесь, что код покрыт тестами до внесения изменений — работа со статическими анализаторами требует определенной техники безопасности.
В основном, были найдены мелкие недочёты и всего одна ошибка. Это неудивительно, поскольку оба фреймворка хорошо оптимизированы и протестированы: найти что-то серьёзное очень трудно.
Во-первых, я хочу подчеркнуть, что статические анализаторы — это такой же инструмент, как, скажем, XDebug. Например, с Php Inspections (EA Extended) анализ возможных ошибок идёт в процессе написания кода. Стоимость такого «внедрения» нулевая, а трудоёмкость рецензирования и обучения коллег заметно снижается.
Во-вторых, хочу поблагодарить Denis Ryabov, который предложил идею и проработал спецификацию для анализа регулярных выражений в Php Inspections (EA Extended), большое ему спасибо за это.
Тема регулярных выражений для 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), большое ему спасибо за это.
Комментарии (21)
NorthDakota
15.06.2015 12:21Пришёл на роботу, прочитал статью, поставил плагин.
И к сожалению, он мне не понравился, так как зачастую «ругается» на вполне адекватные вещи.
Например:
protected $testArray = [ 'testKey' => 'testValue', ]
Ругнулся на запятую, мол php до фени ваша запятая и она будет игнорироватся интерпретатором, уберите её.
Так я знаю что она будет игнорироватся, поэтому и оставил её там. Тем более логика класа подразумевает что, возможно, я или другой разработчик дополнит масив, и вот тогда дифф в гите будет красивым и читабельным. Благодаря вот той самой запятой.
P.S. Хотел найти в настройках плагина «отключение» некоторых фич. Не нашёл…
Reposlav
15.06.2015 12:36Перемещаете курсор на запятую, нажимаете alt+enter или появившуюся лампочку, выбираете «Inspection options->edit inspection profile setting», убираете ненужную галочку. По крайней мере так в PhpStorm 7
kalessil Автор
15.06.2015 12:48Второе — это настроить под себя Code Style инспекции, чтобы сфокусироваться на других сообщениях.
Это как раз о таких вещах: запятые, двойные кавычки, вложенные условия.
Как вариант, Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended): просмотрите всё, что Code Style и настройте под ваши реалии.
t1gor
15.06.2015 16:27Спасибо, поставил. Будем пробовать.
А подскажите еще, пожалуйста, где найти настройки плагина?kalessil Автор
15.06.2015 16:31Пожалуйста. Настроить можно здесь: Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended)
Посмотрите что из Code Style вам подходит, остальные группы можно не трогать — по ним нареканий ещё не было.t1gor
15.06.2015 16:39Settings -> Editor -> Inspections -> PHP -> Php Inspections (EA Extended)
Спасибо, именно то, что искал.
gro
15.06.2015 17:06Когда внезапно начинают пролезать арабские числа, это может быть неприятно. Так что лучше уж [0-9].
kalessil Автор
15.06.2015 17:12На проверенных проектах надо вычищать либо в одну сторону, либо в другую: \d и [0-9] сосуществуют.
Но [0-9] гораздо реже используется, поэтому, выбор — решение команды, зависящие от конкретного проекта.
kirilloid
20.06.2015 23:27+1
и0 === strpos($method, "get")
preg_match('/^get/', $method)
вообще-то не эквиваленты. regex будет работать быстрей на больших строках.
Хотя и эту оптимизацию можно провести через substr.kalessil Автор
21.06.2015 00:33-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.
Placido
В последнем случае вместо инвертирования жадности *? эффективнее использовать инвертированный символьный класс [^<]*. Тогда и модификатор s не потребуется:
kalessil Автор
Этот вариант меняет поведение, по спецификации возможны вложенные теги (во всяком случае мой пулл-реквест с этими правками завернули):
Placido
Согласен.