Rector: ChangeAndIfToEarlyReturnRector should use `continue` within loops, instead of `return`

Created on 26 Nov 2020  路  2Comments  路  Source: rectorphp/rector

Bug Report

early returns introduced in foreach-loops alter logic

Minimal PHP Code Causing Issue

https://getrector.org/demo/8ed4081f-01c5-4bab-9fec-6d001c73fbcb#result

Expected Behaviour

since rector adds return the loop does not iterate all elements, as it did before the change.

input

<?php

declare(strict_types=1);

final class DemoFile
{
    public function abc() {
        foreach($this->columns as $column) {
            // guess validation patterns based on fieldnames
            if (!empty($field_validators[$column->Field]) && empty($this->validates_format_of[$column->Field])) {
                    $this->validates_format_of[$column->Field] = [
                        'with' => $field_validators[$column->Field],
                    ];
            }
        }
    }
}

current result

<?php

declare(strict_types=1);

final class DemoFile
{
    public function abc() {
        foreach ($columns as $column) {
            // guess validation patterns based on fieldnames
            if (empty($field_validators[$column->Field])) {
                return;
            }
            if (!empty($this->validates_format_of[$column->Field])) {
                return;
            }
            $this->validates_format_of[$column->Field] = [
                'with' => $field_validators[$column->Field],
            ];
        }
}

expected result

<?php

declare(strict_types=1);

final class DemoFile
{
    public function abc() {
        foreach ($columns as $column) {
            // guess validation patterns based on fieldnames
            if (empty($field_validators[$column->Field])) {
                continue;
            }
            if (!empty($this->validates_format_of[$column->Field])) {
                continue;
            }
            $this->validates_format_of[$column->Field] = [
                'with' => $field_validators[$column->Field],
            ];
        }
    }
}

Most helpful comment

It seems already fixed. I am closing this.

All 2 comments

Thanks for reporting :+1:

We'll need a failing test for ChangeAndIfToEarlyReturnRector

It's matter of combining input + exptected content to 1 file: https://github.com/rectorphp/rector/blob/master/docs/how_to_add_test_for_rector_rule.md

It seems already fixed. I am closing this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

benr77 picture benr77  路  4Comments

lstrojny picture lstrojny  路  4Comments

pavarnos picture pavarnos  路  4Comments

HDVinnie picture HDVinnie  路  3Comments

carusogabriel picture carusogabriel  路  5Comments