Апгрейд и рефакторинг PHP-проектов — теперь это просто с Rector

[FanatPHP]

Томаш очень крутой чувак. Взять хотя бы его недавний эксперимент по переезду с Симфони на Ларавель, просто по приколу. Жаль только, что он не привел никаких подробностей, только чеклист:

✅ regex Twig to Blade, syntax is 99 % similar
✅ 2 custom @rectorphp rules for controller class names, view() and routes
✅ copy configs from raw project
✅ tidy details

А вам как раз спасибо за такой подробный рассказ!

[SerafimArts]

А вам как раз спасибо за такой подробный рассказ!

Там ещё сам доклад огонь. По моему скромному мнению — один из лучших докладов на конференции.

[Alex-Volodin]

спасибо Кирилл!)

[Alex-Volodin]

спасибо)

[tommyangelo27]

Мы на проектах пришли к такой схеме:

• Ректор запускается в пайплайне после пуша кода в битбакет

• Запускаем в режиме --dry-run, чтобы только показывал ошибки, а не исправлял

• Автор кода может просмотреть рекомендации Ректора, и исправить (вручную или тем же ректором), или, в некоторых случаях, пропустить изменения, по согласованию с членами команды

Сделали так, потому что рефакторинг от Ректора в некоторых случаях ухудшает читаемость кода.

[Alex-Volodin]

Да, тоже хорошее решение.

Мне если какие-то правила начинают мешать или что-то не так рефакторить, то я скипаю сам файл, либо правило, либо правило в этом файле:

<?php
    $rectorConfig->skip([
        CatchExceptionNameMatchingTypeRector::class,
        
        // или

        CatchExceptionNameMatchingTypeRector::class => [
             __DIR__ . '/src/Some.php'
        ],
    ];
    

[SerafimArts]

Сделали так, потому что рефакторинг от Ректора в некоторых случаях ухудшает читаемость кода.

+1. Особо проблем доставляют некоторые правила из CodeQuality и CodingStyle.

Вот список, который пока что собираю, что отключаю (через ->skip([ ... ])) постоянно в каждом проекте, где есть ректор. Отдельно отмечу символами !!! то, что может привести к фаталам и падениям всего проекта после "ректоринга", обратите внимание.

• !!! ClassPropertyAssignToConstructorPromotionRector — Ломает код, заменяя проперти на промоутед варианты (ломает доктрину, JMS или любые другие гидраторы с newInstanceWithoutConstructor()).
• VarConstantCommentRector — Может заменить докблок, например list<string> на string[].
• !!! ReadOnlyPropertyRector — Ломает доктрину, добавляя ридонли для проксей.
• ChangeOrIfReturnToEarlyReturnRector — выражения if ($a || $b) разделяет на 2 разных.
• !!! UnSpreadOperatorRector — Ломает код, заменяя __construct(Some ...$some) на __construct(array $some). Это вообще дичь какая-то, имхо.
• CatchExceptionNameMatchingTypeRector — заменяет catch(Throwable $e) на catch(Throwable $throwable) (зачем? не понятно)
• SplitDoubleAssignRector — Заменяет линейную инициализацию $from = $to = null; на два разных выражения.
• !!! FinalizePublicClassConstantRector — Добавляет final к публичным константам, даже если они где-то переопределяются в будущем.
• FlipTypeControlToUseExclusiveTypeRector — Заменяет строгую проверку $some === null на инвертированную !$some instanceof Example. Опять же, вообще не понятно зачем и кому такое нужно.
• JoinStringConcatRector — Объединяет отформатированные строчки (как в примере чуть ниже) с конкатенацией в одну большую простынестроку, которую фиг прочитаешь:
  

  $message = 'Whoops, something went wrong. Please '
  . 'blah blah blah blah blah blah blah blah blah blah '
  . 'blah blah blah blah blah blah blah blah blah blah '
  . 'and delete your OS';

• ReturnBinaryAndToEarlyReturnRector — заменяет return $some && $any; на 4 строки: if (!$some) { return false; } + return $any.

Пока это всё, продолжаю вести наблюдение. Если у кого есть похожие кейсы, где ректор что-то ломает или ухудшает читаемость — просьба накидать тоже, по возможности, правил с описаниями. Добавлю к себе в коллекцию)

P.S. Извините за фигово отформатированный коммент. Чёрт его знает как сделать его более читаемым.

[SerafimArts]

Вот ещё три правила, которые могут сломать код, которые я обнаружил за последние 2 дня:

