From 84bcfebdf7e9f522c8f9f9e97814c82d3d5d3315 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 28 Nov 2023 21:17:42 +0100 Subject: [PATCH] [CodingStyle] Remove AddArrayDefaultToArrayPropertyRector as based on docblock types and public contract, better use type declaration set instad (#5298) --- .../docs/rector_rules_overview.md | 29 +- config/set/coding-style.php | 2 - ...dArrayDefaultToArrayPropertyRectorTest.php | 28 -- .../Fixture/array_and_type.php.inc | 27 -- .../Fixture/count_on_null.php.inc | 47 --- .../Fixture/fixture.php.inc | 37 --- .../Fixture/scalar_integer.php.inc | 39 --- .../Fixture/skip.php.inc | 31 -- .../Fixture/skip_filled_by_construct.php.inc | 15 - .../Fixture/skip_in_readonly_class.php.inc | 10 - .../Fixture/skip_nullable_array.php.inc | 19 -- .../Fixture/skip_readonly_property.php.inc | 19 -- .../Fixture/static_property.php.inc | 29 -- .../Fixture/two_types.php.inc | 27 -- .../config/configured_rule.php | 10 - .../AddArrayDefaultToArrayPropertyRector.php | 285 ------------------ .../TypeAnalyzer/IterableTypeAnalyzer.php | 35 --- .../NodeManipulator/VisibilityManipulator.php | 3 + src/NodeAnalyzer/PropertyFetchAnalyzer.php | 14 - .../LongAndDependentComplexRectorRule.php | 2 +- 20 files changed, 6 insertions(+), 702 deletions(-) delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/AddArrayDefaultToArrayPropertyRectorTest.php delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/array_and_type.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/count_on_null.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/fixture.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/scalar_integer.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_filled_by_construct.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_in_readonly_class.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_nullable_array.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_readonly_property.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/static_property.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/two_types.php.inc delete mode 100644 rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/config/configured_rule.php delete mode 100644 rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php delete mode 100644 rules/CodingStyle/TypeAnalyzer/IterableTypeAnalyzer.php diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 926261dd876..e986579ebec 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 352 Rules Overview +# 351 Rules Overview
@@ -8,7 +8,7 @@ - [CodeQuality](#codequality) (72) -- [CodingStyle](#codingstyle) (28) +- [CodingStyle](#codingstyle) (27) - [DeadCode](#deadcode) (42) @@ -1572,31 +1572,6 @@ Use ===/!== over ==/!=, it values have the same type ## CodingStyle -### AddArrayDefaultToArrayPropertyRector - -Adds array default value to property to prevent foreach over null error - -- class: [`Rector\CodingStyle\Rector\Class_\AddArrayDefaultToArrayPropertyRector`](../rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php) - -```diff - class SomeClass - { - /** - * @var int[] - */ -- private $values; -+ private $values = []; - - public function isEmpty() - { -- return $this->values === null; -+ return $this->values === []; - } - } -``` - -
- ### ArraySpreadInsteadOfArrayMergeRector Change `array_merge()` to spread operator diff --git a/config/set/coding-style.php b/config/set/coding-style.php index dcb2a109ecf..e4f3014a23f 100644 --- a/config/set/coding-style.php +++ b/config/set/coding-style.php @@ -5,7 +5,6 @@ use Rector\CodingStyle\Rector\ArrowFunction\StaticArrowFunctionRector; use Rector\CodingStyle\Rector\Assign\SplitDoubleAssignRector; use Rector\CodingStyle\Rector\Catch_\CatchExceptionNameMatchingTypeRector; -use Rector\CodingStyle\Rector\Class_\AddArrayDefaultToArrayPropertyRector; use Rector\CodingStyle\Rector\ClassConst\RemoveFinalFromConstRector; use Rector\CodingStyle\Rector\ClassConst\SplitGroupedClassConstantsRector; use Rector\CodingStyle\Rector\ClassMethod\FuncGetArgsToVariadicParamRector; @@ -58,7 +57,6 @@ EncapsedStringsToSprintfRector::class, WrapEncapsedVariableInCurlyBracesRector::class, NewlineBeforeNewAssignSetRector::class, - AddArrayDefaultToArrayPropertyRector::class, MakeInheritedMethodVisibilitySameAsParentRector::class, CallUserFuncArrayToVariadicRector::class, VersionCompareFuncCallToConstantRector::class, diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/AddArrayDefaultToArrayPropertyRectorTest.php b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/AddArrayDefaultToArrayPropertyRectorTest.php deleted file mode 100644 index 57ab993a82b..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/AddArrayDefaultToArrayPropertyRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/array_and_type.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/array_and_type.php.inc deleted file mode 100644 index fc75fb74974..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/array_and_type.php.inc +++ /dev/null @@ -1,27 +0,0 @@ - ------ - diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/count_on_null.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/count_on_null.php.inc deleted file mode 100644 index 61ce721f76b..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/count_on_null.php.inc +++ /dev/null @@ -1,47 +0,0 @@ -values !== null && count($this->values) > 0; - } - - public function isEmptyOtherSide() - { - return null !== $this->values && count($this->values); - } -} - -?> ------ -values) > 0; - } - - public function isEmptyOtherSide() - { - return count($this->values); - } -} - -?> diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/fixture.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/fixture.php.inc deleted file mode 100644 index a04a4c3919c..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,37 +0,0 @@ -values === null; - } -} - -?> ------ -values === []; - } -} - -?> diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/scalar_integer.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/scalar_integer.php.inc deleted file mode 100644 index 3ab8513c29f..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/scalar_integer.php.inc +++ /dev/null @@ -1,39 +0,0 @@ - ------ - diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip.php.inc deleted file mode 100644 index c4fd89ecb67..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip.php.inc +++ /dev/null @@ -1,31 +0,0 @@ -arr = $arr; - } -} - diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_in_readonly_class.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_in_readonly_class.php.inc deleted file mode 100644 index e6fbcf1ec14..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_in_readonly_class.php.inc +++ /dev/null @@ -1,10 +0,0 @@ -values === null; - } -} diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_readonly_property.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_readonly_property.php.inc deleted file mode 100644 index 0b66adcae0b..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/skip_readonly_property.php.inc +++ /dev/null @@ -1,19 +0,0 @@ -changes = $model->getChanges(); - } -} - -?> - diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/static_property.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/static_property.php.inc deleted file mode 100644 index 24a1c055c77..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/static_property.php.inc +++ /dev/null @@ -1,29 +0,0 @@ - ------ - diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/two_types.php.inc b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/two_types.php.inc deleted file mode 100644 index 6ec35ab5978..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/Fixture/two_types.php.inc +++ /dev/null @@ -1,27 +0,0 @@ - ------ - diff --git a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/config/configured_rule.php b/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/config/configured_rule.php deleted file mode 100644 index 4d3b26219e9..00000000000 --- a/rules-tests/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector/config/configured_rule.php +++ /dev/null @@ -1,10 +0,0 @@ -rule(AddArrayDefaultToArrayPropertyRector::class); -}; diff --git a/rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php b/rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php deleted file mode 100644 index c257789da18..00000000000 --- a/rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php +++ /dev/null @@ -1,285 +0,0 @@ -values === null; - } -} -CODE_SAMPLE - , - <<<'CODE_SAMPLE' -class SomeClass -{ - /** - * @var int[] - */ - private $values = []; - - public function isEmpty() - { - return $this->values === []; - } -} -CODE_SAMPLE - ), - ] - ); - } - - /** - * @return array> - */ - public function getNodeTypes(): array - { - return [Class_::class]; - } - - /** - * @param Class_ $node - */ - public function refactor(Node $node): ?Node - { - if ($node->isReadonly()) { - return null; - } - - $changedProperties = $this->collectPropertyNamesWithMissingDefaultArray($node); - if ($changedProperties === []) { - return null; - } - - $this->completeDefaultArrayToPropertyNames($node, $changedProperties); - - // $this->variable !== null && count($this->variable) > 0 → count($this->variable) > 0 - $this->clearNotNullBeforeCount($node, $changedProperties); - - // $this->variable === null → $this->variable === [] - $this->replaceNullComparisonOfArrayPropertiesWithArrayComparison($node, $changedProperties); - - return $node; - } - - /** - * @return string[] - */ - private function collectPropertyNamesWithMissingDefaultArray(Class_ $class): array - { - $propertyNames = []; - $this->traverseNodesWithCallable($class, function (Node $node) use ($class, &$propertyNames) { - if (! $node instanceof Property) { - return null; - } - - foreach ($node->props as $propertyProperty) { - if ($propertyProperty->default instanceof Expr) { - continue; - } - - $varType = $this->resolveVarType($node); - if (! $this->iterableTypeAnalyzer->detect($varType)) { - continue; - } - - if ($this->visibilityManipulator->isReadonly($node)) { - return null; - } - - $propertyName = $this->getName($propertyProperty); - if ($this->constructorAssignDetector->isPropertyAssigned($class, $propertyName)) { - return null; - } - - $propertyNames[] = $propertyName; - } - - return null; - }); - - return $propertyNames; - } - - /** - * @param string[] $propertyNames - */ - private function completeDefaultArrayToPropertyNames(Class_ $class, array $propertyNames): void - { - $this->traverseNodesWithCallable($class, function (Node $node) use ($propertyNames): ?PropertyProperty { - if (! $node instanceof PropertyProperty) { - return null; - } - - if (! $this->isNames($node, $propertyNames)) { - return null; - } - - $node->default = new Array_(); - - return $node; - }); - } - - /** - * @param string[] $propertyNames - */ - private function clearNotNullBeforeCount(Class_ $class, array $propertyNames): void - { - $this->traverseNodesWithCallable($class, function (Node $node) use ($propertyNames): ?Expr { - if (! $node instanceof BooleanAnd) { - return null; - } - - if (! $this->isLocalPropertyOfNamesNotIdenticalToNull($node->left, $propertyNames)) { - return null; - } - - $isNextNodeCountingProperty = (bool) $this->betterNodeFinder->findFirst($node->right, function (Node $node) use ( - $propertyNames - ): bool { - if (! $node instanceof FuncCall) { - return false; - } - - if ($node->isFirstClassCallable()) { - return false; - } - - if (! $this->isName($node, 'count')) { - return false; - } - - $countedArgument = $node->getArgs()[0] -->value; - if (! $countedArgument instanceof PropertyFetch) { - return false; - } - - return $this->isNames($countedArgument, $propertyNames); - }); - - if (! $isNextNodeCountingProperty) { - return null; - } - - return $node->right; - }); - } - - /** - * @param string[] $propertyNames - */ - private function replaceNullComparisonOfArrayPropertiesWithArrayComparison( - Class_ $class, - array $propertyNames - ): void { - // replace comparison to "null" with "[]" - $this->traverseNodesWithCallable($class, function (Node $node) use ($propertyNames): ?BinaryOp { - if (! $node instanceof BinaryOp) { - return null; - } - - if ($this->propertyFetchAnalyzer->isLocalPropertyOfNames( - $node->left, - $propertyNames - ) && $this->valueResolver->isNull($node->right)) { - $node->right = new Array_(); - } - - if ($this->propertyFetchAnalyzer->isLocalPropertyOfNames( - $node->right, - $propertyNames - ) && $this->valueResolver->isNull($node->left)) { - $node->left = new Array_(); - } - - return $node; - }); - } - - private function resolveVarType(Property $property): Type - { - $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($property); - - return $phpDocInfo->getVarType(); - } - - /** - * @param string[] $propertyNames - */ - private function isLocalPropertyOfNamesNotIdenticalToNull(Expr $expr, array $propertyNames): bool - { - if (! $expr instanceof NotIdentical) { - return false; - } - - if ($this->propertyFetchAnalyzer->isLocalPropertyOfNames( - $expr->left, - $propertyNames - ) && $this->valueResolver->isNull($expr->right)) { - return true; - } - - if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($expr->right, $propertyNames)) { - return false; - } - - return $this->valueResolver->isNull($expr->left); - } -} diff --git a/rules/CodingStyle/TypeAnalyzer/IterableTypeAnalyzer.php b/rules/CodingStyle/TypeAnalyzer/IterableTypeAnalyzer.php deleted file mode 100644 index f2e0aafb3db..00000000000 --- a/rules/CodingStyle/TypeAnalyzer/IterableTypeAnalyzer.php +++ /dev/null @@ -1,35 +0,0 @@ -isArray()->yes()) { - return true; - } - - if ($type instanceof UnionType) { - foreach ($type->getTypes() as $unionedType) { - if (! $this->detect($unionedType)) { - return false; - } - } - - return true; - } - - return false; - } -} diff --git a/rules/Privatization/NodeManipulator/VisibilityManipulator.php b/rules/Privatization/NodeManipulator/VisibilityManipulator.php index 2de7040a4e1..9a9d0d3f868 100644 --- a/rules/Privatization/NodeManipulator/VisibilityManipulator.php +++ b/rules/Privatization/NodeManipulator/VisibilityManipulator.php @@ -116,6 +116,9 @@ public function makeReadonly(Class_ | Property | Param $node): void $this->addVisibilityFlag($node, Visibility::READONLY); } + /** + * @api + */ public function isReadonly(Class_ | Property | Param $node): bool { return $this->hasVisibility($node, Visibility::READONLY); diff --git a/src/NodeAnalyzer/PropertyFetchAnalyzer.php b/src/NodeAnalyzer/PropertyFetchAnalyzer.php index 0c81412fdf7..c0702925279 100644 --- a/src/NodeAnalyzer/PropertyFetchAnalyzer.php +++ b/src/NodeAnalyzer/PropertyFetchAnalyzer.php @@ -5,7 +5,6 @@ namespace Rector\Core\NodeAnalyzer; use PhpParser\Node; -use PhpParser\Node\Expr; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; @@ -174,19 +173,6 @@ public function isFilledViaMethodCallInConstructStmts(ClassLike $classLike, stri return false; } - /** - * @param string[] $propertyNames - */ - public function isLocalPropertyOfNames(Expr $expr, array $propertyNames): bool - { - if (! $this->isLocalPropertyFetch($expr)) { - return false; - } - - /** @var PropertyFetch $expr */ - return $this->nodeNameResolver->isNames($expr->name, $propertyNames); - } - private function isTraitLocalPropertyFetch(Node $node): bool { if ($node instanceof PropertyFetch) { diff --git a/utils/PHPStan/Rule/LongAndDependentComplexRectorRule.php b/utils/PHPStan/Rule/LongAndDependentComplexRectorRule.php index c636a92379c..74264c97966 100644 --- a/utils/PHPStan/Rule/LongAndDependentComplexRectorRule.php +++ b/utils/PHPStan/Rule/LongAndDependentComplexRectorRule.php @@ -28,7 +28,7 @@ final class LongAndDependentComplexRectorRule implements Rule /** * @var int */ - private const ALLOWED_TRANSITIONAL_COMPLEXITY = 110; + private const ALLOWED_TRANSITIONAL_COMPLEXITY = 140; private readonly Parser $phpParser;