Ignore trailing if in scope for early exit

[Stadly]

The EarlyExit sniff has the setting ignoreStandaloneIfInScope. What about ignoring ifs that are the trailing part of the scope, and not only ifs that are the only part of the scope? Then also such cases could be ignored:

$errors = [];
foreach ($values as $value) {
    $error = $value->validate();

    if ($error !== null) {
        $errors[] = $error;
    }
}

In addition to the examples shown in #371 and #409.

Thank you for your consideration and great work!

[amorimjuliana]

Same issue here. For those cases, the early return makes the code less readable and more verbose:

$errors = [];
foreach ($values as $value) {
    $error = $value->validate();

    if ($error === null) {
         continue;
    }

    $errors[] = $error;
}

In that sense, I think an option ignoreTrailingIf would be a good addition.

[peldax]

Simple ignore would lead to mixture of EarlyExits and TrailingIfs across codebase. In my opinion you should either require EarlyExit or require TrailingIf.

requireTrailingIf: true would always refactor the code to use TrailingIf, false to EarlyExit.

[jrfnl]

Suggestion: what about ignoring trailing ifs if the content is only one line as early exit really doesn't make a difference in that case ?

function foo() {
    $var = function_call();
    if ($var === true) {
        call_other_function();
    }
}

// Versus
function foo() {
    $var = function_call();
    if ($var !== true) {
        return;
    }

    call_other_function();
}

[kukulich]

New option implemented in 055af01

[Jurigag]

To be honest i like this new option, but don't like it allows no early exit if only one line - it should be more like one instruction, let's take a this example:

 protected function addResultBasedOnCondition($budgetEnded, $timeEnded, $timeAlmostOver, $campaignUnderPerforming): void
    {
        if ($budgetEnded) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_BUDGET_ENDED)
            );
        }
        if ($timeEnded) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_TIME_ENDED)
            );
        }
        if ($timeAlmostOver) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_TIME_ALMOST_OVER)
            );
        }
        if ($campaignUnderPerforming) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_CAMPAIGN_IS_UNDERPERFORMING)
            );
        }
    }

It will require early exit on if ($campaignUnderPerforming) { even if ignoreOneLineTrailingIf is enabled, but if we format it like this:

protected function addResultBasedOnCondition($budgetEnded, $timeEnded, $timeAlmostOver, $campaignUnderPerforming): void
    {
        if ($budgetEnded) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_BUDGET_ENDED)
            );
        }
        if ($timeEnded) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_TIME_ENDED)
            );
        }
        if ($timeAlmostOver) {
            $timeAlertNotificationsCollection->addResult(
                new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_TIME_ALMOST_OVER)
            );
        }
        if ($campaignUnderPerforming) {
            $timeAlertNotificationsCollection->addResult(new TimeAlertNotificationDto($campaignName, $brandName, Notification::TYPE_TIME_ALERT_CAMPAIGN_IS_UNDERPERFORMING));
        }
    }

Then there is no error reported. Can this somehow be improved? @kukulich

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.