easy-coding-standard / easy-coding-standard

The Easiest way to add coding standard to your PHP project
https://tomasvotruba.com/blog/zen-config-in-ecs
MIT License
1.44k stars 80 forks source link

Unable to run fixer on version `11.5.0` #98

Closed milewski closed 1 year ago

milewski commented 1 year ago

Hi I have upgraded to the latest version 11.5.0 and started getting this error:

image image
TomasVotruba commented 1 year ago

Hi,

thanks for reporting.

What rule is responsible for this?

milewski commented 1 year ago

Hi, the issue doesn't seem to be related to any fixer in specific, I have tried to toggle them all but it doesn't seem they are in conflict / infinite loop or things like that, what I noticed is a huge drop in performance between 11.5.0 and 11.4.5, here are my findings:

System:

MacOS Monterey 12.6.6
Processor: 4 GHz Quad-Core Intel Core i7
Memory: 32 GB 1867 MHz DDR3

PHP 8.2.5 (cli) (built: Apr 14 2023 18:04:14) (NTS)
Docker version 24.0.2, build cb74dfc

Running the code in an alpine linux image: `php:8.2-fpm-alpine3.16`

11.5.0:

time php ./vendor/bin/ecs check --fix --clear-cache

real    16m21.703s
user    0m0.195s
sys     0m0.086s

This is the memory / CPU usage of it within the docker container while the task was running:

image

11.4.5:

time php ./vendor/bin/ecs check --fix --clear-cache

real    1m16.937s
user    0m0.212s
sys     0m0.090s

This is the memory / CPU usage of it within the docker container while the task was running:

image
TomasVotruba commented 1 year ago

I see. The problem could be in any rule/set you use in ecs.php. Is this difference same with single rule?

It could be transitional issue with upgrade of php-cs-fixer or PHP_CodeSniffer. How did both packaes versions changed between the 2 ECS version you use?

