[CodeQuality] Add InlineConstructorDefaultToPropertyRector (#1935)

build/target-repository/docs/rector_rules_overview.md

@@ -1,4 +1,4 @@
-# 505 Rules Overview
+# 506 Rules Overview
 
 <br>
 
@@ -6,7 +6,7 @@
 
 - [Arguments](#arguments) (5)
 
-- [CodeQuality](#codequality) (71)
+- [CodeQuality](#codequality) (72)
 
 - [CodingStyle](#codingstyle) (35)
 
@@ -868,6 +868,27 @@ Simplify `in_array` and `array_keys` functions combination into `array_key_exist
 
 <br>
 
+### InlineConstructorDefaultToPropertyRector
+
+Move property default from constructor to property default
+
+- class: [`Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector`](../rules/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector.php)
+
+```diff
+ final class SomeClass
+ {
+-    private $name;
+-
+-    public function __construct()
+-    {
+-        $this->name = 'John';
+-    }
++    private $name = 'John';
+ }
+```
+
+<br>
+
 ### InlineIfToExplicitIfRector
 
 Change inline if to explicit if

rules-tests/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector/Fixture/simple_array.php.inc

@@ -0,0 +1,30 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;
+
+final class SimpleArray
+{
+    private $items;
+
+    public function __construct()
+    {
+        $this->items = ['John', true, 252 => [null]];
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;
+
+final class SimpleArray
+{
+    private $items = ['John', true, 252 => [null]];
+
+    public function __construct()
+    {
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector/Fixture/skip_complex_assign.php.inc

@@ -0,0 +1,13 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;
+
+final class SkipComplexAssign
+{
+    private $name;
+
+    public function __construct($name)
+    {
+        $this->name = $name;
+    }
+}

rules-tests/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector/Fixture/skip_conditional_default.php.inc

@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;
+
+final class SkipConditionalDefault
+{
+    private $name;
+
+    public function __construct()
+    {
+        if (mt_rand(0, 1)) {
+            $this->name = 'John';
+        }
+    }
+}

rules-tests/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector/Fixture/some_class.php.inc

@@ -0,0 +1,30 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;
+
+final class SomeClass
+{
+    private $name;
+
+    public function __construct()
+    {
+        $this->name = 'John';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;
+
+final class SomeClass
+{
+    private $name = 'John';
+
+    public function __construct()
+    {
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector/InlineConstructorDefaultToPropertyRectorTest.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector;
+
+use Iterator;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+use Symplify\SmartFileSystem\SmartFileInfo;
+
+final class InlineConstructorDefaultToPropertyRectorTest extends AbstractRectorTestCase
+{
+    /**
+     * @dataProvider provideData()
+     */
+    public function test(SmartFileInfo $fileInfo): void
+    {
+        $this->doTestFileInfo($fileInfo);
+    }
+
+    /**
+     * @return Iterator<SmartFileInfo>
+     */
+    public function provideData(): Iterator
+    {
+        return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}

rules-tests/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector/config/configured_rule.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return static function (ContainerConfigurator $containerConfigurator): void {
+    $services = $containerConfigurator->services();
+    $services->set(InlineConstructorDefaultToPropertyRector::class);
+};

rules/CodeQuality/NodeAnalyzer/ConstructorPropertyDefaultExprResolver.php

@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\CodeQuality\NodeAnalyzer;
+
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use Rector\CodeQuality\ValueObject\DefaultPropertyExprAssign;
+use Rector\Core\NodeAnalyzer\ExprAnalyzer;
+use Rector\NodeNameResolver\NodeNameResolver;
+
+final class ConstructorPropertyDefaultExprResolver
+{
+    public function __construct(
+        private readonly NodeNameResolver $nodeNameResolver,
+        private readonly ExprAnalyzer $exprAnalyzer
+    ) {
+    }
+
+    /**
+     * @return DefaultPropertyExprAssign[]
+     */
+    public function resolve(ClassMethod $classMethod): array
+    {
+        $stmts = $classMethod->getStmts();
+        if ($stmts === null) {
+            return [];
+        }
+
+        $defaultPropertyExprAssigns = [];
+
+        foreach ($stmts as $stmt) {
+            if (! $stmt instanceof Expression) {
+                continue;
+            }
+
+            $nestedStmt = $stmt->expr;
+            if (! $nestedStmt instanceof Assign) {
+                continue;
+            }
+
+            $assign = $nestedStmt;
+
+            if (! $assign->var instanceof PropertyFetch) {
+                continue;
+            }
+
+            $propertyFetch = $assign->var;
+            if (! $this->nodeNameResolver->isName($propertyFetch->var, 'this')) {
+                continue;
+            }
+
+            $propertyName = $this->nodeNameResolver->getName($propertyFetch->name);
+            if (! is_string($propertyName)) {
+                continue;
+            }
+
+            $expr = $assign->expr;
+            if ($this->exprAnalyzer->isDynamicExpr($expr)) {
+                continue;
+            }
+
+            $defaultPropertyExprAssigns[] = new DefaultPropertyExprAssign($stmt, $propertyName, $expr);
+        }
+
+        return $defaultPropertyExprAssigns;
+    }
+}

rules/CodeQuality/Rector/Class_/InlineConstructorDefaultToPropertyRector.php

@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\CodeQuality\Rector\Class_;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Property;
+use Rector\CodeQuality\NodeAnalyzer\ConstructorPropertyDefaultExprResolver;
+use Rector\Core\Rector\AbstractRector;
+use Rector\Core\ValueObject\MethodName;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\InlineConstructorDefaultToPropertyRectorTest
+ */
+final class InlineConstructorDefaultToPropertyRector extends AbstractRector
+{
+    public function __construct(
+        private readonly ConstructorPropertyDefaultExprResolver $constructorPropertyDefaultExprResolver
+    ) {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition('Move property default from constructor to property default', [
+            new CodeSample(
+                <<<'CODE_SAMPLE'
+final class SomeClass
+{
+    private $name;
+
+    public function __construct()
+    {
+        $this->name = 'John';
+    }
+}
+CODE_SAMPLE
+
+                ,
+                <<<'CODE_SAMPLE'
+final class SomeClass
+{
+    private $name = 'John';
+}
+CODE_SAMPLE
+            ),
+        ]);
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    /**
+     * @param Class_ $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        $constructClassMethod = $node->getMethod(MethodName::CONSTRUCT);
+        if (! $constructClassMethod instanceof ClassMethod) {
+            return null;
+        }
+
+        // resolve property defaults
+        $defaultPropertyExprAssigns = $this->constructorPropertyDefaultExprResolver->resolve($constructClassMethod);
+
+        if ($defaultPropertyExprAssigns === []) {
+            return null;
+        }
+
+        $hasChanged = false;
+        foreach ($defaultPropertyExprAssigns as $defaultPropertyExprAssign) {
+            $property = $node->getProperty($defaultPropertyExprAssign->getPropertyName());
+            if (! $property instanceof Property) {
+                continue;
+            }
+
+            $propertyProperty = $property->props[0];
+            $propertyProperty->default = $defaultPropertyExprAssign->getDefaultExpr();
+
+            $hasChanged = true;
+
+            $this->removeNode($defaultPropertyExprAssign->getAssignExpression());
+        }
+
+        if (! $hasChanged) {
+            return null;
+        }
+
+        return $node;
+    }
+}

rules/CodeQuality/ValueObject/DefaultPropertyExprAssign.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\CodeQuality\ValueObject;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Stmt\Expression;
+
+final class DefaultPropertyExprAssign
+{
+    public function __construct(
+        private readonly Expression $assignExpression,
+        private readonly string $propertyName,
+        private readonly Expr $defaultExpr
+    ) {
+    }
+
+    public function getAssignExpression(): Expression
+    {
+        return $this->assignExpression;
+    }
+
+    public function getPropertyName(): string
+    {
+        return $this->propertyName;
+    }
+
+    public function getDefaultExpr(): Expr
+    {
+        return $this->defaultExpr;
+    }
+}

rules/Php81/NodeAnalyzer/ComplexNewAnalyzer.php

@@ -40,7 +40,7 @@ final class ComplexNewAnalyzer
                 continue;
             }
 
-            if (! $this->exprAnalyzer->isDynamicValue($value)) {
+            if (! $this->exprAnalyzer->isDynamicExpr($value)) {
                 continue;
             }
 

src/NodeAnalyzer/ExprAnalyzer.php

@@ -64,29 +64,29 @@ final class ExprAnalyzer
         return false;
     }
 
-    public function isDynamicValue(Expr $expr): bool
+    public function isDynamicExpr(Expr $expr): bool
     {
         if (! $expr instanceof Array_) {
             if ($expr instanceof Scalar) {
                 return false;
             }
 
-            return ! $this->isAllowedConstFetchOrClassConstFeth($expr);
+            return ! $this->isAllowedConstFetchOrClassConstFetch($expr);
         }
 
         return $this->arrayManipulator->isDynamicArray($expr);
     }
 
-    private function isAllowedConstFetchOrClassConstFeth(Expr $expr): bool
+    private function isAllowedConstFetchOrClassConstFetch(Expr $expr): bool
     {
-        if (! in_array($expr::class, [ConstFetch::class, ClassConstFetch::class], true)) {
-            return false;
+        if ($expr instanceof ConstFetch) {
+            return true;
         }
 
         if ($expr instanceof ClassConstFetch) {
             return $expr->class instanceof Name && $expr->name instanceof Identifier;
         }
 
-        return true;
+        return false;
     }
 }

src/NodeManipulator/ArrayManipulator.php

@@ -121,6 +121,6 @@ final class ArrayManipulator
             return ! $this->isDynamicArray($expr);
         }
 
-        return ! $this->exprAnalyzer->isDynamicValue($expr);
+        return ! $this->exprAnalyzer->isDynamicExpr($expr);
     }
 }

src/NodeManipulator/VariableManipulator.php

@@ -63,7 +63,7 @@ final class VariableManipulator
                     return null;
                 }
 
-                if ($this->exprAnalyzer->isDynamicValue($node->expr)) {
+                if ($this->exprAnalyzer->isDynamicExpr($node->expr)) {
                     return null;
                 }