Fix psalm issues in theming app

After this change, we are down to only one psalm warning for this app
and related to the Application.php. This also make composer
psam:update-baseline not silently ignore new errors.

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
This commit is contained in:
Carl Schwan 2022-01-26 14:26:58 +01:00
parent a145edd00d
commit e3a12b3482
No known key found for this signature in database
GPG key ID: 06B35D38387B67BE
10 changed files with 50 additions and 117 deletions

View file

@ -80,10 +80,9 @@ class LockingController extends OCSController {
}
/**
* @return ILockingProvider
* @throws \RuntimeException
*/
protected function getLockingProvider() {
protected function getLockingProvider(): ILockingProvider {
if ($this->lockingProvider instanceof DBLockingProvider) {
return $this->fakeDBLockingProvider;
}
@ -91,21 +90,17 @@ class LockingController extends OCSController {
}
/**
* @param string $user
* @param string $path
* @return string
* @throws NotFoundException
*/
protected function getPath($user, $path) {
protected function getPath(string $user, string $path): string {
$node = $this->rootFolder->getUserFolder($user)->get($path);
return 'files/' . md5($node->getStorage()->getId() . '::' . trim($node->getInternalPath(), '/'));
}
/**
* @return DataResponse
* @throws OCSException
*/
public function isLockingEnabled() {
public function isLockingEnabled(): DataResponse {
try {
$this->getLockingProvider();
return new DataResponse();
@ -115,13 +110,9 @@ class LockingController extends OCSController {
}
/**
* @param int $type
* @param string $user
* @param string $path
* @return DataResponse
* @throws OCSException
*/
public function acquireLock($type, $user, $path) {
public function acquireLock(int $type, string $user, string $path): DataResponse {
try {
$path = $this->getPath($user, $path);
} catch (NoUserException $e) {
@ -134,7 +125,7 @@ class LockingController extends OCSController {
try {
$lockingProvider->acquireLock($path, $type);
$this->config->setAppValue('testing', 'locking_' . $path, $type);
$this->config->setAppValue('testing', 'locking_' . $path, (string)$type);
return new DataResponse();
} catch (LockedException $e) {
throw new OCSException('', Http::STATUS_LOCKED, $e);
@ -142,13 +133,9 @@ class LockingController extends OCSController {
}
/**
* @param int $type
* @param string $user
* @param string $path
* @return DataResponse
* @throws OCSException
*/
public function changeLock($type, $user, $path) {
public function changeLock(int $type, string $user, string $path): DataResponse {
try {
$path = $this->getPath($user, $path);
} catch (NoUserException $e) {
@ -161,7 +148,7 @@ class LockingController extends OCSController {
try {
$lockingProvider->changeLock($path, $type);
$this->config->setAppValue('testing', 'locking_' . $path, $type);
$this->config->setAppValue('testing', 'locking_' . $path, (string)$type);
return new DataResponse();
} catch (LockedException $e) {
throw new OCSException('', Http::STATUS_LOCKED, $e);
@ -169,13 +156,9 @@ class LockingController extends OCSController {
}
/**
* @param int $type
* @param string $user
* @param string $path
* @return DataResponse
* @throws OCSException
*/
public function releaseLock($type, $user, $path) {
public function releaseLock(int $type, string $user, string $path): DataResponse {
try {
$path = $this->getPath($user, $path);
} catch (NoUserException $e) {
@ -196,13 +179,9 @@ class LockingController extends OCSController {
}
/**
* @param int $type
* @param string $user
* @param string $path
* @return DataResponse
* @throws OCSException
*/
public function isLocked($type, $user, $path) {
public function isLocked(int $type, string $user, string $path): DataResponse {
try {
$path = $this->getPath($user, $path);
} catch (NoUserException $e) {
@ -220,11 +199,7 @@ class LockingController extends OCSController {
throw new OCSException('', Http::STATUS_LOCKED);
}
/**
* @param int $type
* @return DataResponse
*/
public function releaseAll($type = null) {
public function releaseAll(int $type = null): DataResponse {
$lockingProvider = $this->getLockingProvider();
foreach ($this->config->getAppKeys('testing') as $lock) {
@ -232,11 +207,11 @@ class LockingController extends OCSController {
$path = substr($lock, strlen('locking_'));
if ($type === ILockingProvider::LOCK_EXCLUSIVE && (int)$this->config->getAppValue('testing', $lock) === ILockingProvider::LOCK_EXCLUSIVE) {
$lockingProvider->releaseLock($path, $this->config->getAppValue('testing', $lock));
$lockingProvider->releaseLock($path, (int)$this->config->getAppValue('testing', $lock));
} elseif ($type === ILockingProvider::LOCK_SHARED && (int)$this->config->getAppValue('testing', $lock) === ILockingProvider::LOCK_SHARED) {
$lockingProvider->releaseLock($path, $this->config->getAppValue('testing', $lock));
$lockingProvider->releaseLock($path, (int)$this->config->getAppValue('testing', $lock));
} else {
$lockingProvider->releaseLock($path, $this->config->getAppValue('testing', $lock));
$lockingProvider->releaseLock($path, (int)$this->config->getAppValue('testing', $lock));
}
}
}

View file

@ -79,6 +79,7 @@ class UpdateConfig extends Command {
protected function execute(InputInterface $input, OutputInterface $output): int {
$key = $input->getArgument('key');
$value = $input->getArgument('value');
assert(is_string($value) || $value === null, 'At most one value should be provided.');
if ($key === null) {
$output->writeln('Current theming config:');

View file

@ -95,13 +95,9 @@ class IconController extends Controller {
}
$iconFile = $this->imageManager->setCachedImage('icon-' . $app . '-' . str_replace('/', '_',$image), $icon);
}
if ($iconFile !== false) {
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
$response->cacheFor(86400);
return $response;
}
return new NotFoundResponse();
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
$response->cacheFor(86400);
return $response;
}
/**
@ -111,7 +107,8 @@ class IconController extends Controller {
* @NoCSRFRequired
*
* @param $app string app name
* @return FileDisplayResponse|DataDisplayResponse
* @psalm-return FileDisplayResponse|DataDisplayResponse
* @return Response
* @throws \Exception
*/
public function getFavicon(string $app = 'core'): Response {
@ -129,9 +126,7 @@ class IconController extends Controller {
$icon = $this->iconBuilder->getFavicon($app);
$iconFile = $this->imageManager->setCachedImage('favIcon-' . $app, $icon);
}
if ($iconFile !== false) {
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
}
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
}
if ($response === null) {
$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon.png';
@ -148,7 +143,7 @@ class IconController extends Controller {
* @NoCSRFRequired
*
* @param $app string app name
* @return FileDisplayResponse|NotFoundResponse
* @return DataDisplayResponse|FileDisplayResponse
* @throws \Exception
*/
public function getTouchIcon(string $app = 'core'): Response {
@ -165,9 +160,7 @@ class IconController extends Controller {
$icon = $this->iconBuilder->getTouchIcon($app);
$iconFile = $this->imageManager->setCachedImage('touchIcon-' . $app, $icon);
}
if ($iconFile !== false) {
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']);
}
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']);
}
if ($response === null) {
$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon-touch.png';

View file

@ -92,6 +92,7 @@ class ImageManager {
case 'background':
return $this->urlGenerator->imagePath('core', 'background.png') . '?v=' . $cacheBusterCounter;
}
return '';
}
public function getImageUrlAbsolute(string $key, bool $useSvg = true): string {
@ -131,6 +132,9 @@ class ImageManager {
return $folder->getFile($key);
}
/**
* @return array<string, array{mime: string, url: string}>
*/
public function getCustomImages(): array {
$images = [];
foreach ($this->supportedImageKeys as $key) {
@ -192,7 +196,7 @@ class ImageManager {
return $file;
}
public function delete(string $key) {
public function delete(string $key): void {
/* ignore exceptions, since we don't want to fail hard if something goes wrong during cleanup */
try {
$file = $this->appData->getFolder('images')->getFile($key);
@ -208,7 +212,7 @@ class ImageManager {
}
}
public function updateImage(string $key, string $tmpFile) {
public function updateImage(string $key, string $tmpFile): string {
$this->delete($key);
try {
@ -255,7 +259,7 @@ class ImageManager {
* "favicon" images are only allowed to be SVG when imagemagick with SVG support is available.
*
* @param string $key The image key, e.g. "favicon"
* @return array
* @return string[]
*/
private function getSupportedUploadImageFormats(string $key): array {
$supportedFormats = ['image/jpeg', 'image/png', 'image/gif', 'image/webp'];

View file

@ -157,6 +157,10 @@ class ThemingDefaults extends \OC_Defaults {
return $this->config->getAppValue('theming', 'url', $this->url);
}
/**
* @psalm-suppress InvalidReturnStatement
* @psalm-suppress InvalidReturnType
*/
public function getSlogan(?string $lang = null) {
return \OCP\Util::sanitizeHTML($this->config->getAppValue('theming', 'slogan', parent::getSlogan($lang)));
}
@ -404,8 +408,8 @@ class ThemingDefaults extends \OC_Defaults {
* Increases the cache buster key
*/
private function increaseCacheBuster() {
$cacheBusterKey = $this->config->getAppValue('theming', 'cachebuster', '0');
$this->config->setAppValue('theming', 'cachebuster', (int)$cacheBusterKey + 1);
$cacheBusterKey = (int)$this->config->getAppValue('theming', 'cachebuster', '0');
$this->config->setAppValue('theming', 'cachebuster', (string)($cacheBusterKey + 1));
$this->cacheFactory->createDistributed('theming-')->clear();
$this->cacheFactory->createDistributed('imagePath')->clear();
}

View file

@ -155,6 +155,7 @@ class Util {
/**
* @param string $color rgb color value
* @return int[]
* @psalm-return array{0: int, 1: int, 2: int}
*/
public function hexToRGB($color) {
$hex = preg_replace("/[^0-9A-Fa-f]/", '', $color);
@ -162,7 +163,7 @@ class Util {
$hex = $hex[0] . $hex[0] . $hex[1] . $hex[1] . $hex[2] . $hex[2];
}
if (strlen($hex) !== 6) {
return 0;
return [0, 0, 0];
}
return [
hexdec(substr($hex, 0, 2)),
@ -205,9 +206,7 @@ class Util {
$logoFile = null;
try {
$folder = $this->appData->getFolder('images');
if ($folder !== null) {
return $folder->getFile('logo');
}
return $folder->getFile('logo');
} catch (NotFoundException $e) {
}
}

View file

@ -1978,60 +1978,11 @@
<code>$event-&gt;getObjectId()</code>
</InvalidScalarArgument>
</file>
<file src="apps/testing/lib/Controller/LockingController.php">
<InvalidScalarArgument occurrences="5">
<code>$this-&gt;config-&gt;getAppValue('testing', $lock)</code>
<code>$this-&gt;config-&gt;getAppValue('testing', $lock)</code>
<code>$this-&gt;config-&gt;getAppValue('testing', $lock)</code>
<code>$type</code>
<code>$type</code>
</InvalidScalarArgument>
</file>
<file src="apps/theming/lib/AppInfo/Application.php">
<InvalidArgument occurrences="1">
<code>registerEventListener</code>
</InvalidArgument>
</file>
<file src="apps/theming/lib/Controller/IconController.php">
<InvalidReturnStatement occurrences="1">
<code>$response</code>
</InvalidReturnStatement>
<InvalidReturnType occurrences="1">
<code>FileDisplayResponse|NotFoundResponse</code>
</InvalidReturnType>
<RedundantCondition occurrences="3">
<code>$iconFile !== false</code>
<code>$iconFile !== false</code>
<code>$iconFile !== false</code>
</RedundantCondition>
</file>
<file src="apps/theming/lib/ImageManager.php">
<InvalidReturnType occurrences="1">
<code>string</code>
</InvalidReturnType>
</file>
<file src="apps/theming/lib/ThemingDefaults.php">
<InvalidReturnStatement occurrences="1">
<code>\OCP\Util::sanitizeHTML($this-&gt;config-&gt;getAppValue('theming', 'slogan', parent::getSlogan($lang)))</code>
</InvalidReturnStatement>
<InvalidReturnType occurrences="1">
<code>getSlogan</code>
</InvalidReturnType>
<InvalidScalarArgument occurrences="1">
<code>(int)$cacheBusterKey + 1</code>
</InvalidScalarArgument>
</file>
<file src="apps/theming/lib/Util.php">
<InvalidReturnStatement occurrences="1">
<code>0</code>
</InvalidReturnStatement>
<InvalidReturnType occurrences="1">
<code>int[]</code>
</InvalidReturnType>
<RedundantCondition occurrences="1">
<code>$folder !== null</code>
</RedundantCondition>
</file>
<file src="apps/twofactor_backupcodes/lib/AppInfo/Application.php">
<InvalidArgument occurrences="4">
<code>registerEventListener</code>

View file

@ -65,8 +65,13 @@ class Memcached extends Cache implements IMemcache {
// Enable Binary Protocol
//\Memcached::OPT_BINARY_PROTOCOL => true,
];
// by default enable igbinary serializer if available
/** @psalm-suppress RedundantCondition */
/**
* By default enable igbinary serializer if available
*
* Psalm checks depend on if igbinary is installed or not with memcached
* @psalm-suppress RedundantCondition
* @psalm-suppress TypeDoesNotContainType
*/
if (\Memcached::HAVE_IGBINARY) {
$defaultOptions[\Memcached::OPT_SERIALIZER] =
\Memcached::SERIALIZER_IGBINARY;

View file

@ -1130,11 +1130,12 @@ class OC_Util {
* This function is used to sanitize HTML and should be applied on any
* string or array of strings before displaying it on a web page.
*
* @param string|array $value
* @return string|array an array of sanitized strings or a single sanitized string, depends on the input parameter.
* @param string|string[] $value
* @return string|string[] an array of sanitized strings or a single sanitized string, depends on the input parameter.
*/
public static function sanitizeHTML($value) {
if (is_array($value)) {
/** @var string[] $value */
$value = array_map(function ($value) {
return self::sanitizeHTML($value);
}, $value);

View file

@ -435,8 +435,8 @@ class Util {
* This function is used to sanitize HTML and should be applied on any
* string or array of strings before displaying it on a web page.
*
* @param string|array $value
* @return string|array an array of sanitized strings or a single sanitized string, depends on the input parameter.
* @param string|string[] $value
* @return string|string[] an array of sanitized strings or a single sanitized string, depends on the input parameter.
* @since 4.5.0
*/
public static function sanitizeHTML($value) {