From 4373952652c3d206da86f5bedeedbefa2230ac93 Mon Sep 17 00:00:00 2001 From: Can Vural Date: Tue, 14 Dec 2021 15:11:29 +0100 Subject: [PATCH 1/4] Add new ArrayUnpackingRule --- conf/bleedingEdge.neon | 1 + conf/config.level3.neon | 7 ++ conf/config.neon | 2 + src/Rules/Arrays/ArrayUnpackingRule.php | 43 ++++++++++++ .../Rules/Arrays/ArrayUnpackingRuleTest.php | 68 +++++++++++++++++++ .../Rules/Arrays/data/array-unpacking.php | 66 ++++++++++++++++++ 6 files changed, 187 insertions(+) create mode 100644 src/Rules/Arrays/ArrayUnpackingRule.php create mode 100644 tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php create mode 100644 tests/PHPStan/Rules/Arrays/data/array-unpacking.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index cde18d2a3..d0bd30b47 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -4,5 +4,6 @@ parameters: skipCheckGenericClasses: [] explicitMixedInUnknownGenericNew: true arrayFilter: true + arrayUnpacking: true stubFiles: - ../stubs/bleedingEdge/Countable.stub diff --git a/conf/config.level3.neon b/conf/config.level3.neon index 51450cdde..d0f8e774e 100644 --- a/conf/config.level3.neon +++ b/conf/config.level3.neon @@ -1,6 +1,10 @@ includes: - config.level2.neon +conditionalTags: + PHPStan\Rules\Arrays\ArrayUnpackingRule: + phpstan.rules.rule: %featureToggles.arrayUnpacking% + rules: - PHPStan\Rules\Arrays\ArrayDestructuringRule - PHPStan\Rules\Arrays\IterableInForeachRule @@ -74,3 +78,6 @@ services: reportMaybes: %reportMaybes% tags: - phpstan.rules.rule + + - + class: PHPStan\Rules\Arrays\ArrayUnpackingRule diff --git a/conf/config.neon b/conf/config.neon index 2ce812ccf..bbafd8485 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -29,6 +29,7 @@ parameters: - RecursiveCallbackFilterIterator explicitMixedInUnknownGenericNew: false arrayFilter: false + arrayUnpacking: false fileExtensions: - php checkAdvancedIsset: false @@ -207,6 +208,7 @@ parametersSchema: skipCheckGenericClasses: listOf(string()), explicitMixedInUnknownGenericNew: bool(), arrayFilter: bool(), + arrayUnpacking: bool(), ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Arrays/ArrayUnpackingRule.php b/src/Rules/Arrays/ArrayUnpackingRule.php new file mode 100644 index 000000000..8e8577b32 --- /dev/null +++ b/src/Rules/Arrays/ArrayUnpackingRule.php @@ -0,0 +1,43 @@ + + */ +class ArrayUnpackingRule implements Rule +{ + + public function __construct(private PhpVersion $phpVersion) + { + } + + public function getNodeType(): string + { + return ArrayItem::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if ($node->unpack === false || $this->phpVersion->supportsArrayUnpackingWithStringKeys()) { + return []; + } + + $valueType = $scope->getType($node->value); + + if ((new StringType())->isSuperTypeOf($valueType->getIterableKeyType())->no()) { + return []; + } + + return [RuleErrorBuilder::message('Array unpacking cannot be used on array that potentially has string keys.')->build()]; + } + +} diff --git a/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php b/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php new file mode 100644 index 000000000..5d095037f --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php @@ -0,0 +1,68 @@ + + */ +class ArrayUnpackingRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new ArrayUnpackingRule(self::getContainer()->getByType(PhpVersion::class)); + } + + public function testRule(): void + { + if (PHP_VERSION_ID >= 80100) { + $this->markTestSkipped('Test requires PHP version <= 8.0'); + } + + $this->analyse([__DIR__ . '/data/array-unpacking.php'], [ + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 7, + ], + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 18, + ], + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 24, + ], + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 29, + ], + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 40, + ], + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 52, + ], + [ + 'Array unpacking cannot be used on array that potentially has string keys.', + 63, + ], + ]); + } + + public function testRuleOnPHP81(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1+'); + } + + $this->analyse([__DIR__ . '/data/array-unpacking.php'], []); + } + +} diff --git a/tests/PHPStan/Rules/Arrays/data/array-unpacking.php b/tests/PHPStan/Rules/Arrays/data/array-unpacking.php new file mode 100644 index 000000000..7e1afcf9d --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/array-unpacking.php @@ -0,0 +1,66 @@ += 7.4 + +namespace ArrayUnpacking; + +$foo = ['foo' => 'bar', 1, 2, 3]; + +$bar = [...$foo]; + +/** @param array $bar */ +function intKeyedArray(array $bar) +{ + $baz = [...$bar]; +} + +/** @param array $bar */ +function stringKeyedArray(array $bar) +{ + $baz = [...$bar]; +} + +/** @param array $bar */ +function unionKeyedArray(array $bar) +{ + $baz = [...$bar]; +} + +function mixedKeyedArray(array $bar) +{ + $baz = [...$bar]; +} + +/** + * @param array $foo + * @param array $bar + */ +function multipleUnpacking(array $foo, array $bar) +{ + $baz = [ + ...$bar, + ...$foo, + ]; +} + +/** + * @param array $foo + * @param array $bar + */ +function foo(array $foo, array $bar) +{ + $baz = [ + $bar, + ...$foo + ]; +} + +/** + * @param array{foo: string, bar:int} $foo + * @param array{1, 2, 3, 4} $bar + */ +function unpackingArrayShapes(array $foo, array $bar) +{ + $baz = [ + ...$foo, + ...$bar, + ]; +} From 19215687e72c376aeac7781a4a542ed545779047 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 28 Mar 2022 18:03:37 +0200 Subject: [PATCH 2/4] Adjust message --- src/Rules/Arrays/ArrayUnpackingRule.php | 12 ++++++++++-- .../Rules/Arrays/ArrayUnpackingRuleTest.php | 14 +++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/Rules/Arrays/ArrayUnpackingRule.php b/src/Rules/Arrays/ArrayUnpackingRule.php index 8e8577b32..701214b87 100644 --- a/src/Rules/Arrays/ArrayUnpackingRule.php +++ b/src/Rules/Arrays/ArrayUnpackingRule.php @@ -9,6 +9,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\StringType; +use PHPStan\Type\VerbosityLevel; /** * @implements Rule @@ -32,12 +33,19 @@ public function processNode(Node $node, Scope $scope): array } $valueType = $scope->getType($node->value); + $isString = (new StringType())->isSuperTypeOf($valueType->getIterableKeyType()); - if ((new StringType())->isSuperTypeOf($valueType->getIterableKeyType())->no()) { + if ($isString->no()) { return []; } - return [RuleErrorBuilder::message('Array unpacking cannot be used on array that potentially has string keys.')->build()]; + return [ + RuleErrorBuilder::message(sprintf( + 'Array unpacking cannot be used on an array with %sstring keys: %s', + $isString->yes() ? '' : 'potential ', + $valueType->describe(VerbosityLevel::value()), + ))->build() + ]; } } diff --git a/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php b/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php index 5d095037f..8af2da6c3 100644 --- a/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php @@ -26,31 +26,31 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/array-unpacking.php'], [ [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with potential string keys: array{foo: \'bar\', 0: 1, 1: 2, 2: 3}', 7, ], [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with string keys: array', 18, ], [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with potential string keys: array', 24, ], [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with potential string keys: array', 29, ], [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with potential string keys: array', 40, ], [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with potential string keys: array', 52, ], [ - 'Array unpacking cannot be used on array that potentially has string keys.', + 'Array unpacking cannot be used on an array with string keys: array{foo: string, bar: int}', 63, ], ]); From 003aba51a60319205234a4593c570a5ae8554d46 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 28 Mar 2022 18:09:40 +0200 Subject: [PATCH 3/4] Employ RuleLevelHelper --- src/Analyser/MutatingScope.php | 4 +++ src/Node/Expr/GetIterableKeyTypeExpr.php | 34 +++++++++++++++++++ src/Rules/Arrays/ArrayUnpackingRule.php | 23 ++++++++++--- .../Rules/Arrays/ArrayUnpackingRuleTest.php | 28 ++++++++++++++- 4 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 src/Node/Expr/GetIterableKeyTypeExpr.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 7695725ed..887cc838b 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -31,6 +31,7 @@ use PhpParser\NodeFinder; use PhpParser\PrettyPrinter\Standard; use PHPStan\Node\ExecutionEndNode; +use PHPStan\Node\Expr\GetIterableKeyTypeExpr; use PHPStan\Node\Expr\GetIterableValueTypeExpr; use PHPStan\Node\Expr\GetOffsetValueTypeExpr; use PHPStan\Node\Expr\OriginalPropertyTypeExpr; @@ -544,6 +545,9 @@ public function getAnonymousFunctionReturnType(): ?Type /** @api */ public function getType(Expr $node): Type { + if ($node instanceof GetIterableKeyTypeExpr) { + return $this->getType($node->getExpr())->getIterableKeyType(); + } if ($node instanceof GetIterableValueTypeExpr) { return $this->getType($node->getExpr())->getIterableValueType(); } diff --git a/src/Node/Expr/GetIterableKeyTypeExpr.php b/src/Node/Expr/GetIterableKeyTypeExpr.php new file mode 100644 index 000000000..3f0f9f709 --- /dev/null +++ b/src/Node/Expr/GetIterableKeyTypeExpr.php @@ -0,0 +1,34 @@ +getAttributes()); + } + + public function getExpr(): Expr + { + return $this->expr; + } + + public function getType(): string + { + return 'PHPStan_Node_GetIterableKeyTypeExpr'; + } + + /** + * @return string[] + */ + public function getSubNodeNames(): array + { + return []; + } + +} diff --git a/src/Rules/Arrays/ArrayUnpackingRule.php b/src/Rules/Arrays/ArrayUnpackingRule.php index 701214b87..a58746d4a 100644 --- a/src/Rules/Arrays/ArrayUnpackingRule.php +++ b/src/Rules/Arrays/ArrayUnpackingRule.php @@ -5,10 +5,14 @@ use PhpParser\Node; use PhpParser\Node\Expr\ArrayItem; use PHPStan\Analyser\Scope; +use PHPStan\Node\Expr\GetIterableKeyTypeExpr; use PHPStan\Php\PhpVersion; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Rules\RuleLevelHelper; +use PHPStan\Type\ErrorType; use PHPStan\Type\StringType; +use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; /** @@ -17,7 +21,7 @@ class ArrayUnpackingRule implements Rule { - public function __construct(private PhpVersion $phpVersion) + public function __construct(private PhpVersion $phpVersion, private RuleLevelHelper $ruleLevelHelper) { } @@ -32,9 +36,20 @@ public function processNode(Node $node, Scope $scope): array return []; } - $valueType = $scope->getType($node->value); - $isString = (new StringType())->isSuperTypeOf($valueType->getIterableKeyType()); + $stringType = new StringType(); + $typeResult = $this->ruleLevelHelper->findTypeToCheck( + $scope, + new GetIterableKeyTypeExpr($node->value), + '', + static fn (Type $type): bool => $stringType->isSuperTypeOf($type)->no(), + ); + $keyType = $typeResult->getType(); + if ($keyType instanceof ErrorType) { + return $typeResult->getUnknownClassErrors(); + } + + $isString = $stringType->isSuperTypeOf($keyType); if ($isString->no()) { return []; } @@ -43,7 +58,7 @@ public function processNode(Node $node, Scope $scope): array RuleErrorBuilder::message(sprintf( 'Array unpacking cannot be used on an array with %sstring keys: %s', $isString->yes() ? '' : 'potential ', - $valueType->describe(VerbosityLevel::value()), + $scope->getType($node->value)->describe(VerbosityLevel::value()), ))->build() ]; } diff --git a/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php b/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php index 8af2da6c3..4dfc9ff2d 100644 --- a/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php @@ -4,6 +4,7 @@ use PHPStan\Php\PhpVersion; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; use const PHP_VERSION_ID; @@ -13,9 +14,14 @@ class ArrayUnpackingRuleTest extends RuleTestCase { + private bool $checkUnions; + protected function getRule(): Rule { - return new ArrayUnpackingRule(self::getContainer()->getByType(PhpVersion::class)); + return new ArrayUnpackingRule( + self::getContainer()->getByType(PhpVersion::class), + new RuleLevelHelper($this->createReflectionProvider(), true, false, $this->checkUnions, false), + ); } public function testRule(): void @@ -24,6 +30,7 @@ public function testRule(): void $this->markTestSkipped('Test requires PHP version <= 8.0'); } + $this->checkUnions = true; $this->analyse([__DIR__ . '/data/array-unpacking.php'], [ [ 'Array unpacking cannot be used on an array with potential string keys: array{foo: \'bar\', 0: 1, 1: 2, 2: 3}', @@ -56,6 +63,25 @@ public function testRule(): void ]); } + public function testRuleDoNotCheckUnions(): void + { + if (PHP_VERSION_ID >= 80100) { + $this->markTestSkipped('Test requires PHP version <= 8.0'); + } + + $this->checkUnions = false; + $this->analyse([__DIR__ . '/data/array-unpacking.php'], [ + [ + 'Array unpacking cannot be used on an array with string keys: array', + 18, + ], + [ + 'Array unpacking cannot be used on an array with string keys: array{foo: string, bar: int}', + 63, + ], + ]); + } + public function testRuleOnPHP81(): void { if (PHP_VERSION_ID < 80100) { From 67bc65fdfe2864c5b0e47ca350769b69e40976c8 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Mon, 28 Mar 2022 18:14:23 +0200 Subject: [PATCH 4/4] CS --- src/Rules/Arrays/ArrayUnpackingRule.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Rules/Arrays/ArrayUnpackingRule.php b/src/Rules/Arrays/ArrayUnpackingRule.php index a58746d4a..7888da754 100644 --- a/src/Rules/Arrays/ArrayUnpackingRule.php +++ b/src/Rules/Arrays/ArrayUnpackingRule.php @@ -14,6 +14,7 @@ use PHPStan\Type\StringType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; +use function sprintf; /** * @implements Rule @@ -59,7 +60,7 @@ public function processNode(Node $node, Scope $scope): array 'Array unpacking cannot be used on an array with %sstring keys: %s', $isString->yes() ? '' : 'potential ', $scope->getType($node->value)->describe(VerbosityLevel::value()), - ))->build() + ))->build(), ]; }