Правила запахов кода (Code Smell)

Запахи кода -- это паттерны, которые указывают на возможные проблемы. Они не обязательно являются багами, но обозначают места, где код можно улучшить. Эти правила обнаруживают распространенные плохие практики, которых обычно стоит избегать.

Все правила запахов кода можно включать и выключать по отдельности.

---

Булевые аргументы (Boolean Arguments)

Идентификатор правила: code-smell.boolean-argument Серьезность: Warning

Что измеряет

Обнаруживает методы, принимающие параметры типа bool. Булевый аргумент обычно означает, что метод делает две разные вещи в зависимости от флага, что нарушает принцип единственной ответственности.

Пример

// Плохо: что означает `true`? Трудно читать в месте вызова.
$user->save(true);

// Сигнатура метода раскрывает проблему:
public function save(bool $sendNotification): void
{
    // ...сохраняет пользователя...
    if ($sendNotification) {
        // ...отправляет уведомление...
    }
}

Как исправить

1. Разделите на два метода с описательными именами:

   public function save(): void { /* ... */ }
   public function saveAndNotify(): void { /* ... */ }
   

2. Используйте enum, если вариантов больше двух:

   enum SaveMode {
       case Silent;
       case WithNotification;
       case WithAuditLog;
   }
   
   public function save(SaveMode $mode = SaveMode::Silent): void { /* ... */ }
   

Конфигурация

# qmx.yaml
rules:
  code-smell.boolean-argument:
    allowed_prefixes: [is, has, can, should, will, did, was]  # по умолчанию

Булевые параметры, имена которых начинаются с одного из этих префиксов, считаются самодокументируемыми и не помечаются. Например, $isActive, $hasPermission или $is_active (snake_case) будут пропущены с настройками по умолчанию. Установите [], чтобы помечать все булевые параметры.

bin/qmx check src/ --rule-opt="code-smell.boolean-argument:allowed_prefixes=is,has,can"

---

count() в цикле (count() in Loop)

Идентификатор правила: code-smell.count-in-loop Серьезность: Warning

Что измеряет

Обнаруживает вызовы count() (или sizeof()) внутри условий цикла. Когда count() находится в условии цикла, он пересчитывается на каждой итерации, что расточительно.

Пример

// Плохо: count() выполняется на каждой итерации
for ($i = 0; $i < count($items); $i++) {
    processItem($items[$i]);
}

Как исправить

Сохраните результат count() в переменную перед циклом:

// Хорошо: count() выполняется один раз
$itemCount = count($items);
for ($i = 0; $i < $itemCount; $i++) {
    processItem($items[$i]);
}

// Еще лучше: используйте foreach, когда возможно
foreach ($items as $item) {
    processItem($item);
}

---

Отладочный код (Debug Code)

Идентификатор правила: code-smell.debug-code Серьезность: Error

Что измеряет

Обнаруживает отладочные функции, оставленные в коде: var_dump(), print_r(), debug_print_backtrace(), debug_zval_dump() и подобные.

Пример

public function processPayment(Order $order): void
{
    var_dump($order);          // забытый отладочный вывод
    print_r($order->items);   // забытый отладочный вывод

    // основная логика...
}

Как исправить

Удалите все отладочные вызовы перед коммитом. Если нужно инспектировать данные:

• Используйте настоящий логгер ($this->logger->debug(...))
• Используйте отладчик (Xdebug)
• Используйте инструменты профилирования

Внимание

Отладочный вывод может раскрыть конфиденциальную информацию (пароли, токены, персональные данные) конечным пользователям. Поэтому правило имеет уровень Error, а не Warning.

---

Пустой catch (Empty Catch)

Идентификатор правила: code-smell.empty-catch Серьезность: Error

Что измеряет

Обнаруживает блоки catch, которые полностью пусты -- перехватывают исключение и не делают с ним абсолютно ничего. Это беззвучно проглатывает ошибки, делая баги крайне трудными для диагностики.

Пример

// Плохо: исключение беззвучно игнорируется
try {
    $this->sendEmail($user);
} catch (\Exception $e) {
    // ничего -- если отправка не удастся, никто не узнает
}

Как исправить

1. Залогируйте исключение:

   try {
       $this->sendEmail($user);
   } catch (\Exception $e) {
       $this->logger->error('Failed to send email', ['exception' => $e]);
   }
   