• !!! RenameClassRector — Иногда может сломать код, подсунув некорректный класс. В моём случае оно заменило все use статменты, которые ссылались PSR-16 CacheInterface на Symfony CacheInterface, в результате чего код обвалился (т.к. инстансы внутри были PSR, а не симфони).
• !!! AddDefaultValueForUndefinedVariableRector — ломает енамы.

В частности:

private function caseToScalar(\BackedEnum $case): string|int
{
    return $case->value;
}

// Заменило на
private function caseToScalar(\BackedEnum $case): string|int
{
    $case = null;
    return $case->value;
}

• !!! PrivatizeLocalGetterToPropertyRector — ломает код в случае использования каррирования/каста к анонимке.

Может заменить код:

$result = array_map($this->foo(...), $collection); // Первый аргумент анонимка

// На некорректный

$result = array_map($some->property, $collection); // Теперь это скалярный стринг

• ChangeReadOnlyVariableWithDefaultValueToConstantRector — вот это на вкус и цвет.

Есть код, вида:

class SomeException

// + куча методов, вида

public static function fromSomeContext(string $value): self
{
    $message = 'Something went wrong with value ' . $value;
    return new static($message, self::CODE_CONTEXT);
}

Это правило берёт константные строки ($message), которые используются только в этом методе и выносит их все в константы. В результате в классе 100500 констант. Вроде как правильно в теории, но фактически читаемость ухудшается.

[FanatPHP]

Может потянуть на полноценную статью! :)

[SerafimArts]

Да чёрт его знает.

Я просто сейчас (месяц назад) на новом проекте воткнул его в качестве дополнения к кодстайлу. Такой себе phpcs на максималках. Вот сейчас и огребаю по дороге, когда сталкиваюсь с его проблемами.

Ну и более того, тут в списке выше, помимо проблем вида "нельзя везде ставить readonly" или "нельзя везде поля заменять на promoted" есть очевидные баги, которые через пол года уже наверняка пофиксятся, поэтому материал устареет быстро.

Так что пока что в виде комментариев вот так, чтобы те, кто статью прочитали — были более внимательными (у меня уже 16 правил отключено).

[shushu]

А как вы вообще апгрейд делаете?

Из статьи понятно что вы апгрейдите код от версии 7, прямиком к версии 8.

А с деплоем как? Или все на кубернетисах?

Ведь задеплоив код который ожидает версию пхп 8, на машины с версией 7, понятное дело, работать не будет. Каков процесс?

Лично я делал "переходную версию", которая поддерживает как версию 7, так и версию 8. В итоге процесс был такой:

1. Деплой нового кода

2. Апгрейд пхп версии на машинах. Обычно это половина всех нод за раз.

После этого можно использовать фичи из пхп 8 в проекте

Я пытался использовать ректор что бы хотя бы найти все места где код "не работал бы" в пхп 8, но не нашёл способа как это сделать.

[Alex-Volodin]

Из статьи понятно что вы апгрейдите код от версии 7, прямиком к версии 8.

А с деплоем как? Или все на кубернетисах?

Да у нас кубер, до этого был сворм. У нас есть возможность менять образ PHP прямо в проекте и я делаю это вместе с апгрейдом кодом. Всё в рамках одного мердж-реквеста и одного деплоя.

Если у вас версию PHP нужно менять отдельно, то делать "переходную версию" это единственное безопасное решение.

Я пытался использовать ректор что бы хотя бы найти все места где код "не работал бы" в пхп 8, но не нашёл способа как это сделать.

Попробуйте ещё использовать инструмент стат анализа PHPStan. Он может указать например какие методы не существуют.

Ещё у Rector можно насильно указать в какой версии PHP делать рефакторинг. Например у меня локально на компьютере стоит PHP 8.1, но в контейнере в проекте текущая 7.4. Я хочу сделать в проекте 8.0 и чтобы бы код тоже был под 8.0. Для этого в конфиге rector.php указываю

$rectorConfig->phpVersion(PhpVersion::PHP_80);

и теперь я могу своей локальной пыхой (которая 8.1) rector'ить код под 8.0.

И может таким образом он сможет показать какие участки кода нужно улучшить.

[shushu]

Спасибо за совет с PHPStan. Попробую.

С ректором пробовал так, он меняет все подряд, оптимизируя код который бы и так работал бы с пхп8, но используя пхп8 фичи. И изменений было очень много. Поэтому решение не совсем подходило