Skip to content

Commit bbaf603

Browse files
committed
fix(QueryBuilder): Make types even more strict
And put the files in rector strict and psalm strict Signed-off-by: Carl Schwan <carlschwan@kde.org>
1 parent a6a7c2f commit bbaf603

49 files changed

Lines changed: 488 additions & 489 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/dav/lib/DAV/Sharing/SharingMapper.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,6 @@ public function getPrincipalUrisByPrefix(string $resourceType, string $prefix):
165165
->andWhere($query->expr()->eq(
166166
'type',
167167
$query->createNamedParameter($resourceType, IQueryBuilder::PARAM_STR)),
168-
IQueryBuilder::PARAM_STR,
169168
)
170169
->executeQuery();
171170

apps/federatedfilesharing/tests/FederatedShareProviderReshareRemoteTest.php

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OCA\FederatedFileSharing\Tests;
1010

1111
use LogicException;
12+
use OC\DB\QueryBuilder\Parameter;
1213
use OC\Federation\CloudId;
1314
use OC\Share20\Share;
1415
use OCA\FederatedFileSharing\AddressHandler;
@@ -115,7 +116,7 @@ public function testCreateRemoteOwner(): void {
115116
$qb1->method('from')->willReturnSelf();
116117
$qb1->method('where')->willReturnSelf();
117118
$qb1->method('expr')->willReturn($expr1);
118-
$qb1->method('createNamedParameter')->willReturn('');
119+
$qb1->method('createNamedParameter')->willReturn(new Parameter(''));
119120
$qb1->method('executeQuery')->willReturn($result1);
120121

121122
/*
@@ -149,7 +150,7 @@ public function testCreateRemoteOwner(): void {
149150
$qb2->method('from')->willReturnSelf();
150151
$qb2->method('where')->willReturnSelf();
151152
$qb2->method('expr')->willReturn($expr2);
152-
$qb2->method('createNamedParameter')->willReturn('');
153+
$qb2->method('createNamedParameter')->willReturn(new Parameter(''));
153154
$qb2->method('executeQuery')->willReturn($result2);
154155

155156
/*
@@ -194,7 +195,7 @@ public function testCreateRemoteOwner(): void {
194195
$qb4->method('where')->willReturnSelf();
195196
$qb4->method('expr')->willReturn($expr4);
196197
$qb4->method('set')->willReturnSelf();
197-
$qb4->method('createNamedParameter')->willReturn('');
198+
$qb4->method('createNamedParameter')->willReturn(new Parameter(''));
198199

199200
/*
200201
* Mocks for storeRemoteId.
@@ -244,7 +245,7 @@ public function testCreateRemoteOwner(): void {
244245
$qb6->method('from')->willReturnSelf();
245246
$qb6->method('where')->willReturnSelf();
246247
$qb6->method('expr')->willReturn($expr6);
247-
$qb6->method('createNamedParameter')->willReturn('');
248+
$qb6->method('createNamedParameter')->willReturn(new Parameter(''));
248249
$qb6->method('executeQuery')->willReturn($result6);
249250

250251
$queryBuilderMatcher = $this->exactly(7);
@@ -345,7 +346,7 @@ public function testUpdateRemoteOwner(): void {
345346
$qb1->method('update')->willReturnSelf();
346347
$qb1->method('where')->willReturnSelf();
347348
$qb1->method('expr')->willReturn($expr1);
348-
$qb1->method('createNamedParameter')->willReturn('');
349+
$qb1->method('createNamedParameter')->willReturn(new Parameter(''));
349350
$qb1->method('set')->willReturnSelf();
350351

351352
/*
@@ -365,7 +366,7 @@ public function testUpdateRemoteOwner(): void {
365366
$qb2->method('from')->willReturnSelf();
366367
$qb2->method('where')->willReturnSelf();
367368
$qb2->method('expr')->willReturn($expr2);
368-
$qb2->method('createNamedParameter')->willReturn('');
369+
$qb2->method('createNamedParameter')->willReturn(new Parameter(''));
369370
$qb2->method('executeQuery')->willReturn($result2);
370371

371372
$queryBuilderMatcher = $this->exactly(2);

build/psalm-baseline.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4205,4 +4205,13 @@
42054205
<code><![CDATA[getAppValue]]></code>
42064206
</DeprecatedMethod>
42074207
</file>
4208+
<file src="tests/lib/TestCase.php">
4209+
<DeprecatedMethod>
4210+
<code><![CDATA[$container]]></code>
4211+
</DeprecatedMethod>
4212+
<InternalMethod>
4213+
<code><![CDATA[lockFile]]></code>
4214+
<code><![CDATA[unlockFile]]></code>
4215+
</InternalMethod>
4216+
</file>
42084217
</files>

build/rector-strict.php

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* SPDX-License-Identifier: AGPL-3.0-or-later
66
*/
77

8+
use Rector\Php81\Rector\Property\ReadOnlyPropertyRector;
9+
810
$nextcloudDir = dirname(__DIR__);
911

1012
return (require __DIR__ . '/rector-shared.php')
@@ -23,8 +25,15 @@
2325
$nextcloudDir . '/lib/private/Files/Cache/StorageGlobal.php',
2426
$nextcloudDir . '/lib/private/Files/Storage/Wrapper/Wrapper.php',
2527
$nextcloudDir . '/build/psalm/ITypedQueryBuilderTest.php',
26-
$nextcloudDir . '/lib/private/DB/QueryBuilder/TypedQueryBuilder.php',
27-
$nextcloudDir . '/lib/public/DB/QueryBuilder/ITypedQueryBuilder.php',
28+
$nextcloudDir . '/lib/private/DB/QueryBuilder/',
29+
$nextcloudDir . '/lib/public/DB/QueryBuilder/',
30+
$nextcloudDir . '/tests/lib/DB/QueryBuilder/',
31+
])
32+
->withSkip([
33+
$nextcloudDir . '/lib/private/DB/QueryBuilder/Sharded/ShardedQueryBuilder.php', // rector crashes with this file
34+
ReadOnlyPropertyRector::class => [
35+
$nextcloudDir . '/lib/private/DB/QueryBuilder/QueryBuilder.php', // Readonly properties are overwritten in tests
36+
]
2837
])
2938
->withAutoloadPaths([
3039
// ensure rector properly autoload the public interfaces

lib/private/DB/QueryBuilder/CompositeExpression.php

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,15 @@
1515

1616
class CompositeExpression implements ICompositeExpression, \Countable {
1717
public const TYPE_AND = 'AND';
18+
1819
public const TYPE_OR = 'OR';
1920

2021
/**
2122
* @param self::TYPE_* $type
2223
* @param array<ICompositeExpression|string> $parts
2324
*/
2425
public function __construct(
25-
private string $type,
26+
private readonly string $type,
2627
private array $parts = [],
2728
) {
2829
}
@@ -53,8 +54,6 @@ public function add($part): ICompositeExpression {
5354

5455
/**
5556
* Retrieves the amount of expressions on composite expression.
56-
*
57-
* @return integer
5857
*/
5958
#[\Override]
6059
public function count(): int {
@@ -63,8 +62,6 @@ public function count(): int {
6362

6463
/**
6564
* Returns the type of this composite expression (AND/OR).
66-
*
67-
* @return string
6865
*/
6966
#[\Override]
7067
public function getType(): string {
@@ -73,13 +70,12 @@ public function getType(): string {
7370

7471
/**
7572
* Retrieves the string representation of this composite expression.
76-
*
77-
* @return string
7873
*/
7974
public function __toString(): string {
8075
if ($this->count() === 1) {
8176
return (string)$this->parts[0];
8277
}
78+
8379
return '(' . implode(') ' . $this->type . ' (', $this->parts) . ')';
8480
}
8581

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828

2929
class ExpressionBuilder implements IExpressionBuilder {
3030
protected DoctrineExpressionBuilder $expressionBuilder;
31+
3132
protected QuoteHelper $helper;
33+
3234
protected IFunctionBuilder $functionBuilder;
3335

3436
public function __construct(
@@ -43,17 +45,19 @@ public function __construct(
4345

4446
#[Override]
4547
public function andX(ICompositeExpression|string ...$x): ICompositeExpression {
46-
if (empty($x)) {
48+
if ($x === []) {
4749
$this->logger->debug('Calling ' . IQueryBuilder::class . '::' . __FUNCTION__ . ' without parameters is deprecated and will throw soon.', ['exception' => new \Exception('No parameters in call to ' . __METHOD__)]);
4850
}
51+
4952
return new CompositeExpression(CompositeExpression::TYPE_AND, $x);
5053
}
5154

5255
#[Override]
5356
public function orX(ICompositeExpression|string ...$x): ICompositeExpression {
54-
if (empty($x)) {
57+
if ($x === []) {
5558
$this->logger->debug('Calling ' . IQueryBuilder::class . '::' . __FUNCTION__ . ' without parameters is deprecated and will throw soon.', ['exception' => new \Exception('No parameters in call to ' . __METHOD__)]);
5659
}
60+
5761
return new CompositeExpression(CompositeExpression::TYPE_OR, $x);
5862
}
5963

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
@@ -79,6 +81,7 @@ public function castColumn(string|IQueryFunction|ILiteral|IParameter $column, in
7981
$column = $this->helper->quoteColumnName($column);
8082
return new QueryFunction('to_char(' . $column . ')');
8183
}
84+
8285
if ($type === IQueryBuilder::PARAM_INT) {
8386
$column = $this->helper->quoteColumnName($column);
8487
return new QueryFunction('to_number(to_char(' . $column . '))');

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-License-Identifier: AGPL-3.0-or-later

lib/private/DB/QueryBuilder/FunctionBuilder/FunctionBuilder.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
57
* SPDX-License-Identifier: AGPL-3.0-or-later
@@ -37,6 +39,7 @@ public function concat(string|ILiteral|IParameter|IQueryFunction $x, string|ILit
3739
foreach ($args as $item) {
3840
$list[] = $this->helper->quoteColumnName($item);
3941
}
42+
4043
return new QueryFunction(sprintf('CONCAT(%s)', implode(', ', $list)));
4144
}
4245

@@ -52,11 +55,11 @@ public function substring(
5255
string|ILiteral|IParameter|IQueryFunction $start,
5356
null|ILiteral|IParameter|IQueryFunction $length = null,
5457
): IQueryFunction {
55-
if ($length) {
58+
if ($length !== null) {
5659
return new QueryFunction('SUBSTR(' . $this->helper->quoteColumnName($input) . ', ' . $this->helper->quoteColumnName($start) . ', ' . $this->helper->quoteColumnName($length) . ')');
57-
} else {
58-
return new QueryFunction('SUBSTR(' . $this->helper->quoteColumnName($input) . ', ' . $this->helper->quoteColumnName($start) . ')');
5960
}
61+
62+
return new QueryFunction('SUBSTR(' . $this->helper->quoteColumnName($input) . ', ' . $this->helper->quoteColumnName($start) . ')');
6063
}
6164

6265
#[Override]
@@ -87,21 +90,21 @@ public function subtract(
8790

8891
#[Override]
8992
public function count(string|ILiteral|IParameter|IQueryFunction $count = '', string $alias = ''): IQueryFunction {
90-
$alias = $alias ? (' AS ' . $this->helper->quoteColumnName($alias)) : '';
93+
$alias = $alias !== '' && $alias !== '0' ? (' AS ' . $this->helper->quoteColumnName($alias)) : '';
9194
$quotedName = $count === '' ? '*' : $this->helper->quoteColumnName($count);
9295
return new QueryFunction('COUNT(' . $quotedName . ')' . $alias);
9396
}
9497

9598
#[Override]
9699
public function octetLength(string|ILiteral|IParameter|IQueryFunction $field, string $alias = ''): IQueryFunction {
97-
$alias = $alias ? (' AS ' . $this->helper->quoteColumnName($alias)) : '';
100+
$alias = $alias !== '' && $alias !== '0' ? (' AS ' . $this->helper->quoteColumnName($alias)) : '';
98101
$quotedName = $this->helper->quoteColumnName($field);
99102
return new QueryFunction('OCTET_LENGTH(' . $quotedName . ')' . $alias);
100103
}
101104

102105
#[Override]
103106
public function charLength(string|ILiteral|IParameter|IQueryFunction $field, string $alias = ''): IQueryFunction {
104-
$alias = $alias ? (' AS ' . $this->helper->quoteColumnName($alias)) : '';
107+
$alias = $alias !== '' && $alias !== '0' ? (' AS ' . $this->helper->quoteColumnName($alias)) : '';
105108
$quotedName = $this->helper->quoteColumnName($field);
106109
return new QueryFunction('CHAR_LENGTH(' . $quotedName . ')' . $alias);
107110
}

0 commit comments

Comments
 (0)