From 462d7c16eff26f71daa3e8cab1d1ec6bad017bed Mon Sep 17 00:00:00 2001 From: Gert de Pagter Date: Fri, 9 Feb 2024 15:10:39 +0100 Subject: [PATCH 1/2] Add a rule to check that arrays are hinted to doctrine --- .../Doctrine/DBAL/ArrayParameterTypeRule.php | 149 ++++++++++++++++++ .../DBAL/ArrayParameterTypeRuleTest.php | 41 +++++ tests/Rules/Doctrine/DBAL/data/connection.php | 65 ++++++++ 3 files changed, 255 insertions(+) create mode 100644 src/Rules/Doctrine/DBAL/ArrayParameterTypeRule.php create mode 100644 tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php create mode 100644 tests/Rules/Doctrine/DBAL/data/connection.php diff --git a/src/Rules/Doctrine/DBAL/ArrayParameterTypeRule.php b/src/Rules/Doctrine/DBAL/ArrayParameterTypeRule.php new file mode 100644 index 00000000..b8a06d7f --- /dev/null +++ b/src/Rules/Doctrine/DBAL/ArrayParameterTypeRule.php @@ -0,0 +1,149 @@ + + */ +class ArrayParameterTypeRule implements Rule +{ + + private const CONNECTION_QUERY_METHODS_LOWER = [ + 'fetchassociative', + 'fetchnumeric', + 'fetchone', + 'delete', + 'insert', + 'fetchallnumeric', + 'fetchallassociative', + 'fetchallkeyvalue', + 'fetchallassociativeindexed', + 'fetchfirstcolumn', + 'iteratenumeric', + 'iterateassociative', + 'iteratekeyvalue', + 'iterateassociativeindexed', + 'iteratecolumn', + 'executequery', + 'executecachequery', + 'executestatement', + ]; + + public function getNodeType(): string + { + return Node\Expr\MethodCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (! $node->name instanceof Node\Identifier) { + return []; + } + + if (count($node->getArgs()) < 2) { + return []; + } + + $calledOnType = $scope->getType($node->var); + + $connection = 'Doctrine\DBAL\Connection'; + if (! (new ObjectType($connection))->isSuperTypeOf($calledOnType)->yes()) { + return []; + } + + $methodName = $node->name->toLowerString(); + if (! in_array($methodName, self::CONNECTION_QUERY_METHODS_LOWER, true)) { + return []; + } + + $params = $scope->getType($node->getArgs()[1]->value); + + $typesArray = $node->getArgs()[2] ?? null; + $typesArrayType = $typesArray !== null + ? $scope->getType($typesArray->value) + : null; + + foreach ($params->getConstantArrays() as $arrayType) { + $values = $arrayType->getValueTypes(); + $keys = []; + foreach ($values as $i => $value) { + if (!$value->isArray()->yes()) { + continue; + } + + $keys[] = $arrayType->getKeyTypes()[$i]; + } + + if ($keys === []) { + continue; + } + + $typeConstantArrays = $typesArrayType !== null + ? $typesArrayType->getConstantArrays() + : []; + + if ($typeConstantArrays === []) { + return array_map( + static function (ConstantScalarType $type) { + return RuleErrorBuilder::message( + 'Parameter at ' + . $type->describe(VerbosityLevel::precise()) + . ' is an array, but is not hinted as such to doctrine.' + ) + ->identifier('doctrine.parameterType') + ->build(); + }, + $keys + ); + } + + foreach ($typeConstantArrays as $typeConstantArray) { + $issueKeys = []; + foreach ($keys as $key) { + $valueType = $typeConstantArray->getOffsetValueType($key); + + $values = $valueType->getConstantScalarValues(); + if ($values === []) { + $issueKeys[] = $key; + } + + foreach ($values as $scalarValue) { + if (is_int($scalarValue) && !(($scalarValue & Connection::ARRAY_PARAM_OFFSET) !== Connection::ARRAY_PARAM_OFFSET)) { + continue; + } + + $issueKeys[] = $key; + } + + return array_map( + static function (ConstantScalarType $type) { + return RuleErrorBuilder::message( + 'Parameter at ' + . $type->describe(VerbosityLevel::precise()) + . ' is an array, but is not hinted as such to doctrine.' + )->identifier('doctrine.parameterType') + ->build(); + }, + $issueKeys + ); + } + } + } + + return []; + } + +} diff --git a/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php b/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php new file mode 100644 index 00000000..36fc97cb --- /dev/null +++ b/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php @@ -0,0 +1,41 @@ + + */ +final class ArrayParameterTypeRuleTest extends RuleTestCase +{ + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/connection.php'], [ + [ + 'Parameter at 0 is an array, but is not hinted as such to doctrine.', + 11, + ], + [ + "Parameter at 'a' is an array, but is not hinted as such to doctrine.", + 20, + ], + [ + "Parameter at 'a' is an array, but is not hinted as such to doctrine.", + 29, + ], + [ + "Parameter at 'a' is an array, but is not hinted as such to doctrine.", + 40, + ], + ]); + } + + protected function getRule(): Rule + { + return new ArrayParameterTypeRule(); + } + +} diff --git a/tests/Rules/Doctrine/DBAL/data/connection.php b/tests/Rules/Doctrine/DBAL/data/connection.php new file mode 100644 index 00000000..9759fd1f --- /dev/null +++ b/tests/Rules/Doctrine/DBAL/data/connection.php @@ -0,0 +1,65 @@ +executeQuery( + 'SELECT 1 FROM table WHERE a IN (?) AND b = ?', + [ + + $data, + 3 + ] + ); + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + + 'a' => $data, + 'b' => 3 + ] + ); + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + 'a' => $data, + 'b' => 3 + ], + [ + 'b' => ParameterType::INTEGER, + ] + ); + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + 'a' => $data, + 'b' => 3 + ], + [ + 'a' => ParameterType::INTEGER, + 'b' => ParameterType::INTEGER, + ] + ); + + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + 'a' => $data, + 'b' => 3 + ], + [ + 'a' => ArrayParameterType::INTEGER, + 'b' => ParameterType::INTEGER, + ] + ); + +} From e5cab7734e69764b80ec077b3228fe226c81d2fd Mon Sep 17 00:00:00 2001 From: Gert de Pagter Date: Fri, 9 Feb 2024 15:29:17 +0100 Subject: [PATCH 2/2] Deal with older dbal versiosn --- .../DBAL/ArrayParameterTypeRuleTest.php | 38 +++++++++++ .../Doctrine/DBAL/data/connection_dbal2.php | 64 +++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 tests/Rules/Doctrine/DBAL/data/connection_dbal2.php diff --git a/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php b/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php index 36fc97cb..f6567742 100644 --- a/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php +++ b/tests/Rules/Doctrine/DBAL/ArrayParameterTypeRuleTest.php @@ -2,6 +2,8 @@ namespace PHPStan\Rules\Doctrine\DBAL; +use Composer\InstalledVersions; +use Composer\Semver\VersionParser; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; @@ -11,8 +13,44 @@ final class ArrayParameterTypeRuleTest extends RuleTestCase { + public function testRuleOlderDbal(): void + { + if(InstalledVersions::satisfies( + new VersionParser(), + 'doctrine/dbal', + '^3.6 || ^4.0' + )) { + self::markTestSkipped('Test requires dbal 2.'); + } + $this->analyse([__DIR__ . '/data/connection_dbal2.php'], [ + [ + 'Parameter at 0 is an array, but is not hinted as such to doctrine.', + 10, + ], + [ + "Parameter at 'a' is an array, but is not hinted as such to doctrine.", + 19, + ], + [ + "Parameter at 'a' is an array, but is not hinted as such to doctrine.", + 28, + ], + [ + "Parameter at 'a' is an array, but is not hinted as such to doctrine.", + 39, + ], + ]); + } + public function testRule(): void { + if(InstalledVersions::satisfies( + new VersionParser(), + 'doctrine/dbal', + '<3.6' + )) { + self::markTestSkipped('Test requires dbal 3 or 4.'); + } $this->analyse([__DIR__ . '/data/connection.php'], [ [ 'Parameter at 0 is an array, but is not hinted as such to doctrine.', diff --git a/tests/Rules/Doctrine/DBAL/data/connection_dbal2.php b/tests/Rules/Doctrine/DBAL/data/connection_dbal2.php new file mode 100644 index 00000000..c97a3900 --- /dev/null +++ b/tests/Rules/Doctrine/DBAL/data/connection_dbal2.php @@ -0,0 +1,64 @@ +executeQuery( + 'SELECT 1 FROM table WHERE a IN (?) AND b = ?', + [ + + $data, + 3 + ] + ); + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + + 'a' => $data, + 'b' => 3 + ] + ); + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + 'a' => $data, + 'b' => 3 + ], + [ + 'b' => ParameterType::INTEGER, + ] + ); + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + 'a' => $data, + 'b' => 3 + ], + [ + 'a' => ParameterType::INTEGER, + 'b' => ParameterType::INTEGER, + ] + ); + + + $connection->fetchOne( + 'SELECT 1 FROM table WHERE a IN (:a) AND b = :b', + [ + 'a' => $data, + 'b' => 3 + ], + [ + 'a' => Connection::PARAM_INT_ARRAY, + 'b' => ParameterType::INTEGER, + ] + ); + +}