refactor(QueryBuilder): Properly type the query builder by CarlSchwan · Pull Request #57763 · nextcloud/server

CarlSchwan · 2026-01-23T15:57:24Z

Resolves: #

Summary

TODO

...

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)
Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
Milestone added for target branch/version (ex: 32.x for stable32)

lib/public/DB/QueryBuilder/IQueryBuilder.php

lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php

come-nc · 2026-01-27T10:48:11Z

lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php

+		parent::innerJoin($fromAlias, $join, $alias, $condition);
+		return $this;


I think if you properly type the methods with @return $this in phpdoc you do not need to split all these return parent::… into 2 lines, as $this of self and parent is the same.

doesn't work :/

lib/public/DB/QueryBuilder/IFunctionBuilder.php

lib/public/DB/QueryBuilder/IQueryBuilder.php

And make sure the related unit tests are also typed and checked by psalm. Signed-off-by: Carl Schwan <carlschwan@kde.org>

provokateurin

Please add all files you touched to psalm:strict and rector:strict. That will probably result in some more issues being found, but I think it's worth fixing them in such an important part of our codebase.

provokateurin · 2026-02-17T12:19:07Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

@@ -326,17 +325,17 @@ public function setFirstResult($firstResult);
 	 * @return int The position of the first result.
 	 * @since 8.2.0
 	 */
-	public function getFirstResult();
+	public function getFirstResult(): int;

 	/**
 	 * Sets the maximum number of results to retrieve (the "limit").
 	 *
 	 * @param int|null $maxResults The maximum number of results to retrieve.


Suggested change

* @param int|null $maxResults The maximum number of results to retrieve.

* @param positive-int|null $maxResults The maximum number of results to retrieve.

provokateurin · 2026-02-17T12:19:48Z

lib/public/DB/QueryBuilder/IQueryBuilder.php


 	/**
 	 * Gets all defined query parameter types for the query being constructed indexed by parameter index or name.
 	 *
-	 * @return array The currently defined query parameter types indexed by parameter index or name.
+	 * @return list<self::PARAM_*> The currently defined query parameter types indexed by parameter index or name.


Suggested change

* @return list<self::PARAM_*> The currently defined query parameter types indexed by parameter index or name.

* @return array<string|int, self::PARAM_*> The currently defined query parameter types indexed by parameter index or name.

provokateurin · 2026-02-17T12:20:22Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

@@ -267,47 +266,47 @@ public function setParameter($key, $value, $type = null);
 	 *
 	 * @param array $params The query parameters to set.
 	 * @param array $types The query parameters types to set.


Suggested change

* @param array $types The query parameters types to set.

* @param array<string|int, mixed> $params The query parameters to set.

* @param array<string|int, self::PARAM_*> $types The query parameters types to set.

provokateurin · 2026-02-17T12:20:45Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

Suggested change

* @param non-negative-int $firstResult The first result to return.

provokateurin · 2026-02-17T12:21:03Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

@@ -345,7 +344,7 @@ public function setMaxResults($maxResults);
 	 * @return int|null The maximum number of results.


Suggested change

* @return int|null The maximum number of results.

* @return positive-int|null The maximum number of results.

provokateurin · 2026-02-17T12:23:00Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

 	 * @return $this This QueryBuilder instance.
 	 *
 	 * @see where()
 	 * @since 8.2.0
 	 *
 	 * @psalm-taint-sink sql $where
 	 */
-	public function orWhere(...$where);
+	public function orWhere(...$where): self;


Suggested change

public function orWhere(...$where): self;

public function orWhere(mixed ...$where): self;

provokateurin · 2026-02-17T12:23:07Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

 	 * @since 8.2.0
 	 *
 	 * @psalm-taint-sink sql $groupBys
 	 */
-	public function groupBy(...$groupBys);
+	public function groupBy(...$groupBys): self;


Suggested change

public function groupBy(...$groupBys): self;

public function groupBy(mixed ...$groupBys): self;

provokateurin · 2026-02-17T12:23:14Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

 	 * @since 8.2.0
 	 *
 	 * @psalm-taint-sink sql $groupby
 	 */