2. Перебросьте как доменное исключение:

   try {
       $this->sendEmail($user);
   } catch (\Exception $e) {
       throw new NotificationFailedException('Email sending failed', previous: $e);
   }
   

3. Обработайте ошибку явно, если она ожидаема и восстановима.

---

Подавление ошибок (Error Suppression)

Идентификатор правила: code-smell.error-suppression Серьезность: Warning

Что измеряет

Обнаруживает использование оператора подавления ошибок @. Оператор @ скрывает ошибки и предупреждения PHP, затрудняя поиск и исправление проблем.

Пример

// Плохо: если файл не существует, вы не узнаете, почему дальше что-то ломается
$data = @file_get_contents('/path/to/file');

// Плохо: подавление предупреждений от функции
$result = @json_decode($input);

Как исправить

Обрабатывайте ошибки явно:

// Хорошо: проверяем перед вызовом
if (!file_exists($path)) {
    throw new FileNotFoundException($path);
}
$data = file_get_contents($path);

// Хорошо: используем функции обработки ошибок
$result = json_decode($input, flags: JSON_THROW_ON_ERROR);

Конфигурация

# qmx.yaml
rules:
  code-smell.error-suppression:
    allowed_functions:  # по умолчанию: []
      - fopen
      - file_get_contents
      - unlink

Функции, для которых подавление ошибок через @ допустимо. Некоторые функции ввода-вывода возвращают false и генерируют предупреждение при ошибке -- использование @ с ними является распространённой практикой, когда возвращаемое значение проверяется явно. По умолчанию список пуст.

bin/qmx check src/ --rule-opt="code-smell.error-suppression:allowed_functions=fopen,unlink"

---

eval()

Идентификатор правила: code-smell.eval Серьезность: Error

Что измеряет

Обнаруживает использование eval(), которая выполняет произвольный PHP-код из строки. Это серьезный риск безопасности: если пользовательский ввод достигнет eval(), злоумышленник может выполнить любой код на вашем сервере.

Пример

// Плохо: уязвимость безопасности
$formula = $_GET['formula'];
$result = eval("return $formula;");

// Плохо: даже с "безопасным" вводом eval трудно отлаживать и поддерживать
eval('$config = ' . var_export($data, true) . ';');

Как исправить

• Используйте замыкания или вызываемые объекты вместо генерации кода в виде строк.
• Используйте паттерн Strategy вместо динамического выполнения кода.
• Используйте json_decode() для парсинга данных, а не eval().
• Используйте шаблонизаторы для генерации динамического вывода.

---

exit() / die()

Идентификатор правила: code-smell.exit Серьезность: Warning

Что измеряет

Обнаруживает использование exit() и die(). Эти функции немедленно завершают весь PHP-процесс, что:

• Предотвращает правильную обработку ошибок
• Делает код нетестируемым (PHPUnit не может перехватить exit)
• Обходит обработчики завершения и деструкторы

Пример

// Плохо: нетестируемо, нет обработки ошибок
if (!$user->isAdmin()) {
    die('Access denied');
}

// Плохо: предотвращает правильную обработку ответа
if ($error) {
    exit(1);
}

Как исправить

Бросайте исключения вместо этого:

// Хорошо: можно перехватить, протестировать и обработать
if (!$user->isAdmin()) {
    throw new AccessDeniedException('Access denied');
}

Примечание

exit() допустим в точках входа CLI (например, bin/console), где он устанавливает код выхода процесса. Это правило в основном касается использования exit/die внутри логики приложения.

---

goto

Идентификатор правила: code-smell.goto Серьезность: Error

Что измеряет

Обнаруживает использование оператора goto. goto делает поток управления непредсказуемым -- читателю приходится искать целевую метку, которая может быть в любом месте функции. Это делает код очень трудным для чтения и отладки.

Пример

// Плохо: спагетти-поток управления
function process(array $items): void
{
    foreach ($items as $item) {
        if ($item->isInvalid()) {
            goto cleanup;
        }
        // обработка элемента...
    }

    cleanup:
    // код очистки
}

Как исправить

Используйте стандартные конструкции управления потоком:

// Хорошо: понятный поток управления
function process(array $items): void
{
    foreach ($items as $item) {
        if ($item->isInvalid()) {
            $this->cleanup();
            return;
        }
        // обработка элемента...
    }
}

