Comments 23
Почти два экрана ошибок!
Полез разбираться, оказалось ложный шухер.
Анализатор ругается на строки, вида:
$format_12_hour = $element['#hourformat'] == '12-hour'
и
while ($percentage == '100')А за статью спасибо, хотя результат проверки и добавил немного адреналинчику ;-)
Возможности такого анализа ограничены.
Это подходит для прототипирования, но не для production решения. Хотя если аккуратно написать шаблоны, должно помочь.
while ($percentage == '100')
Скорее всего, тут сработало сравнение со строкой через == вместо ===. Это не очень хорошая практика.
$element['#hourformat'] = 12;
$format_12_hour = $element['#hourformat'] == '12-hour';
var_dump($format_12_hour); // bool(true)
Может, конечно, конкретно в вашем случае это не так важно, но в общем случае это источник неприятностей.
Есть лайфхак Yoda conditions. Он помогает избежать ошибок в операциях сравнения, хотя мне такой стиль записи условий не нравится.
PSR-2 не рекомендует к использованию йода-стайл. Обусловлено лучшей читаемостью.
До недавнего времени, он был по умолчанию включён в php-cs-fixer и Style CI. Сейчас уже исправились.
А вы смотрели этот репозиторий? Это проект к php-fig имеет довольно посредственное отношение. Автор этого проекта по сути один человек dereuromark и он меняет многие устоявшиеся нормы, в частности:
Code MUST use 1 tab for indenting, not spaces. Spaces are for alignment and separation of words/text, only tabs are by definition valid indentation characters.
Я не думаю, что стоит рассматривать этот проект всерьез или у вас есть другая информация?
В языках, где = не является выражением, yoda style является нежелательным. :)
От этого обидно, что в PHP приходится такое использовать как защиту от ошибки. Но есть линтеры, которые найдут подозрительные применения присваиваний, так что иногда проще сделать линтеры на CI построже, а код оставить читабельным.
В Python частично решили проблему, там присваивание-как-выражение другой токен имеет, опечатку допустить риск почти нулевой. Но даже если запись другая, всё равно эту фичу лучше использовать с ограничениями.
В go-critic даже проверка есть, yodaStyleExpr, которая просит переписать в нормальную запись.
phpgrep всё же не столько про инспекции. Вы можете с помощью него упрощать рефакторинг, поиск bad practices по вашим собственным правилам. Ещё можно узнавать ответы на вопросы: "как часто в нашем проекте эта функция вызывается с определённой комбинацией аргументов?" или "есть ли у нас места, где тяжёлая для вычисления функция вызывается в цикле?".
Такой анализ можно выполнять и без phpgrep, но это потребует либо больше кода, либо ниндзюцу с регулярными выражениями (если попробуете грепать).
Есть шанс, что позже добавлю опцию replace, чтобы не нужно было руками всё заменять.
if ( preg_match( '/(wowwiki.com|wikia.com|falloutvault.com)/', $url ) ) {
По-моему, тут и экранирование не поможет, можно зарегистрировать домен falloutvault.comxxxx.com
Здесь пройдёт даже
lalalalawikia.comlalalala.lala`'DROP TABLE students;А вот матчить точку можеть быть не обязательно — валидных доменов без точки в этом контексте не будет. Если поставить ^$ в начале и конце строки — будет вполне достаточно, как по мне.
Экранирование точки лучше сделать через preg_quote — есть варианты повышения читабельности в этом кейсе.
Возможно, если регулярка уже не читабельна и при этом всё равно посредственно справляется с задачей, хорошо бы URL распарсить какой-нибудь либой и проверять домен или другую интересующую часть через switch/мапу/ifelse.
Я не эксперт в PHP, но в Go есть url.Parse, например.
u, err := url.Parse(urlString)
if err != nil {
// URL не валиден.
}
switch u.Host {
case "blah.com", "foo.com":
// Good.
default:
// Опционально обработать остальные урлы.
}Если "слишком много кода", то можно это в функцию завернуть.
В PHP это делается так
$allowed_hosts = ['wowwiki.com', 'wikia.com', 'falloutvault.com'];
if (in_array(parse_url($url, PHP_URL_HOST), $allowed_hosts)) {
// ...
}in_array(parse_url($url, PHP_URL_HOST), $allowed_hosts, true)(Да, я знаю, что он опциональный.)
Но и это еще не всё. Читаем документацию:
On seriously malformed URLs, parse_url() may return FALSE.
Поэтому код надо переписать:
$allowed_hosts = ['wowwiki.com', 'wikia.com', 'falloutvault.com'];
$host = parse_url($url, PHP_URL_HOST);
if ($host !== false && in_array($host, $allowed_hosts, true)) {
// ...
}
Принимайте мой PR :D
P.S. На случай $host === false можно отдельно поведение навесить при необходимости.
В изначальном коде не выполнялась проверка на корректность URL и я полагаю, что валидация URL была сделана где-то ранее. То есть, задача этой строчки кода только проверить допустимый домен в URL, а не проверять корректность URL.
И здесь неважен strict mode и false который может вернуть функция parse_url(). В этом примере in_array() всегда будет возвращать false для таких случаев.
Я безусловно согласен с тем, что использование strict mode по умолчанию является хорошей практикой, но в данном случае и без него код будет работать корректно.
И вы забыли еще об одной детали:
If the requested component doesn't exist within the given URL, NULL will be returned.
В изначальном коде не выполнялась проверка на корректность URL
Если вы о go-коде, то она там таки выполнялась.
u, err := url.Parse(urlString)
if err != nil {
// URL не валиден.
}И вы забыли еще об одной детали
Да, вы правы. И это надо тоже учесть.
В этом примере in_array() всегда будет возвращать false для таких случаев.
Ключевое здесь «в этом примере». Если не делать необходимые проверки, не использовать повсеместно строгий режим (полагаясь на то, что «конкретно тут это не важно»), то рано или поздно оно вернется бумерангом в самый неожиданный момент (достаточно, чтобы кто-то где-то что-то исправил и внезапно получилось, что проверка/строгий режим уже стали «конкретно тут важны»).
Спасибо!
Я не совсем понимаю, почему в треде пошли минусы, но здорово, что такой подход в PHP требует совсем незначительного количества дополнительного кода.
Сложно судить, насколько такой подход в PHP распространён, на корпусе из статьи 709 использований, а у preg_match — 5486 (при этом не все из них разбирают URL'ы). Но вроде бы достаточно часто используется. :)
Точка без звёздочки заменяет только один символ. Как сказали выше, добавление ^$ решит проблему.
Спасибо за статью! Запустил на своём проекте и был приятно удивлён что только одна ошибка и именно с случаем точки в регулярном выражении.
Если кто-то использует VS Code для PHP, то теперь под него есть расширение для удобного использования phpgrep.
Ищем баги в PHP коде без статических анализаторов