Pull to refresh

Comments 56

Resharper inspections в PHP варианте! Обязательно проверю свой проект.
До ReSharper далековато (спасибо за наводку), но подход тот же.
Хм… Все «проблемы», которые были найдены — их даже проблемами-то трудно назвать. Это скорее не проблемы, а просто предложения анализатора по приведению некоторых строчек кода в более хороший вид, за исключением пары случаев. Разве нет? :)
Ну оно же как-то работает) Шутка, Symfony2 хороший продукт, и там трудно найти эпичные ошибки (хотя тот же пример с ChainLoader или конструкторами).

Остальные примеры — это улучшение кода, да. Я просто ещё опустил много мелочи: производительность, возможные баги и прочее.
Ну тогда вот — небольшой список. То, что выделено (кроме приведения типов) может оказаться багами:
небольшой список
image

Хм… спасибо, не знал что есть такое расширение, хотя уже давно пользуюсь Штормом. Надо будет попробовать.
На самом деле подобные анализы редко выявляют какие-либо ошибки, но если избавить код от мелких проблем (как в статье, например), о он реально становится лучше.
Скажем так, не напрямую. Но и забытых вар дампов, странных логических конструкций мы исправили достаточно.
Отличное исследование. Следующий шаг — собрать эти правки и оформить в pull request :)
Так всё же просто — fork, push, pull request, а там посмотрим, как отреагируют. Вообще, по моему опыту, мейнтейнеры совершенно нормально относятся даже к небольшим правкам, так что можете смело предлагать изменения. Сейчас, конечно, очередь достаточно большая, но когда это мешало?
Я честно пытался отшутиться, но у меня в контракте есть ограничения на участие в оперсорсе и выступлениях на конференциях.
Видать основная масса PHP программистов не интересуется C++, поэтому вашу шутку, относящуюся к PVS-Studio, не поняли :)
Для обзора была выбрана актуальная версия 2.7 (LTS)

Дык 2.7 еще ж не релизилась. Или я чего-то не знаю?
Подскажите пожалуйста, как использовать данное расширение?
Установил на PhpStom 8.0.2, перезапустил, в списке плагинов активен, а как запустить анализ? Стандартная команда Run Inspections прогоняет только стандартные инспекции, настроек не нашёл.
File > Settings > Inspections, в разделе «PHP» без групп:

Заголовок спойлера

Скорее всего, именно Apple JDK 1.6 у меня. Спасибо!
Если кликнуть правой кнопкой по файлам/папке в навигаторе проекта, то в контекстном меню есть опция «Inspect Code...» — можно часть проекта проверить.

В результатах анализа будет секция PHP — это результаты проверки плагина.
Я вот что не понимаю. Зачем PHPшники так отчаянно превращают PHP в Java? Почему бы не делать на PHP исконно PHPшные задачи, а для статики использовать уже существующую Java? Зачем нам две джавы?
Наверное это начали сами разработчики PHP, когда при реализации в качестве основных источников вдохновения посматривали как раз таки на Java/c#.

В любом случае, статический анализ и прочие полезности никакого отношения к Java не имеют. Статический тайпхинтинг — так же, это лишь удобности для разработчика.
а желание статического тайпхинтинга никак не умаляет динамической природы языка? Не получится ли так, что люди начнут писать «статические» программы на PHP?

С одной стороны, в тексте приводятся довольно невинные примеры. С другой, статья с каких слов начинается: «о необходимости статического анализа в больших проектах уже писали не раз». Именно «необходимости». Это уже звоночек. Если начать всё вот так генерализировать, этот звоночек в конце концов может привести к тому, что заказчики будут требовать статически анализируемый код на PHP. То есть нельзя будет использовать его для того, для чего он собственно и нужен — крутой динамической магии.

