diff --git a/composer.json b/composer.json index 33e4d87ee0a..850a0ece814 100644 --- a/composer.json +++ b/composer.json @@ -81,6 +81,7 @@ "rules-tests", "tests" ], + "Rector\\Utils\\Tests\\": "utils-tests", "E2e\\Parallel\\Reflection\\Resolver\\": [ "e2e/parallel-reflection-resolver/src/", "e2e/no-parallel-reflection-resolver/src" diff --git a/phpstan.neon b/phpstan.neon index 6091df0d97b..db92a0ee7e1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -352,3 +352,5 @@ parameters: - identifier: offsetAccess.invalidOffset path: src/CustomRules/SimpleNodeDumper.php + + - '#Method Rector\\Utils\\Rector\\RemoveRefactorDuplicatedNodeInstanceCheckRector\:\:getInstanceofNodeClass\(\) should return class\-string\|null but returns class\-string#' diff --git a/phpunit.xml b/phpunit.xml index 8607c5b0eb2..9f15cef2ecf 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -13,6 +13,7 @@ tests rules-tests + utils-tests diff --git a/rector.php b/rector.php index b24f7fe82f0..76c431a644c 100644 --- a/rector.php +++ b/rector.php @@ -6,6 +6,7 @@ use Rector\Config\RectorConfig; use Rector\DeadCode\Rector\ConstFetch\RemovePhpVersionIdCheckRector; use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector; +use Rector\Utils\Rector\RemoveRefactorDuplicatedNodeInstanceCheckRector; return RectorConfig::configure() ->withPreparedSets( @@ -18,7 +19,6 @@ naming: true, instanceOf: true, earlyReturn: true, - strictBooleans: true, rectorPreset: true, phpunitCodeQuality: true ) @@ -37,6 +37,7 @@ ]) ->withRootFiles() ->withImportNames(removeUnusedImports: true) + ->withRules([RemoveRefactorDuplicatedNodeInstanceCheckRector::class]) ->withSkip([ StringClassNameToClassConstantRector::class, // tests diff --git a/rules/CodeQuality/Rector/BooleanNot/ReplaceConstantBooleanNotRector.php b/rules/CodeQuality/Rector/BooleanNot/ReplaceConstantBooleanNotRector.php index 9b70b143f1f..0d395974f3a 100644 --- a/rules/CodeQuality/Rector/BooleanNot/ReplaceConstantBooleanNotRector.php +++ b/rules/CodeQuality/Rector/BooleanNot/ReplaceConstantBooleanNotRector.php @@ -69,10 +69,6 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (! $node instanceof BooleanNot) { - return null; - } - if ($this->valueResolver->isFalse($node->expr)) { return new ConstFetch(new Name('true')); } diff --git a/rules/CodingStyle/Rector/FuncCall/FunctionFirstClassCallableRector.php b/rules/CodingStyle/Rector/FuncCall/FunctionFirstClassCallableRector.php index 905da135b3a..5a610bc3e15 100644 --- a/rules/CodingStyle/Rector/FuncCall/FunctionFirstClassCallableRector.php +++ b/rules/CodingStyle/Rector/FuncCall/FunctionFirstClassCallableRector.php @@ -61,12 +61,11 @@ public function getNodeTypes(): array return [FuncCall::class]; } + /** + * @param FuncCall $node + */ public function refactor(Node $node): ?FuncCall { - if (! $node instanceof FuncCall) { - return null; - } - if (! $node->name instanceof Name) { return null; } diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPublicMethodParameterRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPublicMethodParameterRector.php index 18e603a8ba1..842d7ce3581 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPublicMethodParameterRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPublicMethodParameterRector.php @@ -13,6 +13,7 @@ use Rector\NodeAnalyzer\MagicClassMethodAnalyzer; use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer; use Rector\Rector\AbstractRector; +use Rector\ValueObject\MethodName; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -115,7 +116,7 @@ private function shouldSkipClassMethod(ClassMethod $classMethod, Class_ $class): } // parameter is required for contract coupling - if ($this->isName($classMethod->name, '__invoke') && $this->phpAttributeAnalyzer->hasPhpAttribute( + if ($this->isName($classMethod->name, MethodName::INVOKE) && $this->phpAttributeAnalyzer->hasPhpAttribute( $class, 'Symfony\Component\Messenger\Attribute\AsMessageHandler' )) { diff --git a/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php b/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php index ba68a2adfb6..10c793967fb 100644 --- a/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php +++ b/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php @@ -6,7 +6,6 @@ use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Reflection\ClassReflection; -use Rector\FileSystem\FilePathHelper; use Rector\NodeNameResolver\NodeNameResolver; use Rector\Reflection\ReflectionResolver; @@ -15,7 +14,6 @@ public function __construct( private NodeNameResolver $nodeNameResolver, private ReflectionResolver $reflectionResolver, - private FilePathHelper $filePathHelper ) { } @@ -38,11 +36,7 @@ public function isVendorLocked(ClassMethod $classMethod): bool $methodName = $this->nodeNameResolver->getName($classMethod); // has interface vendor lock? → better skip it, as PHPStan has access only to just analyzed classes - if ($this->hasParentInterfaceMethod($classReflection, $methodName)) { - return true; - } - - return $this->hasClassMethodLockMatchingFileName($classReflection, $methodName, '/vendor/'); + return $this->hasParentInterfaceMethod($classReflection, $methodName); } /** @@ -60,37 +54,4 @@ private function hasParentInterfaceMethod(ClassReflection $classReflection, stri return false; } - - private function hasClassMethodLockMatchingFileName( - ClassReflection $classReflection, - string $methodName, - string $filePathPartName - ): bool { - $ancestorClassReflections = [...$classReflection->getParents(), ...$classReflection->getInterfaces()]; - foreach ($ancestorClassReflections as $ancestorClassReflection) { - // parent type - if (! $ancestorClassReflection->hasNativeMethod($methodName)) { - continue; - } - - // is file in vendor? - $fileName = $ancestorClassReflection->getFileName(); - // probably internal class - if ($fileName === null) { - continue; - } - - // not conditions? its a match - if ($filePathPartName === '') { - return true; - } - - $normalizedFileName = $this->filePathHelper->normalizePathAndSchema($fileName); - if (str_contains($normalizedFileName, $filePathPartName)) { - return true; - } - } - - return false; - } } diff --git a/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/Fixture/cover_bare_get_node_types.php.inc b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/Fixture/cover_bare_get_node_types.php.inc new file mode 100644 index 00000000000..456c6bd025a --- /dev/null +++ b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/Fixture/cover_bare_get_node_types.php.inc @@ -0,0 +1,54 @@ + +----- + diff --git a/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/Fixture/some_class.php.inc b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/Fixture/some_class.php.inc new file mode 100644 index 00000000000..f4991045b08 --- /dev/null +++ b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/Fixture/some_class.php.inc @@ -0,0 +1,60 @@ + +----- + diff --git a/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/RemoveRefactorDuplicatedNodeInstanceCheckRectorTest.php b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/RemoveRefactorDuplicatedNodeInstanceCheckRectorTest.php new file mode 100644 index 00000000000..1ae75654141 --- /dev/null +++ b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/RemoveRefactorDuplicatedNodeInstanceCheckRectorTest.php @@ -0,0 +1,28 @@ +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/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/config/configured_rule.php b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/config/configured_rule.php new file mode 100644 index 00000000000..f4ae1bb7e60 --- /dev/null +++ b/utils-tests/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector/config/configured_rule.php @@ -0,0 +1,11 @@ +withRules([ + RemoveRefactorDuplicatedNodeInstanceCheckRector::class, + ]); diff --git a/utils/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector.php b/utils/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector.php new file mode 100644 index 00000000000..35bb54987e5 --- /dev/null +++ b/utils/Rector/RemoveRefactorDuplicatedNodeInstanceCheckRector.php @@ -0,0 +1,165 @@ +getClassReflection(); + + if (! $classReflection instanceof ClassReflection) { + return null; + } + + if (! $classReflection->is('Rector\Rector\AbstractRector')) { + return null; + } + + $refactorClassMethod = $node->getMethod('refactor'); + if (! $refactorClassMethod instanceof ClassMethod) { + return null; + } + + $firstStmt = $refactorClassMethod->stmts[0] ?? null; + if (! $firstStmt instanceof If_) { + return null; + } + + $instanceofNodeClass = $this->matchBooleanNotInstanceOfNodeClass($firstStmt->cond); + if (! is_string($instanceofNodeClass)) { + return null; + } + + $nodeParamTypeClass = $this->matchNodeParamType($refactorClassMethod); + + $getNodeTypesClassMethod = $node->getMethod('getNodeTypes'); + if (! $getNodeTypesClassMethod instanceof ClassMethod) { + return null; + } + + $soleReturn = $getNodeTypesClassMethod->stmts[0] ?? null; + + $nodeTypeClass = null; + if ($soleReturn instanceof Return_) { + Assert::isInstanceOf($soleReturn->expr, Expr::class); + + $nodeTypes = $this->valueResolver->getValue($soleReturn->expr); + if (count($nodeTypes) === 1) { + $nodeTypeClass = $nodeTypes[0]; + } + } + + if ($nodeParamTypeClass !== null) { + if ($nodeParamTypeClass !== $instanceofNodeClass) { + return null; + } + } elseif ($nodeTypeClass !== null) { + if ($nodeTypeClass !== $instanceofNodeClass) { + return null; + } + } else { + return null; + } + + unset($refactorClassMethod->stmts[0]); + + return $node; + } + + private function matchBooleanNotInstanceOfNodeClass(Expr $expr): ?string + { + if (! $expr instanceof BooleanNot) { + return null; + } + + $booleanNot = $expr; + if (! $booleanNot->expr instanceof Instanceof_) { + return null; + } + + return $this->getInstanceofNodeClass($booleanNot->expr); + } + + /** + * @return class-string|null + */ + private function getInstanceofNodeClass(Instanceof_ $instanceof): ?string + { + $checkedClassType = $this->getType($instanceof->class); + if (! $checkedClassType instanceof ObjectType) { + return null; + } + + /** @var ClassReflection $classReflection */ + $classReflection = $checkedClassType->getClassReflection(); + + if (! $classReflection->is(Node::class)) { + return null; + } + + return $classReflection->getName(); + } + + private function matchNodeParamType(ClassMethod $classMethod): ?string + { + $classMethodPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod); + + $paramType = $classMethodPhpDocInfo->getParamType('$node'); + if (! $paramType instanceof ObjectType) { + return null; + } + + if ($paramType instanceof ShortenedObjectType) { + return $paramType->getFullyQualifiedName(); + } + + return $paramType->getClassName(); + } +}