milewski commented 1 year ago
milewski commented 1 year ago
This is my configuration file ```php parallel(); $config->paths([ __DIR__ . '/app', __DIR__ . '/database', __DIR__ . '/config', __DIR__ . '/routes', __DIR__ . '/tests', __DIR__ . '/lang', __DIR__ . '/nova-components', ]); $config->skip([ // IgnoreFixer::class NoBlankLinesAfterClassOpeningFixer::class, ClassDefinitionFixer::class, ]); /** * Or Manually include new fixers */ $options = [ ArrayPushFixer::class => true, BacktickToShellExecFixer::class => false, EregToPregFixer::class => true, MbStrFunctionsFixer::class => false, ModernizeStrposFixer::class => false, NoAliasFunctionsFixer::class => true, NoAliasLanguageConstructCallFixer::class => true, NoMixedEchoPrintFixer::class => true, PowToExponentiationFixer::class => true, RandomApiMigrationFixer::class => true, SetTypeToCastFixer::class => true, ArraySyntaxFixer::class => true, NoMultilineWhitespaceAroundDoubleArrowFixer::class => true, NormalizeIndexBraceFixer::class => true, // NoTrailingCommaInSinglelineArrayFixer::class => true, NoWhitespaceBeforeCommaInArrayFixer::class => true, TrimArraySpacesFixer::class => false, WhitespaceAfterCommaInArrayFixer::class => true, BracesFixer::class => false, CurlyBracesPositionFixer::class => false, EncodingFixer::class => true, NoMultipleStatementsPerLineFixer::class => true, NonPrintableCharacterFixer::class => true, NoTrailingCommaInSinglelineFixer::class => true, OctalNotationFixer::class => true, PsrAutoloadingFixer::class => true, ClassReferenceNameCasingFixer::class => true, ConstantCaseFixer::class => true, IntegerLiteralCaseFixer::class => false, LowercaseKeywordsFixer::class => true, LowercaseStaticReferenceFixer::class => true, MagicConstantCasingFixer::class => true, MagicMethodCasingFixer::class => true, NativeFunctionCasingFixer::class => true, NativeFunctionTypeDeclarationCasingFixer::class => true, CastSpacesFixer::class => true, LowercaseCastFixer::class => true, ModernizeTypesCastingFixer::class => true, NoShortBoolCastFixer::class => true, NoUnsetCastFixer::class => true, ShortScalarCastFixer::class => true, ClassAttributesSeparationFixer::class => false, ClassDefinitionFixer::class => [ 'multi_line_extends_each_single_line' => true, 'single_item_single_line' => true, 'single_line' => true, 'space_before_parenthesis' => true, ], FinalClassFixer::class => false, FinalInternalClassFixer::class => false, FinalPublicMethodForAbstractClassFixer::class => false, NoBlankLinesAfterClassOpeningFixer::class => true, NoNullPropertyInitializationFixer::class => false, NoPhp4ConstructorFixer::class => true, NoUnneededFinalMethodFixer::class => true, OrderedClassElementsFixer::class => false, OrderedInterfacesFixer::class => true, OrderedTraitsFixer::class => true, ProtectedToPrivateFixer::class => true, SelfAccessorFixer::class => false, SelfStaticAccessorFixer::class => true, SingleClassElementPerStatementFixer::class => true, SingleTraitInsertPerStatementFixer::class => false, VisibilityRequiredFixer::class => [ 'elements' => [ 'method', 'property' ] ], CommentToPhpdocFixer::class => true, HeaderCommentFixer::class => false, MultilineCommentOpeningClosingFixer::class => true, NoEmptyCommentFixer::class => true, NoTrailingWhitespaceInCommentFixer::class => true, SingleLineCommentStyleFixer::class => true, NativeConstantInvocationFixer::class => false, ControlStructureBracesFixer::class => true, ControlStructureContinuationPositionFixer::class => true, ElseifFixer::class => false, EmptyLoopBodyFixer::class => [ 'style' => 'braces' ], EmptyLoopConditionFixer::class => true, IncludeFixer::class => true, NoAlternativeSyntaxFixer::class => true, NoBreakCommentFixer::class => false, NoSuperfluousElseifFixer::class => true, // NoTrailingCommaInListCallFixer::class => true, NoUnneededControlParenthesesFixer::class => true, NoUnneededCurlyBracesFixer::class => true, NoUselessElseFixer::class => true, SimplifiedIfReturnFixer::class => false, SwitchCaseSemicolonToColonFixer::class => true, SwitchCaseSpaceFixer::class => true, SwitchContinueToBreakFixer::class => true, TrailingCommaInMultilineFixer::class => [ 'elements' => [ 'arguments', 'arrays', 'match', 'parameters' ] ], YodaStyleFixer::class => false, CombineNestedDirnameFixer::class => true, DateTimeCreateFromFormatCallFixer::class => true, FopenFlagOrderFixer::class => true, FopenFlagsFixer::class => true, FunctionDeclarationFixer::class => true, FunctionTypehintSpaceFixer::class => true, ImplodeCallFixer::class => true, LambdaNotUsedImportFixer::class => true, MethodArgumentSpaceFixer::class => [ 'on_multiline' => 'ignore' ], NativeFunctionInvocationFixer::class => false, NoSpacesAfterFunctionNameFixer::class => true, NoUnreachableDefaultArgumentValueFixer::class => true, NoUselessSprintfFixer::class => true, NullableTypeDeclarationForDefaultNullValueFixer::class => true, PhpdocToParamTypeFixer::class => false, PhpdocToPropertyTypeFixer::class => false, PhpdocToReturnTypeFixer::class => false, RegularCallableCallFixer::class => false, ReturnTypeDeclarationFixer::class => true, SingleLineThrowFixer::class => false, StaticLambdaFixer::class => false, UseArrowFunctionsFixer::class => false, VoidReturnFixer::class => false, FullyQualifiedStrictTypesFixer::class => true, GlobalNamespaceImportFixer::class => [ 'import_classes' => true, 'import_constants' => false, 'import_functions' => false ], GroupImportFixer::class => false, NoLeadingImportSlashFixer::class => true, NoUnneededImportAliasFixer::class => true, NoUnusedImportsFixer::class => true, OrderedImportsFixer::class => true, SingleImportPerStatementFixer::class => true, SingleLineAfterImportsFixer::class => true, CombineConsecutiveIssetsFixer::class => true, CombineConsecutiveUnsetsFixer::class => true, DeclareEqualNormalizeFixer::class => [ 'space' => 'single' ], DeclareParenthesesFixer::class => true, DirConstantFixer::class => true, ErrorSuppressionFixer::class => false, ExplicitIndirectVariableFixer::class => true, FunctionToConstantFixer::class => true, GetClassToClassKeywordFixer::class => true, IsNullFixer::class => false, NoUnsetOnPropertyFixer::class => true, SingleSpaceAfterConstructFixer::class => true, ListSyntaxFixer::class => true, BlankLineAfterNamespaceFixer::class => true, CleanNamespaceFixer::class => true, NoBlankLinesBeforeNamespaceFixer::class => false, NoLeadingNamespaceWhitespaceFixer::class => true, SingleBlankLineBeforeNamespaceFixer::class => true, NoHomoglyphNamesFixer::class => true, AssignNullCoalescingToCoalesceEqualFixer::class => false, BinaryOperatorSpacesFixer::class => true, ConcatSpaceFixer::class => [ 'spacing' => 'one' ], IncrementStyleFixer::class => [ 'style' => 'post' ], LogicalOperatorsFixer::class => true, NewWithBracesFixer::class => true, NoSpaceAroundDoubleColonFixer::class => true, NotOperatorWithSpaceFixer::class => false, NotOperatorWithSuccessorSpaceFixer::class => false, NoUselessConcatOperatorFixer::class => true, NoUselessNullsafeOperatorFixer::class => true, ObjectOperatorWithoutWhitespaceFixer::class => true, OperatorLinebreakFixer::class => true, StandardizeIncrementFixer::class => true, StandardizeNotEqualsFixer::class => true, TernaryOperatorSpacesFixer::class => true, TernaryToElvisOperatorFixer::class => true, TernaryToNullCoalescingFixer::class => true, UnaryOperatorSpacesFixer::class => true, AlignMultilineCommentFixer::class => [ 'comment_type' => 'all_multiline' ], GeneralPhpdocAnnotationRemoveFixer::class => true, GeneralPhpdocTagRenameFixer::class => true, NoBlankLinesAfterPhpdocFixer::class => true, NoEmptyPhpdocFixer::class => true, NoSuperfluousPhpdocTagsFixer::class => true, PhpdocAddMissingParamAnnotationFixer::class => false, PhpdocAlignFixer::class => [ 'align' => 'left' ], PhpdocAnnotationWithoutDotFixer::class => false, PhpdocIndentFixer::class => true, PhpdocInlineTagNormalizerFixer::class => true, PhpdocLineSpanFixer::class => true, PhpdocNoAccessFixer::class => true, PhpdocNoAliasTagFixer::class => true, PhpdocNoEmptyReturnFixer::class => true, PhpdocNoPackageFixer::class => true, PhpdocNoUselessInheritdocFixer::class => true, PhpdocOrderByValueFixer::class => true, PhpdocOrderFixer::class => true, PhpdocReturnSelfReferenceFixer::class => false, PhpdocScalarFixer::class => true, PhpdocSeparationFixer::class => false, PhpdocSingleLineVarSpacingFixer::class => true, PhpdocSummaryFixer::class => false, PhpdocTagCasingFixer::class => true, PhpdocTagTypeFixer::class => true, PhpdocToCommentFixer::class => false, PhpdocTrimConsecutiveBlankLineSeparationFixer::class => true, PhpdocTrimFixer::class => true, PhpdocTypesFixer::class => true, PhpdocTypesOrderFixer::class => [ 'null_adjustment' => 'always_last' ], PhpdocVarAnnotationCorrectOrderFixer::class => true, PhpdocVarWithoutNameFixer::class => true, BlankLineAfterOpeningTagFixer::class => true, EchoTagSyntaxFixer::class => true, FullOpeningTagFixer::class => true, LinebreakAfterOpeningTagFixer::class => true, NoClosingTagFixer::class => true, NoUselessReturnFixer::class => true, ReturnAssignmentFixer::class => true, SimplifiedNullReturnFixer::class => false, MultilineWhitespaceBeforeSemicolonsFixer::class => true, NoEmptyStatementFixer::class => true, NoSinglelineWhitespaceBeforeSemicolonsFixer::class => true, SemicolonAfterInstructionFixer::class => true, SpaceAfterSemicolonFixer::class => true, DeclareStrictTypesFixer::class => true, StrictComparisonFixer::class => true, StrictParamFixer::class => false, EscapeImplicitBackslashesFixer::class => false, ExplicitStringVariableFixer::class => false, HeredocToNowdocFixer::class => false, NoBinaryStringFixer::class => true, NoTrailingWhitespaceInStringFixer::class => true, SimpleToComplexStringVariableFixer::class => true, SingleQuoteFixer::class => true, StringLengthToEmptyFixer::class => true, StringLineEndingFixer::class => true, ArrayIndentationFixer::class => true, BlankLineBeforeStatementFixer::class => [ 'statements' => [ 'do', 'for', 'foreach', 'if', 'return', 'switch', 'try', 'while', 'yield', 'yield_from' ] ], BlankLineBetweenImportGroupsFixer::class => true, CompactNullableTypehintFixer::class => true, HeredocIndentationFixer::class => [ 'indentation' => 'same_as_start' ], IndentationTypeFixer::class => true, LineEndingFixer::class => true, MethodChainingIndentationFixer::class => true, NoExtraBlankLinesFixer::class => [ 'tokens' => [ 'extra', 'use' ] ], NoSpacesAroundOffsetFixer::class => [ 'positions' => [ 'outside' ] ], NoSpacesInsideParenthesisFixer::class => true, NoTrailingWhitespaceFixer::class => true, NoWhitespaceInBlankLineFixer::class => true, SingleBlankLineAtEofFixer::class => true, StatementIndentationFixer::class => true, TypesSpacesFixer::class => [ 'space' => 'none' ], VoidReturnFixer::class => true ]; register_fixers($config, $options); }; function register_fixers(ECSConfig $config, array $options): void { foreach ($options as $fixer => $options) { if (is_bool($options)) { if ($options) { $config->rule($fixer); } else { $config->skip([ $fixer ]); } } if (is_array($options)) { $config->ruleWithConfiguration($fixer, $options); } } } ```
milewski commented 1 year ago

