[CodeQuality] Skip non typed property no default value never assigned…

… on SimplifyEmptyCheckOnEmptyArrayRector (#3171) Co-authored-by: GitHub Action <action@github.com>
rectorphp · Dec 8, 2022 · bbd9d4f · bbd9d4f
1 parent 2848e0d
commit bbd9d4f
Show file tree

Hide file tree

Showing 11 changed files with 265 additions and 20 deletions.
diff --git a/...nEmptyArrayRectorTest/Fixture/non_typed_property_has_default_value_never_assigned.php.inc b/...nEmptyArrayRectorTest/Fixture/non_typed_property_has_default_value_never_assigned.php.inc
@@ -0,0 +1,33 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class NonTypedPropertyHasDefaultValueNeverAssigned
+{
+    /** @var array */
+    public $property = [];
+
+    public function isEmpty(): bool
+    {
+        return empty($this->property);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class NonTypedPropertyHasDefaultValueNeverAssigned
+{
+    /** @var array */
+    public $property = [];
+
+    public function isEmpty(): bool
+    {
+        return $this->property === [];
+    }
+}
+
+?>
diff --git a/...rayRectorTest/Fixture/skip_non_typed_property_has_default_value_assigned_nullable.php.inc b/...rayRectorTest/Fixture/skip_non_typed_property_has_default_value_assigned_nullable.php.inc
@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class SkipNonTypedPropertyHasDefaultValueAssignedNullable
+{
+    /** @var array */
+    public $property = [];
+
+    public function run()
+    {
+        if (rand(0, 1)) {
+            $this->property = null;
+        } else {
+            $this->property = [];
+        }
+    }
+
+    public function isEmpty(): bool
+    {
+        return empty($this->property);
+    }
+}
diff --git a/...est/Fixture/skip_non_typed_property_has_default_value_has_assign_dynamic_property.php.inc b/...est/Fixture/skip_non_typed_property_has_default_value_has_assign_dynamic_property.php.inc
@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class SkipNonTypedPropertyHasDefaultValueHasAssignDynamicProperty
+{
+    /** @var array */
+    public $property = [];
+
+    public function assignDynamicProperty(string $dynamicProperty)
+    {
+        $this->$dynamicProperty = 'value';
+    }
+
+    public function isEmpty(): bool
+    {
+        return empty($this->property);
+    }
+}
diff --git a/...tyArrayRectorTest/Fixture/skip_non_typed_property_no_default_value_never_assigned.php.inc b/...tyArrayRectorTest/Fixture/skip_non_typed_property_no_default_value_never_assigned.php.inc
@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class SkipNonTypedPropertyNoDefaultValueNeverAssigned
+{
+    /** @var array */
+    public $property;
+
+    public function isEmpty(): bool
+    {
+        return empty($this->property);
+    }
+}
diff --git a/...implifyEmptyCheckOnEmptyArrayRectorTest/Fixture/skip_non_typed_property_promotion.php.inc b/...implifyEmptyCheckOnEmptyArrayRectorTest/Fixture/skip_non_typed_property_promotion.php.inc
@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class SkipNonTypedPropertyPromotion
+{
+    /**
+     * @param array $property
+     */
+    public function __construct(private $property)
+    {
+    }
+
+    public function isEmpty(): bool
+    {
+        return empty($this->property);
+    }
+}
diff --git a/..._/SimplifyEmptyCheckOnEmptyArrayRectorTest/Fixture/typed_array_property_promotion.php.inc b/..._/SimplifyEmptyCheckOnEmptyArrayRectorTest/Fixture/typed_array_property_promotion.php.inc
@@ -0,0 +1,35 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class TypedArrayPropertyPromotion
+{
+    public function __construct(private array $property)
+    {
+    }
+
+    public function isEmpty(): bool
+    {
+        return empty($this->property);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRectorTest\Fixture;
+
+final class TypedArrayPropertyPromotion
+{
+    public function __construct(private array $property)
+    {
+    }
+
+    public function isEmpty(): bool
+    {
+        return $this->property === [];
+    }
+}
+
+?>
diff --git a/.../TypedPropertyFromAssignsRector/Fixture/skip_typed_from_default_value_null_assign.php.inc b/.../TypedPropertyFromAssignsRector/Fixture/skip_typed_from_default_value_null_assign.php.inc
@@ -0,0 +1,8 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector\Fixture;
+
+final class SkipTypedFromDefaultValueNullAssign
+{
+    private $name = null;
+}
diff --git a/...r/Property/TypedPropertyFromAssignsRector/Fixture/typed_from_default_value_assign.php.inc b/...r/Property/TypedPropertyFromAssignsRector/Fixture/typed_from_default_value_assign.php.inc
@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector\Fixture;
+
+final class TypedFromDefaultValueAssign
+{
+    private $name = [];
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector\Fixture;
+
+final class TypedFromDefaultValueAssign
+{
+    private array $name = [];
+}
+
+?>
diff --git a/rules/CodeQuality/Rector/Empty_/SimplifyEmptyCheckOnEmptyArrayRector.php b/rules/CodeQuality/Rector/Empty_/SimplifyEmptyCheckOnEmptyArrayRector.php
@@ -14,10 +14,17 @@
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\StaticPropertyFetch;
 use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Stmt\Property;
 use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\ArrayType;
+use PHPStan\Type\MixedType;
 use Rector\Core\NodeAnalyzer\ExprAnalyzer;
+use Rector\Core\PhpParser\AstResolver;
 use Rector\Core\Rector\AbstractScopeAwareRector;
+use Rector\Core\Reflection\ReflectionResolver;
+use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer\AllAssignNodePropertyTypeInferer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -27,7 +34,10 @@
 final class SimplifyEmptyCheckOnEmptyArrayRector extends AbstractScopeAwareRector
 {
     public function __construct(
-        private readonly ExprAnalyzer $exprAnalyzer
+        private readonly ExprAnalyzer $exprAnalyzer,
+        private readonly ReflectionResolver $reflectionResolver,
+        private readonly AstResolver $astResolver,
+        private readonly AllAssignNodePropertyTypeInferer $allAssignNodePropertyTypeInferer
     ) {
     }
 
@@ -77,10 +87,44 @@ private function isAllowedExpr(Expr $expr, Scope $scope): bool
             return ! $this->exprAnalyzer->isNonTypedFromParam($expr);
         }
 
-        if ($expr instanceof PropertyFetch) {
-            return true;
+        if (! $expr instanceof PropertyFetch && ! $expr instanceof StaticPropertyFetch) {
+            return false;
+        }
+
+        if (! $expr->name instanceof Identifier) {
+            return false;
+        }
+
+        $classReflection = $this->reflectionResolver->resolveClassReflection($expr);
+        if (! $classReflection instanceof ClassReflection) {
+            return false;
+        }
+
+        $propertyName = $expr->name->toString();
+        if (! $classReflection->hasNativeProperty($propertyName)) {
+            return false;
+        }
+
+        $phpPropertyReflection = $classReflection->getNativeProperty($propertyName);
+        $nativeType = $phpPropertyReflection->getNativeType();
+
+        if (! $nativeType instanceof MixedType) {
+            return $nativeType instanceof ArrayType;
+        }
+
+        $property = $this->astResolver->resolvePropertyFromPropertyReflection($phpPropertyReflection);
+
+        /**
+         * Skip property promotion mixed type for now, as:
+         *
+         *   - require assign in default param check
+         *   - check all assign of property promotion params under the class
+         */
+        if (! $property instanceof Property) {
+            return false;
         }
 
-        return $expr instanceof StaticPropertyFetch;
+        $type = $this->allAssignNodePropertyTypeInferer->inferProperty($property);
+        return $type instanceof ArrayType;
     }
 }
diff --git a/rules/TypeDeclaration/Matcher/PropertyAssignMatcher.php b/rules/TypeDeclaration/Matcher/PropertyAssignMatcher.php
@@ -8,12 +8,10 @@
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
 use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
-use Rector\NodeNameResolver\NodeNameResolver;
 
 final class PropertyAssignMatcher
 {
     public function __construct(
-        private readonly NodeNameResolver $nodeNameResolver,
         private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer
     ) {
     }
@@ -25,19 +23,16 @@ public function __construct(
      */
     public function matchPropertyAssignExpr(Assign $assign, string $propertyName): ?Expr
     {
-        if ($this->propertyFetchAnalyzer->isPropertyFetch($assign->var)) {
-            if (! $this->nodeNameResolver->isName($assign->var, $propertyName)) {
-                return null;
-            }
-
+        $assignVar = $assign->var;
+        if ($this->propertyFetchAnalyzer->isLocalPropertyFetchName($assignVar, $propertyName)) {
             return $assign->expr;
         }
 
-        if ($assign->var instanceof ArrayDimFetch && $this->propertyFetchAnalyzer->isPropertyFetch($assign->var->var)) {
-            if (! $this->nodeNameResolver->isName($assign->var->var, $propertyName)) {
-                return null;
-            }
+        if (! $assignVar instanceof ArrayDimFetch) {
+            return null;
+        }
 
+        if ($this->propertyFetchAnalyzer->isLocalPropertyFetchName($assignVar->var, $propertyName)) {
             return $assign->expr;
         }
 

diff --git a/rules/TypeDeclaration/TypeInferer/AssignToPropertyTypeInferer.php b/rules/TypeDeclaration/TypeInferer/AssignToPropertyTypeInferer.php
@@ -8,13 +8,18 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\StaticPropertyFetch;
+use PhpParser\Node\Identifier;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\Property;
+use PhpParser\NodeTraverser;
 use PHPStan\Type\ArrayType;
 use PHPStan\Type\MixedType;
 use PHPStan\Type\NullType;
 use PHPStan\Type\Type;
 use Rector\Core\NodeAnalyzer\ExprAnalyzer;
+use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
 use Rector\Core\PhpParser\Node\Value\ValueResolver;
 use Rector\NodeTypeResolver\NodeTypeResolver;
 use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
@@ -39,24 +44,38 @@ public function __construct(
         private readonly TypeFactory $typeFactory,
         private readonly NodeTypeResolver $nodeTypeResolver,
         private readonly ExprAnalyzer $exprAnalyzer,
-        private readonly ValueResolver $valueResolver
+        private readonly ValueResolver $valueResolver,
+        private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer
     ) {
     }
 
     public function inferPropertyInClassLike(Property $property, string $propertyName, ClassLike $classLike): ?Type
     {
         $assignedExprTypes = [];
+        $hasAssignDynamicPropertyValue = false;
 
         $this->simpleCallableNodeTraverser->traverseNodesWithCallable($classLike->stmts, function (Node $node) use (
             $propertyName,
-            &$assignedExprTypes
-        ) {
+            &$assignedExprTypes,
+            &$hasAssignDynamicPropertyValue
+        ): ?int {
             if (! $node instanceof Assign) {
                 return null;
             }
 
             $expr = $this->propertyAssignMatcher->matchPropertyAssignExpr($node, $propertyName);
             if (! $expr instanceof Expr) {
+                if (! $this->propertyFetchAnalyzer->isLocalPropertyFetch($node->var)) {
+                    return null;
+                }
+
+                /** @var PropertyFetch|StaticPropertyFetch $assignVar */
+                $assignVar = $node->var;
+                if (! $assignVar->name instanceof Identifier) {
+                    $hasAssignDynamicPropertyValue = true;
+                    return NodeTraverser::STOP_TRAVERSAL;
+                }
+
                 return null;
             }
 
@@ -69,17 +88,33 @@ public function inferPropertyInClassLike(Property $property, string $propertyNam
             return null;
         });
 
+        if ($hasAssignDynamicPropertyValue) {
+            return null;
+        }
+
         if ($this->shouldAddNullType($classLike, $propertyName, $assignedExprTypes)) {
             $assignedExprTypes[] = new NullType();
         }
 
+        return $this->resolveTypeWithVerifyDefaultValue($property, $assignedExprTypes);
+    }
+
+    /**
+     * @param Type[] $assignedExprTypes
+     */
+    private function resolveTypeWithVerifyDefaultValue(Property $property, array $assignedExprTypes): ?Type
+    {
+        $defaultPropertyValue = $property->props[0]->default;
         if ($assignedExprTypes === []) {
+            // not typed, never assigned, but has default value, then pull type from default value
+            if (! $property->type instanceof Node && $defaultPropertyValue instanceof Expr) {
+                return $this->nodeTypeResolver->getType($defaultPropertyValue);
+            }
+
             return null;
         }
 
         $inferredType = $this->typeFactory->createMixedPassedOrUnionType($assignedExprTypes);
-        $defaultPropertyValue = $property->props[0]->default;
-
         if ($this->shouldSkipWithDifferentDefaultValueType($defaultPropertyValue, $inferredType)) {
             return null;
         }