Это не про «прямо сейчас», а про развитие на годы вперед, куда это всё придет. Гадание что будет, что случится, чем сердце успокоится, дальняя дорога или казенный дом.
Тайпхинтинг никак вообще не должен мешать вам творить вашу «крутую динамическую магию». Статический же анализ в рамках динамического языка все-равно необходим (и не важно, пишите вы на PHP или Ruby), так как это значительно упрощает отлов глупых ошибок.
Например, магический метод, который обращение к несуществующему полю превращает в запрос к базе данных, и назад возвращает совершенно что угодно, что там в базе нашлось. Переменные переменных. Рекурсивные инклуды. Препроцессоры типа CCPP. Посмотрим, как с таким кодом справится тайпчекер, он же совершенно unsafe. Тайпчекер-то мне не помешает, это я ему случайно могу помешать, сводя всю работу к нулю.

А если уж нужна статика, зачем ее тащить в PHP, если можно взять язык с таким же синтаксисом, в котором уже всё есть? Те же Java и C#. В Java динамику, кстати, не тащат совсем. Когда понадобилась динамика, просто создали другие языки на той же виртуальной машине. Более того, в саму виртуальную машину джавы добавили поддержку JavaScript, прямо в базовом наборе инструментов. Может, имеет смысл сделать другой язык для PHPшной виртуальной машины, если так нужна PHPшная виртуальная машина?
PHP был создан для решения задач разработки web-приложений максимально быстро а не для магии. Я не говорю что магией пользоваться совсем так уж плохо, просто это не особо хорошо в рамках реализации бизнес-логики, отладки, поддержки приложения. С этой точки зрения тайпхинтинг, статический анализ в купе с автотестами и прочими приблудами существенно упрощают сопровождение проекта.

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

Магия скажем очень помогает при реализации каких-то вещей в духе прокси-классов и т.п. И да, магия основанная на рефлексии и кодо-генерации меня лично радует намного больше.
Почему вы тогда не возьмете языки, построенные на статике, рефлексии, кодогенерации и других милых вашему сердцу вещах? Например, упомянутые Java, C#. Там уже все это есть, все отлично работает, сто лет как.

Мне в пхп хочется видеть наоборот больше динамики, раз уж в нем она возможна. Это дает выбор инструментов. Нужно на динамике быстро наколбасить прототип — пхп. Нужна динамика с прототипами — жс. Нужна статика — джава. Нужна статика под виндовс — сишарп. Итп. Для каждой задачи — свой инструмент. Сейчас же я вижу, что пхп превращается в джаву, то есть становится на один инструмент меньше. Если пхп похож на джаву, нет совершенно никакого смысла выбирать пхп, он умер. Это печально, в этом классе остается только руби.
Как-то вы узко мыслите.
Те возможности который предоставляет PHP куда больше чем «на динамике быстро наколбасить прототип». PHP даёт возможность писать, в том числе, большие и сложные системы, при этом при написании таких систем уже применяются другие практики, в том числе использование тайпхинтинга где это возможно и т.д. И это не делает язык похожим на другие языки, это подход схож с другими подходами на других языках. И это не лишает язык его преимуществ. И при выборе языка для решения какой-то задачи критериев для выбора куда больше чем нудна статика или динамика.
Только ситхи все возводят в абсолют.
Это тоже не ошибка, а упрощение кода, и можно переписать как "'content_type' => $response->headers->get('Content-Type') ?: 'text/html'".

$response->headers->get('Content-Type', 'text/html') так не лучше?
По многим причинам Hack/HHVM мы не можем использовать.

Костыли пришлось писать для примитивных типов (статика — необходимое зло в данном случае):

class ..._SimpleTypeHint {
    public static function throwExceptionIfArgumentIsNotString($givenArgument);
    public static function throwExceptionIfArgumentIsNotInteger($givenArgument);
    public static function throwExceptionIfArgumentIsNotFloat($givenArgument);
    public static function throwExceptionIfArgumentIsNotBoolean($givenArgument);
}

class ..._SimpleTypeCasting {
    public static function toInteger($mixedValueToCast);
    public static function toFloat($mixedValueToCast);
    public static function toBoolean($mixedValueToCast);
}


А вот так выглядит худший случай использования:

class SomeClass {
	/** @var int|null $someId */
    public function doSomethingWithEntityId($someId)
    {
        if ($someId !== null) {
            ..._SimpleTypeHint::throwExceptionIfArgumentIsNotInteger($someId);
        }
		
		...
	}
}
Остальные типы (массивы, классы и интерфейсы), типо-зависимые операции сравнения и поиска — часть языка и родного API.
… и использующий шаблон проектирования MVC ...

