Комментарии 51
Сначала мы даже хотели пропатчить PHP. Нам хотелось, чтобы если функция принимает какой-то скалярный тип (скажем, int), а на вход пришёл другой скалярный тип (например, float), то не кидался бы TypeError (который по сути своей исключение), а происходила бы конвертация типа, а также логирование этого события в error.log
Очень странное желание, обычно при разработке руководствуются правилом, что если происходит какая-то ошибка, то программа должна «упасть» как можно раньше. А здесь вы, по сути, хотели, чтобы функция уже получившая какие-то некорректные данные, во-первых продолжила работу с ними, а во-вторых, делала какие-то неявные преобразования, над уже ошибочными данными, еще больше усугубляя ситуацию
Но про патч я всё-таки поясню.
В типичном PHP-коде, написаном без strict_types, все эти неявные преобразования уже происходят. Такова система типов PHP, по крайней мере до времён strict_types.
PHP-программисты часто используют например
var_export($some_var,1) и это работает так, как программист ожидает, хотя вторым параметром надо передавать boolean. Патч показал бы нам такие места.
Этот патч PHP предназначен, скорее, не для постоянной работы, а для смягчения перехода на проверку типов, чтобы в проде не падало от незначительных неточностей.
функция, уже получившая какие-то некорректные данные, во-первых продолжила работу с ними, а во-вторых, делала какие-то неявные преобразования
Учитывая, что такое поведение являлось абсолютно нормальным в течение первых 20 лет существования РНР, а "неявные преобразования" до сих остаются одной из ключевых фич языка, то предложение, чтобы сайт с миллиардной аудиторией падал при срабатывании type juggling выглядит, на мой взгляд, несколько оторванным от реальности.
первых 20 лет существования РНР
PHP 20 летней давности — это явно не пример для подражания
чтобы сайт с миллиардной аудиторией падал при срабатывании type juggling
Прямо таки с милиардной?)) Очень хорошие показатели, учитывая, что по статистике ООН 50% населения планеты не имеют доступа к интернету.
А если серьезно, то необязательно раскладывать новый функционал на 100% аудитории, можно разложить ветку с изменениями для 1% или еще меньшей доли, и пофиксить баги, если вдруг они появятся + покрывать тестами. Также можно поэтапно вводить strict type, например только для новых/изменненых файлов, но не пытаться делать вид что ничего не произошло и продолжать работу при, например, строке, приведенной к инту.
Срок "первые 20 лет" включает в себя не только код 20-летней давности, но и 5- и 3-летней.
Миллиарда у них, насколько я знаю, нет, но величины сравнимые.
1% от 100 миллионов — это миллион. Вам бы хотелось попасть в эту фокус-группу? Пусть даже 10 тысяч — зачем экспериментировать на посетителях, если можно обойтись без этого?
Лично я воспринял абзац про "пропатчить РНР" не как предложение "сделать вид, что ничего не произошло" и оставить все как есть — в этом случае ваше возмущение было бы оправданным — а как graceful refactoring — залогировать ошибки и исправить потенциально некорректный код.
В целом, мне кажется что ваши предложения, будучи формально правильными, являются несколько оторванными от реальности, поскольку не учитывают контекст задачи и требования бизнес-процессов.
В целом, мне кажется что ваши предложения, будучи формально правильными, являются несколько оторванными от реальности, поскольку не учитывают контекст задачи и требования бизнес-процессов.
Да тут, я соглассен что издержки будут. Но архитектурные ограничения — это стратегические инвестиции в будушее, их смысл уменьшить стоимость поддержки кода через год, три, пять. Допустим, сейчас мы создадим некоторый дискомфорт для малой части пользователей, но это окупится в дальнейшем тем, что у нас не будет фантомных трудноловимых багов, которые будут у 100% юзеров.
Но про бизнесс процессы это вы верно заметили, в большинстве проектов менеджменту насрать что будет через год и тем более 5 лишь бы сегодня закрыть побольше бизнес задач с как можно меньшими проблемами, а инициативы разрабов по поводу инвестиций в будущее не приветствуются. Так что, может я и не прав.
Насколько я знаю ситуацию конрeктно в Баду, это в первую очередь технологическая компания. И учитывая, сколько всего она сделала для РНР сообщества (взять один только php-fpm или фикс проблемы с opcache в 7.0.4), то она не вписывается в нарисованную вами мрачную картину засилья неадекватного менеджмента.
Не говоря уже о том, что отказ от намеренного показа 500 ошибки вместо работающего сайта вряд ли можно назвать вопиющим произволом.
Вы опять сбиваетесь на рассуждения о каком-то абстрактном сферическом менеджменте в вакууме, как раньше рассуждали об абстрактном подходе к обработке ошибок вообще. В то время как в конкретном посте не было ни слова про "закрыть побольше бизнес задач с как можно меньшими проблемами" и негативную реакцию на "инициативы разрабов по поводу инвестиций в будущее". Как раз наоборот — насколько я понимаю все инициативы исходили исключительно от разработчиков.
мы создадим некоторый дискомфорт для малой части пользователей, но это окупится в дальнейшем тем, что у нас не будет фантомных трудноловимых багов, которые будут у 100% юзеров.
Речь шла о том, как отловить фантомные трудноуловимые баги, не создавая дискомфорта для юзеров.
Так что анализатор в целом может помочь, но расслабляться ни в коем случае нельзя.
Так что анализатор в целом может помочь, но расслабляться ни в коем случае нельзя.Всё верно. Именно поэтому мы используем все 3 анализатора + тесты + ручное тестирование
Немного дополню: в PHP есть довольно старый баг (один из сотен, обнаруженных более 10 лет назад и до сих пор не исправленных), связанный с доступом к элементам массива. Точнее не совсем массива. Проблема в том, что при попытке обратиться по какому-либо ключу к любому скалярному типу, отличному от строки, мы не получим ошибку/предупреждение (пруф: https://3v4l.org/D3c2q), т.е. конструкция
$a = null;
var_dump($a['one']['two']['three']);Выполнится успешно и выдаст в ответе NULL, что не всегда является ожидаемым результатом. Статические анализаторы (при условии корректно заполненного phpdoc, конечно же) позволяют отловить такие участки.
declare(strict_types = 1);
function parse(string $fileContent): array
{
return [$fileContent];
}
parse(file_get_contents('/someWrongPath'));В этом случае будет сгенерирован warning, многие современные фреймворки конвертируют их в Exception-ы. Анализаторы, как мне кажется, считают что эти случаи можно не анализировать.
Зачем анализатор в принципе нужен, многие современные фреймворки и ошибки вполне себе в исключения конвертируют.
У нас явная ошибка, вызванная ленью\глупостью (нужное подчеркнуть). Почему она должна быть проигнорирован анализатором? Потому, что в исключение сконвертируется и кем-то где-то будет перехвачено?
На такой пример psalm ругается — getpsalm.org/r/075628070d, так что я думаю, что дело как раз в обработке ошибок.
просто напомню. Это допущение нужно по одной простой причине — нет никакой возможности убать ложно-положительное срабатывание вот тут:
if (!is_file($filePath)) {
return;
}
// 99.99% ситуаций тут будет строка,
// но psalm об этом никак не узнает и будет думать что string | false
$fileContent = file_get_content($filePath); То есть увы — все это поиски компромисов в условиях языка, конструкции которого плохо дружат с выводом типов. С чем я несогласен — так это с невозможностью переопределить тайпинги легко и просто. Но думаю это как раз пофиксят в ближайшем будущем.
Хочешь красивый вывод типов с минимальным количеством ложных срабатываний — добро пожаловать в дивный новый мир отказа от любых if-ов, while-ов и прочих вещей (ну то есть привет ФП с их Maybe и Either, map fold и т.д.)
Psalm и в fgets(fopen('zzz', 'rt')) не замечает ничего плохого :(
Поэтому всегда пред обращению к элементу массива приходится делать isset/array_key_exists
Вот пример первого попавшегося файла и на который в будущем будут похожи все файлы.
<?php
/*
* This file is part of sebastian/global-state.
*
* (c) Sebastian Bergmann <sebastian@phpunit.de>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
declare(strict_types=1);
namespace SebastianBergmann\GlobalState;
/**
* Exports parts of a Snapshot as PHP code.
*/
class CodeExporterЭто нужно просто взять и убрать.
<?php
declare(strict_types=1);
For the full copyright and license information, please view the LICENSE file that was distributed with this source code.А еще радуют такие вот блоки, которые вполне могли бы заменяться с новым {}, но никто не заменяет.
use SebastianBergmann\PHPCPD\Detector\Detector;
use SebastianBergmann\PHPCPD\Detector\Strategy\DefaultStrategy;
use SebastianBergmann\PHPCPD\Log\PMD;
use SebastianBergmann\PHPCPD\Log\Text;
use SebastianBergmann\FinderFacade\FinderFacade;
use Symfony\Component\Console\Command\Command as AbstractCommand;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;Ну а папка vendor все жирнеет и жирнеет: 100, 200 и 300 мб — не редкость.
Короче, нужно менять это. Наболело.
upd: прошу прощения, ошибся, это phpdcd
Убирать namespace и class не нужно.
Группировать импорты — мне не нравится идея. Плагин, который я использую, с ними может не работать. И их все равно ведь руками никто не пишет. И синтаксис импортов со скобками некрасивый по моему субъективному мнению.
Тогда вообще никакой код писать и править нельзя. Так как если у вас нет покрытия тестами, то из-за этого могут всплыть ошибки на продакшене.
Очень часто можно встретить это противопоставление — типа 100% покрытие кода vs статический анализ. Это противопоставление в корне не верно.
Во первых 100% покрытие кода тестов ничего нам не гарантирует. Точно так же как и 100% покрытие кода типами (есть суровый внешний мир, и он может нести много сюрпризов и несоответствий с типами о которых мы узнаем только в рантайме, да и пых уж простите не хаскель и не окамл).
На эту тему вообще есть замечательный докладик: Ideology — Gary Bernhardt from Strange Loop 2015
Мы постоянно ищем возможности как для ускорения разработки, так и для повышения качества кода.
А может, все таки, уйти на более подходящую технологию: руби, питон, и т.д.? Сам уже в начале года попрощался с пыхой, проработав с ней более 10 лет, ибо уже в конец заеб… лся (ушел полностью на руби). На огромных проектах пыха — это всегда легаси дичь. Что не внедряй, что не придумывай, все равно это будут тонны костылей и новых граблей. С нестрогой типизацией ничего нельзя сделать. На сколько разработчик не был бы опытным и даже гениальным, все равно произойдет ситуация, когда авто приведение типа будет ломать логику. И эту ошибку не всегда можно заметить и очень сложно отловить. Особенно при работе со всякими разнородными массивами, где не всегда ясно, какого типа данные лежат, могут лежать или должны лежать. Плюс на пыхе разные разработчики используют разные подходы, которые не всегда очевидны для других. Три опытных пыхера хрен когда сойдутся во мнении, как нужно реализовать функционал. И каждый будет отстаивать свое решение. Работал на двух громаднейших порталах, один по финансам, другой по недвижимости. Оба на пыхе. Это незабываемые для меня годы! Через год сами разработчики уже в собственном коде не могут разобраться. Любые улучшения/плюшки/фишки с течением времени легко умножаются на ноль. Какой бы хороший код и логику не писал, все превращается в легаси кашу, которую вынуждены поддерживать и «развивать» команды по 30!!! человек. И как бы мы не работали (канбан/скрам), какие бы методологии и подходы не вырабатывали, все равно выбор ЯП является основополагающим. На новом проекте я ушел полностью на руби. После пыхи, где элементарное действие кодится сотней строк, я просто прозреваю, как тоже самое делается 5-10 строками. Я наконец увидел, что программирование может приносить не боль, а удовольствие. Если что, то на питоне и яве я тоже пишу, но очень редко. Пыху считаю днищем, хотя работал с ней с 2000 года!
а сколько разработчик не был бы опытным и даже гениальным, все равно произойдет ситуация, когда авто приведение типа будет ломать логику.
Наличие или отсутствие автоматического приведения ничего не меняет — логика всё равно сломается. Например, для упомянутого вами Ruby будет что-то вроде:
+': no implicit conversion of Integer into String (TypeError) from main.rb:1:in `<main>'
Если у вас нет статического анализа, вы эту проблему в обоих случаях можете найти только во время выполнения. В обоих случаях остаётся только надеяться на то, что это место покрыто тестами, или его проверят вручную и найдут фейл.
Но не надо отчаиваться, для Ruby тоже есть тулзы для статического анализа https://github.com/mre/awesome-static-analysis#ruby
Наличие или отсутствие автоматического приведения ничего не меняет — логика всё равно сломается.
Ну не скажите. Например, '3' + 2 в пыхе выведет 5, а руби словит эксепшн. Значит в руби интерпретатор свалится с указанием проблемы, и вы сможете быстро ее пофиксить. А вот в пыхе это может привести к неправильному расчету в какой-то формуле, что уже реально проблематично. Это такой пример из головы, но суть, думаю ясна.
Но не надо отчаиваться, для Ruby тоже есть тулзы для статического анализа
Спасибо за ссылку :)
Значит в руби интерпретатор свалится с указанием проблемы, и вы сможете быстро ее пофиксить. А вот в пыхе это может привести к неправильному расчету в какой-то формуле, что уже реально проблематично.
Поведение PHP и Ruby будет разным — это правда, но это не меняет сути.
Ключевой момент в обоих случаях: проблемный код должен кто-то
- исполнить (человек/тест/автоматика)
- проверить результат (есть ли ошибка).
Если что-то из этого не произошло, то код с ошибкой будут исполнять пользователи на продакшене в обоих случаях.
Вспомнил анекдот: У джуна не проходили тесты, но он смог решить проблему — он их все закомментировал :))
Хотя я и не представляю, как разработчик может не оттестировав свой код лить его на прод.
Если лить код на прод, не проверяя ничем, уже ничего не поможет. :) Даже при наличии статических анализаторов можно на них забить — это крайности.
Обычно всё-таки проблемы бывают в случаях, когда правка затрагивает кучу сценариев и все их сложно проверить, либо есть какой-то неочевидный сценарий, о котором забыли.
Через год сами разработчики уже в собственном коде не могут разобратьсяУ нас 12-летний легаси и мы как-то справляемся :) С этой проблемой очень помогают правильно настроенные процессы. В частности у нас все коммиты привязаны к тикетам в jira. Поэтому сделав git blame и посмотрев описание коммит а, я вижу в каком тикете были сделаны данные изменения и обычно из тикета ясны почти все детали.
Что касатеся перехода на другие языки, то я в начале статьи писал, что у нас миллионы строк php-кода. Их нельзя по-быстрому переписать на руби или питон. Но у нас есть движение в сторону более активного использования Go.
В общем, я желаю вам удачи от всей души. После таких проектов нужно брать годовой отпуск и не просыхать сутками на пролет ))
многое в отчётах анализаторов — просто неточности, например, неправильно указанные типы в PHPDoc. Некоторые программисты пренебрежительно относятся к таким ошибкам — код ведь работает.Фу такими быть. Тут уж либо поддерживать мета-информацию соответствующей действительности, либо вообще удалить — меньше потенциального вреда.
Например,
declare(strict_types=1);
class Model {
/**
* @var string
*/
public $a = ''; // Тут строка и не ожидается ничего кроме строки
}
function getA(): bool
{
return true;
}
$m = new Model();
$m->a = getA(); // Тут не будет ошибки для PHP 7.2 и PhpStorm Php Inspections (EA Extended), а вот Psalm найдет ее, при условии что есть phpdoc
В PHP 8 будут введены типы для свойств классов, а там еще немного останется для полной типизации языка
Давно бы запилили типизированный аналог пыха по типу TypeScript для JS и горя не знали. Но, видимо, динамика со всеми своими недостатками (которые на больших проектах превращаются в полнейшее уродство), чем то прельщает программистов.
а там еще немного останется для полной типизации языка
class Foo
{
/**
* @var Bar[]
*/
private array $bars;
}это я к тому что в целом фича с тайпхинтами для пропертей вообще ничего не добавляет с точки зрения возможности описания типов. От слова совсем. Без дженериков, без алгебры типов (хотя бы простые штуки типа type Foo = "foo" | "bar", без array shapes и прочего — ни о какой "полной типизации" даже речи быть не может.
А с учетом того что пых любые проверки типов обязан еще и в рантайме делать (потому что так повелось и делать иначе будет уже не консистентно), сложность всего этого вырастает на несколько порядков.
- строгая и статическая типизация — разные вещи. Строгая — запрет на неявные касты типов в рантайме. Увы и ах поменять поведение операторов даже с волшебными
declareуже не выйдет. Слишком много кода сломается. Тут надежда как раз на анализаторы типа psalm и других, которые подобные касты могут отловить в компайл тайме и наругать человека. - статическая типизация имеет смысл только при наличии возможности описывать все типы (с чем в php все плохо даже на уровне анализаторов, опять же потому что нет никаких стандартов и все выдумывают свои) + возможность вывода типов (мы же ленивые, да?) + проверка этих типов самим PHP на этапе компиляции/трансляции в опкоды.
- PHP давным давно принял довольно странное решение — проверять типы в рантайме. Что это означает? Да все просто — вся информация о типах обязана быть в рантайме. А это дополнительные накладные расходы, начиная с увеличения количества занимаемой памяти (что невилирует все старания для php7 в этом плане), заканчивая накладными расходами на рантайм (на вход передается массив чисел, но нам надо в этом убедиться. Либо нужно мутить какие-то хидден классы для типов либо O(n)). Именно по этой причине до сих пор нет ни дженериков ни uniton/intersection types ни чего-быто нибыло еще что хоть немного упростило бы жизнь разработчикам инструментов типа того же psalm.
Сторонние инструменты безусловно неплохо, но когда стат. анализатор и бьютифайер непосредственно интегрированы в среду разработки — это значительно упрощает работу с ними. Плагины и те не особо решают.
Незаслуженно никем не упомянут стат. анализатор в зенд студио.
если я правильно помню — он там совсем простенький, без вывода типов и в целом примитивный был. Даже по сравнению с тем что дает phpstorm.
Да и потом, просто раз уж вы ухватились за слова "Zend" — Phan например — творение Расмуса.
но когда стат. анализатор и бьютифайер непосредственно интегрированы в среду разработки
У такой стратегии развития есть несколько очевидных минусов:
- сложно расширять
- у каждой IDE должен быть свой анализатор.
Сегодня в целом есть тренд чуть другой, более рациональной стратегии — интеграция статического анализатора прямо в компилятор языка (language server). Профит — вам не надо ждать обновлений IDE что бы у вас появились все новые плюшки после обновления компилятора.
Да и потом, интеграция в PhpStorm того же psalm — дело не хитрое, можете даже плагин написать. Просто оно как бы не особо надо. А претифайеры и прочее — это вообще делается на раз два — добавляете ватчеры на файлы и вжух.
И повторюсь — основной плюс независимых инструментов — расширяемость.
Статический анализ PHP-кода на примере PHPStan, Phan и Psalm