Используйте циклы, функции, ранние возвраты или исключения -- все они выражают намерение яснее, чем goto.

---

Суперглобальные переменные (Superglobals)

Идентификатор правила: code-smell.superglobals Серьезность: Warning

Что измеряет

Обнаруживает прямой доступ к суперглобальным переменным PHP: $_GET, $_POST, $_REQUEST, $_SERVER, $_SESSION, $_COOKIE, $_FILES, $_ENV.

Прямой доступ к суперглобальным переменным создает скрытые зависимости от глобального состояния, делая код трудным для тестирования и непредсказуемым.

Пример

// Плохо: жестко связан с глобальным состоянием
class UserController
{
    public function register(): void
    {
        $name = $_POST['name'];
        $email = $_POST['email'];
        $ip = $_SERVER['REMOTE_ADDR'];
        // ...
    }
}

Как исправить

Используйте внедрение зависимостей с объектами запроса:

// Хорошо: зависимости явные и тестируемые
class UserController
{
    public function register(ServerRequestInterface $request): ResponseInterface
    {
        $body = $request->getParsedBody();
        $name = $body['name'];
        $email = $body['email'];
        $ip = $request->getServerParams()['REMOTE_ADDR'];
        // ...
    }
}

Распространенные абстракции запросов:

• PSR-7: ServerRequestInterface (фреймворко-независимый)
• Symfony: Request из HttpFoundation
• Laravel: Illuminate\Http\Request

---

Избыточное внедрение в конструктор (Constructor Over-injection)

Идентификатор правила: code-smell.constructor-overinjection

Что измеряет

Обнаруживает конструкторы со слишком большим количеством внедрённых зависимостей. Длинный список параметров конструктора в кодовых базах с DI -- прямой сигнал нарушения принципа единственной ответственности (SRP): класс имеет слишком много коллабораторов.

Пороговые значения

Значение	Серьёзность	Что означает
8	Warning	Слишком много зависимостей, рассмотрите разделение
12+	Error	Класс явно нарушает SRP

Пример

// Плохо: конструктор зависит от слишком многих сервисов
class OrderProcessor
{
    public function __construct(
        private UserRepository $users,
        private ProductRepository $products,
        private InventoryService $inventory,
        private PricingService $pricing,
        private TaxCalculator $tax,
        private ShippingService $shipping,
        private NotificationService $notifications,
        private AuditLogger $audit,
        private MetricsCollector $metrics,
    ) {}
}

Как исправить

1. Разделите класс на меньшие, сфокусированные сервисы (например, OrderValidator, OrderFulfiller, OrderNotifier).
2. Используйте Фасад или Медиатор для группировки связанных зависимостей за единым интерфейсом.
3. Внедрите Command Bus -- вместо прямого внедрения обработчиков отправляйте команды.

Конфигурация

# qmx.yaml
rules:
  code-smell.constructor-overinjection:
    warning: 8
    error: 12

bin/qmx check src/ --rule-opt="code-smell.constructor-overinjection:warning=6"

---

Длинный список параметров (Long Parameter List)

Идентификатор правила: code-smell.long-parameter-list

Что измеряет

Обнаруживает методы и функции со слишком большим количеством параметров. Длинный список параметров затрудняет правильный вызов метода, его тестирование и часто указывает на то, что метод делает слишком много. Рассмотрите использование объекта параметров или разделение метода.

Пороговые значения

Стандартные пороги (методы и функции):

Значение	Серьёзность	Что означает
4	Warning	Стоит сгруппировать параметры
6+	Error	Слишком много параметров, необходим рефакторинг

Пороги для VO конструкторов (readonly класс, все promoted, пустое тело):

Значение	Серьёзность	Что означает
8	Warning	Стоит разделить value object
12+	Error	Слишком много полей, разделите value object

Исключение для конструкторов Value Object

Readonly классы с promoted-параметрами конструктора (PHP 8.2+) рассматриваются как Value Objects. readonly class с 6 promoted-свойствами и без логики в теле — это корректный дизайн, типизированный контейнер данных, а не признак плохой декомпозиции. Для таких конструкторов используются отдельные, повышенные пороговые значения.

Критерии определения VO (все должны быть истинны):

