Skip to content

CommandConstantReturnCodeRector appends unreachable return after a break-less while(true) loop #9852

Description

@NathanTaco

Bug Report

Rector version: 2.6.2

Rule: CommandConstantReturnCodeRector (pulled in via RectorConfig::withComposerBased(symfony: true) / SymfonySetList::SYMFONY_CODE_QUALITY in a Symfony 8.1 app)

Minimal reproduction

Input — a command whose only loop is while (true) { ... } with no break, where every exit path is an explicit return from inside the loop body:

<?php

use Symfony\Component\Console\Command\Command;

class ExampleCommand extends Command
{
    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        while (true) {
            if ($this->shouldStop()) {
                return Command::SUCCESS;
            }

            if ($this->hasFailed()) {
                return Command::FAILURE;
            }

            $this->tick();
        }
    }
}

This compiles and analyses cleanly today: PHPStan already proves the method never falls off the end without returning, because the loop has no break and both branches that leave the loop return directly.

Actual output (--fix)

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        while (true) {
            if ($this->shouldStop()) {
                return Command::SUCCESS;
            }

            if ($this->hasFailed()) {
                return Command::FAILURE;
            }

            $this->tick();
        }

        return Command::SUCCESS;
    }

The rule appends return Command::SUCCESS; unconditionally at the end of execute() without checking whether the method already covers every path. Here it clearly doesn't: the while (true) loop has no break, so control can never reach the appended statement — it's provably dead code.

PHPStan (level 7+, deadCode.unreachable from the standard rule set) then flags it:

Line   src/Command/.../ExampleCommand.php
110    Unreachable statement - code above always terminates.
       🪪  deadCode.unreachable

Impact

We hit this on two real commands in production code (WmsProductsCommand, WcStockSimulatorCommand — both interactive/daemon-style commands whose while (true) loop is the entire method body). A single --fix run silently introduced 2 new PHPStan errors across otherwise-unrelated files, only caught because CI runs phpstan analyse after rector process.

Expected behavior

Before appending a fallback return Command::SUCCESS;, the rule should check reachability of the end of the method body (e.g. via the same reachability analysis PHPStan itself and Rector's own dead-code rules already perform) and skip insertion when the preceding statement is a loop with no break whose every branch already returns/throws.

Workaround

Skip the rule in rector.php:

->withSkip([
    CommandConstantReturnCodeRector::class,
])

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions