From 5313bc680f572c2a41305e550b63d94ab5407b9b Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Wed, 20 May 2026 03:58:29 +0200 Subject: [PATCH 1/5] feat: Add PHPStan extension inferring SQL return shapes Adds a custom PHPStan extension that parses the literal SQL string passed to `Connection::getPArray()`, `getPRow()`, and `getGenerator()` and narrows the return type to a constant array shape derived from the SELECT list. Also adds a `PlaceholderCountRule` that flags mismatches between literal `?` placeholders and the literal `$params` array length. The extension is shipped inside this package and auto-registered for downstream consumers via `extra.phpstan.includes`, picked up by phpstan-extension-installer. Schema-agnostic: column shapes are inferred, all values typed as `mixed`. SELECT *, UNION, non-literal SQL, and non-SELECT statements gracefully fall back to the declared return type. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitignore | 2 + composer.json | 8 + phpstan-baseline.neon | 77 ++++++++ phpstan-extension.neon | 23 +++ phpstan.neon | 3 + .../GetGeneratorReturnTypeExtension.php | 56 ++++++ src/PHPStan/GetPArrayReturnTypeExtension.php | 57 ++++++ src/PHPStan/GetPRowReturnTypeExtension.php | 62 ++++++ src/PHPStan/PlaceholderCountRule.php | 181 ++++++++++++++++++ src/PHPStan/SqlReturnShapeAnalyser.php | 127 ++++++++++++ tests/PHPStan/PlaceholderCountRuleTest.php | 34 ++++ tests/PHPStan/SqlReturnShapeAnalyserTest.php | 59 ++++++ tests/PHPStan/data/placeholder-count.php | 15 ++ tests/PHPStan/data/return-types.php | 40 ++++ tests/PHPStan/phpstan-fixtures.neon | 7 + 15 files changed, 751 insertions(+) create mode 100644 phpstan-extension.neon create mode 100644 src/PHPStan/GetGeneratorReturnTypeExtension.php create mode 100644 src/PHPStan/GetPArrayReturnTypeExtension.php create mode 100644 src/PHPStan/GetPRowReturnTypeExtension.php create mode 100644 src/PHPStan/PlaceholderCountRule.php create mode 100644 src/PHPStan/SqlReturnShapeAnalyser.php create mode 100644 tests/PHPStan/PlaceholderCountRuleTest.php create mode 100644 tests/PHPStan/SqlReturnShapeAnalyserTest.php create mode 100644 tests/PHPStan/data/placeholder-count.php create mode 100644 tests/PHPStan/data/return-types.php create mode 100644 tests/PHPStan/phpstan-fixtures.neon 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..4e4a4135 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,2 +1,79 @@ parameters: ignoreErrors: + - + message: '#^Method Artemeon\\Database\\Connection\:\:dbsafeParams\(\) should return list\ but returns list\\.$#' + identifier: return.type + count: 1 + path: src/Connection.php + + - + message: '#^Parameter \#1 \$params of method Artemeon\\Database\\Connection\:\:dbsafeParams\(\) expects array\, list\ given\.$#' + identifier: argument.type + count: 9 + path: src/Connection.php + + - + message: '#^Offset ''cnt'' might not exist on array\{cnt\?\: mixed\}\.$#' + identifier: offsetAccess.notFound + count: 2 + path: tests/ConnectionMultiInsertTest.php + + - + message: '#^Offset ''val'' might not exist on 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\{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\{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\{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\{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..b0a1a771 --- /dev/null +++ b/src/PHPStan/GetPRowReturnTypeExtension.php @@ -0,0 +1,62 @@ +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; + } + + $constantArrays = $rowType->getConstantArrays(); + if (count($constantArrays) !== 1) { + return null; + } + + $constant = $constantArrays[0]; + $optional = ConstantArrayTypeBuilder::createEmpty(); + foreach ($constant->getKeyTypes() as $i => $keyType) { + $optional->setOffsetValueType($keyType, $constant->getValueTypes()[$i], true); + } + + return $optional->getArray(); + } +} diff --git a/src/PHPStan/PlaceholderCountRule.php b/src/PHPStan/PlaceholderCountRule.php new file mode 100644 index 00000000..6d17e0b5 --- /dev/null +++ b/src/PHPStan/PlaceholderCountRule.php @@ -0,0 +1,181 @@ + + */ +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..75db9e2e --- /dev/null +++ b/src/PHPStan/SqlReturnShapeAnalyser.php @@ -0,0 +1,127 @@ + + */ + 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 $rendered; + } + + 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..c7c499c9 --- /dev/null +++ b/tests/PHPStan/PlaceholderCountRuleTest.php @@ -0,0 +1,34 @@ + + */ +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..4eb69aaf --- /dev/null +++ b/tests/PHPStan/SqlReturnShapeAnalyserTest.php @@ -0,0 +1,59 @@ +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('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..8a1aedf9 --- /dev/null +++ b/tests/PHPStan/data/return-types.php @@ -0,0 +1,40 @@ +getPArray('SELECT id, name FROM users'); + assertType('list', $rows); +} + +function selectRow(ConnectionInterface $conn): void +{ + $row = $conn->getPRow('SELECT id, name FROM users'); + assertType('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 From 1c96739054015b211ce3f9aa2ab42acb572d1d7f Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Wed, 20 May 2026 04:23:16 +0200 Subject: [PATCH 2/5] fix: Pint formatting and PHPStan baseline drift Auto-applied Pint formatting (blank lines before continue/return, @internal annotation on RuleTestCase, blank line between import groups). Removed two stale Connection.php baseline entries that PHPStan on CI doesn't report, which were failing the baseline-match check. Co-Authored-By: Claude Opus 4.7 (1M context) --- phpstan-baseline.neon | 12 ------------ src/PHPStan/PlaceholderCountRule.php | 13 +++++++++++++ tests/PHPStan/PlaceholderCountRuleTest.php | 1 + tests/PHPStan/SqlReturnShapeAnalyserTest.php | 1 + tests/PHPStan/data/return-types.php | 3 ++- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4e4a4135..7687db2d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,17 +1,5 @@ parameters: ignoreErrors: - - - message: '#^Method Artemeon\\Database\\Connection\:\:dbsafeParams\(\) should return list\ but returns list\\.$#' - identifier: return.type - count: 1 - path: src/Connection.php - - - - message: '#^Parameter \#1 \$params of method Artemeon\\Database\\Connection\:\:dbsafeParams\(\) expects array\, list\ given\.$#' - identifier: argument.type - count: 9 - path: src/Connection.php - - message: '#^Offset ''cnt'' might not exist on array\{cnt\?\: mixed\}\.$#' identifier: offsetAccess.notFound diff --git a/src/PHPStan/PlaceholderCountRule.php b/src/PHPStan/PlaceholderCountRule.php index 6d17e0b5..e6ea1fed 100644 --- a/src/PHPStan/PlaceholderCountRule.php +++ b/src/PHPStan/PlaceholderCountRule.php @@ -49,6 +49,7 @@ public function processNode(Node $node, Scope $scope): array foreach ($callerType->getObjectClassReflections() as $classReflection) { if ($classReflection->getName() === $interface->getName() || $classReflection->is($interface->getName())) { $isConnection = true; + break; } } @@ -108,6 +109,7 @@ private function countPlaceholders(string $query): int if ($char === "\n") { $inLineComment = false; } + continue; } @@ -116,58 +118,69 @@ private function countPlaceholders(string $query): int $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; } diff --git a/tests/PHPStan/PlaceholderCountRuleTest.php b/tests/PHPStan/PlaceholderCountRuleTest.php index c7c499c9..4133bba5 100644 --- a/tests/PHPStan/PlaceholderCountRuleTest.php +++ b/tests/PHPStan/PlaceholderCountRuleTest.php @@ -10,6 +10,7 @@ /** * @extends RuleTestCase + * @internal */ final class PlaceholderCountRuleTest extends RuleTestCase { diff --git a/tests/PHPStan/SqlReturnShapeAnalyserTest.php b/tests/PHPStan/SqlReturnShapeAnalyserTest.php index 4eb69aaf..35089e4e 100644 --- a/tests/PHPStan/SqlReturnShapeAnalyserTest.php +++ b/tests/PHPStan/SqlReturnShapeAnalyserTest.php @@ -17,6 +17,7 @@ if (count($arrays) !== 1) { return []; } + return array_map(static fn ($k) => $k->getValue(), $arrays[0]->getKeyTypes()); }; diff --git a/tests/PHPStan/data/return-types.php b/tests/PHPStan/data/return-types.php index 8a1aedf9..489ba6c7 100644 --- a/tests/PHPStan/data/return-types.php +++ b/tests/PHPStan/data/return-types.php @@ -5,6 +5,7 @@ namespace Artemeon\Database\Tests\PHPStan\Data; use Artemeon\Database\ConnectionInterface; + use function PHPStan\Testing\assertType; function selectList(ConnectionInterface $conn): void @@ -23,6 +24,7 @@ function selectGenerator(ConnectionInterface $conn): \Generator { $gen = $conn->getGenerator('SELECT id FROM users'); assertType('Generator', $gen); + return $gen; } @@ -37,4 +39,3 @@ function dynamicSqlFallsBack(ConnectionInterface $conn, string $sql): void $rows = $conn->getPArray($sql); assertType('array>', $rows); } - From 01e3214360bf877c509b58acf99716f48fa5d415 Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Wed, 20 May 2026 04:32:04 +0200 Subject: [PATCH 3/5] fix: Model getPRow return type as union of empty + all-required Previously getPRow returned `array{k1?: mixed, k2?: mixed}` (each key independently optional). After narrowing away the empty case with `if ($row === []) return null;`, PHPStan left every key still optional, making downstream calls that need all keys present (e.g. Token::fromRow) fail typing. Now returns `array{}|array{k1: mixed, k2: mixed, ...}` so the empty check narrows cleanly to the all-required shape. Co-Authored-By: Claude Opus 4.7 (1M context) --- phpstan-baseline.neon | 12 ++++++------ src/PHPStan/GetPRowReturnTypeExtension.php | 14 +++++--------- tests/PHPStan/data/return-types.php | 2 +- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7687db2d..65565330 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,13 +1,13 @@ parameters: ignoreErrors: - - message: '#^Offset ''cnt'' might not exist on array\{cnt\?\: mixed\}\.$#' + 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\{val\?\: mixed\}\.$#' + message: '#^Offset ''val'' might not exist on array\{\}\|array\{val\: mixed\}\.$#' identifier: offsetAccess.notFound count: 2 path: tests/ConnectionRoundTest.php @@ -25,13 +25,13 @@ parameters: path: tests/ConnectionTest.php - - message: '#^Offset ''temp_bigint_new'' might not exist on array\{temp_id\?\: mixed, temp_bigint_new\?\: mixed\}\.$#' + 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\{temp_id\?\: mixed, temp_bigint_new\?\: mixed\}\.$#' + 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 @@ -43,7 +43,7 @@ parameters: path: tests/ConnectionTest.php - - message: '#^Offset ''cnt'' might not exist on array\{cnt\?\: mixed\}\.$#' + message: '#^Offset ''cnt'' might not exist on array\{\}\|array\{cnt\: mixed\}\.$#' identifier: offsetAccess.notFound count: 2 path: tests/ConnectionTxTest.php @@ -61,7 +61,7 @@ parameters: path: tests/ConnectionUpsertTest.php - - message: '#^Offset ''cnt'' might not exist on array\{cnt\?\: mixed\}\.$#' + message: '#^Offset ''cnt'' might not exist on array\{\}\|array\{cnt\: mixed\}\.$#' identifier: offsetAccess.notFound count: 1 path: tests/ConnectionUpsertTest.php diff --git a/src/PHPStan/GetPRowReturnTypeExtension.php b/src/PHPStan/GetPRowReturnTypeExtension.php index b0a1a771..79d448df 100644 --- a/src/PHPStan/GetPRowReturnTypeExtension.php +++ b/src/PHPStan/GetPRowReturnTypeExtension.php @@ -11,6 +11,7 @@ use PHPStan\Type\Constant\ConstantArrayTypeBuilder; use PHPStan\Type\DynamicMethodReturnTypeExtension; use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; final class GetPRowReturnTypeExtension implements DynamicMethodReturnTypeExtension { @@ -46,17 +47,12 @@ public function getTypeFromMethodCall(MethodReflection $methodReflection, Method return null; } - $constantArrays = $rowType->getConstantArrays(); - if (count($constantArrays) !== 1) { - return null; + if ($rowType->getConstantArrays() === []) { + return $rowType; } - $constant = $constantArrays[0]; - $optional = ConstantArrayTypeBuilder::createEmpty(); - foreach ($constant->getKeyTypes() as $i => $keyType) { - $optional->setOffsetValueType($keyType, $constant->getValueTypes()[$i], true); - } + $empty = ConstantArrayTypeBuilder::createEmpty()->getArray(); - return $optional->getArray(); + return TypeCombinator::union($rowType, $empty); } } diff --git a/tests/PHPStan/data/return-types.php b/tests/PHPStan/data/return-types.php index 489ba6c7..756b93aa 100644 --- a/tests/PHPStan/data/return-types.php +++ b/tests/PHPStan/data/return-types.php @@ -17,7 +17,7 @@ function selectList(ConnectionInterface $conn): void function selectRow(ConnectionInterface $conn): void { $row = $conn->getPRow('SELECT id, name FROM users'); - assertType('array{id?: mixed, name?: mixed}', $row); + assertType('array{}|array{id: mixed, name: mixed}', $row); } function selectGenerator(ConnectionInterface $conn): \Generator From 3d00b4f49d89e73ed055642b199dbde76f2c88cb Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Wed, 20 May 2026 04:41:34 +0200 Subject: [PATCH 4/5] fix: Strip wrapping parens when deriving column key from SELECT expr `SELECT DISTINCT(col) FROM t` is parsed as DISTINCT modifier + the expression `(col)`, so the key fell back to the verbatim text `(col)` instead of `col`. Strip matched outer parentheses (validated to be balanced) and unquote the inner identifier before using it. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/PHPStan/SqlReturnShapeAnalyser.php | 34 +++++++++++++++++++- tests/PHPStan/SqlReturnShapeAnalyserTest.php | 5 +++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/PHPStan/SqlReturnShapeAnalyser.php b/src/PHPStan/SqlReturnShapeAnalyser.php index 75db9e2e..f199aff3 100644 --- a/src/PHPStan/SqlReturnShapeAnalyser.php +++ b/src/PHPStan/SqlReturnShapeAnalyser.php @@ -101,7 +101,39 @@ private function keyFor(Expression $expr): ?string return null; } - return $rendered; + 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 diff --git a/tests/PHPStan/SqlReturnShapeAnalyserTest.php b/tests/PHPStan/SqlReturnShapeAnalyserTest.php index 35089e4e..dc0a363f 100644 --- a/tests/PHPStan/SqlReturnShapeAnalyserTest.php +++ b/tests/PHPStan/SqlReturnShapeAnalyserTest.php @@ -38,6 +38,11 @@ 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('falls back to array for SELECT *', function () use ($analyser): void { $type = $analyser->analyseRow('SELECT * FROM users'); expect($type)->not()->toBeNull(); From c35195a3d2b0b5113cb74a067b02013730cc4317 Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Mon, 25 May 2026 14:32:53 +0200 Subject: [PATCH 5/5] test: Lock in GROUP BY return-shape inference Adds a regression test verifying that GROUP BY queries with aggregates (COUNT, MAX, MIN, SUM), with and without HAVING clauses, yield the expected key list. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/PHPStan/SqlReturnShapeAnalyserTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/PHPStan/SqlReturnShapeAnalyserTest.php b/tests/PHPStan/SqlReturnShapeAnalyserTest.php index dc0a363f..4c33348c 100644 --- a/tests/PHPStan/SqlReturnShapeAnalyserTest.php +++ b/tests/PHPStan/SqlReturnShapeAnalyserTest.php @@ -43,6 +43,15 @@ 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();