1. Класс имеет модификатор readonly
2. Все параметры конструктора — promoted-свойства (имеют модификатор видимости)
3. Тело конструктора пустое (нет операторов)

Если любое условие не выполнено, применяются стандартные пороги.

Пример

// Плохо: слишком много параметров, трудно запомнить порядок
public function createUser(
    string $name,
    string $email,
    string $phone,
    string $address,
    string $city,
    string $country,
    string $zipCode,
): User {
    // ...
}

// OK: readonly VO с promoted-свойствами использует повышенные пороги
final readonly class CreateUserRequest
{
    public function __construct(
        public string $name,
        public string $email,
        public string $phone,
        public string $address,
        public string $city,
        public string $country,
        public string $zipCode,  // 7 параметров — ниже vo-warning=8
    ) {}
}

Как исправить

1. Используйте объект параметров (DTO):

   final readonly class CreateUserRequest
   {
       public function __construct(
           public string $name,
           public string $email,
           public string $phone,
           public Address $address,
       ) {}
   }
   
   public function createUser(CreateUserRequest $request): User { /* ... */ }
   

2. Разделите метод, если параметры относятся к разным ответственностям.

Конфигурация

# qmx.yaml
rules:
  code-smell.long-parameter-list:
    warning: 4        # стандартные методы/функции
    error: 6
    vo-warning: 8     # readonly VO конструкторы
    vo-error: 12

bin/qmx check src/ --rule-opt="code-smell.long-parameter-list:warning=5"
bin/qmx check src/ --rule-opt="code-smell.long-parameter-list:error=8"
bin/qmx check src/ --rule-opt="code-smell.long-parameter-list:vo-warning=10"
bin/qmx check src/ --rule-opt="code-smell.long-parameter-list:vo-error=15"

---

Идентичные подвыражения (Identical Sub-expression)

Идентификатор правила: code-smell.identical-subexpression Серьезность: Warning

Что измеряет

Обнаруживает идентичные подвыражения, указывающие на ошибки копирования или логические баги. Правило выявляет четыре паттерна:

1. Одинаковые операнды в бинарных операциях -- одно и то же выражение по обе стороны оператора (например, $a === $a, $a - $a, $a && $a).
2. Дублирующиеся условия в цепочках if/elseif -- одно и то же условие проверяется дважды, что делает вторую ветку мёртвым кодом.
3. Одинаковые ветки тернарного оператора -- тернарный оператор, в котором ветки «истина» и «ложь» совпадают, что делает условие бессмысленным.
4. Дублирующиеся условия в match -- повторяющиеся условия в выражении match, где выполнится только первый вариант.

Операторы с легитимным использованием одинаковых операндов не помечаются: +, *, ., &, |, <<, >>.

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

Пример

class OrderService
{
    public function validate(Order $order): bool
    {
        // Плохо: одинаковые операнды -- всегда true, вероятно опечатка
        if ($order->total === $order->total) {
            // ...
        }

        // Плохо: вычитание значения из самого себя -- всегда 0
        $diff = $order->price - $order->price;

        // Плохо: дублирующееся условие -- вторая ветка -- мёртвый код
        if ($order->isPaid()) {
            return true;
        } elseif ($order->isPaid()) {
            return false;
        }

        // Плохо: одинаковые ветки тернарного оператора -- условие бессмысленно
        $status = $order->isActive() ? 'pending' : 'pending';

        // Плохо: дублирующееся условие match
        return match ($order->type) {
            'retail' => $this->handleRetail($order),
            'wholesale' => $this->handleWholesale($order),
            'retail' => $this->handleSpecial($order),  // никогда не выполнится
        };
    }
}

Как исправить

Это почти всегда баги -- проверьте каждое вхождение и исправьте предполагаемую логику:

1. Одинаковые операнды: Одна из сторон обычно является опечаткой. Замените на правильную переменную:

   // Было: $order->total === $order->total (всегда true)
   // Исправление: сравнить с ожидаемым значением
   if ($order->total === $order->expectedTotal) {
       // ...
   }
   

2. Дублирующиеся условия: Удалите дублирующую ветку или исправьте условие:

   if ($order->isPaid()) {
       return true;
   } elseif ($order->isRefunded()) {
       return false;
   }
   

3. Одинаковые ветки тернарного оператора: Либо условие не нужно, либо одна из веток содержит неверное значение:

   $status = $order->isActive() ? 'active' : 'pending';
   

