Php-cs-fixer: Fixer takes almost a WHOLE DAY to finish on ALREADY FIXED code

[mvorisek]

Version: latest stable 2.16

Code:

https://github.com/mvorisek/TCPDF/tree/psr12_cs

Command:

vendor\friendsofphp\php-cs-fixer\php-cs-fixer fix -v

Fixer output:

Fixed all files in 74065.269 seconds, 522.000 MB memory used

I think it is related with https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/4656 even the main issue there is with an unfixed code.

This issue should considered as fix asap bug as performance like this makes CS Fixer almost impossible to run with CI not mentioning the usability on the programmer side.

[mvorisek]

@SpacePossum array_indentation rule is very slow even on fixed code, is there something that can be easily improved?

steps to reproduce:

1. download https://raw.githubusercontent.com/tecnickcom/TCPDF/6.3.5/fonts/cid0cs.php
2. config:

<?php

$finder = PhpCsFixer\Finder::create()
    ->in([__DIR__])
    ->exclude([
        'vendor',
    ]);

return PhpCsFixer\Config::create()
    ->setRiskyAllowed(true)
    ->setRules([
        '@PhpCsFixer' => true,
        '@Symfony:risky' => true,
        '@PHP71Migration' => true,
        '@PHP71Migration:risky' => true,

        // required by PSR-12
        'concat_space' => [
            'spacing' => 'one',
        ],

        // disable some too strict rules
        'phpdoc_types_order' => [
            'null_adjustment' => 'always_last',
            'sort_algorithm' => 'none',
        ],
        'single_line_throw' => false,
        'yoda_style' => [
            'equal' => false,
            'identical' => false,
        ],
        'native_function_invocation' => false,
        'void_return' => false,
        'blank_line_before_statement' => [
            'statements' => ['break', 'continue', 'declare', 'return', 'throw', 'exit'],
        ],
        'combine_consecutive_issets' => false,
        'combine_consecutive_unsets' => false,
        'multiline_whitespace_before_semicolons' => false,
        'no_superfluous_elseif' => false,
        'ordered_class_elements' => false,
        'php_unit_internal_class' => false,
        'php_unit_test_class_requires_covers' => false,
        'phpdoc_add_missing_param_annotation' => false,
        'return_assignment' => false,
        'comment_to_phpdoc' => false,
        'list_syntax' => ['syntax' => 'short'],
        'general_phpdoc_annotation_remove' => [
            'annotations' => ['author', 'copyright', 'throws'],
        ],
        'nullable_type_declaration_for_default_null_value' => [
            'use_nullable_type_declaration' => false,
        ],

        // disable too destructive formating for now
        'single_line_comment_style' => false,
        'random_api_migration' => [
            'replacements' => [
                'mt_rand' => 'random_int',
                'rand' => 'random_int',
            ],
        ],
        'declare_strict_types' => false,
        'php_unit_set_up_tear_down_visibility' => true,
    ])
    ->setFinder($finder);

1. fix code with fixer
2. delete cache and run fixer again, notice array_indentation rule is very slow

[keradus]

Observation (not root cause of the problem, maybe worth a dedicated ticket ?)
TokenizerLinter looks way slower than ProcessLinter on big files

ker@dus:~/github/PHP-CS-Fixer λ php php-cs-fixer fix --rules=array_indentation debug/cid0cs.php 
Loaded config default from "/home/d.ruminski/github/PHP-CS-Fixer/.php_cs.dist".
Paths from configuration file have been overridden by paths provided as command arguments.
string(41) "PhpCsFixer\Linter\ProcessLinter::lintFile"
string(18) "ProcessLinter time"
float(0.0019989013671875)

ker@dus:~/github/PHP-CS-Fixer λ php php-cs-fixer fix --rules=array_indentation debug/cid0cs.php 
Loaded config default from "/home/d.ruminski/github/PHP-CS-Fixer/.php_cs.dist".
Paths from configuration file have been overridden by paths provided as command arguments.
string(43) "PhpCsFixer\Linter\TokenizerLinter::lintFile"
string(20) "TokenizerLinter time"
float(6.3825531005859)

[keradus]

@mvorisek , the example file you gave has 698517 tokens... :sob:

[mvorisek]

@mvorisek , the example file you gave has 698517 tokens... 😭

that itself should be fine, but the code complexity is probably too bad

[keradus]

@mvorisek , for now, I would suggest you to exclude the file from fixing in your config file.

1.7 MB file (or 700k tokens) is very huge PHP file, out of regular human-written files. Isn't it autogerenated? Do you expect someone to modify this file manually, or do a review of it ?

I fully agree that it would be great if the tool would be able to process the file in a reasonable time. I spent 2 hours on this topic and unfortunately cannot bring any valuable improvement in that time. If you want, please investigate the topic on your side and we will gladly accept a PR.

As 2MB file of code is our of regular usage, I would call it out of scope for core team to take care of this request - we have limited time capacity and we are in need to focus on topics that will impact majority of community, like PHP 8 support.

For that, I'm closing the issue on GitHub. Feel free everyone to continue the discussion on the topic, and I'm very happy to accept PR for it.
I hope you will understand my decision.

[mvorisek]

Please reopen. There are much more files that big, the tcpdf.php itself is about 1MB too. I think this should be fixed as for smaller files the tital time is lower, but probably still slow enough with a lot of space to improve due non linear complexity.

[SpacePossum]

Besides the tokenizer I think the alignment fixers issues are the same as https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/3996
Until someone figured out a way to fix this I would suggest disabling the alignment rules or skip the files (also based on the findings of Keradus).

[mvorisek]

@SpacePossum @keradus reading the discussion, comment https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/3996#issuecomment-456733931 tells that https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/4250 might be a fix for this. Can that PR be merged soon?

[keradus]

4250 is not actively working on, we are focusing on PHP v8 support.

also, #4250 will help only when the fixing itself is slow due to huge amount of tokens to be inserted. in concern you spotted, the fixer is slow for already fixed file, due to amount of arrays and distance between opening and ending of them measured in thousands of tokens

[SpacePossum]

The issue with the alignment fixers is that these insert placeholder tokens even if there is nothing to fix, as such these are always slow as on big token sets. I agree on the focus though, we cannot do PHP8 and PSR12 while also looking into this.