From 1250b2ffbda604e281f985811f1f8aff41d3e158 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 13 Nov 2024 16:26:35 +0100 Subject: [PATCH] [php74] Skip conditinal assign in RestoreDefaultNullToNullableTypePropertyRector as most likely desired to assign or fail --- .../skip_assigned_in_construct.php.inc | 2 +- .../Fixture/skip_checked.php.inc | 10 ++++- ...efaultNullToNullableTypePropertyRector.php | 9 ++-- .../Rector/FunctionLike/MixedTypeRector.php | 5 +-- .../ConstructorAssignDetector.php | 42 ++++++++----------- .../StatementDepthAttributeDecorator.php | 28 +++++++++++++ src/NodeTypeResolver/Node/AttributeKey.php | 5 +++ 7 files changed, 64 insertions(+), 37 deletions(-) create mode 100644 src/NodeDecorator/StatementDepthAttributeDecorator.php diff --git a/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_assigned_in_construct.php.inc b/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_assigned_in_construct.php.inc index a319ee3d995..c5feb1994ce 100644 --- a/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_assigned_in_construct.php.inc +++ b/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_assigned_in_construct.php.inc @@ -2,7 +2,7 @@ namespace Rector\Tests\Php74\Rector\Property\RestoreDefaultNullToNullableTypePropertyRector\Fixture; -class SkipAssignedInConstruct +final class SkipAssignedInConstruct { public ?string $name; diff --git a/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_checked.php.inc b/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_checked.php.inc index 8e0634dc174..388959e976e 100644 --- a/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_checked.php.inc +++ b/rules-tests/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_checked.php.inc @@ -6,8 +6,14 @@ final class SkipChecked { private ?string $name; - public function __construct(?string $name) + public function __construct(array $items) { - $this->name = $name; + foreach ($items as $item) { + $this->name = $item; + } + + if (! isset($this->item)) { + throw new \InvalidArgumentException(); + } } } diff --git a/rules/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector.php b/rules/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector.php index aca18cfa533..1a91f990919 100644 --- a/rules/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector.php +++ b/rules/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector.php @@ -71,7 +71,7 @@ public function refactor(Node $node): ?Node $hasChanged = false; foreach ($node->getProperties() as $property) { - if ($this->shouldSkip($property, $node)) { + if ($this->shouldSkipProperty($property, $node)) { continue; } @@ -93,7 +93,7 @@ public function provideMinPhpVersion(): int return PhpVersionFeature::TYPED_PROPERTIES; } - private function shouldSkip(Property $property, Class_ $class): bool + private function shouldSkipProperty(Property $property, Class_ $class): bool { if ($property->type === null) { return true; @@ -103,8 +103,7 @@ private function shouldSkip(Property $property, Class_ $class): bool return true; } - $onlyProperty = $property->props[0]; - if ($onlyProperty->default instanceof Expr) { + if ($property->props[0]->default instanceof Expr) { return true; } @@ -118,7 +117,7 @@ private function shouldSkip(Property $property, Class_ $class): bool // is variable assigned in constructor $propertyName = $this->getName($property); - return $this->constructorAssignDetector->isPropertyAssigned($class, $propertyName); + return $this->constructorAssignDetector->isPropertyAssignedConditionally($class, $propertyName); } private function isReadonlyProperty(Property $property): bool diff --git a/rules/Php80/Rector/FunctionLike/MixedTypeRector.php b/rules/Php80/Rector/FunctionLike/MixedTypeRector.php index 95cb308bd9d..bbe5c887dad 100644 --- a/rules/Php80/Rector/FunctionLike/MixedTypeRector.php +++ b/rules/Php80/Rector/FunctionLike/MixedTypeRector.php @@ -94,11 +94,8 @@ public function refactor(Node $node): ?Node $this->refactorParamTypes($node, $phpDocInfo); $hasChanged = $this->paramTagRemover->removeParamTagsIfUseless($phpDocInfo, $node, new MixedType()); - if ($this->hasChanged) { - return $node; - } - if ($hasChanged) { + if ($this->hasChanged || $hasChanged) { return $node; } diff --git a/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php b/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php index 13862a4d9d6..f518d4bcca3 100644 --- a/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php +++ b/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php @@ -10,6 +10,7 @@ use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Identifier; use PhpParser\Node\Stmt; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Else_; @@ -19,6 +20,8 @@ use PhpParser\NodeTraverser; use PHPStan\Type\ObjectType; use Rector\NodeAnalyzer\PropertyFetchAnalyzer; +use Rector\NodeDecorator\StatementDepthAttributeDecorator; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\NodeTypeResolver\NodeTypeResolver; use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser; use Rector\PhpParser\Comparing\NodeComparator; @@ -28,11 +31,6 @@ final readonly class ConstructorAssignDetector { - /** - * @var string - */ - private const IS_FIRST_LEVEL_STATEMENT = 'first_level_stmt'; - public function __construct( private NodeTypeResolver $nodeTypeResolver, private PropertyAssignMatcher $propertyAssignMatcher, @@ -43,7 +41,12 @@ public function __construct( ) { } - public function isPropertyAssigned(ClassLike $classLike, string $propertyName): bool + public function isPropertyAssignedConditionally(Class_ $class, string $propertyName): bool + { + return $this->isPropertyAssigned($class, $propertyName, true); + } + + public function isPropertyAssigned(ClassLike $classLike, string $propertyName, bool $allowConditional = false): bool { $initializeClassMethods = $this->matchInitializeClassMethod($classLike); if ($initializeClassMethods === []) { @@ -52,12 +55,12 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName): $isAssignedInConstructor = false; - $this->decorateFirstLevelStatementAttribute($initializeClassMethods); + StatementDepthAttributeDecorator::decorateClassMethods($initializeClassMethods); foreach ($initializeClassMethods as $initializeClassMethod) { $this->simpleCallableNodeTraverser->traverseNodesWithCallable((array) $initializeClassMethod->stmts, function ( Node $node - ) use ($propertyName, &$isAssignedInConstructor): ?int { + ) use ($propertyName, &$isAssignedInConstructor, $allowConditional): ?int { if ($this->isIfElseAssign($node, $propertyName)) { $isAssignedInConstructor = true; return NodeTraverser::STOP_TRAVERSAL; @@ -77,10 +80,15 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName): return NodeTraverser::STOP_TRAVERSAL; } - $isFirstLevelStatement = $assign->getAttribute(self::IS_FIRST_LEVEL_STATEMENT); + $isFirstLevelStatement = $assign->getAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT); // cannot be nested if ($isFirstLevelStatement !== true) { + if ($allowConditional) { + $isAssignedInConstructor = true; + return NodeTraverser::STOP_TRAVERSAL; + } + return null; } @@ -139,22 +147,6 @@ private function matchAssignExprToPropertyName(Node $node, string $propertyName) return $this->propertyAssignMatcher->matchPropertyAssignExpr($node, $propertyName); } - /** - * @param ClassMethod[] $classMethods - */ - private function decorateFirstLevelStatementAttribute(array $classMethods): void - { - foreach ($classMethods as $classMethod) { - foreach ((array) $classMethod->stmts as $methodStmt) { - $methodStmt->setAttribute(self::IS_FIRST_LEVEL_STATEMENT, true); - - if ($methodStmt instanceof Expression) { - $methodStmt->expr->setAttribute(self::IS_FIRST_LEVEL_STATEMENT, true); - } - } - } - } - /** * @return ClassMethod[] */ diff --git a/src/NodeDecorator/StatementDepthAttributeDecorator.php b/src/NodeDecorator/StatementDepthAttributeDecorator.php new file mode 100644 index 00000000000..d26a8e28a25 --- /dev/null +++ b/src/NodeDecorator/StatementDepthAttributeDecorator.php @@ -0,0 +1,28 @@ +stmts as $methodStmt) { + $methodStmt->setAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT, true); + + if ($methodStmt instanceof Expression) { + $methodStmt->expr->setAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT, true); + } + } + } + } +} diff --git a/src/NodeTypeResolver/Node/AttributeKey.php b/src/NodeTypeResolver/Node/AttributeKey.php index 3305711dcd7..099684f6b96 100644 --- a/src/NodeTypeResolver/Node/AttributeKey.php +++ b/src/NodeTypeResolver/Node/AttributeKey.php @@ -285,4 +285,9 @@ final class AttributeKey * @var string */ public const ATTRIBUTE_COMMENT = 'attribute_comment'; + + /** + * @var string + */ + public const IS_FIRST_LEVEL_STATEMENT = 'first_level_stmt'; }