diff --git a/.gitignore b/.gitignore index e5995bd5..aaebabf4 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ vendor .phpunit.cache .phpunit.result.cache composer.lock +/phpstan/ +/phpstan-tests/ diff --git a/composer.json b/composer.json index e0d84c48..8392a234 100644 --- a/composer.json +++ b/composer.json @@ -16,6 +16,7 @@ "require": { "php": ">=8.4", "psr/log": "^1|^2|^3", + "phpmyadmin/sql-parser": "^5.10", "symfony/process": "^5.0|^6.0|^7.0", "symfony/polyfill-mbstring": "^1.15" }, @@ -51,5 +52,12 @@ "phpstan/extension-installer": true, "pestphp/pest-plugin": true } + }, + "extra": { + "phpstan": { + "includes": [ + "phpstan-extension.neon" + ] + } } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 364905f7..65565330 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,2 +1,67 @@ parameters: ignoreErrors: + - + message: '#^Offset ''cnt'' might not exist on array\{\}\|array\{cnt\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 2 + path: tests/ConnectionMultiInsertTest.php + + - + message: '#^Offset ''val'' might not exist on array\{\}\|array\{val\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 2 + path: tests/ConnectionRoundTest.php + + - + message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertCount\(\) with 12 and array\{temp_char10\: mixed\} will always evaluate to false\.$#' + identifier: method.impossibleType + count: 1 + path: tests/ConnectionTest.php + + - + message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertCount\(\) with 16 and array\{temp_char10\: mixed\} will always evaluate to false\.$#' + identifier: method.impossibleType + count: 1 + path: tests/ConnectionTest.php + + - + message: '#^Offset ''temp_bigint_new'' might not exist on array\{\}\|array\{temp_id\: mixed, temp_bigint_new\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 2 + path: tests/ConnectionTest.php + + - + message: '#^Offset ''temp_id'' might not exist on array\{\}\|array\{temp_id\: mixed, temp_bigint_new\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 2 + path: tests/ConnectionTest.php + + - + message: '#^Offset int\<0, 15\> does not exist on array\{temp_char10\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 1 + path: tests/ConnectionTest.php + + - + message: '#^Offset ''cnt'' might not exist on array\{\}\|array\{cnt\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 2 + path: tests/ConnectionTxTest.php + + - + message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertCount\(\) with 3 and \*NEVER\* will always evaluate to false\.$#' + identifier: method.impossibleType + count: 1 + path: tests/ConnectionUpsertTest.php + + - + message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertCount\(\) with 3 and array\{array\\} will always evaluate to false\.$#' + identifier: method.impossibleType + count: 1 + path: tests/ConnectionUpsertTest.php + + - + message: '#^Offset ''cnt'' might not exist on array\{\}\|array\{cnt\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 1 + path: tests/ConnectionUpsertTest.php diff --git a/phpstan-extension.neon b/phpstan-extension.neon new file mode 100644 index 00000000..5c528281 --- /dev/null +++ b/phpstan-extension.neon @@ -0,0 +1,23 @@ +services: + - + class: Artemeon\Database\PHPStan\SqlReturnShapeAnalyser + + - + class: Artemeon\Database\PHPStan\GetPArrayReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Artemeon\Database\PHPStan\GetPRowReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Artemeon\Database\PHPStan\GetGeneratorReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Artemeon\Database\PHPStan\PlaceholderCountRule + tags: + - phpstan.rules.rule diff --git a/phpstan.neon b/phpstan.neon index a1fb39e0..4a531c71 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,8 +1,11 @@ includes: - phpstan-baseline.neon + - phpstan-extension.neon parameters: level: 9 paths: - src - tests + excludePaths: + - tests/PHPStan/data diff --git a/src/PHPStan/GetGeneratorReturnTypeExtension.php b/src/PHPStan/GetGeneratorReturnTypeExtension.php new file mode 100644 index 00000000..3156b8f0 --- /dev/null +++ b/src/PHPStan/GetGeneratorReturnTypeExtension.php @@ -0,0 +1,56 @@ +getName() === 'getGenerator'; + } + + public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type + { + $args = $methodCall->getArgs(); + if ($args === []) { + return null; + } + + $queryType = $scope->getType($args[0]->value); + $constantStrings = $queryType->getConstantStrings(); + if (count($constantStrings) !== 1) { + return null; + } + + $rowType = $this->analyser->analyseRow($constantStrings[0]->getValue()); + if ($rowType === null) { + return null; + } + + return new GenericObjectType(Generator::class, [ + new IntegerType(), + $rowType, + ]); + } +} diff --git a/src/PHPStan/GetPArrayReturnTypeExtension.php b/src/PHPStan/GetPArrayReturnTypeExtension.php new file mode 100644 index 00000000..3231d306 --- /dev/null +++ b/src/PHPStan/GetPArrayReturnTypeExtension.php @@ -0,0 +1,57 @@ +getName() === 'getPArray'; + } + + public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type + { + $args = $methodCall->getArgs(); + if ($args === []) { + return null; + } + + $queryType = $scope->getType($args[0]->value); + $constantStrings = $queryType->getConstantStrings(); + if (count($constantStrings) !== 1) { + return null; + } + + $rowType = $this->analyser->analyseRow($constantStrings[0]->getValue()); + if ($rowType === null) { + return null; + } + + return TypeCombinator::intersect( + new ArrayType(new IntegerType(), $rowType), + new AccessoryArrayListType(), + ); + } +} diff --git a/src/PHPStan/GetPRowReturnTypeExtension.php b/src/PHPStan/GetPRowReturnTypeExtension.php new file mode 100644 index 00000000..79d448df --- /dev/null +++ b/src/PHPStan/GetPRowReturnTypeExtension.php @@ -0,0 +1,58 @@ +getName() === 'getPRow'; + } + + public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type + { + $args = $methodCall->getArgs(); + if ($args === []) { + return null; + } + + $queryType = $scope->getType($args[0]->value); + $constantStrings = $queryType->getConstantStrings(); + if (count($constantStrings) !== 1) { + return null; + } + + $rowType = $this->analyser->analyseRow($constantStrings[0]->getValue()); + if ($rowType === null) { + return null; + } + + if ($rowType->getConstantArrays() === []) { + return $rowType; + } + + $empty = ConstantArrayTypeBuilder::createEmpty()->getArray(); + + return TypeCombinator::union($rowType, $empty); + } +} diff --git a/src/PHPStan/PlaceholderCountRule.php b/src/PHPStan/PlaceholderCountRule.php new file mode 100644 index 00000000..e6ea1fed --- /dev/null +++ b/src/PHPStan/PlaceholderCountRule.php @@ -0,0 +1,194 @@ + + */ +final class PlaceholderCountRule implements Rule +{ + private const array METHODS = ['getPArray' => true, 'getPRow' => true, 'getGenerator' => true, '_pQuery' => true]; + + public function __construct(private readonly ReflectionProvider $reflectionProvider) + { + } + + public function getNodeType(): string + { + return MethodCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Identifier) { + return []; + } + + $methodName = $node->name->toString(); + if (!isset(self::METHODS[$methodName])) { + return []; + } + + $callerType = $scope->getType($node->var); + if (!$this->reflectionProvider->hasClass(ConnectionInterface::class)) { + return []; + } + + $interface = $this->reflectionProvider->getClass(ConnectionInterface::class); + $isConnection = false; + foreach ($callerType->getObjectClassReflections() as $classReflection) { + if ($classReflection->getName() === $interface->getName() || $classReflection->is($interface->getName())) { + $isConnection = true; + + break; + } + } + + if (!$isConnection) { + return []; + } + + $args = $node->getArgs(); + if (count($args) < 2) { + return []; + } + + $queryType = $scope->getType($args[0]->value); + $constantStrings = $queryType->getConstantStrings(); + if (count($constantStrings) !== 1) { + return []; + } + + $paramsType = $scope->getType($args[1]->value); + $arrays = $paramsType->getConstantArrays(); + if (count($arrays) !== 1) { + return []; + } + + $paramsCount = count($arrays[0]->getValueTypes()); + $placeholderCount = $this->countPlaceholders($constantStrings[0]->getValue()); + + if ($paramsCount === $placeholderCount) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf( + 'SQL has %d placeholder(s) but %d parameter(s) given to %s().', + $placeholderCount, + $paramsCount, + $methodName, + ))->identifier('artemeon.database.placeholderCount')->build(), + ]; + } + + private function countPlaceholders(string $query): int + { + $count = 0; + $length = strlen($query); + $inSingle = false; + $inDouble = false; + $inLineComment = false; + $inBlockComment = false; + + for ($i = 0; $i < $length; $i++) { + $char = $query[$i]; + $next = $i + 1 < $length ? $query[$i + 1] : ''; + + if ($inLineComment) { + if ($char === "\n") { + $inLineComment = false; + } + + continue; + } + + if ($inBlockComment) { + if ($char === '*' && $next === '/') { + $inBlockComment = false; + $i++; + } + + continue; + } + + if ($inSingle) { + if ($char === '\\' && $next !== '') { + $i++; + + continue; + } + if ($char === "'") { + if ($next === "'") { + $i++; + + continue; + } + $inSingle = false; + } + + continue; + } + + if ($inDouble) { + if ($char === '\\' && $next !== '') { + $i++; + + continue; + } + if ($char === '"') { + if ($next === '"') { + $i++; + + continue; + } + $inDouble = false; + } + + continue; + } + + if ($char === "'") { + $inSingle = true; + + continue; + } + + if ($char === '"') { + $inDouble = true; + + continue; + } + + if ($char === '-' && $next === '-') { + $inLineComment = true; + $i++; + + continue; + } + + if ($char === '/' && $next === '*') { + $inBlockComment = true; + $i++; + + continue; + } + + if ($char === '?') { + $count++; + } + } + + return $count; + } +} diff --git a/src/PHPStan/SqlReturnShapeAnalyser.php b/src/PHPStan/SqlReturnShapeAnalyser.php new file mode 100644 index 00000000..f199aff3 --- /dev/null +++ b/src/PHPStan/SqlReturnShapeAnalyser.php @@ -0,0 +1,159 @@ + + */ + private array $cache = []; + + public function analyseRow(string $query): ?Type + { + $key = md5($query); + if (array_key_exists($key, $this->cache)) { + return $this->cache[$key]; + } + + return $this->cache[$key] = $this->doAnalyseRow($query); + } + + private function doAnalyseRow(string $query): ?Type + { + try { + $parser = new Parser($query); + } catch (\Throwable) { + return null; + } + + if (count($parser->statements) !== 1) { + return null; + } + + $statement = $parser->statements[0]; + if (!$statement instanceof SelectStatement) { + return null; + } + + if ($statement->union !== []) { + return null; + } + + if ($statement->expr === null) { + return null; + } + + $builder = ConstantArrayTypeBuilder::createEmpty(); + foreach ($statement->expr as $expr) { + if (!$expr instanceof Expression) { + return null; + } + + $key = $this->keyFor($expr); + if ($key === null) { + return $this->fallbackRow(); + } + + $builder->setOffsetValueType( + new ConstantStringType($key), + new MixedType(), + ); + } + + return $builder->getArray(); + } + + private function keyFor(Expression $expr): ?string + { + if ($expr->alias !== null && $expr->alias !== '') { + return $this->unquote($expr->alias); + } + + if ($expr->column !== null && $expr->column !== '') { + if ($expr->column === '*') { + return null; + } + + return $this->unquote($expr->column); + } + + $rendered = trim((string) $expr->expr); + if ($rendered === '' || $rendered === '*') { + return null; + } + + return $this->stripWrappingParens($rendered); + } + + private function stripWrappingParens(string $expression): string + { + while (strlen($expression) >= 2 && $expression[0] === '(' && substr($expression, -1) === ')') { + $inner = trim(substr($expression, 1, -1)); + if ($inner === '' || !$this->parensBalancedAtTopLevel($inner)) { + break; + } + $expression = $inner; + } + + return $this->unquote($expression); + } + + private function parensBalancedAtTopLevel(string $expression): bool + { + $depth = 0; + $length = strlen($expression); + for ($i = 0; $i < $length; $i++) { + $char = $expression[$i]; + if ($char === '(') { + $depth++; + } elseif ($char === ')') { + $depth--; + if ($depth < 0) { + return false; + } + } + } + + return $depth === 0; + } + + private function unquote(string $identifier): string + { + $identifier = trim($identifier); + if ($identifier === '') { + return $identifier; + } + + $first = $identifier[0]; + $last = $identifier[strlen($identifier) - 1]; + if (($first === '`' && $last === '`') || ($first === '"' && $last === '"')) { + return substr($identifier, 1, -1); + } + + return $identifier; + } + + private function fallbackRow(): Type + { + return new ArrayType(new StringType(), new MixedType()); + } +} diff --git a/tests/PHPStan/PlaceholderCountRuleTest.php b/tests/PHPStan/PlaceholderCountRuleTest.php new file mode 100644 index 00000000..4133bba5 --- /dev/null +++ b/tests/PHPStan/PlaceholderCountRuleTest.php @@ -0,0 +1,35 @@ + + * @internal + */ +final class PlaceholderCountRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new PlaceholderCountRule($this->createReflectionProvider()); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/placeholder-count.php'], [ + [ + 'SQL has 2 placeholder(s) but 1 parameter(s) given to getPArray().', + 12, + ], + [ + 'SQL has 1 placeholder(s) but 0 parameter(s) given to getPRow().', + 13, + ], + ]); + } +} diff --git a/tests/PHPStan/SqlReturnShapeAnalyserTest.php b/tests/PHPStan/SqlReturnShapeAnalyserTest.php new file mode 100644 index 00000000..4c33348c --- /dev/null +++ b/tests/PHPStan/SqlReturnShapeAnalyserTest.php @@ -0,0 +1,74 @@ +analyseRow($sql); + if ($type === null) { + return []; + } + $arrays = $type->getConstantArrays(); + if (count($arrays) !== 1) { + return []; + } + + return array_map(static fn ($k) => $k->getValue(), $arrays[0]->getKeyTypes()); +}; + +it('infers shape from a simple column list', function () use ($keysOf): void { + expect($keysOf('SELECT id, name FROM users'))->toBe(['id', 'name']); +}); + +it('uses aliases as keys when present', function () use ($keysOf): void { + $keys = $keysOf('SELECT name AS user_name, COUNT(*) AS total FROM users GROUP BY name'); + expect($keys)->toContain('user_name')->toContain('total'); +}); + +it('strips backticks and double quotes from identifiers', function () use ($keysOf): void { + expect($keysOf('SELECT `id`, "name" FROM users'))->toBe(['id', 'name']); +}); + +it('lets later JOIN columns overwrite earlier ones with the same name', function () use ($keysOf): void { + expect($keysOf('SELECT a.id, b.id FROM a JOIN b ON a.id = b.id'))->toBe(['id']); +}); + +it('strips wrapping parentheses around column references', function () use ($keysOf): void { + expect($keysOf('SELECT DISTINCT(stats_handler) FROM cache'))->toBe(['stats_handler']); + expect($keysOf('SELECT (`name`) FROM users'))->toBe(['name']); +}); + +it('infers shape from GROUP BY queries with aggregates', function () use ($keysOf): void { + expect($keysOf('SELECT user_id, COUNT(*) AS total FROM orders GROUP BY user_id')) + ->toBe(['user_id', 'total']); + expect($keysOf('SELECT category, MAX(price) AS max_p, MIN(price) AS min_p FROM products GROUP BY category')) + ->toBe(['category', 'max_p', 'min_p']); + expect($keysOf('SELECT user_id, SUM(amount) AS total FROM o GROUP BY user_id HAVING SUM(amount) > 100')) + ->toBe(['user_id', 'total']); +}); + +it('falls back to array for SELECT *', function () use ($analyser): void { + $type = $analyser->analyseRow('SELECT * FROM users'); + expect($type)->not()->toBeNull(); + expect($type?->getConstantArrays())->toBe([]); + expect($type?->getIterableKeyType()->describe(\PHPStan\Type\VerbosityLevel::precise()))->toBe('string'); +}); + +it('returns null for non-SELECT statements', function () use ($analyser): void { + expect($analyser->analyseRow('INSERT INTO users (id) VALUES (1)'))->toBeNull(); + expect($analyser->analyseRow('UPDATE users SET id = 1'))->toBeNull(); + expect($analyser->analyseRow('DELETE FROM users'))->toBeNull(); +}); + +it('returns null for UNION queries', function () use ($analyser): void { + expect($analyser->analyseRow('SELECT id FROM a UNION SELECT id FROM b'))->toBeNull(); +}); + +it('returns null when SQL fails to parse cleanly', function () use ($analyser): void { + expect($analyser->analyseRow('not even sql'))->toBeNull(); +}); diff --git a/tests/PHPStan/data/placeholder-count.php b/tests/PHPStan/data/placeholder-count.php new file mode 100644 index 00000000..33075d21 --- /dev/null +++ b/tests/PHPStan/data/placeholder-count.php @@ -0,0 +1,15 @@ +getPArray('SELECT id FROM users WHERE a = ? AND b = ?', [1, 2]); + $conn->getPArray('SELECT id FROM users WHERE a = ? AND b = ?', [1]); + $conn->getPRow('SELECT id FROM users WHERE a = ?', []); + $conn->getPArray("SELECT id FROM users WHERE name = 'a?b' AND id = ?", [1]); +} diff --git a/tests/PHPStan/data/return-types.php b/tests/PHPStan/data/return-types.php new file mode 100644 index 00000000..756b93aa --- /dev/null +++ b/tests/PHPStan/data/return-types.php @@ -0,0 +1,41 @@ +getPArray('SELECT id, name FROM users'); + assertType('list', $rows); +} + +function selectRow(ConnectionInterface $conn): void +{ + $row = $conn->getPRow('SELECT id, name FROM users'); + assertType('array{}|array{id: mixed, name: mixed}', $row); +} + +function selectGenerator(ConnectionInterface $conn): \Generator +{ + $gen = $conn->getGenerator('SELECT id FROM users'); + assertType('Generator', $gen); + + return $gen; +} + +function selectStarFallsBack(ConnectionInterface $conn): void +{ + $rows = $conn->getPArray('SELECT * FROM users'); + assertType('list>', $rows); +} + +function dynamicSqlFallsBack(ConnectionInterface $conn, string $sql): void +{ + $rows = $conn->getPArray($sql); + assertType('array>', $rows); +} diff --git a/tests/PHPStan/phpstan-fixtures.neon b/tests/PHPStan/phpstan-fixtures.neon new file mode 100644 index 00000000..b3b3d9e8 --- /dev/null +++ b/tests/PHPStan/phpstan-fixtures.neon @@ -0,0 +1,7 @@ +includes: + - ../../phpstan-extension.neon + +parameters: + level: 9 + paths: + - data/return-types.php