From abd008ba860a5a27f8efc7ca8d0ba9c9737cb30d Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 22 Jun 2024 21:41:53 +0900 Subject: [PATCH] [CodeQuality] Deprecate GetClassToInstanceOfRector as can create invalid comparison (#6005) --- config/set/instanceof.php | 2 - .../Fixture/fixture.php.inc | 47 --------- .../self_should_not_be_namespaced.php.inc | 23 ----- .../Fixture/skip_parent_usage.php.inc | 13 --- .../static_should_not_be_namespaced.php.inc | 23 ----- .../GetClassToInstanceOfRectorTest.php | 28 ------ .../Source/SomeParent.php | 7 -- .../config/configured_rule.php | 9 -- .../Identical/GetClassToInstanceOfRector.php | 99 +++---------------- 9 files changed, 12 insertions(+), 239 deletions(-) delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/fixture.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/self_should_not_be_namespaced.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/skip_parent_usage.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/static_should_not_be_namespaced.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/GetClassToInstanceOfRectorTest.php delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Source/SomeParent.php delete mode 100644 rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/config/configured_rule.php diff --git a/config/set/instanceof.php b/config/set/instanceof.php index 67a22c50530..9305d1c8240 100644 --- a/config/set/instanceof.php +++ b/config/set/instanceof.php @@ -4,7 +4,6 @@ use Rector\CodeQuality\Rector\FuncCall\InlineIsAInstanceOfRector; use Rector\CodeQuality\Rector\Identical\FlipTypeControlToUseExclusiveTypeRector; -use Rector\CodeQuality\Rector\Identical\GetClassToInstanceOfRector; use Rector\Config\RectorConfig; use Rector\DeadCode\Rector\If_\RemoveDeadInstanceOfRector; use Rector\Instanceof_\Rector\Ternary\FlipNegatedTernaryInstanceofRector; @@ -15,7 +14,6 @@ return static function (RectorConfig $rectorConfig): void { $rectorConfig->rules([ EmptyOnNullableObjectToInstanceOfRector::class, - GetClassToInstanceOfRector::class, InlineIsAInstanceOfRector::class, FlipTypeControlToUseExclusiveTypeRector::class, RemoveDeadInstanceOfRector::class, diff --git a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/fixture.php.inc b/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/fixture.php.inc deleted file mode 100644 index 68a29adcd7b..00000000000 --- a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,47 +0,0 @@ - ------ - diff --git a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/self_should_not_be_namespaced.php.inc b/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/self_should_not_be_namespaced.php.inc deleted file mode 100644 index 4b5a20f97c0..00000000000 --- a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/self_should_not_be_namespaced.php.inc +++ /dev/null @@ -1,23 +0,0 @@ - ------ - diff --git a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/skip_parent_usage.php.inc b/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/skip_parent_usage.php.inc deleted file mode 100644 index 38e8a1fe774..00000000000 --- a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/skip_parent_usage.php.inc +++ /dev/null @@ -1,13 +0,0 @@ -test(new SkipParentUsage()); diff --git a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/static_should_not_be_namespaced.php.inc b/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/static_should_not_be_namespaced.php.inc deleted file mode 100644 index b30d4e9914b..00000000000 --- a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Fixture/static_should_not_be_namespaced.php.inc +++ /dev/null @@ -1,23 +0,0 @@ - ------ - diff --git a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/GetClassToInstanceOfRectorTest.php b/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/GetClassToInstanceOfRectorTest.php deleted file mode 100644 index 6e7dc761ba8..00000000000 --- a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/GetClassToInstanceOfRectorTest.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/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Source/SomeParent.php b/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Source/SomeParent.php deleted file mode 100644 index b11c8f3971f..00000000000 --- a/rules-tests/CodeQuality/Rector/Identical/GetClassToInstanceOfRector/Source/SomeParent.php +++ /dev/null @@ -1,7 +0,0 @@ -withRules([GetClassToInstanceOfRector::class]); diff --git a/rules/CodeQuality/Rector/Identical/GetClassToInstanceOfRector.php b/rules/CodeQuality/Rector/Identical/GetClassToInstanceOfRector.php index d5b84da18d4..071040e0652 100644 --- a/rules/CodeQuality/Rector/Identical/GetClassToInstanceOfRector.php +++ b/rules/CodeQuality/Rector/Identical/GetClassToInstanceOfRector.php @@ -7,36 +7,17 @@ use PhpParser\Node; use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\BinaryOp\NotIdentical; -use PhpParser\Node\Expr\BooleanNot; -use PhpParser\Node\Expr\ClassConstFetch; -use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Expr\Instanceof_; -use PhpParser\Node\Name; -use PhpParser\Node\Name\FullyQualified; -use PhpParser\Node\Scalar\String_; -use Rector\Enum\ObjectReference; -use Rector\NodeManipulator\BinaryOpManipulator; -use Rector\Php71\ValueObject\TwoNodeMatch; -use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\CodeQuality\Rector\Identical\GetClassToInstanceOfRector\GetClassToInstanceOfRectorTest + * @deprecated Deprecated since 1.1.2, as can create invalid checks. See https://github.com/rectorphp/rector/issues/8639 + * Use PHPStan to find those case and upgrade manually with care instead. */ final class GetClassToInstanceOfRector extends AbstractRector { - /** - * @var string[] - */ - private const NO_NAMESPACED_CLASSNAMES = ['self', 'static']; - - public function __construct( - private readonly BinaryOpManipulator $binaryOpManipulator, - private readonly ValueResolver $valueResolver - ) { - } + private bool $hasWarned = false; public function getRuleDefinition(): RuleDefinition { @@ -64,75 +45,19 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - $twoNodeMatch = $this->binaryOpManipulator->matchFirstAndSecondConditionNode( - $node, - fn (Node $node): bool => $this->isClassReference($node), - fn (Node $node): bool => $this->isGetClassFuncCallNode($node) - ); - - if (! $twoNodeMatch instanceof TwoNodeMatch) { - return null; - } - - /** @var ClassConstFetch|String_ $firstExpr */ - $firstExpr = $twoNodeMatch->getFirstExpr(); - - /** @var FuncCall $secondExpr */ - $secondExpr = $twoNodeMatch->getSecondExpr(); - - if ($secondExpr->isFirstClassCallable()) { - return null; - } - - if (! isset($secondExpr->getArgs()[0])) { + if ($this->hasWarned) { return null; } - $firstArg = $secondExpr->getArgs()[0]; - - $varNode = $firstArg->value; - - if ($firstExpr instanceof String_) { - $className = $this->valueResolver->getValue($firstExpr); - } else { - $className = $this->getName($firstExpr->class); - } - - if ($className === null) { - return null; - } - - if ($className === ObjectReference::PARENT) { - return null; - } - - $class = in_array($className, self::NO_NAMESPACED_CLASSNAMES, true) - ? new Name($className) - : new FullyQualified($className); - $instanceof = new Instanceof_($varNode, $class); - if ($node instanceof NotIdentical) { - return new BooleanNot($instanceof); - } - - return $instanceof; - } - - private function isClassReference(Node $node): bool - { - if (! $node instanceof ClassConstFetch) { - // might be - return $node instanceof String_; - } - - return $this->isName($node->name, 'class'); - } + trigger_error( + sprintf( + 'The "%s" rule was deprecated, as its functionality caused bugs. See https://github.com/rectorphp/rector/issues/8639', + self::class, + ) + ); - private function isGetClassFuncCallNode(Node $node): bool - { - if (! $node instanceof FuncCall) { - return false; - } + $this->hasWarned = true; - return $this->isName($node, 'get_class'); + return null; } }