Рефакторинг #7: Улучшение стиля кодирования в PHP с помощью Rector

Продолжая предыдущий пост, мы изучаем другие конфигурации Rector, которые помогают нам улучшить наш код.

Забавно, но Rector — это инструмент для рефакторинга, но я бы соврал, если бы не использовал его для изучения пары хороших практик. Действительно, у них есть каталог рефакторингов, готовых к использованию и расширению.

В любом случае, переходим к конфигурационным наборам SetList::CODING_STYLE.

    $rectorConfig->sets([
        LevelSetList::UP_TO_PHP_74,
        SetList::CODE_QUALITY,
        SetList::CODING_STYLE,
    ]);

Обновления, которые мне очень нравятся

Мы начнем с некоторых первоклассных улучшений, которые, как мне кажется, должны применяться в каждом PHP-проекте.

Новая строка после утверждения

 class Invoice extends Model
 {
-    protected $appends = ['order_subtotal', 'total'];
-    protected $guarded = [];

+    protected $appends = ['order_subtotal', 'total'];
+
+    protected $guarded = [];

Это может показаться немного навязчивым, в том смысле, что могут быть случаи, когда вы захотите упаковать строки вместе, но я не думаю, что у вас возникнут с этим проблемы. Этот рефакторинг затрагивает только тот код, для которого вы действительно хотите иметь пространство для дыхания, например, методы и свойства классов, условия, циклы и т.д. На самом деле вот полный список по состоянию на август 2022 года.

Разделять несколько утверждений use.

 class Delivered
 {
-    use Dispatchable, InteractsWithSockets, SerializesModels;

+    use Dispatchable;
+    use InteractsWithSockets;
+    use SerializesModels;

Мне нравится вот это. Размещая утверждения use одно под другим, мне легче их найти. Также, скрытые жемчужины, если вы добавите/удалите одну из них, они появятся как добавления/удаления при просмотре git diff, вместо изменения в одну строку, на которое уйдет секунда.

Обратите внимание, что правило с новыми строками выше не влияет на утверждения use, они по-прежнему хорошо упакованы вместе.

Упрощение экранирования кавычек

-    return ['message' => 'You're already verified.'];

+    return ['message' => "You're already verified."];

О, я виноват в этом. Это довольно частое явление — начинать строку с одинарных кавычек, когда она приятно вписывается в соседние строки, а потом обнаружить, что нужно использовать ‘you’re’ с неудобным обратным слешем. То же самое с использованием двойных кавычек. Какой прекрасный рефактор.

Оберните переменные фигурными скобками

-    return "Service $service does not exist!";

+    return "Service {$service} does not exist!";

Действительно ли мне нужен этот рефакторинг? Довольно давно PHPStorm предложил использовать фигурные скобки во всех случаях. В последнее время я вижу, что он выступает за удаление ненужных фигурных скобок. Мои мысли сейчас разделились, но, похоже, мне изначально нравилось иметь их в строках. Поэтому я оставляю скобки и говорю PHPStorm, чтобы он пока не жаловался.

Преобразовать switch в if-else.

-    switch ($event->type) {
-        case 'payment_intent.succeeded':
-            $this->handlePaymentIntentSucceeded($event);
-            break;
-        default:
-            abort(404);
-    }

+    if ($event->type == 'payment_intent.succeeded') {
+        $this->handlePaymentIntentSucceeded($event);
+    } else {
+        abort(404);
+    }

Это отличный пример того, как разработчик при написании кода думает о будущих сценариях использования, делая вещи более сложными для чтения. Если ваш switch содержит только один case, лучше использовать старый добрый if. Кстати, я бы никогда не подумал делать этот рефакторинг вручную, этот инструмент просто супер.

Сопоставьте исключения catch с их типами

-    } catch (Card $e) {
-        $body = $e->getJsonBody();

+    } catch (Card $card) {
+        $body = $card->getJsonBody();

Сначала у меня были некоторые сомнения по поводу этого правила. Мне нравилась короткая версия $e, она была красивой и чистой, но я думаю, что правильное имя исключения дает больше контекста. Хорошо, хорошо, я оставляю этот вариант.

Унаследованный метод с той же видимостью, что и родительский

     use RefreshDatabase;

-    public function setUp():void

+    protected function setUp():void
     {
         parent::setUp();

Я не замечал этого раньше, но у меня было много тестовых классов, где я устанавливал функцию setUp как public. Я помню, как сделал это однажды в одном тесте, и с тех пор я копировал этот метод 😂.

Не делайте этой ошибки, генерируйте свои тесты с помощью php artisan make:test ExampleTest, и изменяйте заглушки по своему усмотрению с помощью php artisan stub:publish.

Модификации, к которым у меня смешанные чувства

Статические стрелочные функции и закрытия

    $schedule->command('some:dummy-command')
-        ->when(fn() => Carbon::now()->endOfWeek()->isToday());

+        ->when(static fn() => Carbon::now()->endOfWeek()->isToday());

Существуют два правила, которые на самом деле встречаются довольно часто: префикс стрелочных функций и закрытий с static всегда, когда это возможно. То есть, если закрытию не нужна ссылка $this, его можно сделать статическим. Однако я отказываюсь от этих правил, поскольку RFC отмечает, что они, вероятно, не нужны.

Статические замыкания используются редко: В основном они используются для предотвращения циклов $this, которые делают поведение GC менее предсказуемым. Большинству кода не нужно беспокоиться об этом.

Вы можете пропустить правила, просто добавив их в конфигурацию ректора, например, так:

    $rectorConfig->skip([
        // ...
        StaticArrowFunctionRector::class,
        StaticClosureRector::class,
    ]);

Преобразовать пост-инкременты в пре-инкременты

-    $count++;

+    ++$count;

Этот вопрос заставил меня сильно задуматься. Зачем мне это нужно? Я проверил выпуск и PR, которые добавили это правило, и выяснил, что это зеркальное правило из PHPStan. Я знаю, что есть некоторые крайние случаи, когда разработчики будут ++ везде, где только можно, в операторах if и ключах массивов, но, черт возьми, это правило сделало много кода немного подозрительным. Или я просто не привык к этому и это на самом деле довольно распространено. Я пропущу это сейчас, возможно, мы снова пересечемся в будущем (PostIncDecToPreIncDecRector).

Преобразование встроенных переменных в sprintfs

-    "Order canceled with reason {$reason}."

+    sprintf('Order canceled with reason %s', $reason)

Я не уверен, что понимаю, как версия sprintf является более читабельной, чем инкапсированная. Возможно, если бы я хотел сделать какое-то форматирование, но в существующем проекте форматирование уже было сделано другим способом. Это очень похоже на C, если не учитывать и этот вариант (EncapsedStringsToSprintfRector).

Здесь мы пока делаем паузу. Этот список, конечно, неполный, невозможно охватить все рефакторинги Code Style, которые он может сделать. Мы продолжим изучение других рефакторингов, таких как мертвый код и ранние возвраты, в другом посте. До встречи 🏠.