-	public function addGroupBy(...$groupBy);
+	public function addGroupBy(...$groupBy): self;


Suggested change

public function addGroupBy(...$groupBy): self;

public function addGroupBy(mixed ...$groupBy): self;

provokateurin · 2026-02-17T12:23:27Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

 	 * @since 8.2.0
 	 *
 	 * @psalm-taint-sink sql $having
 	 */
-	public function having(...$having);
+	public function having(...$having): self;


Suggested change

public function having(...$having): self;

public function having(mixed ...$having): self;

provokateurin · 2026-02-17T12:24:35Z

lib/public/DB/QueryBuilder/IQueryBuilder.php

mixed should be replaced by string|IParameter|IQueryFunction|ILiteral in almost all places.
Also non-empty-string could be used on most methods, but it's not that important.

CarlSchwan added this to the Nextcloud 34 milestone Jan 23, 2026

CarlSchwan self-assigned this Jan 23, 2026

CarlSchwan added 2. developing Work in progress feature: database Database related DB labels Jan 23, 2026

github-advanced-security bot found potential problems Jan 23, 2026

View reviewed changes

lib/public/DB/QueryBuilder/IQueryBuilder.php Fixed Show fixed Hide fixed

CarlSchwan force-pushed the carl/type-querybuilder branch 7 times, most recently from 4949aa8 to 27c24e9 Compare January 26, 2026 10:02

github-advanced-security bot found potential problems Jan 26, 2026

View reviewed changes

lib/public/DB/QueryBuilder/IQueryBuilder.php Dismissed Show dismissed Hide dismissed

CarlSchwan force-pushed the carl/type-querybuilder branch 2 times, most recently from 75af4a1 to ca05129 Compare January 27, 2026 10:17

CarlSchwan added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 27, 2026

CarlSchwan marked this pull request as ready for review January 27, 2026 10:18

CarlSchwan requested review from a team, SebastianKrupinski and hamza221 as code owners January 27, 2026 10:18

CarlSchwan requested review from Altahrim, ArtificialOwl, icewind1991 and sorbaugh and removed request for a team January 27, 2026 10:18

come-nc reviewed Jan 27, 2026

View reviewed changes

CarlSchwan force-pushed the carl/type-querybuilder branch 2 times, most recently from 97b14e1 to efaca86 Compare January 28, 2026 17:31

refactor(QueryBuilder): Properly type the query builder

4020f91

And make sure the related unit tests are also typed and checked by psalm. Signed-off-by: Carl Schwan <carlschwan@kde.org>

CarlSchwan force-pushed the carl/type-querybuilder branch from efaca86 to 4020f91 Compare January 30, 2026 14:28

provokateurin requested changes Feb 17, 2026

View reviewed changes

		parent::innerJoin($fromAlias, $join, $alias, $condition);
		return $this;

	* @param int\|null $maxResults The maximum number of results to retrieve.
	* @param positive-int\|null $maxResults The maximum number of results to retrieve.

	* @return list<self::PARAM_*> The currently defined query parameter types indexed by parameter index or name.
	* @return array<string\|int, self::PARAM_*> The currently defined query parameter types indexed by parameter index or name.

	* @param array $types The query parameters types to set.
	* @param array<string\|int, mixed> $params The query parameters to set.
	* @param array<string\|int, self::PARAM_*> $types The query parameters types to set.


	* @param non-negative-int $firstResult The first result to return.

		@@ -345,7 +344,7 @@ public function setMaxResults($maxResults);
		* @return int\|null The maximum number of results.

	* @return int\|null The maximum number of results.
	* @return positive-int\|null The maximum number of results.

	public function orWhere(...$where): self;
	public function orWhere(mixed ...$where): self;

	public function groupBy(...$groupBys): self;
	public function groupBy(mixed ...$groupBys): self;

	public function addGroupBy(...$groupBy): self;
	public function addGroupBy(mixed ...$groupBy): self;

	public function having(...$having): self;
	public function having(mixed ...$having): self;

Uh oh!

Conversation

CarlSchwan commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

TODO

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

provokateurin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

CarlSchwan commented Jan 23, 2026 •

edited

Loading