diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 5f4e2d114a..3e8b7ad2fa 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -21,3 +21,4 @@ parameters: reportMethodPurityOverride: true checkDynamicConstantNameValues: true unusedLabel: true + arrayColumnObjectArrays: true diff --git a/conf/config.level5.neon b/conf/config.level5.neon index 534d58dde0..aa38361014 100644 --- a/conf/config.level5.neon +++ b/conf/config.level5.neon @@ -12,6 +12,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.checkPrintfParameterTypes% PHPStan\Rules\DateIntervalInstantiationRule: phpstan.rules.rule: %featureToggles.checkDateIntervalConstructor% + PHPStan\Rules\Functions\ArrayColumnRule: + phpstan.rules.rule: %featureToggles.arrayColumnObjectArrays% autowiredAttributeServices: # registers rules with #[RegisteredRule] attribute @@ -26,3 +28,8 @@ services: checkStrictPrintfPlaceholderTypes: %checkStrictPrintfPlaceholderTypes% - class: PHPStan\Rules\DateIntervalInstantiationRule + - + class: PHPStan\Rules\Functions\ArrayColumnRule + arguments: + treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% + treatPhpDocTypesAsCertainTip: %tips.treatPhpDocTypesAsCertain% diff --git a/conf/config.neon b/conf/config.neon index 0e09a3ed22..9b5d96b56a 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -48,6 +48,7 @@ parameters: reportMethodPurityOverride: false checkDynamicConstantNameValues: false unusedLabel: false + arrayColumnObjectArrays: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index dcd030439b..6a38faced1 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -50,6 +50,7 @@ parametersSchema: reportMethodPurityOverride: bool() checkDynamicConstantNameValues: bool() unusedLabel: bool() + arrayColumnObjectArrays: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Functions/ArrayColumnRule.php b/src/Rules/Functions/ArrayColumnRule.php new file mode 100644 index 0000000000..5ffb910ffe --- /dev/null +++ b/src/Rules/Functions/ArrayColumnRule.php @@ -0,0 +1,140 @@ + + */ +final class ArrayColumnRule implements Rule +{ + + public function __construct( + private readonly ReflectionProvider $reflectionProvider, + private readonly ArrayColumnHelper $arrayColumnHelper, + private readonly bool $treatPhpDocTypesAsCertain, + private readonly bool $treatPhpDocTypesAsCertainTip, + ) + { + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!($node->name instanceof Node\Name)) { + return []; + } + + // array_column() requires at least the array and the column key - bail + // out before the more expensive reflection lookups below. + if (count($node->getArgs()) < 2) { + return []; + } + + if (!$this->reflectionProvider->hasFunction($node->name, $scope)) { + return []; + } + + $functionReflection = $this->reflectionProvider->getFunction($node->name, $scope); + if ($functionReflection->getName() !== 'array_column') { + return []; + } + + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $node->getArgs(), + $functionReflection->getVariants(), + $functionReflection->getNamedArgumentsVariants(), + ); + + $normalizedFuncCall = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $node); + if ($normalizedFuncCall === null) { + return []; + } + + $args = $normalizedFuncCall->getArgs(); + if (count($args) < 2) { + return []; + } + + $arrayArg = $args[0]->value; + $valueType = $scope->getType($arrayArg)->getIterableValueType(); + $nativeValueType = $scope->getNativeType($arrayArg)->getIterableValueType(); + + $errors = []; + foreach ($this->checkColumn($args[1]->value, $valueType, $nativeValueType, '#2 $column_key', $scope) as $error) { + $errors[] = $error; + } + + if (count($args) >= 3) { + foreach ($this->checkColumn($args[2]->value, $valueType, $nativeValueType, '#3 $index_key', $scope) as $error) { + $errors[] = $error; + } + } + + return $errors; + } + + /** + * @return list + */ + private function checkColumn(Node\Expr $columnExpr, Type $valueType, Type $nativeValueType, string $parameter, Scope $scope): array + { + $checkedValueType = $this->treatPhpDocTypesAsCertain ? $valueType : $nativeValueType; + + $columnType = $scope->getType($columnExpr); + $missingProperties = $this->arrayColumnHelper->findMissingObjectProperties($checkedValueType, $columnType); + if ($missingProperties === []) { + return []; + } + + $nativeMissingPropertyNames = []; + foreach ($this->arrayColumnHelper->findMissingObjectProperties($nativeValueType, $columnType) as $nativeMissingProperty) { + $nativeMissingPropertyNames[$nativeMissingProperty->getValue()] = true; + } + + $errors = []; + foreach ($missingProperties as $propertyNameType) { + $errorBuilder = RuleErrorBuilder::message(sprintf( + 'Parameter %s of function array_column expects a valid property name, %s given, but %s does not have such property.', + $parameter, + $propertyNameType->describe(VerbosityLevel::value()), + $checkedValueType->describe(VerbosityLevel::typeOnly()), + ))->identifier('arrayColumn.property'); + + if ( + $this->treatPhpDocTypesAsCertain + && $this->treatPhpDocTypesAsCertainTip + && !isset($nativeMissingPropertyNames[$propertyNameType->getValue()]) + ) { + $errorBuilder->treatPhpDocTypesAsCertainTip(); + } + + $errors[] = $errorBuilder->build(); + } + + return $errors; + } + +} diff --git a/src/Type/Php/ArrayColumnHelper.php b/src/Type/Php/ArrayColumnHelper.php index aeb414875f..94268cb16f 100644 --- a/src/Type/Php/ArrayColumnHelper.php +++ b/src/Type/Php/ArrayColumnHelper.php @@ -11,6 +11,7 @@ use PHPStan\Type\ArrayType; use PHPStan\Type\Constant\ConstantArrayType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\IntegerType; use PHPStan\Type\MixedType; use PHPStan\Type\NeverType; @@ -150,6 +151,77 @@ public function handleConstantArray(ConstantArrayType $arrayType, Type $columnTy return $builder->getArray(); } + /** + * Returns the property names from $columnType that the objects contained in + * $iterableValueType certainly lack, so that reading them via array_column() + * would yield nothing. + * + * Unlike the type-inference path (getOffsetOrProperty), this resolves + * properties at the ClassReflection level so that concrete and non-final + * element classes are reported - matching how AccessPropertiesRule reports + * direct property fetches. + * + * @return list + */ + public function findMissingObjectProperties(Type $iterableValueType, Type $columnType): array + { + // array_column() reads object properties (never ArrayAccess offsets), so + // only check when the elements are definitely objects. Array elements use + // offset access, scalars never have the member - leave those to other rules. + if (!$iterableValueType->isObject()->yes()) { + return []; + } + + $propertyNameTypes = $columnType->getConstantStrings(); + if ($propertyNameTypes === []) { + return []; + } + + $missing = []; + foreach ($propertyNameTypes as $propertyNameType) { + if (!$this->isObjectPropertyMissing($iterableValueType, $propertyNameType->getValue())) { + continue; + } + + $missing[] = $propertyNameType; + } + + return $missing; + } + + private function isObjectPropertyMissing(Type $iterableValueType, string $propertyName): bool + { + $classReflections = $iterableValueType->getObjectClassReflections(); + if ($classReflections === []) { + return false; + } + + foreach ($classReflections as $classReflection) { + if ($classReflection->isEnum()) { + // Enum cases expose the read-only `name` property, and backed + // enums additionally expose `value`. Any other name is missing. + if ($propertyName === 'name') { + return false; + } + if ($propertyName === 'value' && $classReflection->isBackedEnum()) { + return false; + } + continue; + } + if ($classReflection->hasInstanceProperty($propertyName)) { + return false; + } + if ($classReflection->allowsDynamicProperties()) { + return false; + } + if ($classReflection->hasNativeMethod('__isset') && $classReflection->hasNativeMethod('__get')) { + return false; + } + } + + return true; + } + /** * @return array{Type, TrinaryLogic} */ diff --git a/tests/PHPStan/Rules/Functions/ArrayColumnRuleTest.php b/tests/PHPStan/Rules/Functions/ArrayColumnRuleTest.php new file mode 100644 index 0000000000..8b2fb5a92c --- /dev/null +++ b/tests/PHPStan/Rules/Functions/ArrayColumnRuleTest.php @@ -0,0 +1,103 @@ + + */ +class ArrayColumnRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new ArrayColumnRule( + self::createReflectionProvider(), + self::getContainer()->getByType(ArrayColumnHelper::class), + $this->shouldTreatPhpDocTypesAsCertain(), + true, + ); + } + + #[RequiresPhp('>= 8.2')] + public function testRule(): void + { + $tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting treatPhpDocTypesAsCertain: false in your %configurationFile%.'; + $this->analyse([__DIR__ . '/data/array-column.php'], [ + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'wrong_key' given, but ArrayColumnRuleTest\\NonFinalObject does not have such property.", + 72, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 76, + $tipText, + ], + [ + "Parameter #3 \$index_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 78, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 79, + $tipText, + ], + [ + "Parameter #3 \$index_key of function array_column expects a valid property name, 'missing2' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 79, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\Suit does not have such property.", + 87, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'value' given, but ArrayColumnRuleTest\\PureSuit does not have such property.", + 93, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\PureSuit does not have such property.", + 94, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'wrong_key' given, but ArrayColumnRuleTest\\NonFinalObject does not have such property.", + 108, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject|ArrayColumnRuleTest\\NonFinalObject does not have such property.", + 119, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'Price' given, but DateTimeImmutable does not have such property.", + 130, + $tipText, + ], + [ + "Parameter #2 \$column_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 140, + $tipText, + ], + [ + "Parameter #3 \$index_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 141, + $tipText, + ], + [ + "Parameter #3 \$index_key of function array_column expects a valid property name, 'missing' given, but ArrayColumnRuleTest\\FinalObject does not have such property.", + 142, + $tipText, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Functions/data/array-column.php b/tests/PHPStan/Rules/Functions/data/array-column.php new file mode 100644 index 0000000000..9b478782a5 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/array-column.php @@ -0,0 +1,143 @@ += 8.2 + +namespace ArrayColumnRuleTest; + +class NonFinalObject +{ + + /** @var string */ + public $key = 'as'; + +} + +final class FinalObject +{ + + public int $id = 1; + + public string $name = 'a'; + + private int $secret = 2; + +} + +class MagicObject +{ + + public function __get(string $name): int + { + return 1; + } + + public function __isset(string $name): bool + { + return true; + } + +} + +#[\AllowDynamicProperties] +class DynamicObject +{ + +} + +enum Suit: string +{ + + case Hearts = 'H'; + +} + +enum PureSuit +{ + + case Hearts; + +} + +/** + * @param NonFinalObject[] $a + * @param FinalObject[] $b + * @param MagicObject[] $c + * @param DynamicObject[] $d + * @param Suit[] $e + * @param array> $f + * @param list> $g + * @param PureSuit[] $h + */ +function test(array $a, array $b, array $c, array $d, array $e, array $f, array $g, array $h): void +{ + array_column($a, 'key'); + array_column($a, 'wrong_key'); + + array_column($b, 'id'); + array_column($b, 'name'); + array_column($b, 'missing'); + array_column($b, 'name', 'id'); + array_column($b, 'name', 'missing'); + array_column($b, 'missing', 'missing2'); + array_column($b, 'secret'); + + array_column($c, 'anything'); + array_column($d, 'anything'); + + array_column($e, 'value'); + array_column($e, 'name'); + array_column($e, 'missing'); + + array_column($f, 'col'); + array_column($g, 'missing'); + + array_column($h, 'name'); + array_column($h, 'value'); + array_column($h, 'missing'); +} + +/** + * @param FinalObject[] $b + */ +function dynamicColumnName(array $b, string $column): void +{ + array_column($b, $column); +} + +function bug5101(): void +{ + $ar = [new NonFinalObject(), new NonFinalObject()]; + array_column($ar, 'wrong_key'); + array_column($ar, 'key'); +} + +/** + * @param array $union + */ +function unionElements(array $union): void +{ + array_column($union, 'key'); // exists on NonFinalObject + array_column($union, 'id'); // exists on FinalObject + array_column($union, 'missing'); // exists on neither +} + +class HelloWorld +{ + + /** + * @param list<\DateTimeImmutable> $dates + */ + public function bug9671(array $dates): void + { + array_column($dates, 'Price'); + } + +} + +/** + * @param FinalObject[] $b + */ +function namedArguments(array $b): void +{ + array_column($b, column_key: 'missing'); + array_column($b, index_key: 'missing', column_key: 'name'); + array_column(array: $b, column_key: 'name', index_key: 'missing'); +}