Php-cs-fixer: php-cs-fixer is removing variables defaulted to null.

Created on 31 Jan 2017  Â·  4Comments  Â·  Source: FriendsOfPHP/PHP-CS-Fixer

The PHP version you are using:

PHP 7.1.0

PHP CS Fixer version you are using:

PHP CS Fixer 2.0.0

The command you use to run the fixer (and the configuration file you are using if any).

php-cs-fixer fix

Config:

return PhpCsFixer\Config::create()
    ->setRiskyAllowed(true)
    ->setRules(array(
        '@Symfony' => true,
        '@Symfony:risky' => true,
        'array_syntax' => array('syntax' => 'long'),
        'combine_consecutive_unsets' => true,
        // one should use PHPUnit methods to set up expected exception instead of annotations
        'general_phpdoc_annotation_remove' => array('expectedException', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp'),
        'heredoc_to_nowdoc' => true,
        'no_extra_consecutive_blank_lines' => array('break', 'continue', 'extra', 'return', 'throw', 'use', 'parenthesis_brace_block', 'square_brace_block', 'curly_brace_block'),
        'no_unreachable_default_argument_value' => true,
        'no_useless_else' => true,
        'no_useless_return' => true,
        'ordered_class_elements' => true,
        'ordered_imports' => true,
        'php_unit_strict' => true,
        'phpdoc_add_missing_param_annotation' => true,
        'phpdoc_order' => true,
        'psr4' => true,
        'strict_comparison' => true,
        'strict_param' => true,
    ))
    ->setFinder(
        PhpCsFixer\Finder::create()
            ->exclude('tests/Fixtures')
            ->in(__DIR__)
    )
;

A minimal sample of valid PHP code that is not fixed correctly (if applicable).

The following code is changed from:


public function sidePanelAction(Request $request, $selected_category_id, $count_posts, $show_active_only)

The fixed version of that code after you run the fixer and the version you expected.


public function sidePanelAction(Request $request, $selected_category_id = null, $count_posts, $show_active_only)

php-cs-fixer is removed the "= null" for the variables selected_category_id.

This is only an issue when rendering esi within Symfony:

{{ render_esi(controller('AcmeBlogBundle:Category:sidePanel', {'selected_category_id': category.id|default(null), 'count_posts': false, 'show_active_only': true })) }}

Symfony will automatically strip the 'selected_category_id' variable because it is null. So it results in not sending that to the action. Now, because the variable isn't defined as null, it results in an error.

Controller "Acme\BlogBundle\Controller\CategoryController::sidePanelAction()" requires that you provide a value for the "$selected_category_id" argument (because there is no default value or because there is a non optional argument after this one).
500 Internal Server Error - RuntimeException
kinquestion

Most helpful comment

Looks like you switched before and after in your description, and this is a result of running the no_unreachable_default_argument_value fixer. Seems legit, though.

All 4 comments

Looks like you switched before and after in your description, and this is a result of running the no_unreachable_default_argument_value fixer. Seems legit, though.

Agreed with @localheinz

On a side note the fixer doesn't fix the Twig statement.
So what might be happening is some part of SF and/or Twig uses reflection to fill in the default if set on the method signature. After it is removed by the fixer this does break you setup.
That is just me guessing, would be nice to find out.

In the mean time you can disable the fixer in your configuration.

Looking at what the fixer did I think there is no bug in it. The change was valid and was configured to happen, however this caused an issue when using the fixed code with another system (possibly SF).
Since we cannot detect if code execution relies on another system that values the method/function signature differently than PHP itself; we cannot determine if the change is risky.
The user of the fixer does know this and as such should not ask the fixer to change the code.

We try our best to make sure the tool only makes changes that do not effect code execution and if it possible does we document it (including marking the fixer as risky). However the scope of this is PHP executing it, therefor I'm closing this issue.

Thank you @trsteel88 for opening this issue and taking time to talk to us. To resolve your issue please remove the no_unreachable_default_argument_value rule from your configuration.
Feel free to comment or ask about this more here :)

Thanks for the help. Updating no_unreachable_default_argument_value to false resolved the problem.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

grachevko picture grachevko  Â·  3Comments

Bilge picture Bilge  Â·  3Comments

kcloze picture kcloze  Â·  3Comments

ro0NL picture ro0NL  Â·  3Comments

aidantwoods picture aidantwoods  Â·  3Comments