I have been reviewing the change logs for versions 11.4.5..11.5.0 in an attempt to identify what caused such a significant drop in performance. However, it has proven to be quite challenging. Are there any specific changes that you can recall off the top of your mind that might be responsible? This way, I can focus solely on that particular section of the code.

TomasVotruba commented 1 year ago

Hi, could you try current dev-main?

We've implemented few changes and also upgraded the PHP CS Fixer version that might affect the issue

milewski commented 1 year ago

It is not working, I have tried with dev-main and 11.4.5 again, this time switching between versions upgraded / downgraded a bunch of dependencies:

Package operations: 18 installs, 2 updates, 1 removal
  - Downloading symplify/easy-coding-standard (dev-main 2d7427e)
  - Removing symfony/polyfill-intl-normalizer (v1.27.0)
  - Installing react/event-loop (v1.4.0): Extracting archive
  - Installing evenement/evenement (v3.0.1): Extracting archive
  - Installing react/stream (v1.3.0): Extracting archive
  - Installing react/promise (v3.0.0): Extracting archive
  - Installing react/cache (v1.2.0): Extracting archive
  - Installing react/dns (v1.11.0): Extracting archive
  - Installing react/socket (v1.13.0): Extracting archive
  - Installing react/child-process (v0.6.5): Extracting archive
  - Installing clue/ndjson-react (v1.3.0): Extracting archive
  - Installing symplify/easy-parallel (11.1.27): Extracting archive
  - Downgrading nette/utils (v4.0.0 => v3.2.9): Extracting archive
  - Installing symplify/rule-doc-generator-contracts (11.1.26): Extracting archive
  - Installing symfony/stopwatch (v6.3.0): Extracting archive
  - Installing symfony/filesystem (v6.3.1): Extracting archive
  - Installing doctrine/annotations (2.0.1): Extracting archive
  - Installing composer/xdebug-handler (3.0.3): Extracting archive
  - Installing friendsofphp/php-cs-fixer (v3.22.0): Extracting archive
  - Installing symplify/coding-standard (11.4.1): Extracting archive
  - Installing squizlabs/php_codesniffer (3.7.2): Extracting archive
  - Upgrading symplify/easy-coding-standard (11.4.5 => dev-main 2d7427e): Extracting archive

