Skip to content
Open

PHPStan #4344

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ updates:
- /vendor-bin/cs-fixer
- /vendor-bin/openapi-extractor
- /vendor-bin/phpunit
- /vendor-bin/psalm
- /vendor-bin/rector
- /vendor-bin/phpstan
schedule:
interval: weekly
day: saturday
Expand Down
59 changes: 0 additions & 59 deletions .github/workflows/psalm.yml

This file was deleted.

1 change: 0 additions & 1 deletion .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
->getFinder()
->ignoreVCSIgnored(true)
->notPath('build')
->notPath('tests/stubs')
->notPath('l10n')
->notPath('src')
->notPath('vendor')
Expand Down
235 changes: 0 additions & 235 deletions LICENSES/AGPL-3.0-only.txt

This file was deleted.

24 changes: 0 additions & 24 deletions REUSE.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,30 +53,6 @@ precedence = "aggregate"
SPDX-FileCopyrightText = "2018-2024 Google LLC"
SPDX-License-Identifier = "Apache-2.0"

[[annotations]]
path = ["tests/stubs/doctrine_dbal_**"]
precedence = "aggregate"
SPDX-FileCopyrightText = "none"
SPDX-License-Identifier = "MIT"

[[annotations]]
path = ["tests/stubs/symfony_component_**"]
precedence = "aggregate"
SPDX-FileCopyrightText = "none"
SPDX-License-Identifier = "MIT"

[[annotations]]
path = ["tests/stubs/stecman_component_**"]
precedence = "aggregate"
SPDX-FileCopyrightText = "none"
SPDX-License-Identifier = "MIT"

[[annotations]]
path = ["tests/stubs/icewind_streams_**"]
precedence = "aggregate"
SPDX-FileCopyrightText = "none"
SPDX-License-Identifier = "MIT"

