Skip to content

Commit

Permalink
Implement ArrayColumnRule
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm committed Dec 4, 2024
1 parent 0cbd46b commit 3eb96d9
Show file tree
Hide file tree
Showing 8 changed files with 293 additions and 0 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ parameters:
explicitMixedInUnknownGenericNew: true
explicitMixedForGlobalVariables: true
explicitMixedViaIsArray: true
arrayColumn: true
arrayFilter: true
arrayUnpacking: true
arrayValues: true
Expand Down
8 changes: 8 additions & 0 deletions conf/config.level5.neon
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ parameters:
checkArgumentsPassedByReference: true

conditionalTags:
PHPStan\Rules\Functions\ArrayColumnRule:
phpstan.rules.rule: %featureToggles.arrayColumn%
PHPStan\Rules\Functions\ArrayFilterRule:
phpstan.rules.rule: %featureToggles.arrayFilter%
PHPStan\Rules\Functions\ArrayValuesRule:
Expand Down Expand Up @@ -56,3 +58,9 @@ services:
class: PHPStan\Rules\Functions\ImplodeParameterCastableToStringRule
-
class: PHPStan\Rules\Functions\SortParameterCastableToStringRule

-
class: PHPStan\Rules\Functions\ArrayColumnRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
treatPhpDocTypesAsCertainTip: %tips.treatPhpDocTypesAsCertain%
4 changes: 4 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ parameters:
explicitMixedInUnknownGenericNew: false
explicitMixedForGlobalVariables: false
explicitMixedViaIsArray: false
arrayColumn: false
arrayFilter: false
arrayUnpacking: false
arrayValues: false
Expand Down Expand Up @@ -1192,6 +1193,9 @@ services:
tags:
- phpstan.broker.dynamicFunctionReturnTypeExtension

-
class: PHPStan\Type\Php\ArrayColumnHelper

-
class: PHPStan\Type\Php\ArrayCombineFunctionReturnTypeExtension
tags:
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ parametersSchema:
explicitMixedInUnknownGenericNew: bool(),
explicitMixedForGlobalVariables: bool(),
explicitMixedViaIsArray: bool(),
arrayColumn: bool(),
arrayFilter: bool(),
arrayUnpacking: bool(),
arrayValues: bool(),
Expand Down
111 changes: 111 additions & 0 deletions src/Rules/Functions/ArrayColumnRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\NeverType;
use PHPStan\Type\Php\ArrayColumnHelper;
use PHPStan\Type\VerbosityLevel;
use function count;
use function sprintf;

/**
* @implements Rule<Node\Expr\FuncCall>
*/
final class ArrayColumnRule implements Rule
{

public function __construct(
private readonly ReflectionProvider $reflectionProvider,
private readonly bool $treatPhpDocTypesAsCertain,
private bool $treatPhpDocTypesAsCertainTip,
private ArrayColumnHelper $arrayColumnHelper,
)
{
}

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

public function processNode(Node $node, Scope $scope): array
{
if (!($node->name instanceof Node\Name)) {
return [];
}

$args = $node->getArgs();
if (count($args) < 2) {
return [];
}

if (!$this->reflectionProvider->hasFunction($node->name, $scope)) {
return [];
}

$functionReflection = $this->reflectionProvider->getFunction($node->name, $scope);
if ($functionReflection->getName() !== 'array_column') {
return [];
}

$indexKeyType = null;
if ($this->treatPhpDocTypesAsCertain) {
$arrayType = $scope->getType($args[0]->value);
$columnKeyType = $scope->getType($args[1]->value);
if (count($args) >= 3) {
$indexKeyType = $scope->getType($args[2]->value);
}
} else {
$arrayType = $scope->getNativeType($args[0]->value);
$columnKeyType = $scope->getNativeType($args[1]->value);
if (count($args) >= 3) {
$indexKeyType = $scope->getNativeType($args[2]->value);
}
}

$errors = [];
if ($columnKeyType->isNull()->no()) {
[$returnValueType] = $this->arrayColumnHelper->getReturnValueType($arrayType, $columnKeyType, $scope);
if ($returnValueType instanceof NeverType) {
$errorBuilder = RuleErrorBuilder::message(sprintf(
'Cannot access column %s on %s.',
$columnKeyType->describe(VerbosityLevel::value()),
$arrayType->getIterableValueType()->describe(VerbosityLevel::value()),
))->identifier('arrayColumn.column');

if ($this->treatPhpDocTypesAsCertainTip) {
$errorBuilder->treatPhpDocTypesAsCertainTip();
}

$errors[] = $errorBuilder->build();
}
}

if ($indexKeyType !== null && $indexKeyType->isNull()->no()) {
$returnValueType = $this->arrayColumnHelper->getReturnIndexType($arrayType, $indexKeyType, $scope);
if ($returnValueType instanceof NeverType) {
$errorBuilder = RuleErrorBuilder::message(sprintf(
'Cannot access column %s on %s.',
$indexKeyType->describe(VerbosityLevel::value()),
$arrayType->getIterableValueType()->describe(VerbosityLevel::value()),
))->identifier('arrayColumn.index');

$nativeArrayType = $scope->getNativeType($args[0]->value);
if ($this->treatPhpDocTypesAsCertainTip) {
$errorBuilder->treatPhpDocTypesAsCertainTip();
}

$errors[] = $errorBuilder->build();
}
}

return $errors;
}

}
130 changes: 130 additions & 0 deletions tests/PHPStan/Rules/Functions/ArrayColumnRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPStan\Type\Php\ArrayColumnHelper;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<ArrayColumnRule>
*/
class ArrayColumnRuleTest extends RuleTestCase
{

private bool $treatPhpDocTypesAsCertain = true;

protected function getRule(): Rule
{
return new ArrayColumnRule(
$this->createReflectionProvider(),
$this->treatPhpDocTypesAsCertain,
true,
self::getContainer()->getByType(ArrayColumnHelper::class),
);
}

public function testFile(): void
{
$expectedErrors = [];

$this->analyse([__DIR__ . '/../../Analyser/nsrt/array-column-php7.php'], $expectedErrors);
}

public function testFilePhp82(): void
{
if (PHP_VERSION_ID < 80200) {
$this->markTestSkipped('Test requires PHP 8.2');
}

$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
$expectedErrors = [
[
"Cannot access column 'column' on *NEVER*.",
30,
$tipText,
],
[
"Cannot access column 'column' on *NEVER*.",
31,
$tipText,
],
[
"Cannot access column 'key' on *NEVER*.",
31,
$tipText,
],
[
"Cannot access column 'key' on *NEVER*.",
32,
$tipText,
],
[
"Cannot access column 'foo' on array{column: string, key: string}.",
76,
$tipText,
],
[
"Cannot access column 'foo' on array{column: string, key: string}.",
77,
$tipText,
],
[
"Cannot access column 'nodeName' on ArrayColumn82\Foo.",
216,
$tipText,
],
[
"Cannot access column 'nodeName' on ArrayColumn82\Foo.",
217,
$tipText,
],
[
"Cannot access column 'tagName' on ArrayColumn82\Foo.",
217,
$tipText,
],
];

$this->analyse([__DIR__ . '/../../Analyser/nsrt/array-column-php82.php'], $expectedErrors);
}

public function testBug5101(): void
{
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';

// in PHP < 8.2 dynamic properties can exist any time
$expectedErrors = [];
if (PHP_VERSION_ID >= 80200) {
$expectedErrors = [
[
"Cannot access column 'y' on Bug5101\FinalFooBar.",
22,
$tipText,
],
];
}

$this->analyse([__DIR__ . '/data/bug-5101.php'], $expectedErrors);
}

public function testBug12188(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1');
}

$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
$expectedErrors = [
[
"Cannot access column 'value' on Bug12188\Foo::A|Bug12188\Foo::B.",
14,
$tipText,
],
];

$this->analyse([__DIR__ . '/data/bug-12188.php'], $expectedErrors);
}

}
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-12188.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php declare(strict_types = 1); // lint >= 8.1

namespace Bug12188;

enum Foo
{
case A;
case B;
}

function doFoo() {
$arr = [Foo::A, Foo::B];

var_dump(array_column($arr, 'value'));
}
23 changes: 23 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-5101.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php declare(strict_types = 1); // lint >= 7.4

namespace Bug5101;

class FooBar
{
public $x;
}

final class FinalFooBar
{
public $x;
}

/** @param array<FooBar> $arrClass */
function doFoo(array $arrClass) {
$arrFinalClass = [new FinalFooBar()];

var_dump(array_column($arrClass, 'x'));
var_dump(array_column($arrClass, 'y'));
var_dump(array_column($arrFinalClass, 'x'));
var_dump(array_column($arrFinalClass, 'y'));
}

0 comments on commit 3eb96d9

Please sign in to comment.