From 5ca93fdca969579c4060713c25e00285f2f9c6e9 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Fri, 29 Oct 2021 21:00:19 +0700 Subject: [PATCH] [Php80] Skip union callable on ClassPropertyAssignToConstructorPromotionRector (#1101) * Add failing test fixture for ClassPropertyAssignToConstructorPromotionRector * Closes #1100 * cs fix * Add PropertyAnalyzer * [ci-review] Rector Rectify * rectify * clean up * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify Co-authored-by: zingimmick Co-authored-by: GitHub Action --- ...nullable_callable_without_typehint.php.inc | 14 ++++++ ...kip_union_typed_with_callable_type.php.inc | 14 ++++++ .../Rector/Property/TypedPropertyRector.php | 22 ++++---- ...ertyAssignToConstructorPromotionRector.php | 27 +++++++++- src/NodeAnalyzer/PropertyAnalyzer.php | 50 +++++++++++++++++++ 5 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_nullable_callable_without_typehint.php.inc create mode 100644 rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_union_typed_with_callable_type.php.inc create mode 100644 src/NodeAnalyzer/PropertyAnalyzer.php diff --git a/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_nullable_callable_without_typehint.php.inc b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_nullable_callable_without_typehint.php.inc new file mode 100644 index 00000000000..b8f8eccfad5 --- /dev/null +++ b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_nullable_callable_without_typehint.php.inc @@ -0,0 +1,14 @@ +cb = $cb; + } +} diff --git a/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_union_typed_with_callable_type.php.inc b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_union_typed_with_callable_type.php.inc new file mode 100644 index 00000000000..40cdc6efc10 --- /dev/null +++ b/rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_union_typed_with_callable_type.php.inc @@ -0,0 +1,14 @@ +cb = $cb; + } +} diff --git a/rules/Php74/Rector/Property/TypedPropertyRector.php b/rules/Php74/Rector/Property/TypedPropertyRector.php index 4fa9e4468b6..5813e139a80 100644 --- a/rules/Php74/Rector/Property/TypedPropertyRector.php +++ b/rules/Php74/Rector/Property/TypedPropertyRector.php @@ -16,6 +16,7 @@ use PHPStan\Type\Type; use PHPStan\Type\UnionType; use Rector\Core\Contract\Rector\ConfigurableRectorInterface; +use Rector\Core\NodeAnalyzer\PropertyAnalyzer; use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\PhpVersionFeature; @@ -71,7 +72,8 @@ public function __construct( private VarTagRemover $varTagRemover, private ReflectionProvider $reflectionProvider, private PropertyFetchAnalyzer $propertyFetchAnalyzer, - private FamilyRelationsAnalyzer $familyRelationsAnalyzer + private FamilyRelationsAnalyzer $familyRelationsAnalyzer, + private PropertyAnalyzer $propertyAnalyzer ) { } @@ -246,14 +248,6 @@ private function shouldSkipNonClassLikeType(Name|NullableType|PhpParserUnionType return false; } - if ($typeName === 'null') { - return true; - } - - if ($typeName === 'callable') { - return true; - } - if (! $this->classLikeTypeOnly) { return false; } @@ -311,6 +305,14 @@ private function shouldSkipProperty(Property $property): bool return true; } - return $this->privatePropertyOnly && ! $property->isPrivate(); + if (! $this->privatePropertyOnly) { + return $this->propertyAnalyzer->hasForbiddenType($property); + } + + if ($property->isPrivate()) { + return $this->propertyAnalyzer->hasForbiddenType($property); + } + + return true; } } diff --git a/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php b/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php index 06028e083dd..cd9bf94ddc8 100644 --- a/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php +++ b/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php @@ -11,10 +11,12 @@ use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Property; +use PhpParser\Node\UnionType; use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode; use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger; use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey; use Rector\Core\NodeAnalyzer\ParamAnalyzer; +use Rector\Core\NodeAnalyzer\PropertyAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\MethodName; use Rector\Core\ValueObject\PhpVersionFeature; @@ -39,7 +41,8 @@ public function __construct( private VariableRenamer $variableRenamer, private VarTagRemover $varTagRemover, private ParamAnalyzer $paramAnalyzer, - private PhpDocTypeChanger $phpDocTypeChanger + private PhpDocTypeChanger $phpDocTypeChanger, + private PropertyAnalyzer $propertyAnalyzer ) { } @@ -106,6 +109,10 @@ public function refactor(Node $node): ?Node continue; } + if ($this->propertyAnalyzer->hasForbiddenType($property)) { + continue; + } + $this->removeNode($property); $this->removeNode($promotionCandidate->getAssign()); @@ -184,6 +191,22 @@ private function shouldSkipParam(Param $param): bool $type = $param->type; } - return $type instanceof Identifier && $this->isName($type, 'callable'); + if (! $type instanceof UnionType) { + return false; + } + + foreach ($type->types as $type) { + if (! $type instanceof Identifier) { + continue; + } + + if (! $this->isName($type, 'callable')) { + continue; + } + + return true; + } + + return false; } } diff --git a/src/NodeAnalyzer/PropertyAnalyzer.php b/src/NodeAnalyzer/PropertyAnalyzer.php new file mode 100644 index 00000000000..c89c4f6390a --- /dev/null +++ b/src/NodeAnalyzer/PropertyAnalyzer.php @@ -0,0 +1,50 @@ +nodeTypeResolver->getType($property); + if ($propertyType instanceof NullType) { + return true; + } + + if ($this->isCallableType($propertyType)) { + return true; + } + + if (! $propertyType instanceof UnionType) { + return false; + } + + $types = $propertyType->getTypes(); + foreach ($types as $type) { + if ($this->isCallableType($type)) { + return true; + } + } + + return false; + } + + private function isCallableType(Type $type): bool + { + return $type instanceof CallableType; + } +}