Using dev-main with $config->parallel();

time php ./vendor/bin/ecs check --fix --clear-cache

 [ERROR] Child process timed out after 120 seconds in                                                                   
         /srv/vendor/symplify/easy-parallel/src/ValueObject/ParallelProcess.php:105                                     

 [ERROR] Reached system errors count limit of 50, exiting...                                                            

real    2m31.286s
user    0m0.132s
sys     0m0.071s

Using dev-main with $config->disableParallel();

without parallel, I can see a progress bar, but it is very very slow, it took 20 minute to finish 😮

time php ./vendor/bin/ecs check --fix --clear-cache
 888/888 [â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“] 100%

    ---------- begin diff ----------
    .... a bunch of diffs ......
    ----------- end diff -----------

Applied checkers:

 * PhpCsFixer\Fixer\Whitespace\IndentationTypeFixer
 * PhpCsFixer\Fixer\Whitespace\StatementIndentationFixer

 [OK] 31 errors successfully fixed and no other errors found!                                                           

real    20m2.612s
user    0m0.226s
sys     0m0.127s

Using 11.4.5 with $config->parallel();, Finishes in 2 minutes..

Package operations: 1 install, 2 updates, 18 removals
  - Removing symplify/rule-doc-generator-contracts (11.1.26)
  - Removing symplify/easy-parallel (11.1.27)
  - Removing symplify/coding-standard (11.4.1)
  - Removing symfony/stopwatch (v6.3.0)
  - Removing symfony/filesystem (v6.3.1)
  - Removing squizlabs/php_codesniffer (3.7.2)
  - Removing react/stream (v1.3.0)
  - Removing react/socket (v1.13.0)
  - Removing react/promise (v3.0.0)
  - Removing react/event-loop (v1.4.0)
  - Removing react/dns (v1.11.0)
  - Removing react/child-process (v0.6.5)
  - Removing react/cache (v1.2.0)
  - Removing friendsofphp/php-cs-fixer (v3.22.0)
  - Removing evenement/evenement (v3.0.1)
  - Removing doctrine/annotations (2.0.1)
  - Removing composer/xdebug-handler (3.0.3)
  - Removing clue/ndjson-react (v1.3.0)
  - Installing symfony/polyfill-intl-normalizer (v1.27.0): Extracting archive
  - Upgrading nette/utils (v3.2.9 => v4.0.0): Extracting archive
  - Upgrading symplify/easy-coding-standard (dev-main 2d7427e => 11.4.5): Extracting archive
