diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 1163db1d93015..160cb0a31b219 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3417,9 +3417,6 @@ - - - diff --git a/build/psalm/ITypedQueryBuilderTest.php b/build/psalm/ITypedQueryBuilderTest.php new file mode 100644 index 0000000000000..0967f83106de4 --- /dev/null +++ b/build/psalm/ITypedQueryBuilderTest.php @@ -0,0 +1,37 @@ +getTypedQueryBuilder(); + +$qb->selectColumns('a', 'b'); +$qb->selectColumns('c'); + +$qb->selectColumnDistinct('d'); +$qb->selectColumnDistinct('e'); + +$qb->selectAlias('f', 'g'); +$qb->selectAlias($qb->func()->lower('h'), 'i'); + +/** @psalm-check-type-exact $result = \OCP\DB\IResult<'a'|'b'|'c'|'d'|'e'|'g'|'i'> */ +$result = $qb->executeQuery(); + +/** @psalm-check-type-exact $rows = array<'a'|'b'|'c'|'d'|'e'|'g'|'i', mixed>|false */ +$rows = $result->fetch(\PDO::FETCH_ASSOC); + +/** @psalm-check-type-exact $rows = array<'a'|'b'|'c'|'d'|'e'|'g'|'i', mixed>|false */ +$rows = $result->fetchAssociative(); + +/** @psalm-check-type-exact $rows = list> */ +$rows = $result->fetchAll(\PDO::FETCH_ASSOC); + +/** @psalm-check-type-exact $rows = list> */ +$rows = $result->fetchAllAssociative(); diff --git a/build/rector-strict.php b/build/rector-strict.php index 529eace3b6743..d6a39afe5a447 100644 --- a/build/rector-strict.php +++ b/build/rector-strict.php @@ -12,6 +12,9 @@ $nextcloudDir . '/build/rector-strict.php', $nextcloudDir . '/core/BackgroundJobs/ExpirePreviewsJob.php', $nextcloudDir . '/lib/public/IContainer.php', + $nextcloudDir . '/build/psalm/ITypedQueryBuilderTest.php', + $nextcloudDir . '/lib/private/DB/QueryBuilder/TypedQueryBuilder.php', + $nextcloudDir . '/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php', ]) ->withPreparedSets( deadCode: true, diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index b2008b286ca25..5b6de5ff356d1 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -327,6 +327,7 @@ 'OCP\\DB\\QueryBuilder\\IParameter' => $baseDir . '/lib/public/DB/QueryBuilder/IParameter.php', 'OCP\\DB\\QueryBuilder\\IQueryBuilder' => $baseDir . '/lib/public/DB/QueryBuilder/IQueryBuilder.php', 'OCP\\DB\\QueryBuilder\\IQueryFunction' => $baseDir . '/lib/public/DB/QueryBuilder/IQueryFunction.php', + 'OCP\\DB\\QueryBuilder\\ITypedQueryBuilder' => $baseDir . '/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php', 'OCP\\DB\\QueryBuilder\\Sharded\\IShardMapper' => $baseDir . '/lib/public/DB/QueryBuilder/Sharded/IShardMapper.php', 'OCP\\DB\\Types' => $baseDir . '/lib/public/DB/Types.php', 'OCP\\Dashboard\\IAPIWidget' => $baseDir . '/lib/public/Dashboard/IAPIWidget.php', @@ -1657,6 +1658,7 @@ 'OC\\DB\\QueryBuilder\\Sharded\\ShardDefinition' => $baseDir . '/lib/private/DB/QueryBuilder/Sharded/ShardDefinition.php', 'OC\\DB\\QueryBuilder\\Sharded\\ShardQueryRunner' => $baseDir . '/lib/private/DB/QueryBuilder/Sharded/ShardQueryRunner.php', 'OC\\DB\\QueryBuilder\\Sharded\\ShardedQueryBuilder' => $baseDir . '/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php', + 'OC\\DB\\QueryBuilder\\TypedQueryBuilder' => $baseDir . '/lib/private/DB/QueryBuilder/TypedQueryBuilder.php', 'OC\\DB\\ResultAdapter' => $baseDir . '/lib/private/DB/ResultAdapter.php', 'OC\\DB\\SQLiteMigrator' => $baseDir . '/lib/private/DB/SQLiteMigrator.php', 'OC\\DB\\SQLiteSessionInit' => $baseDir . '/lib/private/DB/SQLiteSessionInit.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index fb4a73103f5b3..fbee07dafc6b4 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -368,6 +368,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\DB\\QueryBuilder\\IParameter' => __DIR__ . '/../../..' . '/lib/public/DB/QueryBuilder/IParameter.php', 'OCP\\DB\\QueryBuilder\\IQueryBuilder' => __DIR__ . '/../../..' . '/lib/public/DB/QueryBuilder/IQueryBuilder.php', 'OCP\\DB\\QueryBuilder\\IQueryFunction' => __DIR__ . '/../../..' . '/lib/public/DB/QueryBuilder/IQueryFunction.php', + 'OCP\\DB\\QueryBuilder\\ITypedQueryBuilder' => __DIR__ . '/../../..' . '/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php', 'OCP\\DB\\QueryBuilder\\Sharded\\IShardMapper' => __DIR__ . '/../../..' . '/lib/public/DB/QueryBuilder/Sharded/IShardMapper.php', 'OCP\\DB\\Types' => __DIR__ . '/../../..' . '/lib/public/DB/Types.php', 'OCP\\Dashboard\\IAPIWidget' => __DIR__ . '/../../..' . '/lib/public/Dashboard/IAPIWidget.php', @@ -1698,6 +1699,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\DB\\QueryBuilder\\Sharded\\ShardDefinition' => __DIR__ . '/../../..' . '/lib/private/DB/QueryBuilder/Sharded/ShardDefinition.php', 'OC\\DB\\QueryBuilder\\Sharded\\ShardQueryRunner' => __DIR__ . '/../../..' . '/lib/private/DB/QueryBuilder/Sharded/ShardQueryRunner.php', 'OC\\DB\\QueryBuilder\\Sharded\\ShardedQueryBuilder' => __DIR__ . '/../../..' . '/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php', + 'OC\\DB\\QueryBuilder\\TypedQueryBuilder' => __DIR__ . '/../../..' . '/lib/private/DB/QueryBuilder/TypedQueryBuilder.php', 'OC\\DB\\ResultAdapter' => __DIR__ . '/../../..' . '/lib/private/DB/ResultAdapter.php', 'OC\\DB\\SQLiteMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/SQLiteMigrator.php', 'OC\\DB\\SQLiteSessionInit' => __DIR__ . '/../../..' . '/lib/private/DB/SQLiteSessionInit.php', diff --git a/lib/private/DB/ArrayResult.php b/lib/private/DB/ArrayResult.php index 614d4f77322ae..251000e29ae3a 100644 --- a/lib/private/DB/ArrayResult.php +++ b/lib/private/DB/ArrayResult.php @@ -14,6 +14,8 @@ /** * Wrap an array or rows into a result interface + * + * @template-implements IResult */ class ArrayResult implements IResult { protected int $count; diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 081ef017aa301..3eef60c6dafb7 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -34,6 +34,7 @@ use OC\DB\QueryBuilder\Sharded\ShardDefinition; use OC\SystemConfig; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\DB\QueryBuilder\ITypedQueryBuilder; use OCP\DB\QueryBuilder\Sharded\IShardMapper; use OCP\Diagnostics\IEventLogger; use OCP\EventDispatcher\IEventDispatcher; @@ -247,6 +248,14 @@ public function getStats(): array { * Returns a QueryBuilder for the connection. */ public function getQueryBuilder(): IQueryBuilder { + return $this->getInnerQueryBuilder(); + } + + public function getTypedQueryBuilder(): ITypedQueryBuilder { + return $this->getInnerQueryBuilder(); + } + + private function getInnerQueryBuilder(): IQueryBuilder&ITypedQueryBuilder { $this->queriesBuilt++; $builder = new QueryBuilder( diff --git a/lib/private/DB/ConnectionAdapter.php b/lib/private/DB/ConnectionAdapter.php index 60b549dabcf2d..d9e3e7ec5490b 100644 --- a/lib/private/DB/ConnectionAdapter.php +++ b/lib/private/DB/ConnectionAdapter.php @@ -17,6 +17,7 @@ use OCP\DB\IPreparedStatement; use OCP\DB\IResult; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\DB\QueryBuilder\ITypedQueryBuilder; use OCP\IDBConnection; /** @@ -32,6 +33,10 @@ public function getQueryBuilder(): IQueryBuilder { return $this->inner->getQueryBuilder(); } + public function getTypedQueryBuilder(): ITypedQueryBuilder { + return $this->inner->getTypedQueryBuilder(); + } + public function prepare($sql, $limit = null, $offset = null): IPreparedStatement { try { return new PreparedStatement( diff --git a/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php b/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php index c7eda9b337c7b..33901ace1d4ee 100644 --- a/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/ExtendedQueryBuilder.php @@ -16,7 +16,7 @@ /** * Base class for creating classes that extend the builtin query builder */ -abstract class ExtendedQueryBuilder implements IQueryBuilder { +abstract class ExtendedQueryBuilder extends TypedQueryBuilder { public function __construct( protected IQueryBuilder $builder, ) { @@ -100,7 +100,7 @@ public function select(...$selects) { return $this; } - public function selectAlias($select, $alias) { + public function selectAlias($select, $alias): self { $this->builder->selectAlias($select, $alias); return $this; } diff --git a/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php b/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php index 7750ce5056a8d..2a9c2eb9124bc 100644 --- a/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php +++ b/lib/private/DB/QueryBuilder/Partitioned/PartitionedQueryBuilder.php @@ -92,7 +92,7 @@ public function addSelect(...$select) { return $this; } - public function selectAlias($select, $alias) { + public function selectAlias($select, $alias): self { $this->selects[] = ['select' => $select, 'alias' => $alias]; return $this; } diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index faf196e241c5f..1129970265c66 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -30,8 +30,9 @@ use OCP\IDBConnection; use Override; use Psr\Log\LoggerInterface; +use RuntimeException; -class QueryBuilder implements IQueryBuilder { +class QueryBuilder extends TypedQueryBuilder { private \Doctrine\DBAL\Query\QueryBuilder $queryBuilder; private QuoteHelper $helper; private bool $automaticTablePrefix = true; @@ -242,7 +243,7 @@ private function prepareForExecute() { public function executeQuery(?IDBConnection $connection = null): IResult { if ($this->getType() !== \Doctrine\DBAL\Query\QueryBuilder::SELECT) { - throw new \RuntimeException('Invalid query type, expected SELECT query'); + throw new RuntimeException('Invalid query type, expected SELECT query'); } $this->prepareForExecute(); @@ -259,7 +260,7 @@ public function executeQuery(?IDBConnection $connection = null): IResult { public function executeStatement(?IDBConnection $connection = null): int { if ($this->getType() === \Doctrine\DBAL\Query\QueryBuilder::SELECT) { - throw new \RuntimeException('Invalid query type, expected INSERT, DELETE or UPDATE statement'); + throw new RuntimeException('Invalid query type, expected INSERT, DELETE or UPDATE statement'); } $this->prepareForExecute(); @@ -476,7 +477,7 @@ public function select(...$selects) { * * @return $this This QueryBuilder instance. */ - public function selectAlias($select, $alias) { + public function selectAlias($select, $alias): self { $this->queryBuilder->addSelect( $this->helper->quoteColumnName($select) . ' AS ' . $this->helper->quoteColumnName($alias) ); @@ -524,18 +525,18 @@ public function selectDistinct($select) { * ->leftJoin('u', 'phonenumbers', 'u.id = p.user_id'); * * - * @param mixed ...$selects The selection expression. + * @param mixed ...$select The selection expression. * * @return $this This QueryBuilder instance. */ - public function addSelect(...$selects) { - if (count($selects) === 1 && is_array($selects[0])) { - $selects = $selects[0]; + public function addSelect(...$select) { + if (count($select) === 1 && is_array($select[0])) { + $select = $select[0]; } - $this->addOutputColumns($selects); + $this->addOutputColumns($select); $this->queryBuilder->addSelect( - $this->helper->quoteColumnNames($selects) + $this->helper->quoteColumnNames($select) ); return $this; diff --git a/lib/private/DB/QueryBuilder/TypedQueryBuilder.php b/lib/private/DB/QueryBuilder/TypedQueryBuilder.php new file mode 100644 index 0000000000000..385550f4ea261 --- /dev/null +++ b/lib/private/DB/QueryBuilder/TypedQueryBuilder.php @@ -0,0 +1,40 @@ + + */ +abstract class TypedQueryBuilder implements ITypedQueryBuilder { + private function validateColumn(string $column): void { + if (str_contains($column, '.') || trim($column) === '*') { + throw new RuntimeException('Only column names are allowed, got: ' . $column); + } + } + + public function selectColumns(string ...$columns): static { + foreach ($columns as $column) { + $this->validateColumn($column); + } + + /** @psalm-suppress InternalMethod */ + return $this->select(...$columns); + } + + public function selectColumnDistinct(string $column): static { + $this->validateColumn($column); + + /** @psalm-suppress InternalMethod */ + return $this->selectDistinct($column); + } +} diff --git a/lib/private/DB/ResultAdapter.php b/lib/private/DB/ResultAdapter.php index 0f365b6732189..b403e78673eb2 100644 --- a/lib/private/DB/ResultAdapter.php +++ b/lib/private/DB/ResultAdapter.php @@ -15,6 +15,8 @@ /** * Adapts DBAL 2.6 API for DBAL 3.x for backwards compatibility of a leaked type + * + * @template-implements IResult */ class ResultAdapter implements IResult { public function __construct( diff --git a/lib/public/DB/IResult.php b/lib/public/DB/IResult.php index 4f7e048907c3f..daca3be31bab7 100644 --- a/lib/public/DB/IResult.php +++ b/lib/public/DB/IResult.php @@ -27,6 +27,7 @@ * } * ``` * + * @template-covariant S of string * @since 21.0.0 */ #[Consumable(since: '21.0.0')] @@ -41,7 +42,7 @@ public function closeCursor(): bool; /** * @param PDO::FETCH_* $fetchMode * - * @return ($fetchMode is PDO::FETCH_ASSOC ? array : ($fetchMode is PDO::FETCH_NUM ? list : mixed))|false + * @return ($fetchMode is PDO::FETCH_ASSOC ? array : ($fetchMode is PDO::FETCH_NUM ? list : mixed))|false * * @since 21.0.0 * @note Since 33.0.0, prefer using fetchAssociative/fetchNumeric/fetchOne or iterateAssociate/iterateNumeric instead. @@ -51,7 +52,7 @@ public function fetch(int $fetchMode = PDO::FETCH_ASSOC); /** * Returns the next row of the result as an associative array or FALSE if there are no more rows. * - * @return array|false + * @return array|false * * @since 33.0.0 */ @@ -78,7 +79,7 @@ public function fetchOne(); /** * @param PDO::FETCH_* $fetchMode * - * @return list<($fetchMode is PDO::FETCH_ASSOC ? array : ($fetchMode is PDO::FETCH_NUM ? list : mixed))> + * @return list<($fetchMode is PDO::FETCH_ASSOC ? array : ($fetchMode is PDO::FETCH_NUM ? list : mixed))> * * @since 21.0.0 * @note Since 33.0.0, prefer using fetchAllAssociative/fetchAllNumeric/fetchFirstColumn or iterateAssociate/iterateNumeric instead. @@ -88,7 +89,7 @@ public function fetchAll(int $fetchMode = PDO::FETCH_ASSOC): array; /** * Returns an array containing all the result rows represented as associative arrays. * - * @return list> + * @return list> * @since 33.0.0 */ public function fetchAllAssociative(): array; @@ -136,7 +137,7 @@ public function iterateNumeric(): Traversable; /** * Returns an iterator over rows represented as associative arrays. * - * @return Traversable> + * @return Traversable> * * @since 33.0.0 */ diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index 34e23b17ed270..32a505292c20e 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -386,7 +386,7 @@ public function select(...$selects); * @psalm-taint-sink sql $select * @psalm-taint-sink sql $alias */ - public function selectAlias($select, $alias); + public function selectAlias($select, $alias): self; /** * Specifies an item that is to be returned uniquely in the query result. diff --git a/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php b/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php new file mode 100644 index 0000000000000..cc5d852514d4c --- /dev/null +++ b/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php @@ -0,0 +1,76 @@ + + */ + #[Override] + public function executeQuery(?IDBConnection $connection = null): IResult; + + /** + * @inheritDoc + * @internal This method does not work with {@see self}. Use {@see self::selectColumns()} or {@see self::selectAlias()} instead. + */ + #[Override] + public function select(...$selects); + + /** + * @template NewS of string + * @param NewS ...$columns The columns to select. They are not allowed to contain table or alias prefixes.Use {@see self::selectAlias()} for that. + * @psalm-this-out self + * @since 34.0.0 + */ + public function selectColumns(string ...$columns): self; + + /** + * @inheritDoc + * @internal This method does not work with {@see self}. Use {@see self::selectColumnDistinct()} or {@see self::selectAlias()} instead. + */ + #[Override] + public function selectDistinct($select); + + /** + * @template NewS of string + * @param NewS $column The column to select distinct. It is are not allowed to contain table or alias prefixes. Use {@see self::selectAlias()} for that. + * @psalm-this-out self + * @since 34.0.0 + */ + public function selectColumnDistinct(string $column): self; + + /** + * @inheritDoc + * @internal This method does not work with {@see self}. Use {@see self::selectColumns()} or {@see self::selectAlias()} instead. + */ + #[Override] + public function addSelect(...$select); + + /** + * @inheritDoc + * @param mixed $select + * @template NewS of string + * @param NewS $alias + * @psalm-this-out self + * @psalm-suppress LessSpecificImplementedReturnType + */ + #[Override] + public function selectAlias($select, $alias): self; +} diff --git a/lib/public/IDBConnection.php b/lib/public/IDBConnection.php index 05ac0da2d7a2e..68fd174ce7aec 100644 --- a/lib/public/IDBConnection.php +++ b/lib/public/IDBConnection.php @@ -17,6 +17,7 @@ use OCP\DB\IPreparedStatement; use OCP\DB\IResult; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\DB\QueryBuilder\ITypedQueryBuilder; /** * Interface IDBConnection @@ -57,6 +58,13 @@ interface IDBConnection { */ public function getQueryBuilder(); + /** + * Gets the ITypedQueryBuilder for the connection. + * + * @since 34.0.0 + */ + public function getTypedQueryBuilder(): ITypedQueryBuilder; + /** * Used to abstract the Nextcloud database access away * @param string $sql the sql query with ? placeholder for params diff --git a/psalm-strict.xml b/psalm-strict.xml index 900470bcbab17..03290626ca53d 100644 --- a/psalm-strict.xml +++ b/psalm-strict.xml @@ -18,6 +18,9 @@ + + + @@ -29,4 +32,11 @@ + + + + + + +