[TypedPropertyRector] Remove private property only, to keep other rules work separately (#1496)

* remove private property only, to keep other rules work separately

* allow in final protected

* allow in final protected

* allow changing only final/protected properties

* bump docs

* Update rules/Php74/Rector/Property/TypedPropertyRector.php

Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>

* cleanup complexity

* remove test fixture that can be overriden by parent

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
Co-authored-by: GitHub Action <action@github.com>

build/target-repository/docs/rector_rules_overview.md

@@ -1,4 +1,4 @@
-# 505 Rules Overview
+# 506 Rules Overview
 
 <br>
 
@@ -38,7 +38,7 @@
 
 - [DowngradePhp74](#downgradephp74) (12)
 
-- [DowngradePhp80](#downgradephp80) (23)
+- [DowngradePhp80](#downgradephp80) (24)
 
 - [DowngradePhp81](#downgradephp81) (8)
 
@@ -5411,6 +5411,25 @@ Remove reflection `getAttributes()` class method code
 
 <br>
 
+### DowngradeReflectionPropertyGetDefaultValueRector
+
+Downgrade `ReflectionProperty->getDefaultValue()`
+
+- class: [`Rector\DowngradePhp80\Rector\MethodCall\DowngradeReflectionPropertyGetDefaultValueRector`](../rules/DowngradePhp80/Rector/MethodCall/DowngradeReflectionPropertyGetDefaultValueRector.php)
+
+```diff
+ class SomeClass
+ {
+     public function run(ReflectionProperty $reflectionProperty)
+     {
+-        return $reflectionProperty->getDefaultValue();
++        return $reflectionProperty->getDeclaringClass()->getDefaultProperties()[$reflectionProperty->getName()] ?? null;
+     }
+ }
+```
+
+<br>
+
 ### DowngradeStaticTypeDeclarationRector
 
 Remove "static" return and param type, add a `"@param` `$this"` and `"@return` `$this"` tag instead
@@ -7747,26 +7766,8 @@ Add null default to properties with PHP 7.4 property nullable type
 
 Changes property `@var` annotations from annotation to type.
 
-:wrench: **configure it!**
-
 - class: [`Rector\Php74\Rector\Property\TypedPropertyRector`](../rules/Php74/Rector/Property/TypedPropertyRector.php)
 
-```php
-use Rector\Php74\Rector\Property\TypedPropertyRector;
-use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
-
-return static function (ContainerConfigurator $containerConfigurator): void {
-    $services = $containerConfigurator->services();
-
-    $services->set(TypedPropertyRector::class)
-        ->configure([
-            TypedPropertyRector::PRIVATE_PROPERTY_ONLY => false,
-        ]);
-};
-```
-
-↓
-
 ```diff
  final class SomeClass
  {
@@ -8247,10 +8248,10 @@ Change docs to intersection types, where possible (properties are covered by Typ
  final class SomeClass
  {
 -    /**
--     * @param string&int $types
+-     * @param Foo&Bar $types
 -     */
 -    public function process($types)
-+    public function process(string&int $types)
++    public function process(Foo&Bar $types)
      {
      }
  }

phpstan.neon

@@ -612,4 +612,4 @@ parameters:
         # will be desolved
         -
             path: rules/Php74/Rector/Property/TypedPropertyRector.php
-            message: '#Class cognitive complexity is 31, keep it under 30#'
+            message: '#Class cognitive complexity is \d+, keep it under 30#'

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/assert_choice.php.inc

@@ -4,13 +4,13 @@ namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
 
 use Symfony\Component\Validator\Constraints as Assert;
 
-class AssertChoice
+final class AssertChoice
 {
     /**
      * @var string
      * @Assert\Choice({"chalet", "apartment"})
      */
-    public $type;
+    protected $type;
 }
 
 ?>
@@ -21,12 +21,12 @@ namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
 
 use Symfony\Component\Validator\Constraints as Assert;
 
-class AssertChoice
+final class AssertChoice
 {
     /**
      * @Assert\Choice({"chalet", "apartment"})
      */
-    public string $type;
+    protected string $type;
 }
 
 ?>

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/child_class_extends_typed_base_class.php.inc

@@ -1,26 +0,0 @@
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeParent;
-
-class ChildClass extends SomeParent
-{
-    /**
-     * @var bool
-     */
-    protected $anAlreadyReplacedPropertyInParentClass = true;
-}
-?>
------
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeParent;
-
-class ChildClass extends SomeParent
-{
-    protected bool $anAlreadyReplacedPropertyInParentClass = true;
-}
-?>

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/child_class_has_another_property.php.inc

@@ -1,43 +0,0 @@
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeChildOfSomeParent;
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeParent;
-
-class ChildClassHasAnotherProperty
-{
-    /**
-     * @var SomeChildOfSomeParent
-     */
-    public $someChildOfSomeParent;
-}
-
-final class ChildClass extends ChildClassHasAnotherProperty
-{
-    /**
-     * @var SomeParent
-     */
-    public $someParent;
-}
-
-?>
------
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeChildOfSomeParent;
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeParent;
-
-class ChildClassHasAnotherProperty
-{
-    public \Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeChildOfSomeParent $someChildOfSomeParent;
-}
-
-final class ChildClass extends ChildClassHasAnotherProperty
-{
-    public \Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeParent $someParent;
-}
-
-?>

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/filled_by_parent_class.php.inc

@@ -1,30 +0,0 @@
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\AnotherClass;
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\FillerAbstract;
-
-class FilledByParentClass extends FillerAbstract
-{
-    /**
-     * @var AnotherClass
-     */
-    protected $property;
-}
-
-?>
------
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\AnotherClass;
-use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\FillerAbstract;
-
-class FilledByParentClass extends FillerAbstract
-{
-    protected ?\Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\AnotherClass $property = null;
-}
-
-?>

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/parent_class.php.inc

@@ -1,35 +0,0 @@
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-class ParentClass
-{
-    /**
-     * @var bool
-     */
-    protected $active = false;
-}
-
-class ChildClass extends ParentClass
-{
-    /**
-     * @var bool
-     */
-    protected $active = true;
-}
-?>
------
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
-
-class ParentClass
-{
-    protected bool $active = false;
-}
-
-class ChildClass extends ParentClass
-{
-    protected bool $active = true;
-}
-?>

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/skip_protected_property_with_parent.php.inc

@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
+
+use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\AbstractParentClass;
+use Symfony\Component\Validator\Constraints as Assert;
+
+final class SkipProtectedPropertyWithParent extends AbstractParentClass
+{
+    /**
+     * @var string
+     * @Assert\Choice({"chalet", "apartment"})
+     */
+    protected $type;
+}

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/skip_public_property.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
+
+use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeChildOfSomeParent;
+use Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source\SomeParent;
+
+final class SkipPublicProperty
+{
+    /**
+     * @var SomeChildOfSomeParent
+     */
+    public $someChildOfSomeParent;
+}

rules-tests/Php74/Rector/Property/TypedPropertyRector/Fixture/skip_unioned_type.php.inc

@@ -2,12 +2,12 @@
 
 namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Fixture;
 
-class SkipUnionedType
+final class SkipUnionedType
 {
     /**
      * @var bool|int
      */
-    public $cantTouchThis = true;
+    protected $cantTouchThis = true;
 
     public function setNumber()
     {

rules-tests/Php74/Rector/Property/TypedPropertyRector/FixturePrivatePropertyOnly/fixture.php.inc

@@ -1,50 +0,0 @@
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\FixturePrivatePropertyOnly;
-
-class Fixture
-{
-    /**
-     * @var bool
-     */
-    private $privateProperty;
-
-    /**
-     * @var bool
-     */
-    protected $protectedProperty;
-}
-
-class ExtendsFixture extends Fixture
-{
-    /**
-     * @var bool
-     */
-    protected $protectedProperty;
-}
-
-?>
------
-<?php
-
-namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\FixturePrivatePropertyOnly;
-
-class Fixture
-{
-    private bool $privateProperty;
-
-    /**
-     * @var bool
-     */
-    protected $protectedProperty;
-}
-
-class ExtendsFixture extends Fixture
-{
-    /**
-     * @var bool
-     */
-    protected $protectedProperty;
-}
-
-?>

rules-tests/Php74/Rector/Property/TypedPropertyRector/FixtureUnionIntersectionTypes/include_unioned_type.php.inc

@@ -2,12 +2,12 @@
 
 namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\FixtureUnionIntersectionTypes;
 
-class IncludeUnionedType
+final class IncludeUnionedType
 {
     /**
      * @var bool|int
      */
-    public $cantTouchThis = true;
+    private $cantTouchThis = true;
 
     public function setNumber()
     {
@@ -29,9 +29,9 @@ class IncludeUnionedType
 
 namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\FixtureUnionIntersectionTypes;
 
-class IncludeUnionedType
+final class IncludeUnionedType
 {
-    public bool|int $cantTouchThis = true;
+    private bool|int $cantTouchThis = true;
 
     public function setNumber()
     {

rules-tests/Php74/Rector/Property/TypedPropertyRector/FixtureUnionIntersectionTypes/set_if_else.php.inc

@@ -2,9 +2,9 @@
 
 namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\FixtureUnionIntersectionTypes;
 
-class SetIfElse
+final class SetIfElse
 {
-    public $stringOrInteger = 'hi';
+    protected $stringOrInteger = 'hi';
 
     public function setNumber()
     {
@@ -22,9 +22,9 @@ class SetIfElse
 
 namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\FixtureUnionIntersectionTypes;
 
-class SetIfElse
+final class SetIfElse
 {
-    public int|string $stringOrInteger = 'hi';
+    protected int|string $stringOrInteger = 'hi';
 
     public function setNumber()
     {

rules-tests/Php74/Rector/Property/TypedPropertyRector/PrivatePropertyOnlyTest.php

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

rules-tests/Php74/Rector/Property/TypedPropertyRector/Source/AbstractParentClass.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Php74\Rector\Property\TypedPropertyRector\Source;
+
+abstract class AbstractParentClass
+{
+
+}

rules-tests/Php74/Rector/Property/TypedPropertyRector/config/private_property_only.php

@@ -1,18 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-use Rector\Core\Configuration\Option;
-use Rector\Php74\Rector\Property\TypedPropertyRector;
-use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
-
-return static function (ContainerConfigurator $containerConfigurator): void {
-    $parameters = $containerConfigurator->parameters();
-    $parameters->set(Option::AUTO_IMPORT_NAMES, true);
-
-    $services = $containerConfigurator->services();
-    $services->set(TypedPropertyRector::class)
-        ->configure([
-            TypedPropertyRector::PRIVATE_PROPERTY_ONLY => true,
-        ]);
-};

rules/Php74/Rector/Property/TypedPropertyRector.php

@@ -11,12 +11,13 @@ use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\Property;
 use PhpParser\Node\Stmt\Trait_;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\Generic\TemplateType;
 use PHPStan\Type\MixedType;
 use PHPStan\Type\NullType;
 use PHPStan\Type\Type;
 use PHPStan\Type\UnionType;
-use Rector\Core\Contract\Rector\AllowEmptyConfigurableRectorInterface;
 use Rector\Core\NodeAnalyzer\PropertyAnalyzer;
 use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
 use Rector\Core\PhpParser\AstResolver;
@@ -31,7 +32,7 @@ use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
 use Rector\TypeDeclaration\TypeInferer\PropertyTypeInferer;
 use Rector\VendorLocker\VendorLockResolver;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
-use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 /**
@@ -42,19 +43,8 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
  * @see \Rector\Tests\Php74\Rector\Property\TypedPropertyRector\DoctrineTypedPropertyRectorTest
  * @see \Rector\Tests\Php74\Rector\Property\TypedPropertyRector\ImportedTest
  */
-final class TypedPropertyRector extends AbstractRector implements AllowEmptyConfigurableRectorInterface, MinPhpVersionInterface
+final class TypedPropertyRector extends AbstractRector implements MinPhpVersionInterface
 {
-    /**
-     * @var string
-     */
-    final public const PRIVATE_PROPERTY_ONLY = 'PRIVATE_PROPERTY_ONLY';
-
-    /**
-     * If want to keep BC, it can be set to true
-     * @see https://3v4l.org/spl4P
-     */
-    private bool $privatePropertyOnly = false;
-
     public function __construct(
         private readonly PropertyTypeInferer $propertyTypeInferer,
         private readonly VendorLockResolver $vendorLockResolver,
@@ -73,7 +63,7 @@ final class TypedPropertyRector extends AbstractRector implements AllowEmptyConf
         return new RuleDefinition(
             'Changes property `@var` annotations from annotation to type.',
             [
-                new ConfiguredCodeSample(
+                new CodeSample(
                     <<<'CODE_SAMPLE'
 final class SomeClass
 {
@@ -90,10 +80,6 @@ final class SomeClass
     private int $count;
 }
 CODE_SAMPLE
-                    ,
-                    [
-                        self::PRIVATE_PROPERTY_ONLY => false,
-                    ]
                 ),
             ]
         );
@@ -112,7 +98,8 @@ CODE_SAMPLE
      */
     public function refactor(Node $node): ?Node
     {
-        if ($this->shouldSkipProperty($node)) {
+        $scope = $node->getAttribute(AttributeKey::SCOPE);
+        if ($this->shouldSkipProperty($node, $scope)) {
             return null;
         }
 
@@ -167,14 +154,6 @@ CODE_SAMPLE
         return $node;
     }
 
-    /**
-     * @param mixed[] $configuration
-     */
-    public function configure(array $configuration): void
-    {
-        $this->privatePropertyOnly = $configuration[self::PRIVATE_PROPERTY_ONLY] ?? false;
-    }
-
     public function provideMinPhpVersion(): int
     {
         return PhpVersionFeature::TYPED_PROPERTIES;
@@ -234,7 +213,7 @@ CODE_SAMPLE
         $onlyProperty->default = $this->nodeFactory->createNull();
     }
 
-    private function shouldSkipProperty(Property $property): bool
+    private function shouldSkipProperty(Property $property, Scope $scope): bool
     {
         // type is already set → skip
         if ($property->type !== null) {
@@ -246,8 +225,13 @@ CODE_SAMPLE
             return true;
         }
 
-        $trait = $this->betterNodeFinder->findParentType($property, Trait_::class);
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return true;
+        }
+
         // skip trait properties, as they ar unpredictable based on class context they appear in
+        $trait = $this->betterNodeFinder->findParentType($property, Trait_::class);
         if ($trait instanceof Trait_) {
             return true;
         }
@@ -259,15 +243,12 @@ CODE_SAMPLE
             return true;
         }
 
-        if (! $this->privatePropertyOnly) {
-            return $this->propertyAnalyzer->hasForbiddenType($property);
-        }
-
         if ($property->isPrivate()) {
             return $this->propertyAnalyzer->hasForbiddenType($property);
         }
 
-        return true;
+        // is we're in final class, the type can be changed
+        return ! ($property->isProtected() && $classReflection->isFinal() && $classReflection->getParents() === []);
     }
 
     private function isModifiedByTrait(ClassLike $classLike, string $propertyName): bool