Skip to content

Commit

Permalink
EnforceIteratorToArrayPreserveKeysRule
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Sep 12, 2023
1 parent c588201 commit 83547c9
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 0 deletions.
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ parameters:
superclassToSuffixMapping: []
enforceEnumMatch:
enabled: true
enforceIteratorToArrayPreserveKeys:
enabled: true
enforceListReturn:
enabled: true
enforceNativeReturnTypehint:
Expand Down Expand Up @@ -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<T>` 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
Expand Down
9 changes: 9 additions & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ parameters:
superclassToSuffixMapping: []
enforceEnumMatch:
enabled: true
enforceIteratorToArrayPreserveKeys:
enabled: true
enforceListReturn:
enabled: true
enforceNativeReturnTypehint:
Expand Down Expand Up @@ -112,6 +114,9 @@ parametersSchema:
enforceEnumMatch: structure([
enabled: bool()
])
enforceIteratorToArrayPreserveKeys: structure([
enabled: bool()
])
enforceListReturn: structure([
enabled: bool()
])
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -304,6 +311,8 @@ services:
superclassToSuffixMapping: %shipmonkRules.classSuffixNaming.superclassToSuffixMapping%
-
class: ShipMonk\PHPStan\Rule\EnforceEnumMatchRule
-
class: ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule
-
class: ShipMonk\PHPStan\Rule\EnforceListReturnRule
-
Expand Down
49 changes: 49 additions & 0 deletions src/Rule/EnforceIteratorToArrayPreserveKeysRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\Rule;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use function count;

/**
* @implements Rule<FuncCall>
*/
class EnforceIteratorToArrayPreserveKeysRule implements Rule
{

public function getNodeType(): string
{
return FuncCall::class;
}

/**
* @param FuncCall $node
* @return list<RuleError>
*/
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 [];
}

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()];
}

}
25 changes: 25 additions & 0 deletions tests/Rule/EnforceIteratorToArrayPreserveKeysRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php declare(strict_types = 1);

namespace Rule;

use PHPStan\Rules\Rule;
use ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule;
use ShipMonk\PHPStan\RuleTestCase;

/**
* @extends RuleTestCase<EnforceIteratorToArrayPreserveKeysRule>
*/
class EnforceIteratorToArrayPreserveKeysRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new EnforceIteratorToArrayPreserveKeysRule();
}

public function test(): void
{
$this->analyseFile(__DIR__ . '/data/EnforceIteratorToArrayPreserveKeysRule/code.php');
}

}
26 changes: 26 additions & 0 deletions tests/Rule/data/EnforceIteratorToArrayPreserveKeysRule/code.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace EnforceIteratorToArrayPreserveKeysRule;

$objectAsKey = function () {
yield new \stdClass => 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);

0 comments on commit 83547c9

Please sign in to comment.