time php ./vendor/bin/ecs check --fix --clear-cache
 888/888 [â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“â–“] 100%

    ---------- begin diff ----------
    .... a bunch of diffs ......
    ----------- end diff -----------

Applied checkers:

 * PhpCsFixer\Fixer\Whitespace\IndentationTypeFixer
 * PhpCsFixer\Fixer\Whitespace\StatementIndentationFixer

 [OK] 31 errors successfully fixed and no other errors found!                                                           

real    2m11.463s
user    0m0.200s
sys     0m0.104s
TomasVotruba commented 1 year ago

I see. This would require on project debugging, as it could be any of those rules.

milewski commented 1 year ago

But what I don't get is why would this be an issue with the rules? since switching between 11.4.5 and 11.5.0 no rules are upgraded/downgraded/modified, it seems to me that something changed on how the tasks are spawned in parallel which consumes more memory / needs more CPU which causes every rule to run slower and timeout, then when the parallel process manager reaches a threshold of 50 crashes it aborts the whole process.

Also, it is not a specific rule that is timing out.. it is all of them, I have tried to disable/enable them all one by one to pinpoint exactly which rule is causing this but reach no conclusive answer, the problem seems to appear when I have around ~40 of them enabled at the same time...

Another thing I found out was that I run it on a much beefier machine (i9 - 32GB - Win) and it was able to complete in ~1m on dev-main and ~20s on 11.4.5, so im pretty convince something changed in 11.5.0 that is causing severe performance issue...

TomasVotruba commented 1 year ago

It's often the rule, because we didn't change anything in architecture and no other reports are filled. Also we include ~7 sets that transitional include over 200-300 external rules.

One fix in them can cause performance drop in a single project, because the fix didn't take into account some specific use case. That's why we always look for specific rule and file to make the fix. The ECS is basically a wrapper around 3rd party rules, so the rule performance wrap it out of our scope.

We cannot do anything about this without having particular file and rule. At time being, I'd recommend you staying with 11.4.5 and in couple months try 12+ which will include newer version of php-cs-fixer and PHP_CodeSniffer.

E.g. since 11.4.5 the php-cs-fixer itself released 3 new patch versions which can cause this:

Screenshot from 2023-07-26 10-05-00

Screenshot from 2023-07-26 10-05-44

Thanks for understanding