Php-cs-fixer: NoUselessElse fixer applies invalid fix

[localheinz]

Noticed that applying the no_useless_else fixer on

return array_map(function ($name, $options) use ($isMany) {
    if (array_key_exists('model', $options)) {
        $alias = $options['model'];
    } else {
        $alias = Inflector::singular($name);
    }

    return new Relation(
        $name,
        $alias,
        $isMany
    );
}, array_keys($value), array_values($value));

results in

return array_map(function ($name, $options) use ($isMany) {
    if (array_key_exists('model', $options)) {
        $alias = $options['model'];
    }
    $alias = Inflector::singular($name);

    return new Relation(
        $name,
        $alias,
        $isMany
    );
}, array_keys($value), array_values($value));

Any idea? Using https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/tag/v2.0.0-alpha on

PHP 5.6.21 (cli) (built: Apr 29 2016 02:31:44)
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies

/cc @SpacePossum

Related to #1773.

[soullivaneuh]

Also got an issue with no_useless_else

This:

if (null !== $options['user']) {
    $qb = $er->getByOwnerQB($options['user']);
} else {
    $qb = $er->createQueryBuilder('s');
}

Is converted to this:

if (null !== $options['user']) {
    $qb = $er->getByOwnerQB($options['user']);
}
$qb = $er->createQueryBuilder('s');

But in this case, $qb will be overridden even if $options['user'] is not null.

[localheinz]

@Soullivaneuh

Looks like the exact same thing, right?! 😃

[soullivaneuh]

@localheinz Well, indeed. End of the day, you know... :-)

[greg0ire]

"Me too" :trollface:

[SpacePossum]

just a little longer, fix is on it way...

[SpacePossum]

btw. @Soullivaneuh could you share a bit more of the code being changed incorrectly, it is prop. because of the tokens found just before the if (I hope, otherwise there is another issue..)

[soullivaneuh]

@SpacePossum Of course!

public function configureOptions(OptionsResolver $resolver)
{
    $resolver
        ->setDefaults([
            'label' => 'form.server',
            'empty_value' => 'form.server_choose',
            'translation_domain' => 'global',
            'class' => 'AppBundle:Server',
            'choice_label' => 'nameForSelect',
            'user' => null,
            'test_compliant' => null,
            'ip_network' => null,
            'ip_system' => null,
            'query_builder' => function (Options $options) {
                return function (ServerRepository $er) use ($options) {
                    if (null !== $options['user']) {
                        $qb = $er->getByOwnerQB($options['user']);
                    } else {
                        $qb = $er->createQueryBuilder('s');
                    }
                    $qb->leftJoin('s.ipV4s', 'ip');

                    if (null !== $options['test_compliant']) {
                        $qb->andWhere('s.testCompliant = :testCompliant')->setParameter('testCompliant', $options['test_compliant']);
                    }

                    if (null !== $options['ip_network']) {
                        $qb->andWhere('ip.network = :ip_network')->setParameter('ip_network', $options['ip_network']);
                    }

                    if (null !== $options['ip_system']) {
                        $qb->andWhere('ip.systemIp = :ip_system')->setParameter('ip_system', $options['ip_system']);
                    }

                    return $qb
                        ->andWhere('s.status = :status')->setParameter('status', Status::GOOD)
                        ->orderBy('s.name, s.alias')
                    ;
                };
            },
        ])
        ->setAllowedTypes('user', ['AppBundle\Entity\User', 'int', 'null'])
        ->setAllowedTypes('test_compliant', ['bool', 'null'])
        ->setAllowedValues('ip_network', [null, IpV4::NETWORK_PUBLIC, IpV4::NETWORK_PRIVATE])
        ->setAllowedTypes('ip_system', ['bool', 'null'])
    ;
}

[SpacePossum]

Thanks!
Indeed same issue, the return before the if, will fix :)

[soullivaneuh]

@keradus This should be closed, should not be?

[keradus]

indeed, thx.