From 2efd564640e94edf09f95f3872adb11e059f5fb8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 31 Jan 2024 15:25:11 +0100 Subject: [PATCH] [stabilize] Deprecate FinalizePublicClassConstantRector as not reliable and causes uncontroller changed (#5534) --- config/set/php81.php | 2 - phpstan.neon | 2 + .../FinalizePublicClassConstantRectorTest.php | 28 -------- .../Fixture/class_inheritance.php.inc | 29 -------- .../Fixture/skip_already_final.php.inc | 8 --- .../Fixture/skip_anonymous_class.php.inc | 7 -- .../Fixture/skip_final_class.php.inc | 8 --- .../Fixture/some_class.php.inc | 21 ------ .../config/configured_rule.php | 13 ---- .../FinalizePublicClassConstantRector.php | 71 ++++--------------- .../NodeManipulator/VisibilityManipulator.php | 3 + ...ddMethodCallBasedStrictParamTypeRector.php | 17 ++--- utils/Command/MissingInSetCommand.php | 3 + 13 files changed, 31 insertions(+), 181 deletions(-) delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/FinalizePublicClassConstantRectorTest.php delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/class_inheritance.php.inc delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/skip_already_final.php.inc delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/skip_anonymous_class.php.inc delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/skip_final_class.php.inc delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/some_class.php.inc delete mode 100644 rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/config/configured_rule.php diff --git a/config/set/php81.php b/config/set/php81.php index 91d7916d1bd..f7cc2c6fe78 100644 --- a/config/set/php81.php +++ b/config/set/php81.php @@ -6,7 +6,6 @@ use Rector\Php81\Rector\Array_\FirstClassCallableRector; use Rector\Php81\Rector\Class_\MyCLabsClassToEnumRector; use Rector\Php81\Rector\Class_\SpatieEnumClassToEnumRector; -use Rector\Php81\Rector\ClassConst\FinalizePublicClassConstantRector; use Rector\Php81\Rector\ClassMethod\NewInInitializerRector; use Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector; use Rector\Php81\Rector\MethodCall\MyCLabsMethodCallToEnumConstRector; @@ -19,7 +18,6 @@ ReturnNeverTypeRector::class, MyCLabsClassToEnumRector::class, MyCLabsMethodCallToEnumConstRector::class, - FinalizePublicClassConstantRector::class, ReadOnlyPropertyRector::class, SpatieEnumClassToEnumRector::class, SpatieEnumMethodCallToEnumConstRector::class, diff --git a/phpstan.neon b/phpstan.neon index e1073336d0f..9c8a50c1de3 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -384,6 +384,7 @@ parameters: - '#Call to method PHPUnit\\Framework\\Assert\:\:assert(.*?) will always evaluate to (true|false)#' - '#Doing instanceof PHPStan\\Type\\.* is error\-prone and deprecated(\. Use Type\:\:.*\(\) (or .* )?instead)?#' + - '#Fetching class constant class of deprecated class Rector\\Php81\\Rector\\ClassConst\\FinalizePublicClassConstantRector#' # phpstan 1.10.0 - '#Parameter 3 should use "PHPStan\\Reflection\\ParameterReflectionWithPhpDocs" type as the only type passed to this method#' @@ -463,6 +464,7 @@ parameters: # optional as changes behavior, should be used explicitly outside PHP upgrade - '#Register "Rector\\Php73\\Rector\\FuncCall\\JsonThrowOnErrorRector" service to "php73\.php" config set#' + - '#Register "Rector\\Php81\\Rector\\ClassConst\\FinalizePublicClassConstantRector" service to "php81\.php" config set#' # soon to be extended - diff --git a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/FinalizePublicClassConstantRectorTest.php b/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/FinalizePublicClassConstantRectorTest.php deleted file mode 100644 index 4fad919676a..00000000000 --- a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/FinalizePublicClassConstantRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/class_inheritance.php.inc b/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/class_inheritance.php.inc deleted file mode 100644 index 568784cb4f1..00000000000 --- a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/class_inheritance.php.inc +++ /dev/null @@ -1,29 +0,0 @@ - ------ - diff --git a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/skip_already_final.php.inc b/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/skip_already_final.php.inc deleted file mode 100644 index 15b4b17b482..00000000000 --- a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/Fixture/skip_already_final.php.inc +++ /dev/null @@ -1,8 +0,0 @@ - ------ - diff --git a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/config/configured_rule.php b/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/config/configured_rule.php deleted file mode 100644 index af308e06210..00000000000 --- a/rules-tests/Php81/Rector/ClassConst/FinalizePublicClassConstantRector/config/configured_rule.php +++ /dev/null @@ -1,13 +0,0 @@ -rule(FinalizePublicClassConstantRector::class); - - $rectorConfig->phpVersion(PhpVersionFeature::FINAL_CLASS_CONSTANTS); -}; diff --git a/rules/Php81/Rector/ClassConst/FinalizePublicClassConstantRector.php b/rules/Php81/Rector/ClassConst/FinalizePublicClassConstantRector.php index d506858e0aa..3c61180b799 100644 --- a/rules/Php81/Rector/ClassConst/FinalizePublicClassConstantRector.php +++ b/rules/Php81/Rector/ClassConst/FinalizePublicClassConstantRector.php @@ -6,29 +6,18 @@ use PhpParser\Node; use PhpParser\Node\Stmt\Class_; -use PHPStan\Analyser\Scope; -use PHPStan\Reflection\ReflectionProvider; -use Rector\FamilyTree\Reflection\FamilyRelationsAnalyzer; -use Rector\Privatization\NodeManipulator\VisibilityManipulator; -use Rector\Rector\AbstractScopeAwareRector; +use Rector\Rector\AbstractRector; use Rector\ValueObject\PhpVersionFeature; use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @changelog https://php.watch/versions/8.1/final-class-const - * - * @see \Rector\Tests\Php81\Rector\ClassConst\FinalizePublicClassConstantRector\FinalizePublicClassConstantRectorTest + * @deprecated This was deprecated, as its functionality caused bugs. Without knowing the full dependency tree, its risky to change */ -final class FinalizePublicClassConstantRector extends AbstractScopeAwareRector implements MinPhpVersionInterface +final class FinalizePublicClassConstantRector extends AbstractRector implements MinPhpVersionInterface { - public function __construct( - private readonly FamilyRelationsAnalyzer $familyRelationsAnalyzer, - private readonly ReflectionProvider $reflectionProvider, - private readonly VisibilityManipulator $visibilityManipulator - ) { - } + private bool $hasWarned = false; public function getRuleDefinition(): RuleDefinition { @@ -63,42 +52,22 @@ public function getNodeTypes(): array /** * @param Class_ $node */ - public function refactorWithScope(Node $node, Scope $scope): ?Node + public function refactor(Node $node): ?Node { - if ($node->isFinal()) { + if ($this->hasWarned) { return null; } - if (! $scope->isInClass()) { - return null; - } + trigger_error( + sprintf( + 'The "%s" rule was deprecated, as its functionality caused bugs. Without knowing the full dependency tree, its risky to change.', + self::class + ) + ); - $classReflection = $scope->getClassReflection(); - if ($classReflection->isAnonymous()) { - return null; - } + sleep(3); - $hasChanged = false; - foreach ($node->getConstants() as $classConst) { - if (! $classConst->isPublic()) { - continue; - } - - if ($classConst->isFinal()) { - continue; - } - - if ($this->isClassHasChildren($node)) { - continue; - } - - $hasChanged = true; - $this->visibilityManipulator->makeFinal($classConst); - } - - if ($hasChanged) { - return $node; - } + $this->hasWarned = true; return null; } @@ -107,16 +76,4 @@ public function provideMinPhpVersion(): int { return PhpVersionFeature::FINAL_CLASS_CONSTANTS; } - - private function isClassHasChildren(Class_ $class): bool - { - $className = (string) $this->nodeNameResolver->getName($class); - if (! $this->reflectionProvider->hasClass($className)) { - return false; - } - - $classReflection = $this->reflectionProvider->getClass($className); - - return $this->familyRelationsAnalyzer->getChildrenOfClassReflection($classReflection) !== []; - } } diff --git a/rules/Privatization/NodeManipulator/VisibilityManipulator.php b/rules/Privatization/NodeManipulator/VisibilityManipulator.php index 8aaf112bdbf..4cc18f9e1b8 100644 --- a/rules/Privatization/NodeManipulator/VisibilityManipulator.php +++ b/rules/Privatization/NodeManipulator/VisibilityManipulator.php @@ -54,6 +54,9 @@ public function makeNonAbstract(ClassMethod | Class_ $node): void $node->flags -= Class_::MODIFIER_ABSTRACT; } + /** + * @api + */ public function makeFinal(Class_ | ClassMethod | ClassConst $node): void { $this->addVisibilityFlag($node, Visibility::FINAL); diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddMethodCallBasedStrictParamTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddMethodCallBasedStrictParamTypeRector.php index 267109e2352..18954b2a3b4 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddMethodCallBasedStrictParamTypeRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddMethodCallBasedStrictParamTypeRector.php @@ -37,10 +37,10 @@ public function __construct( public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( - 'Change private classMethod param type to strict type, based on passed strict types', + 'Change private method param type to strict type, based on passed strict types', [ - new CodeSample( - <<<'CODE_SAMPLE' + new CodeSample( + <<<'CODE_SAMPLE' final class SomeClass { public function run(int $value) @@ -53,8 +53,8 @@ private function resolve($value) } } CODE_SAMPLE - , - <<<'CODE_SAMPLE' + , + <<<'CODE_SAMPLE' final class SomeClass { public function run(int $value) @@ -67,9 +67,10 @@ private function resolve(int $value) } } CODE_SAMPLE - ), - - ]); + ), + + ] + ); } /** diff --git a/utils/Command/MissingInSetCommand.php b/utils/Command/MissingInSetCommand.php index dc883704d02..48ef6e6f6a7 100644 --- a/utils/Command/MissingInSetCommand.php +++ b/utils/Command/MissingInSetCommand.php @@ -9,6 +9,7 @@ use Rector\Contract\Rector\ConfigurableRectorInterface; use Rector\DeadCode\Rector\ClassMethod\RemoveNullTagValueNodeRector; use Rector\Php73\Rector\FuncCall\JsonThrowOnErrorRector; +use Rector\Php81\Rector\ClassConst\FinalizePublicClassConstantRector; use Rector\Privatization\Rector\Class_\FinalizeClassesWithoutChildrenRector; use Rector\TypeDeclaration\Rector\BooleanAnd\BinaryOpNullableToInstanceofRector; use Rector\TypeDeclaration\Rector\StmtsAwareInterface\DeclareStrictTypesRector; @@ -39,6 +40,8 @@ final class MissingInSetCommand extends Command // personal preference, enable on purpose ArraySpreadInsteadOfArrayMergeRector::class, FinalizeClassesWithoutChildrenRector::class, + // deprecated + FinalizePublicClassConstantRector::class, ]; public function __construct(