diff --git a/README.md b/README.md index a56370e..e913762 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ parameters: superclassToSuffixMapping: [] enforceEnumMatch: enabled: true + enforceIteratorToArrayPreserveKeys: + enabled: true enforceListReturn: enabled: true enforceNativeReturnTypehint: @@ -240,6 +242,19 @@ PHPStan even adds tip about `match` in those cases since `1.10.11`. For those reasons, this rule detects any always-true/false enum comparisons and forces you to rewrite it to `match ($enum)`. +### enforceIteratorToArrayPreserveKeys +- Enforces presence of second parameter in [iterator_to_array](https://www.php.net/manual/en/function.iterator-to-array.php) call (`$preserve_keys`) as the default value `true` is generally dangerous (risk of data loss / failure) +- You can use both `true` and `false` there, but doing so is intentional choice now + +```php +$fn = function () { + yield new stdClass => 1; +}; + +iterator_to_array($fn()); // denied, would fail +``` + + ### enforceListReturn - Enforces usage of `list` when list is always returned from a class method or function - When only single return with empty array is present in the method, it is not considered as list diff --git a/rules.neon b/rules.neon index 1743f13..14bd85b 100644 --- a/rules.neon +++ b/rules.neon @@ -11,6 +11,8 @@ parameters: superclassToSuffixMapping: [] enforceEnumMatch: enabled: true + enforceIteratorToArrayPreserveKeys: + enabled: true enforceListReturn: enabled: true enforceNativeReturnTypehint: @@ -112,6 +114,9 @@ parametersSchema: enforceEnumMatch: structure([ enabled: bool() ]) + enforceIteratorToArrayPreserveKeys: structure([ + enabled: bool() + ]) enforceListReturn: structure([ enabled: bool() ]) @@ -216,6 +221,8 @@ conditionalTags: phpstan.rules.rule: %shipmonkRules.classSuffixNaming.enabled% ShipMonk\PHPStan\Rule\EnforceEnumMatchRule: phpstan.rules.rule: %shipmonkRules.enforceEnumMatch.enabled% + ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule: + phpstan.rules.rule: %shipmonkRules.enforceIteratorToArrayPreserveKeys.enabled% ShipMonk\PHPStan\Rule\EnforceNativeReturnTypehintRule: phpstan.rules.rule: %shipmonkRules.enforceNativeReturnTypehint.enabled% ShipMonk\PHPStan\Rule\EnforceListReturnRule: @@ -304,6 +311,8 @@ services: superclassToSuffixMapping: %shipmonkRules.classSuffixNaming.superclassToSuffixMapping% - class: ShipMonk\PHPStan\Rule\EnforceEnumMatchRule + - + class: ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule - class: ShipMonk\PHPStan\Rule\EnforceListReturnRule - diff --git a/src/Rule/EnforceIteratorToArrayPreserveKeysRule.php b/src/Rule/EnforceIteratorToArrayPreserveKeysRule.php new file mode 100644 index 0000000..c0a33e9 --- /dev/null +++ b/src/Rule/EnforceIteratorToArrayPreserveKeysRule.php @@ -0,0 +1,57 @@ + + */ +class EnforceIteratorToArrayPreserveKeysRule implements Rule +{ + + public function getNodeType(): string + { + return FuncCall::class; + } + + /** + * @param FuncCall $node + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Name) { + return []; + } + + if ($node->name->toString() !== 'iterator_to_array') { + return []; + } + + if (count($node->getArgs()) >= 2) { + return []; + } + + if (count($node->getArgs()) === 0) { + return []; + } + + if ($node->getArgs()[0]->unpack) { + return []; // not trying to analyse what is being unpacked as this is very non-standard approach here + } + + return [RuleErrorBuilder::message('Calling iterator_to_array without 2nd parameter $preserve_keys. Default value true might cause failures or data loss.') + ->line($node->getLine()) + ->identifier('iteratorToArrayWithoutPreserveKeys') + ->build()]; + } + +} diff --git a/tests/Rule/EnforceIteratorToArrayPreserveKeysRuleTest.php b/tests/Rule/EnforceIteratorToArrayPreserveKeysRuleTest.php new file mode 100644 index 0000000..bb5c911 --- /dev/null +++ b/tests/Rule/EnforceIteratorToArrayPreserveKeysRuleTest.php @@ -0,0 +1,25 @@ + + */ +class EnforceIteratorToArrayPreserveKeysRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new EnforceIteratorToArrayPreserveKeysRule(); + } + + public function test(): void + { + $this->analyseFile(__DIR__ . '/data/EnforceIteratorToArrayPreserveKeysRule/code.php'); + } + +} diff --git a/tests/Rule/data/EnforceIteratorToArrayPreserveKeysRule/code.php b/tests/Rule/data/EnforceIteratorToArrayPreserveKeysRule/code.php new file mode 100644 index 0000000..54745e9 --- /dev/null +++ b/tests/Rule/data/EnforceIteratorToArrayPreserveKeysRule/code.php @@ -0,0 +1,31 @@ + 1; +}; + +$dataLoss = function () { + yield 1 => 1; + yield 1 => 2; +}; + +$noKeys = function () { + yield 1; + yield 2; +}; + + +iterator_to_array($objectAsKey()); // error: Calling iterator_to_array without 2nd parameter $preserve_keys. Default value true might cause failures or data loss. +iterator_to_array($dataLoss()); // error: Calling iterator_to_array without 2nd parameter $preserve_keys. Default value true might cause failures or data loss. +iterator_to_array($noKeys()); // error: Calling iterator_to_array without 2nd parameter $preserve_keys. Default value true might cause failures or data loss. + +iterator_to_array($objectAsKey(), false); +iterator_to_array($dataLoss(), false); +iterator_to_array($noKeys(), true); +iterator_to_array(... [$noKeys(), true]); +iterator_to_array( + preserve_keys: true, + iterator: $noKeys() +);