From 84a76a2f2e0d2009d0dd62ea7be619cf5a79c85e Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 18 Oct 2026 15:15:01 +0200 Subject: [PATCH] make exposed downgrade rules idempotent on repeated application These rules kept transforming their own output when a rule runs more than once over the same file: - DowngradeFlexibleHeredocSyntaxRector: marker so a flattened doc is skipped - DowngradeNewMethodCallWithoutParenthesesRector: guard checked the wrong token position - DowngradeIsEnumRector: skip isEnum() already wrapped in a method_exists ternary - DowngradePhp73JsonConstRector: clean the flag in-place, so the error check is appended once - DowngradeCovariantReturnTypeRector: only downgrade to a wider ancestor type - DowngradeProcOpenArrayCommandArgRector: skip a command arg already wrapped --- phpstan.neon | 1 + .../DowngradePhp73JsonConstRector.php | 14 +++++++++++ .../DowngradeFlexibleHeredocSyntaxRector.php | 12 +++++++++- .../DowngradeCovariantReturnTypeRector.php | 6 +++++ ...DowngradeProcOpenArrayCommandArgRector.php | 8 +++++++ .../MethodCall/DowngradeIsEnumRector.php | 23 +++++++++++++++++-- ...eNewMethodCallWithoutParenthesesRector.php | 9 +++++++- 7 files changed, 69 insertions(+), 4 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 86fe6158..0d6e44bf 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -116,6 +116,7 @@ parameters: - rules/DowngradePhp80/Rector/FuncCall/DowngradeSubstrFalsyRector.php - rules/DowngradePhp74/Rector/MethodCall/DowngradeReflectionGetTypeRector.php - rules/DowngradePhp80/Rector/MethodCall/DowngradeReflectionGetAttributesRector.php + - rules/DowngradePhp81/Rector/MethodCall/DowngradeIsEnumRector.php - rules/DowngradePhp73/Rector/String_/DowngradeFlexibleHeredocSyntaxRector.php - rules/DowngradePhp80/Rector/Expression/DowngradeMatchToSwitchRector.php diff --git a/rules/DowngradePhp73/Rector/ConstFetch/DowngradePhp73JsonConstRector.php b/rules/DowngradePhp73/Rector/ConstFetch/DowngradePhp73JsonConstRector.php index eda16219..367331af 100644 --- a/rules/DowngradePhp73/Rector/ConstFetch/DowngradePhp73JsonConstRector.php +++ b/rules/DowngradePhp73/Rector/ConstFetch/DowngradePhp73JsonConstRector.php @@ -169,6 +169,20 @@ private function refactorExpression(Expression $expression): ?array return null; } + // clean the flag from the call args here; returning an array replaces the Expression, + // so the traverser never descends into the ConstFetch to clean it, and the expression + // would match again every pass and keep appending the error check + foreach ($funcCall->getArgs() as $arg) { + if (! $arg->value instanceof ConstFetch && ! $arg->value instanceof BitwiseOr) { + continue; + } + + $cleanedValue = $this->jsonConstCleaner->clean($arg->value, [JsonConstant::THROW_ON_ERROR]); + if ($cleanedValue instanceof Expr) { + $arg->value = $cleanedValue; + } + } + $nodes = [$expression]; $nodes[] = new If_( new NotIdentical( diff --git a/rules/DowngradePhp73/Rector/String_/DowngradeFlexibleHeredocSyntaxRector.php b/rules/DowngradePhp73/Rector/String_/DowngradeFlexibleHeredocSyntaxRector.php index 38b48470..1c5fa882 100644 --- a/rules/DowngradePhp73/Rector/String_/DowngradeFlexibleHeredocSyntaxRector.php +++ b/rules/DowngradePhp73/Rector/String_/DowngradeFlexibleHeredocSyntaxRector.php @@ -23,6 +23,11 @@ final class DowngradeFlexibleHeredocSyntaxRector extends AbstractRector */ private const array HERENOW_DOC_KINDS = [String_::KIND_HEREDOC, String_::KIND_NOWDOC]; + /** + * Marker left on the doc indentation attribute once flattened, so a later pass skips the node + */ + private const string REMOVED_DOC_INDENTATION = '__REMOVED__'; + public function __construct( private readonly FollowedByNewlineOnlyMaybeWithSemicolonAnalyzer $followedByNewlineOnlyMaybeWithSemicolonAnalyzer ) { @@ -80,7 +85,12 @@ public function refactor(Node $node): ?Node return null; } - $node->setAttribute(AttributeKey::DOC_INDENTATION, '__REMOVED__'); + // already flattened on a previous pass, re-running would keep appending newlines + if ($docIndentation === self::REMOVED_DOC_INDENTATION) { + return null; + } + + $node->setAttribute(AttributeKey::DOC_INDENTATION, self::REMOVED_DOC_INDENTATION); $node->setAttribute(AttributeKey::ORIGINAL_NODE, null); $tokens = $this->getFile() diff --git a/rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php b/rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php index 7b181a29..25622faf 100644 --- a/rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php +++ b/rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php @@ -244,6 +244,12 @@ private function resolveMatchingReturnType( continue; } + // only downgrade to a wider ancestor type; skipping a narrower one keeps the rule + // idempotent, otherwise a multi-level hierarchy flips the type back and forth + if (! $parentReturnType->accepts($returnType, true)->yes()) { + continue; + } + // This is an ancestor class with a different return type return $parentReturnType; } diff --git a/rules/DowngradePhp74/Rector/FuncCall/DowngradeProcOpenArrayCommandArgRector.php b/rules/DowngradePhp74/Rector/FuncCall/DowngradeProcOpenArrayCommandArgRector.php index cf856978..0d86e1e0 100644 --- a/rules/DowngradePhp74/Rector/FuncCall/DowngradeProcOpenArrayCommandArgRector.php +++ b/rules/DowngradePhp74/Rector/FuncCall/DowngradeProcOpenArrayCommandArgRector.php @@ -59,6 +59,14 @@ public function refactor(Node $node): ?FuncCall $firstArg = $node->getArgs()[0]; + // already wrapped by this rule on a previous pass; the wrapped ternary type is string|array, + // so the string check below does not catch it and the rule would keep nesting the wrap + if ($firstArg->value instanceof Ternary + && $firstArg->value->cond instanceof FuncCall + && $this->isName($firstArg->value->cond, 'is_array')) { + return null; + } + $commandType = $this->getType($firstArg->value); if ($commandType->isString()->yes()) { return null; diff --git a/rules/DowngradePhp81/Rector/MethodCall/DowngradeIsEnumRector.php b/rules/DowngradePhp81/Rector/MethodCall/DowngradeIsEnumRector.php index 4c376429..820660df 100644 --- a/rules/DowngradePhp81/Rector/MethodCall/DowngradeIsEnumRector.php +++ b/rules/DowngradePhp81/Rector/MethodCall/DowngradeIsEnumRector.php @@ -6,6 +6,8 @@ use PhpParser\Node; use PhpParser\Node\Arg; +use PhpParser\Node\Expr; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Scalar\String_; @@ -19,6 +21,8 @@ */ final class DowngradeIsEnumRector extends AbstractRector { + private const string IS_IF_TERNARY = 'is_if_ternary'; + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Downgrades isEnum() on class reflection', [ @@ -52,18 +56,33 @@ public function run(ReflectionClass $reflectionClass) */ public function getNodeTypes(): array { - return [MethodCall::class]; + return [Ternary::class, MethodCall::class]; } /** - * @param MethodCall $node + * @param Ternary|MethodCall $node */ public function refactor(Node $node): ?Node { + if ($node instanceof Ternary) { + if ($node->if instanceof Expr + && $node->cond instanceof FuncCall + && $this->isName($node->cond, 'method_exists')) { + $node->if->setAttribute(self::IS_IF_TERNARY, true); + } + + return null; + } + if (! $this->isName($node->name, 'isEnum')) { return null; } + // already wrapped by this rule on a previous pass + if ($node->getAttribute(self::IS_IF_TERNARY) === true) { + return null; + } + if (! $this->isObjectType($node->var, new ObjectType('ReflectionClass'))) { return null; } diff --git a/rules/DowngradePhp84/Rector/MethodCall/DowngradeNewMethodCallWithoutParenthesesRector.php b/rules/DowngradePhp84/Rector/MethodCall/DowngradeNewMethodCallWithoutParenthesesRector.php index a6c815fc..d9541372 100644 --- a/rules/DowngradePhp84/Rector/MethodCall/DowngradeNewMethodCallWithoutParenthesesRector.php +++ b/rules/DowngradePhp84/Rector/MethodCall/DowngradeNewMethodCallWithoutParenthesesRector.php @@ -62,7 +62,14 @@ public function refactor(Node $node): ?Node return null; } - if ((string) $oldTokens[$node->getStartTokenPos()] === '(') { + // source already wraps the new in parentheses + if ((string) $oldTokens[$startTokenPos] === '(') { + return null; + } + + // idempotency: on a previous pass the opening paren was prepended to the New_ var token, + // so check that token too, otherwise the rule keeps wrapping the same call every pass + if (str_starts_with((string) $oldTokens[$node->var->getStartTokenPos()], '(')) { return null; }