From 50db37cfd68dc8458609b98dbeffadfa8bf57def Mon Sep 17 00:00:00 2001 From: Carlos Granados Date: Fri, 28 Mar 2025 07:17:27 +0100 Subject: [PATCH] Fix cases where rule was applied even when no code changed --- .../TemplateAnnotationToThisRenderRector.php | 33 +++++++++++-------- ...ypeOptionNameFromTypeToEntryTypeRector.php | 10 ++++-- .../StringToArrayArgumentProcessRector.php | 14 ++++++-- .../ConsoleExecuteReturnIntRector.php | 2 ++ 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/rules/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector.php b/rules/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector.php index 77835565..bffc75da 100644 --- a/rules/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/TemplateAnnotationToThisRenderRector.php @@ -204,14 +204,15 @@ private function refactorClassMethod( return null; } - $this->refactorStmtsAwareNode( + $hasChangedNode = $this->refactorStmtsAwareNode( $node, $templateDoctrineAnnotationTagValueNode, $hasThisRenderOrReturnsResponse, $classMethod ); - - $hasChanged = true; + if ($hasChangedNode) { + $hasChanged = true; + } return null; }); @@ -256,10 +257,10 @@ private function refactorReturn( DoctrineAnnotationTagValueNode $templateDoctrineAnnotationTagValueNode, bool $hasThisRenderOrReturnsResponse, ClassMethod $classMethod - ): void { + ): bool { // nothing we can do if (! $return->expr instanceof Expr) { - return; + return false; } // create "$this->render('template.file.twig.html', ['key' => 'value']);" method call @@ -269,7 +270,7 @@ private function refactorReturn( $classMethod ); - $this->refactorReturnWithValue( + return $this->refactorReturnWithValue( $return, $hasThisRenderOrReturnsResponse, $thisRenderMethodCall, @@ -295,7 +296,7 @@ private function refactorReturnWithValue( MethodCall $thisRenderMethodCall, ClassMethod $classMethod, DoctrineAnnotationTagValueNode $doctrineAnnotationTagValueNode - ): void { + ): bool { /** @var Expr $lastReturnExpr */ $lastReturnExpr = $return->expr; @@ -309,7 +310,7 @@ private function refactorReturnWithValue( $return->expr = $thisRenderMethodCall; } elseif ($returnStaticType instanceof MixedType) { // nothing we can do - return; + return false; } $isArrayOrResponseType = $this->arrayUnionResponseTypeAnalyzer->isArrayUnionResponseType( @@ -319,12 +320,13 @@ private function refactorReturnWithValue( // skip as the original class method has to change first if ($isArrayOrResponseType) { - return; + return false; } // already response $this->removeDoctrineAnnotationTagValueNode($classMethod, $doctrineAnnotationTagValueNode); $this->returnTypeDeclarationUpdater->updateClassMethod($classMethod, SymfonyClass::RESPONSE); + return true; } private function removeDoctrineAnnotationTagValueNode( @@ -342,11 +344,12 @@ private function refactorStmtsAwareNode( DoctrineAnnotationTagValueNode $templateDoctrineAnnotationTagValueNode, bool $hasThisRenderOrReturnsResponse, ClassMethod $classMethod - ): void { + ): bool { if ($stmtsAware->stmts === null) { - return; + return false; } + $hasChanged = false; foreach ($stmtsAware->stmts as $stmt) { if (! $stmt instanceof Return_) { continue; @@ -354,15 +357,19 @@ private function refactorStmtsAwareNode( // just created node, skip it if ($stmt->getAttributes() === []) { - return; + return false; } - $this->refactorReturn( + $hasChangedReturn = $this->refactorReturn( $stmt, $templateDoctrineAnnotationTagValueNode, $hasThisRenderOrReturnsResponse, $classMethod ); + if ($hasChangedReturn) { + $hasChanged = true; + } } + return $hasChanged; } } diff --git a/rules/Symfony27/Rector/MethodCall/ChangeCollectionTypeOptionNameFromTypeToEntryTypeRector.php b/rules/Symfony27/Rector/MethodCall/ChangeCollectionTypeOptionNameFromTypeToEntryTypeRector.php index c13e2167..71db08a8 100644 --- a/rules/Symfony27/Rector/MethodCall/ChangeCollectionTypeOptionNameFromTypeToEntryTypeRector.php +++ b/rules/Symfony27/Rector/MethodCall/ChangeCollectionTypeOptionNameFromTypeToEntryTypeRector.php @@ -113,13 +113,17 @@ public function refactor(Node $node): ?Node return null; } - $this->refactorOptionsArray($optionsArray); + $hasChanged = $this->refactorOptionsArray($optionsArray); + if (! $hasChanged) { + return null; + } return $node; } - private function refactorOptionsArray(Array_ $optionsArray): void + private function refactorOptionsArray(Array_ $optionsArray): bool { + $hasChanged = false; foreach ($optionsArray->items as $arrayItem) { if (! $arrayItem instanceof ArrayItem) { continue; @@ -135,7 +139,9 @@ private function refactorOptionsArray(Array_ $optionsArray): void } $arrayItem->key = new String_($newName); + $hasChanged = true; } } + return $hasChanged; } } diff --git a/rules/Symfony42/Rector/New_/StringToArrayArgumentProcessRector.php b/rules/Symfony42/Rector/New_/StringToArrayArgumentProcessRector.php index 90868180..6963de2c 100644 --- a/rules/Symfony42/Rector/New_/StringToArrayArgumentProcessRector.php +++ b/rules/Symfony42/Rector/New_/StringToArrayArgumentProcessRector.php @@ -111,7 +111,11 @@ private function processArgumentPosition(New_|MethodCall $node, int $argumentPos return null; } - $this->processStringType($node, $argumentPosition, $activeArgValue); + $hasChanged = $this->processStringType($node, $argumentPosition, $activeArgValue); + + if (! $hasChanged) { + return null; + } return $node; } @@ -121,25 +125,29 @@ private function shouldSkipProcessMethodCall(MethodCall $methodCall): bool return in_array($methodName, self::EXCLUDED_PROCESS_METHOD_CALLS, true); } - private function processStringType(New_|MethodCall $expr, int $argumentPosition, Expr $firstArgumentExpr): void + private function processStringType(New_|MethodCall $expr, int $argumentPosition, Expr $firstArgumentExpr): bool { if ($firstArgumentExpr instanceof Concat) { $arrayNode = $this->nodeTransformer->transformConcatToStringArray($firstArgumentExpr); $expr->args[$argumentPosition] = new Arg($arrayNode); - return; + return true; } $args = $expr->getArgs(); + $hasChanged = false; if ($firstArgumentExpr instanceof FuncCall && $this->isName($firstArgumentExpr, 'sprintf')) { $arrayNode = $this->nodeTransformer->transformSprintfToArray($firstArgumentExpr); if ($arrayNode instanceof Array_) { $args[$argumentPosition]->value = $arrayNode; + $hasChanged = true; } } elseif ($firstArgumentExpr instanceof String_) { $parts = $this->splitProcessCommandToItems($firstArgumentExpr->value); $args[$argumentPosition]->value = $this->nodeFactory->createArray($parts); + $hasChanged = true; } + return $hasChanged; } /** diff --git a/rules/Symfony44/Rector/ClassMethod/ConsoleExecuteReturnIntRector.php b/rules/Symfony44/Rector/ClassMethod/ConsoleExecuteReturnIntRector.php index f46e7441..3e88640c 100644 --- a/rules/Symfony44/Rector/ClassMethod/ConsoleExecuteReturnIntRector.php +++ b/rules/Symfony44/Rector/ClassMethod/ConsoleExecuteReturnIntRector.php @@ -90,6 +90,8 @@ public function refactor(Node $node): ?Node return null; } + $this->hasChanged = false; + $this->refactorReturnTypeDeclaration($executeClassMethod); $this->addReturn0ToExecuteClassMethod($executeClassMethod);