4. Дублирующиеся условия match: Удалите дубликат или исправьте значение условия.

---

Неиспользуемые приватные члены (Unused Private Members)

Идентификатор правила: code-smell.unused-private Серьезность: Warning

Что измеряет

Обнаруживает приватные методы, свойства и константы, которые объявлены, но нигде не используются внутри класса. Неиспользуемые приватные члены -- это мёртвый код: они создают шум, увеличивают когнитивную нагрузку и могут указывать на незавершённый рефакторинг.

Правило учитывает граничные случаи:

• Магические методы: классы с __call/__callStatic пропускают проверку методов; классы с __get/__set пропускают проверку свойств
• Промоутнутые свойства: свойства, продвигаемые через конструктор, отслеживаются корректно
• Анонимные классы: приватные члены анонимных классов изолированы и не «утекают» в родительский класс
• Исключённые типы: интерфейсы, трейты и перечисления не анализируются
• Паттерны доступа: распознаёт $this->method(), self::method(), static::method(), доступ к свойствам и константам
• Разрешение трейтов: вызовы методов, определённых в трейтах, используемых тем же классом (в том же файле), распознаются, что снижает количество ложных срабатываний

Пример

class OrderService
{
    private string $unusedField = '';  // нигде не читается и не записывается

    private const LEGACY_LIMIT = 100;  // нигде не используется

    public function process(Order $order): void
    {
        // ... использует $order, но не обращается к $unusedField и LEGACY_LIMIT
    }

    private function oldHelper(): void  // нигде не вызывается
    {
        // осталось от предыдущей реализации
    }
}

Как исправить

• Удалите неиспользуемый член, если это действительно мёртвый код.
• Измените видимость на protected или public, если член используется подклассами или внешним кодом.
• Если член намеренно сохранён для будущего использования, подавите предупреждение с помощью @qmx-ignore code-smell.unused-private.

---

Недостижимый код (Unreachable Code)

Идентификатор правила: code-smell.unreachable-code

Что измеряет

Обнаруживает код, который никогда не может быть выполнен, потому что находится после терминального оператора (return, throw, exit/die, continue, break, goto). Мёртвый код добавляет шум, сбивает с толку читателей и может указывать на логическую ошибку.

Пример

public function process(Order $order): string
{
    if ($order->isPaid()) {
        return 'processed';
    }

    return 'pending';

    // Плохо: этот код никогда не выполнится
    $this->logger->info('Processing complete');
    $this->notify($order);
}

Как исправить

Удалите недостижимый код. Если код должен был выполняться, исправьте поток управления:

public function process(Order $order): string
{
    if ($order->isPaid()) {
        $this->logger->info('Processing complete');
        $this->notify($order);
        return 'processed';
    }

    return 'pending';
}

Конфигурация

# qmx.yaml
rules:
  code-smell.unreachable-code:
    warning: 1
    error: 1

bin/qmx check src/ --rule-opt="code-smell.unreachable-code:warning=1"
bin/qmx check src/ --rule-opt="code-smell.unreachable-code:error=1"

---

Конфигурация

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

# qmx.yaml
rules:
  code-smell.boolean-argument:
    enabled: true
    allowed_prefixes: [is, has, can, should, will, did, was]
  code-smell.debug-code:
    enabled: true
  code-smell.empty-catch:
    enabled: true
  code-smell.eval:
    enabled: false    # выключить, если есть легитимное использование eval
  code-smell.exit:
    enabled: true
  code-smell.goto:
    enabled: true
  code-smell.superglobals:
    enabled: true
  code-smell.constructor-overinjection:
    warning: 8
    error: 12
  code-smell.count-in-loop:
    enabled: true
  code-smell.error-suppression:
    enabled: true
    allowed_functions: []
  code-smell.long-parameter-list:
    warning: 4
    error: 6
  code-smell.unreachable-code:
    warning: 1
    error: 1
  code-smell.unused-private:
    enabled: true
  code-smell.identical-subexpression:
    enabled: true

Также можно отключить отдельные правила через опцию --disable-rule в CLI:

# Отключить конкретное правило
bin/qmx check src/ --disable-rule=code-smell.exit

# Отключить все правила запахов кода сразу (сопоставление по префиксу)
bin/qmx check src/ --disable-rule=code-smell