Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new ArrayUnpackingRule #856

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Expand Up @@ -4,5 +4,6 @@ parameters:
skipCheckGenericClasses: []
explicitMixedInUnknownGenericNew: true
arrayFilter: true
arrayUnpacking: true
stubFiles:
- ../stubs/bleedingEdge/Countable.stub
7 changes: 7 additions & 0 deletions 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
Expand Down Expand Up @@ -74,3 +78,6 @@ services:
reportMaybes: %reportMaybes%
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\Arrays\ArrayUnpackingRule
2 changes: 2 additions & 0 deletions conf/config.neon
Expand Up @@ -29,6 +29,7 @@ parameters:
- RecursiveCallbackFilterIterator
explicitMixedInUnknownGenericNew: false
arrayFilter: false
arrayUnpacking: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down Expand Up @@ -207,6 +208,7 @@ parametersSchema:
skipCheckGenericClasses: listOf(string()),
explicitMixedInUnknownGenericNew: bool(),
arrayFilter: bool(),
arrayUnpacking: bool(),
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
4 changes: 4 additions & 0 deletions src/Analyser/MutatingScope.php
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
34 changes: 34 additions & 0 deletions src/Node/Expr/GetIterableKeyTypeExpr.php
@@ -0,0 +1,34 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node\Expr;

use PhpParser\Node\Expr;
use PHPStan\Node\VirtualNode;

class GetIterableKeyTypeExpr extends Expr implements VirtualNode
{

public function __construct(private Expr $expr)
{
parent::__construct($expr->getAttributes());
}

public function getExpr(): Expr
{
return $this->expr;
}

public function getType(): string
{
return 'PHPStan_Node_GetIterableKeyTypeExpr';
}

/**
* @return string[]
*/
public function getSubNodeNames(): array
{
return [];
}

}
67 changes: 67 additions & 0 deletions src/Rules/Arrays/ArrayUnpackingRule.php
@@ -0,0 +1,67 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Arrays;

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;
use function sprintf;

/**
* @implements Rule<ArrayItem>
*/
class ArrayUnpackingRule implements Rule
{

public function __construct(private PhpVersion $phpVersion, private RuleLevelHelper $ruleLevelHelper)
{
}

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

public function processNode(Node $node, Scope $scope): array
{
if ($node->unpack === false || $this->phpVersion->supportsArrayUnpackingWithStringKeys()) {
return [];
}

$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 [];
}

return [
RuleErrorBuilder::message(sprintf(
'Array unpacking cannot be used on an array with %sstring keys: %s',
$isString->yes() ? '' : 'potential ',
$scope->getType($node->value)->describe(VerbosityLevel::value()),
))->build(),
];
}

}
94 changes: 94 additions & 0 deletions tests/PHPStan/Rules/Arrays/ArrayUnpackingRuleTest.php
@@ -0,0 +1,94 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Arrays;

use PHPStan\Php\PhpVersion;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleLevelHelper;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<ArrayUnpackingRule>
*/
class ArrayUnpackingRuleTest extends RuleTestCase
{

private bool $checkUnions;

protected function getRule(): Rule
{
return new ArrayUnpackingRule(
self::getContainer()->getByType(PhpVersion::class),
new RuleLevelHelper($this->createReflectionProvider(), true, false, $this->checkUnions, false),
);
}

public function testRule(): void
{
if (PHP_VERSION_ID >= 80100) {
$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}',
7,
],
[
'Array unpacking cannot be used on an array with string keys: array<string, string>',
18,
],
[
'Array unpacking cannot be used on an array with potential string keys: array<string>',
24,
],
[
'Array unpacking cannot be used on an array with potential string keys: array',
29,
],
[
'Array unpacking cannot be used on an array with potential string keys: array<int|string, string>',
40,
],
[
'Array unpacking cannot be used on an array with potential string keys: array<int|string, string>',
52,
],
[
'Array unpacking cannot be used on an array with string keys: array{foo: string, bar: int}',
63,
],
]);
}

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<string, string>',
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) {
$this->markTestSkipped('Test requires PHP 8.1+');
}

$this->analyse([__DIR__ . '/data/array-unpacking.php'], []);
}

}
66 changes: 66 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/array-unpacking.php
@@ -0,0 +1,66 @@
<?php // lint >= 7.4

namespace ArrayUnpacking;

$foo = ['foo' => 'bar', 1, 2, 3];

$bar = [...$foo];

/** @param array<int, string> $bar */
function intKeyedArray(array $bar)
{
$baz = [...$bar];
}

/** @param array<string, string> $bar */
function stringKeyedArray(array $bar)
{
$baz = [...$bar];
}

/** @param array<array-key, string> $bar */
function unionKeyedArray(array $bar)
{
$baz = [...$bar];
}

function mixedKeyedArray(array $bar)
{
$baz = [...$bar];
}

/**
* @param array<mixed, string> $foo
* @param array<int, string> $bar
*/
function multipleUnpacking(array $foo, array $bar)
{
$baz = [
...$bar,
...$foo,
];
}

/**
* @param array<mixed, string> $foo
* @param array<string, string> $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,
];
}