refactor(Files\Storage): Strong-type properties #49064
Conversation
provokateurin
added
2. developing
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
Signed-off-by: provokateurin <kate@provokateurin.de>
provokateurin
force-pushed
the
refactor/storage/strong-type-properties
branch
from
679152a
to
a0e2749
Compare
| @@ -171,7 +171,7 @@ private function parseUnixItem(string $item, string $directory): array { | |||
| ]; | |||
| } | |||
|
|
|||
| private function normalizePermissions(string $permissions) { | |||
| private function normalizePermissions(string $permissions): mixed { | |||
There was a problem hiding this comment.
| private function normalizePermissions(string $permissions): mixed { | |
| private function normalizePermissions(string $permissions): string { |
But we might need to update the code for psalm to trust us on this, unless psalm has a stub for array_reduce with templates and stuff.
| * @return ICacheEntry | ||
| */ | ||
| private function getSourceRootInfo() { | ||
| private function getSourceRootInfo(): ICacheEntry|false|null { |
There was a problem hiding this comment.
I saw this as well and I think it can be improved.
I was planning to do that later, but I can also do it in this PR.
| if (is_null($this->sourceRootInfo)) { | ||
| if (is_null($this->superShare->getNodeCacheEntry())) { |
There was a problem hiding this comment.
| if (is_null($this->sourceRootInfo)) { | |
| if (is_null($this->superShare->getNodeCacheEntry())) { | |
| if (is_null($this->sourceRootInfo)) { | |
| $this->sourceRootInfo = $this->superShare->getNodeCacheEntry() | |
| if (is_null($this->sourceRootInfo)) { |
This would avoid the else and calling the method twice
| /** | ||
| * @var array<string, bool> | ||
| * for which path we execute the repair step to avoid recursions | ||
| */ |
There was a problem hiding this comment.
| /** | |
| * @var array<string, bool> | |
| * for which path we execute the repair step to avoid recursions | |
| */ | |
| /** | |
| * @var array<string, bool> for which path we execute the repair step to avoid recursions | |
| */ |
| @@ -47,7 +44,7 @@ public function getUnjailedPath(string $path): string { | |||
| /** | |||
| * This is separate from Wrapper::getWrapperStorage so we can get the jailed storage consistently even if the jail is inside another wrapper | |||
| */ | |||
| public function getUnjailedStorage(): IStorage { | |||
| public function getUnjailedStorage(): ?IStorage { | |||
| use OCP\Files\Cache\ICacheEntry; | ||
| use OCP\Files\FileInfo; | ||
| use OCP\Files\Storage\IStorage; | ||
|
|
||
| class Quota extends Wrapper { | ||
| /** @var callable|null */ | ||
| protected $quotaCallback; | ||
| /** @var int|float|null int on 64bits, float on 32bits for bigint */ |
| /** @var callable|null */ | ||
| protected $quotaCallback; | ||
| /** @var int|float|null int on 64bits, float on 32bits for bigint */ | ||
| protected ?\Closure $quotaCallback; |
| class Wrapper implements Storage, ILockingStorage, IWriteStreamStorage { | ||
| protected ?Storage $storage; | ||
| public ?ICache $cache = null; | ||
| public IScanner $scanner; |
There was a problem hiding this comment.
scanner is not set in constructor, why is this one not nullable?
I don't think so. Any property is either private and the subclass has it's own independent definition or it's protected and the subclass uses the definition of the parent and can not override it either. |
I was thinking of return types, but it seems you did not add those to Common/Wrapper, so maybe it will be fine. |
|
The return types I already added in the other PR a few weeks ago. |
Summary
Added strong types where possible (mostly with rector and a modified config file).
Rector also added some return types and other cleanups.
Checklist
Signed-off-by: provokateurin kate@provokateurin.de