Symfony2 не использует шаблон проектирования MVC, поправьте, пожалуйста.
Посмотрел на расширение и увидел там «un-necessary double quotes» и даже грустно стало :(
Будем надеяться, что английский в названиях и сообщениях поправят.
Да не, я скорее про то, что двойные кавычки или одинарные — не важно
Пока там нет инлайн-переменных, то разницы нет.

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

Я правда не помню, что происходит внутри APC, там разница будет минимальна.
Opcache на то и опкеш, чтобы кешировать байткод.
Весь этот поиск будет произведен во время компиляции, то есть один раз. Этой микрооптимизацией можно смело пренебрегать направо и налево, потому что это не важно — у всех включен опкеш.
Начиная с php5.4 все чуть веселее. Все строки (и хэши для ключей массивов) php старается инициализировать только один раз и затем шарит это все в рамках одного процесса и затем просто реюзает. За счет этого нам не приходится перевыделять память да и сама память существенно экономится. opcache просто дает возможность шарить все в рамках всего пула процессов делая оптимизацию еще более эффективной.

Но в целом да, если при компиляции интерполяции строк замечено небыло и значение можно получить на этапе компиляции — разницы вот вообще никакой. Это по идее замедляет только разбор кода что в случае использования opcache копейки. Всегда можно завармапить кэш при деплое.

Вообще большая часть этих холиварных вопросов типа «что быстрее echo или print» на корню решаются opcache.
Не нужны тут никакие варианты, можно забить на эту проверку.
Если вас она так волнует — вам надо в панике убегать с Symfony, потому что весь кеш шаблонов Twig на двойных кавычках :)
А можете рассказать, чем вы пользуетесь для статического анализа и как?

Доводы по поводу кавычек были конструктивны, хочу продолжить дискуссию в более интересном русле.
Все теми же инспекциями в phpstorm, поэтому и заинтересовался плагином из топика и с удовольствием его попробую :)
О, расскажите потом (что отключили, что оставили)?

Хочу сравнить с нашим опытом (мы пока всё используем, но была пара предложений от коллег).
Могу посоветовать на данном этапе внедрить какую-то систему аналитики — мониторить что люди отключают и т.д. что бы выборка была более репрезентативна.
Платформа не даёт таких инструментов, плагин может только экспортировать инспекции и аста-лависта. Второй момент — я немного параноик на тему сбора статистики плагинами от стороннего вендора.
У нас в команде пока нет каких-то жестко формализованных правил писать код, потихоньку к этому идем, так что и общих настроек инспекций нет. Зато есть код ревью, негласные соглашения по способам решении некоторых задач и голова на плечах :)
Ну и есть CodeSniffer, для которого есть набор правил PSR2 + несколько своих кастомных, который висит на receive-hook'е git'а на удаленном сервере и не дает пушить.
У каждого разработчика свое видение мира, строгих разделений между добром и злом нет, как и времени. Когда-нибудь появится время и что-то получится «стандартизировать» и описать какие-то правила для тулзов, которые тоже можно будет повесить на хуки и, тем самым, избегать некоторых вещей еще до того, как код уйдет на ревью.
А пока как есть, такие дела :}
Когда начнёте формализоать, придётся повоевать.

Мы просто решили: вот плагин, смотрите на что жалуется, чините и обучайтесь. Код должен быть «зелёным».

Если есть жалобы от анализатора, сразу отправляем на доработку. НО легаси код вычищают архитекторы, попутно оставляя todo и deprecated.

На гит переезжаем по-проектно и с ним гораздо комфортнее организовывать хуки — в целом у нас похожий подход.
В большинстве анализаторов удручает, что определённые правила нельзя менять для некоторого куска кода.
Как в jslint/jshint, например. Для какого-нибудь блока можно указать комментарий, отключающий какую-то проверку.
Типа, «да, я понял твоё указание, но именно здесь так надо.»
В таких случаях можно подавить ложные срабатывания на уровне выражения или всего файла. Здесь есть информацияо том, как это делается.
Sign up to leave a comment.

Articles