[[annotations]]
path = ["openapi.json", "src/types/openapi/openapi.ts"]
precedence = "aggregate"
Expand Down
8 changes: 3 additions & 5 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@
"post-install-cmd": [
"@composer bin all install --ansi"
],
"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './vendor-bin/*' -not -path './tests/stubs/*' -print0 | xargs -0 -n1 php -l",
"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './vendor-bin/*' -print0 | xargs -0 -n1 php -l",
"cs:check": "php-cs-fixer fix --dry-run --diff",
"cs:fix": "php-cs-fixer fix",
"psalm": "psalm --threads=$(nproc) --no-cache",
"psalm:update-baseline": "psalm --threads=$(nproc) --no-cache --update-baseline",
"psalm:fix": "psalm --no-cache --alter --issues=InvalidReturnType,InvalidNullableReturnType,MissingParamType,InvalidFalsableReturnType,MissingOverrideAttribute",
"test:unit": "phpunit -c tests/phpunit.xml",
"test:unit:coverage": "XDEBUG_MODE=coverage phpunit -c tests/phpunit.xml",
"rector": "rector && composer cs:fix",
"openapi": "generate-spec && npm run typescript:generate"
"openapi": "generate-spec && npm run typescript:generate",
"phpstan": "phpstan analyse -c phpstan.neon"
},
"config": {
"allow-plugins": {
Expand Down
11 changes: 10 additions & 1 deletion lib/ACL/ACLCacheWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ public function __construct(
parent::__construct($cache);
}

/**
* @param array<string, Rule[]> $rules
*/
private function getACLPermissionsForPath(string $path, array $rules = []): int {
if ($rules) {
$permissions = $this->aclManager->getPermissionsForPathFromRules($this->folderId, $path, $rules);
Expand All @@ -43,9 +46,12 @@ private function getACLPermissionsForPath(string $path, array $rules = []): int
return $canRead ? $permissions : 0;
}

/**
* @param array<string, Rule[]> $rules
*/
#[\Override]
protected function formatCacheEntry($entry, array $rules = []): ICacheEntry|false {
if (isset($entry['permissions'])) {
if (isset($entry['permissions']) && is_int($entry['permissions']) && is_string($entry['path'])) {
$entry['scan_permissions'] ??= $entry['permissions'];
$entry['permissions'] &= $this->getACLPermissionsForPath($entry['path'], $rules);
if (!$entry['permissions']) {
Expand All @@ -56,6 +62,9 @@ protected function formatCacheEntry($entry, array $rules = []): ICacheEntry|fals
return $entry;
}

/**
* @return array<ICacheEntry|false>
*/
#[\Override]
public function getFolderContentsById($fileId, ?string $mimeTypeFilter = null): array {
/** @psalm-suppress TooManyArguments Remove this in a few days */
Expand Down
19 changes: 17 additions & 2 deletions lib/ACL/ACLStorageWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OCA\GroupFolders\ACL;

use Icewind\Streams\IteratorDirectory;
use OC\Files\Storage\Storage;
use OC\Files\Storage\Wrapper\Wrapper;
use OCP\Constants;
use OCP\Files\Cache\ICache;
Expand All @@ -22,6 +23,9 @@ class ACLStorageWrapper extends Wrapper implements IConstructableStorage {
private readonly int $folderId;
private readonly int $storageId;

/**
* @param array{storage: Storage, acl_manager: ACLManager, in_share: bool, folder_id: int, storage_id: int} $arguments
*/
public function __construct(array $arguments) {
parent::__construct($arguments);
$this->aclManager = $arguments['acl_manager'];
Expand Down Expand Up @@ -76,7 +80,7 @@ public function isSharable(string $path): bool {

#[\Override]
public function getPermissions(string $path): int {
return $this->storage->getPermissions($path) & $this->getACLPermissionsForPath($path);
return $this->getWrapperStorage()->getPermissions($path) & $this->getACLPermissionsForPath($path);
}

#[\Override]
Expand Down Expand Up @@ -212,7 +216,7 @@ public function getCache(string $path = '', ?IStorage $storage = null): ICache {
public function getMetaData(string $path): ?array {
$data = parent::getMetaData($path);

if (is_array($data) && isset($data['permissions'])) {
if (is_array($data) && isset($data['permissions']) && is_int($data['permissions'])) {
$data['scan_permissions'] ??= $data['permissions'];
$data['permissions'] &= $this->getACLPermissionsForPath($path);
}
Expand Down Expand Up @@ -331,10 +335,21 @@ public function getDirectDownload(string $path): array|false {
return parent::getDirectDownload($path);
}

/**
* @return \Traversable<array<string, mixed>>
*/
#[\Override]
public function getDirectoryContent(string $directory): \Traversable {
$content = $this->getWrapperStorage()->getDirectoryContent($directory);
foreach ($content as $data) {
if (!isset($data['permissions']) || !is_int($data['permissions'])) {
throw new \RuntimeException('permissions is not an integer.');
}

if (!isset($data['name']) || !is_string($data['name'])) {
throw new \RuntimeException('name is not a string.');
}

$data['scan_permissions'] ??= $data['permissions'];
$data['permissions'] &= $this->getACLPermissionsForPath(rtrim($directory, '/') . '/' . $data['name']);

Expand Down
43 changes: 39 additions & 4 deletions lib/ACL/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ public function xmlSerialize(Writer $writer): void {
$writer->write($data);
}

/**
* @return array{
* mapping: array{
* type: 'user'|'group'|'dummy'|'circle',
* id: string,
* },
* mask: int,
* permissions: int,
* }
*/
#[\Override]
public function jsonSerialize(): array {
return [
Expand All @@ -125,19 +135,44 @@ public function jsonSerialize(): array {
public static function xmlDeserialize(Reader $reader): Rule {
$elements = \Sabre\Xml\Deserializer\keyValue($reader);

$mappingType = $elements[self::MAPPING_TYPE];
if (!is_string($mappingType)) {
throw new \RuntimeException(self::MAPPING_TYPE . ' is not a string.');
}

if (!in_array($mappingType, ['user', 'group', 'circle'])) {
throw new \RuntimeException(self::MAPPING_TYPE . ' does not have a valid value.');
}

$mappingId = $elements[self::MAPPING_ID];
if (!is_string($mappingId)) {
throw new \RuntimeException(self::MAPPING_ID . ' is not a string.');
}

$mask = $elements[self::MASK];
if (!is_string($mask)) {
throw new \RuntimeException(self::MASK . ' is not a string.');
}

$permission = $elements[self::PERMISSIONS];
if (!is_string($permission)) {
throw new \RuntimeException(self::PERMISSIONS . ' is not a string.');
}

return new Rule(
new UserMapping(
$elements[self::MAPPING_TYPE],
$elements[self::MAPPING_ID]
$mappingType,
$mappingId,
),
-1,
(int)$elements[self::MASK],
(int)$elements[self::PERMISSIONS]
(int)$mask,
(int)$permission,
);
}

/**
* merge multiple rules that apply on the same file where allow overwrites deny
* @param Rule[] $rules
*/
public static function mergeRules(array $rules): Rule {
// or'ing the masks to get a new mask that masks all set permissions
Expand Down
19 changes: 19 additions & 0 deletions lib/ACL/RuleManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ public function __construct(
) {
}

/**
* @param array{mapping_type?: 'user'|'group'|'dummy'|'circle', mapping_id: string, fileid: int, mask: int, permissions: int} $data
*/
private function createRule(array $data): ?Rule {
if (!isset($data['mapping_type'])) {
return null;
Expand Down Expand Up @@ -58,6 +61,7 @@ public function getRulesForFilesById(IUser $user, array $fileIds): array {
$query->expr()->eq('mapping_id', $query->createNamedParameter($userMapping->getId()))
), $userMappings)));

/** @var list<array{mapping_type: 'user'|'group'|'circle', mapping_id: string, fileid: int, mask: int, permissions: int}> $rows */
$rows = $query->executeQuery()->fetchAll();

$result = [];
Expand Down Expand Up @@ -94,6 +98,7 @@ public function getRulesForFilesByPath(IUser $user, int $storageId, array $fileP
$query->expr()->eq('mapping_id', $query->createNamedParameter($userMapping->getId()))
), $userMappings)));

/** @var list<array{fileid: int, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int, permissions: int, path: string}> $rows */
$rows = array_merge($rows, $query->executeQuery()->fetchAll());
}

Expand Down Expand Up @@ -127,6 +132,7 @@ public function getRulesForFilesByIds(IUser $user, array $fileIds): array {
$query->expr()->eq('a.mapping_id', $query->createNamedParameter($userMapping->getId()))
), $userMappings)));

/** @var list<array{fileid: int, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int, permissions: int, path: string, storage: int}> $rows */
$rows = array_merge($rows, $query->executeQuery()->fetchAll());
}

Expand Down Expand Up @@ -161,6 +167,7 @@ public function getRulesForFilesByParent(IUser $user, int $storageId, int $paren
)
);

/** @var list<array{mapping_type: null|'user'|'group'|'circle', mapping_id: string, fileid: int, mask: int, permissions: int, path: string}> $rows */
$rows = $query->executeQuery()->fetchAll();

$result = [];
Expand Down Expand Up @@ -193,9 +200,15 @@ public function getAllRulesForPaths(int $storageId, array $filePaths): array {

$rows = $query->executeQuery()->fetchAll();

/** @var list<array{fileid: int, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int, permissions: int, path: string}> $rows */
return $this->rulesByPath($rows);
}

/**
* @param list<array{path: string, mapping_type?: 'user'|'group'|'dummy'|'circle', mapping_id: string, fileid: int, mask: int, permissions: int}> $rows
* @param array<string, list<Rule>> $result
* @return array<string, list<Rule>>
*/
private function rulesByPath(array $rows, array $result = []): array {
foreach ($rows as $row) {
$rule = $this->createRule($row);
Expand All @@ -210,6 +223,10 @@ private function rulesByPath(array $rows, array $result = []): array {
return $result;
}

/**
* @param list<array{storage: int, path: string, mapping_type?: 'user'|'group'|'dummy'|'circle', mapping_id: string, fileid: int, mask: int, permissions: int}> $rows
* @return array<int, array<string, list<Rule>>>
*/
private function rulesByFileId(array $rows): array {
$result = [];
foreach ($rows as $row) {
Expand Down Expand Up @@ -244,6 +261,7 @@ public function getAllRulesForPrefix(int $storageId, string $prefix): array {

$rows = $query->executeQuery()->fetchAll();

/** @var list<array{fileid: int, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int, permissions: int, path: string}> $rows */
return $this->rulesByPath($rows);
}

Expand Down Expand Up @@ -272,6 +290,7 @@ public function getRulesForPrefix(IUser $user, int $storageId, string $prefix):

$rows = $query->executeQuery()->fetchAll();

/** @var list<array{fileid: int, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int, permissions: int, path: string}> $rows */
return $this->rulesByPath($rows);
}

Expand Down
3 changes: 2 additions & 1 deletion lib/ACL/UserMapping/UserMappingManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function __construct(
#[\Override]
public function getMappingsForUser(IUser $user, bool $userAssignable = true): array {
$groupMappings = array_values(array_map(fn (IGroup $group): UserMapping => new UserMapping('group', $group->getGID(), $group->getDisplayName()), $this->groupManager->getUserGroups($user)));
$circleMappings = array_values(array_map(fn (Circle $circle): UserMapping => new UserMapping('circle', $circle->getSingleId(), $circle->getDisplayName()), $this->getUserCircles($user->getUID())));
$circleMappings = array_map(fn (Circle $circle): UserMapping => new UserMapping('circle', $circle->getSingleId(), $circle->getDisplayName()), $this->getUserCircles($user->getUID()));

return array_merge([
new UserMapping('user', $user->getUID(), $user->getDisplayName()),
Expand Down Expand Up @@ -91,6 +91,7 @@ private function getCircle(string $groupId): ?Circle {

/**
* returns list of circles a user is member of
* @return list<Circle>
*/
private function getUserCircles(string $userId): array {
$circlesManager = $this->getCirclesManager();
Expand Down
Loading
Loading