diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_attribute_decorated_args.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_attribute_decorated_args.php.inc deleted file mode 100644 index 4e192c6932c..00000000000 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_attribute_decorated_args.php.inc +++ /dev/null @@ -1,14 +0,0 @@ - $config - * - * @phpstan-param CustomScalarConfig $config - */ - public function __construct(array $config) - { - parent::__construct($config); - } -} diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_parent_does_not_has_default_param.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_parent_does_not_has_default_param.php.inc deleted file mode 100644 index 57be0e36c98..00000000000 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_parent_does_not_has_default_param.php.inc +++ /dev/null @@ -1,13 +0,0 @@ -d->format('Y-m-d'); - } -} diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_protected_parent_construct.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_protected_parent_construct.php.inc deleted file mode 100644 index e86ca95a2b2..00000000000 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_protected_parent_construct.php.inc +++ /dev/null @@ -1,13 +0,0 @@ - ------ - diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/with_equal_default_param_value.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/with_equal_default_param_value.php.inc deleted file mode 100644 index f904501bfab..00000000000 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/with_equal_default_param_value.php.inc +++ /dev/null @@ -1,23 +0,0 @@ - ------ - diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/RemoveParentDelegatingConstructorRectorTest.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/RemoveParentDelegatingConstructorRectorTest.php deleted file mode 100644 index e65a7af8078..00000000000 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/RemoveParentDelegatingConstructorRectorTest.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/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Source/ProtectedConstructClass.php b/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Source/ProtectedConstructClass.php deleted file mode 100644 index afd87e3c665..00000000000 --- a/rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Source/ProtectedConstructClass.php +++ /dev/null @@ -1,12 +0,0 @@ -withRules([RemoveParentDelegatingConstructorRector::class]); diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingClassMethodRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingClassMethodRector.php index d41c24010b5..a43d794af75 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingClassMethodRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingClassMethodRector.php @@ -77,7 +77,7 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?int { - // constructors are handled by RemoveParentDelegatingConstructorRector + // skip constructors, as removing a delegating constructor is risky and usually intentional if ($this->isName($node, MethodName::CONSTRUCT)) { return null; } diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php index 7520d64c6a6..e9dc2886133 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php +++ b/rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php @@ -5,40 +5,18 @@ namespace Rector\DeadCode\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\Arg; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\StaticCall; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_; -use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Expression; -use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter; -use PHPStan\Reflection\ClassReflection; -use PHPStan\Reflection\ExtendedMethodReflection; -use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; -use Rector\Enum\ObjectReference; -use Rector\PhpParser\Node\Value\ValueResolver; -use Rector\PHPStan\ScopeFetcher; -use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; use Rector\RuleDoc\CodeSample\CodeSample; use Rector\RuleDoc\RuleDefinition; -use Rector\StaticTypeMapper\StaticTypeMapper; -use Rector\ValueObject\MethodName; /** - * @see \Rector\Tests\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\RemoveParentDelegatingConstructorRectorTest + * @deprecated This rule is deprecated, as removing a delegating constructor is risky and such constructors are nearly always kept on purpose. */ -final class RemoveParentDelegatingConstructorRector extends AbstractRector +final class RemoveParentDelegatingConstructorRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly StaticTypeMapper $staticTypeMapper, - private readonly ValueResolver $valueResolver, - private readonly PhpDocInfoFactory $phpDocInfoFactory - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -92,264 +70,9 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Class_ { - $constructClassMethod = $node->getMethod(MethodName::CONSTRUCT); - if (! $constructClassMethod instanceof ClassMethod) { - return null; - } - - if ($constructClassMethod->stmts === null || count($constructClassMethod->stmts) !== 1) { - return null; - } - - if ($constructClassMethod->isFinal()) { - return null; - } - - if (! $constructClassMethod->isPublic()) { - return null; - } - - $soleStmt = $constructClassMethod->stmts[0]; - $parentCallArgs = $this->matchParentConstructorCallArgs($soleStmt); - if ($parentCallArgs === null) { - return null; - } - - // match count and order - if (! $this->isParameterAndArgCountAndOrderIdentical($constructClassMethod)) { - return null; - } - - $parentMethodReflection = $this->matchParentConstructorReflection($node); - if (! $parentMethodReflection instanceof ExtendedMethodReflection) { - return null; - } - - // removing public constructor would expose the narrower parent one - if (! $parentMethodReflection->isPublic()) { - return null; - } - - // match parameter types and parent constructor types - if (! $this->areConstructorAndParentParameterTypesMatching($constructClassMethod, $parentMethodReflection)) { - return null; - } - - if ($this->doAttributeDecoratedParametersExist($constructClassMethod)) { - return null; - } - - // keep when the docblock refines parameter types beyond the native signature, - // e.g. @param array or @phpstan-param SomeShape $config — removing - // the constructor would drop that type information - if ($this->hasParamRefiningDocblock($constructClassMethod)) { - return null; - } - - foreach ($node->stmts as $key => $stmt) { - if ($stmt === $constructClassMethod) { - unset($node->stmts[$key]); - $node->stmts = array_values($node->stmts); - - return $node; - } - } - - return null; - } - - private function hasParamRefiningDocblock(ClassMethod $classMethod): bool - { - $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod); - - return $phpDocInfo->hasByNames(['@param', '@phpstan-param', '@psalm-param']); - } - - private function matchParentConstructorReflection(Class_ $class): ?ExtendedMethodReflection - { - $scope = ScopeFetcher::fetch($class); - - $classReflection = $scope->getClassReflection(); - if (! $classReflection instanceof ClassReflection) { - return null; - } - - $parentClassReflection = $classReflection->getParentClass(); - if (! $parentClassReflection instanceof ClassReflection) { - return null; - } - - if (! $parentClassReflection->hasConstructor()) { - return null; - } - - return $parentClassReflection->getConstructor(); - } - - /** - * Looking for parent::__construct() - * - * @return Arg[]|null - */ - private function matchParentConstructorCallArgs(Stmt $stmt): ?array - { - if (! $stmt instanceof Expression) { - return null; - } - - if (! $stmt->expr instanceof StaticCall) { - return null; - } - - $staticCall = $stmt->expr; - if ($staticCall->isFirstClassCallable()) { - return null; - } - - if (! $this->isName($staticCall->class, ObjectReference::PARENT)) { - return null; - } - - if (! $this->isName($staticCall->name, MethodName::CONSTRUCT)) { - return null; - } - - return $staticCall->getArgs(); - } - - private function isParameterAndArgCountAndOrderIdentical(ClassMethod $classMethod): bool - { - if ($classMethod->stmts === null || ! isset($classMethod->stmts[0])) { - return false; - } - - $soleStmt = $classMethod->stmts[0]; - - $parentCallArgs = $this->matchParentConstructorCallArgs($soleStmt); - if ($parentCallArgs === null) { - return false; - } - - $constructorParams = $classMethod->getParams(); - if (count($constructorParams) !== count($parentCallArgs)) { - return false; - } - - // match passed names in the same order - $paramNames = []; - foreach ($constructorParams as $constructorParam) { - $paramNames[] = $this->getName($constructorParam->var); - } - - $argNames = []; - foreach ($parentCallArgs as $parentCallArg) { - $argValue = $parentCallArg->value; - if (! $argValue instanceof Variable) { - return false; - } - - $argNames[] = $this->getName($argValue); - } - - return $paramNames === $argNames; - } - - private function areConstructorAndParentParameterTypesMatching( - ClassMethod $classMethod, - ExtendedMethodReflection $extendedMethodReflection - ): bool { - foreach ($classMethod->getParams() as $position => $param) { - if ($param->isPromoted() && $param->isPrivate()) { - return false; - } - - $parameterType = $param->type; - - // no type override - if ($parameterType === null) { - if ($param->default instanceof Expr && $this->isDifferentDefaultValue( - $param->default, - $extendedMethodReflection, - $position - )) { - return false; - } - - continue; - } - - $parametersSelector = $extendedMethodReflection->getOnlyVariant(); - - foreach ($parametersSelector->getParameters() as $index => $parameterReflection) { - if ($index !== $position) { - continue; - } - - $parentParameterType = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode( - $parameterReflection->getType(), - TypeKind::PARAM - ); - - if (! $this->nodeComparator->areNodesEqual($parameterType, $parentParameterType)) { - return false; - } - - if (! $param->default instanceof Expr) { - continue; - } - - if ($this->isDifferentDefaultValue($param->default, $extendedMethodReflection, $index)) { - return false; - } - } - } - - return true; - } - - private function isDifferentDefaultValue( - Expr $defaultExpr, - ExtendedMethodReflection $extendedMethodReflection, - int $index - ): bool { - $methodName = $extendedMethodReflection->getName(); - // native reflection is needed to get exact default value - if ($extendedMethodReflection->getDeclaringClass()->getNativeReflection()->hasMethod($methodName)) { - $parentMethod = $extendedMethodReflection->getDeclaringClass() - ->getNativeReflection() - ->getMethod($methodName); - $nativeParentParameterReflection = $parentMethod->getParameters()[$index] ?? null; - - if (! $nativeParentParameterReflection instanceof ReflectionParameter) { - return false; - } - - // when child has default value, but parent does not have default value, - // mark as different - if (! $nativeParentParameterReflection->isDefaultValueAvailable()) { - return true; - } - - $parentDefault = $nativeParentParameterReflection->getDefaultValue(); - if (! $this->valueResolver->isValue($defaultExpr, $parentDefault)) { - return true; - } - } - - return false; - } - - private function doAttributeDecoratedParametersExist(ClassMethod $classMethod): bool - { - $constructorParams = $classMethod->getParams(); - foreach ($constructorParams as $constructorParam) { - foreach ($constructorParam->attrGroups as $attrGroup) { - if ($attrGroup->attrs !== []) { - return true; - } - } - } - - return false; + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as removing a delegating constructor is risky and such constructors are nearly always kept on purpose', + self::class + )); } } diff --git a/src/Config/Level/DeadCodeLevel.php b/src/Config/Level/DeadCodeLevel.php index 5f8db19ffd3..402a6c68327 100644 --- a/src/Config/Level/DeadCodeLevel.php +++ b/src/Config/Level/DeadCodeLevel.php @@ -24,7 +24,6 @@ use Rector\DeadCode\Rector\ClassMethod\RemoveMixedDocblockOverruledByNativeTypeRector; use Rector\DeadCode\Rector\ClassMethod\RemoveNullTagValueNodeRector; use Rector\DeadCode\Rector\ClassMethod\RemoveParentDelegatingClassMethodRector; -use Rector\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector; use Rector\DeadCode\Rector\ClassMethod\RemoveReturnTagIncompatibleWithNativeTypeRector; use Rector\DeadCode\Rector\ClassMethod\RemoveTestsOverriddenPrivateMethodParameterRector; use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedConstructorParamRector; @@ -165,7 +164,6 @@ final class DeadCodeLevel UnwrapFutureCompatibleIfPhpVersionRector::class, RemoveParentCallWithoutParentRector::class, - RemoveParentDelegatingConstructorRector::class, RemoveParentDelegatingClassMethodRector::class, RemoveDeadConditionAboveReturnRector::class, diff --git a/tests/Issues/IssuePropertyPromoRemoveDelegatingParent/config/configured_rule.php b/tests/Issues/IssuePropertyPromoRemoveDelegatingParent/config/configured_rule.php index 454a89aec1a..d7b2958f886 100644 --- a/tests/Issues/IssuePropertyPromoRemoveDelegatingParent/config/configured_rule.php +++ b/tests/Issues/IssuePropertyPromoRemoveDelegatingParent/config/configured_rule.php @@ -3,11 +3,9 @@ declare(strict_types=1); use Rector\Config\RectorConfig; -use Rector\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector; use Rector\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector; return RectorConfig::configure() ->withRules([ ClassPropertyAssignToConstructorPromotionRector::class, - RemoveParentDelegatingConstructorRector::class, ]);