From 1b4b6f46737bd1598215a2758c88d9cf15c32fd5 Mon Sep 17 00:00:00 2001 From: Jaggob <37583151+Jaggob@users.noreply.github.com> Date: Mon, 8 Jun 2026 10:49:11 +0200 Subject: [PATCH] chore(psalm): reduce the baseline noise (suppress + stubs + redundant casts) (#122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part 1 of the Psalm baseline burn-down (#122): clear the trivial/artifact findings so the baseline reflects only real type issues (131 -> 34). - Suppress MissingOverrideAttribute project-wide (33): the #[\Override] attribute is PHP 8.3+, but the app supports PHP 8.1+ (info.xml). - Resolve the OCP-resolution artifacts (36): - tests/psalm/missing-class-stubs.php defines the two unshipped OC internals (OC\Hooks\Emitter, OC\User\NoUserException) that nextcloud/ocp's OCP\Files\IRootFolder references, so the real OCP type files load and Psalm gets their actual API instead of UndefinedClass. - Targeted suppressions for genuinely-external classes not in nextcloud/ocp and not cleanly stubbable (Doctrine\DBAL\Schema\Table, \OC, OC\Security\CSRF\CsrfTokenManager, the Files/Files_Sharing/Viewer events, and OCP\Image whose concrete internal base needs 25 IImage methods). - Remove 31 RedundantCasts (casts on parse_url()/preg_match() results Psalm already types) across 11 services — behaviour-neutral. PHPUnit 409 + Psalm green (baseline now 34 real type issues, tackled in the follow-up); full Playwright 23/23 on NC 33. Part of #122. --- .../PadFileLockRetryExhaustedException.php | 2 +- lib/Listeners/CSPListener.php | 6 +- lib/Service/AdminSettingsValidator.php | 2 +- lib/Service/AllowlistNormalizer.php | 8 +- lib/Service/EtherpadClient.php | 6 +- lib/Service/EtherpadHealthCheckService.php | 2 +- lib/Service/ExternalPadExportFetcher.php | 18 +- lib/Service/LifecycleService.php | 2 +- lib/Service/PadFileService.php | 4 +- lib/Service/PadOpenService.php | 4 +- lib/Service/TrustedEmbedOriginsNormalizer.php | 8 +- psalm-baseline.xml | 235 +----------------- psalm.xml | 26 ++ tests/psalm/missing-class-stubs.php | 37 +++ tests/psalm/ocp-autoload.php | 5 + 15 files changed, 108 insertions(+), 257 deletions(-) create mode 100644 tests/psalm/missing-class-stubs.php diff --git a/lib/Exception/PadFileLockRetryExhaustedException.php b/lib/Exception/PadFileLockRetryExhaustedException.php index 7ba6bec..b7d6561 100644 --- a/lib/Exception/PadFileLockRetryExhaustedException.php +++ b/lib/Exception/PadFileLockRetryExhaustedException.php @@ -16,7 +16,7 @@ public function __construct( private int $retryAttempts, private LockedException $lockedException, ) { - parent::__construct($lockedException->getMessage(), (int)$lockedException->getCode(), $lockedException); + parent::__construct($lockedException->getMessage(), $lockedException->getCode(), $lockedException); } public function getRetryAttempts(): int { diff --git a/lib/Listeners/CSPListener.php b/lib/Listeners/CSPListener.php index c6d7fbe..8352dce 100644 --- a/lib/Listeners/CSPListener.php +++ b/lib/Listeners/CSPListener.php @@ -86,9 +86,9 @@ private function normalizeExternalAllowlistEntry(string $entry): ?string { if (!is_array($parts)) { return null; } - $scheme = strtolower((string)($parts['scheme'] ?? '')); - $host = strtolower((string)($parts['host'] ?? '')); - $port = isset($parts['port']) ? (int)$parts['port'] : 443; + $scheme = strtolower($parts['scheme'] ?? ''); + $host = strtolower($parts['host'] ?? ''); + $port = isset($parts['port']) ? $parts['port'] : 443; if ($scheme !== 'https' || $host === '' || $port <= 0 || $port > 65535) { return null; } diff --git a/lib/Service/AdminSettingsValidator.php b/lib/Service/AdminSettingsValidator.php index f9798d5..f135bb0 100644 --- a/lib/Service/AdminSettingsValidator.php +++ b/lib/Service/AdminSettingsValidator.php @@ -124,7 +124,7 @@ private function normalizeEtherpadApiHost(string $rawHost, string $fallbackPubli throw new AdminValidationException('etherpad_api_host', $this->l10n->t('Etherpad API URL must not include query or fragment.')); } - $scheme = strtolower((string)$parts['scheme']); + $scheme = strtolower($parts['scheme']); if (!in_array($scheme, ['http', 'https'], true)) { throw new AdminValidationException('etherpad_api_host', $this->l10n->t('Etherpad API URL must use http or https.')); } diff --git a/lib/Service/AllowlistNormalizer.php b/lib/Service/AllowlistNormalizer.php index 9e8c133..914ee2c 100644 --- a/lib/Service/AllowlistNormalizer.php +++ b/lib/Service/AllowlistNormalizer.php @@ -47,10 +47,10 @@ private function normalizeUrlEntry(string $entry): string { ); } - $scheme = strtolower((string)($parts['scheme'] ?? '')); - $host = strtolower((string)($parts['host'] ?? '')); - $port = isset($parts['port']) ? (int)$parts['port'] : 443; - $path = (string)($parts['path'] ?? ''); + $scheme = strtolower($parts['scheme'] ?? ''); + $host = strtolower($parts['host'] ?? ''); + $port = isset($parts['port']) ? $parts['port'] : 443; + $path = $parts['path'] ?? ''; if ($scheme !== 'https' || $host === '' || $port <= 0 || $port > 65535 || ($path !== '' && $path !== '/') || isset($parts['user']) || isset($parts['pass']) || isset($parts['query']) || isset($parts['fragment']) diff --git a/lib/Service/EtherpadClient.php b/lib/Service/EtherpadClient.php index f05c789..1bb5be1 100644 --- a/lib/Service/EtherpadClient.php +++ b/lib/Service/EtherpadClient.php @@ -270,9 +270,9 @@ public function normalizeOrigin(string $url): string { if ($parts === false || empty($parts['scheme']) || empty($parts['host'])) { return ''; } - $scheme = strtolower((string)$parts['scheme']); - $host = strtolower((string)$parts['host']); - $port = isset($parts['port']) ? (int)$parts['port'] : null; + $scheme = strtolower($parts['scheme']); + $host = strtolower($parts['host']); + $port = isset($parts['port']) ? $parts['port'] : null; $isDefaultPort = ($scheme === 'https' && $port === 443) || ($scheme === 'http' && $port === 80); if ($port === null || $isDefaultPort) { return $scheme . '://' . $host; diff --git a/lib/Service/EtherpadHealthCheckService.php b/lib/Service/EtherpadHealthCheckService.php index 20c4329..56a71cf 100644 --- a/lib/Service/EtherpadHealthCheckService.php +++ b/lib/Service/EtherpadHealthCheckService.php @@ -51,7 +51,7 @@ public function check(ValidatedAdminSettings $settings): HealthCheckResult { // path leaks the literal '{detail}' through to consumers in // some catalog setups. Doing the substitution here removes that // surface area. - $template = (string)$this->l10n->t('Etherpad connection test failed: {detail}'); + $template = $this->l10n->t('Etherpad connection test failed: {detail}'); throw new AdminHealthCheckException( str_replace('{detail}', $detail, $template), 0, diff --git a/lib/Service/ExternalPadExportFetcher.php b/lib/Service/ExternalPadExportFetcher.php index 4028034..61428db 100644 --- a/lib/Service/ExternalPadExportFetcher.php +++ b/lib/Service/ExternalPadExportFetcher.php @@ -284,9 +284,9 @@ private function normalizeAllowlistOrigin(string $entry): string { if (!is_array($parts)) { return ''; } - $scheme = strtolower((string)($parts['scheme'] ?? '')); - $host = strtolower((string)($parts['host'] ?? '')); - $port = isset($parts['port']) ? (int)$parts['port'] : 443; + $scheme = strtolower($parts['scheme'] ?? ''); + $host = strtolower($parts['host'] ?? ''); + $port = isset($parts['port']) ? $parts['port'] : 443; if ($scheme !== 'https' || $host === '' || $port <= 0 || $port > 65535) { return ''; } @@ -308,10 +308,10 @@ private function parsePublicPadUrl(string $padUrl): array { throw new EtherpadClientException('Public pad URL must not contain credentials.'); } - $scheme = strtolower((string)($parts['scheme'] ?? '')); - $host = strtolower((string)($parts['host'] ?? '')); - $port = isset($parts['port']) ? (int)$parts['port'] : 443; - $path = (string)($parts['path'] ?? ''); + $scheme = strtolower($parts['scheme'] ?? ''); + $host = strtolower($parts['host'] ?? ''); + $port = isset($parts['port']) ? $parts['port'] : 443; + $path = $parts['path'] ?? ''; // `+` is literal in URL path segments — only query/form bodies treat // it as a space. Using urldecode here turned `/p/team+pad` into // pad-id `team pad`, then re-encoded to `/p/team%20pad` at fetch @@ -325,8 +325,8 @@ private function parsePublicPadUrl(string $padUrl): array { throw new EtherpadClientException('Public pad URL must match /p/{padId}.'); } - $basePath = rtrim((string)$matches[1], '/'); - $padId = trim((string)$matches[2]); + $basePath = rtrim($matches[1], '/'); + $padId = trim($matches[2]); if ($padId === '') { throw new EtherpadClientException('Invalid public pad URL.'); } diff --git a/lib/Service/LifecycleService.php b/lib/Service/LifecycleService.php index c52e584..39e0266 100644 --- a/lib/Service/LifecycleService.php +++ b/lib/Service/LifecycleService.php @@ -202,7 +202,7 @@ public function handleTrash(File $file): array { if ($this->isTestFaultActive(self::TEST_FAULT_TRASH_WRITE_FAIL)) { throw new \RuntimeException('Injected test fault: trash_write_fail'); } - $file->putContent((string)$updatedContent); + $file->putContent($updatedContent); $snapshotPersisted = true; } catch (LockedException $e) { // Trash operation can hold a lock on the file node; do not block state transition/deletion. diff --git a/lib/Service/PadFileService.php b/lib/Service/PadFileService.php index 1439e7d..7734a18 100644 --- a/lib/Service/PadFileService.php +++ b/lib/Service/PadFileService.php @@ -102,7 +102,7 @@ public function parseLegacyOwnpadShortcut(string $content): ?array { return null; } - $url = trim((string)$matches[1]); + $url = trim($matches[1]); if ($url === '' || preg_match('#^https?://#i', $url) !== 1) { return null; } @@ -118,7 +118,7 @@ public function parseLegacyOwnpadShortcut(string $content): ?array { if (preg_match('~/p/([^/?#]+)$~', $decodedPath, $padMatches) !== 1) { return null; } - $padId = trim((string)$padMatches[1]); + $padId = trim($padMatches[1]); if ($padId === '') { return null; } diff --git a/lib/Service/PadOpenService.php b/lib/Service/PadOpenService.php index 088f745..64d6e55 100644 --- a/lib/Service/PadOpenService.php +++ b/lib/Service/PadOpenService.php @@ -69,13 +69,13 @@ private function openNode(string $uid, string $displayName, File $node, string $ throw new \RuntimeException('Could not resolve file ID.'); } - $pad = $this->padFileService->readPad((string)$content); + $pad = $this->padFileService->readPad($content); $padId = $pad->padId; $accessMode = $pad->accessMode; $padUrl = $pad->padUrl; $isExternal = $pad->isExternal; $snapshot = $isExternal - ? $this->snapshotExtractor->extract((string)$content) + ? $this->snapshotExtractor->extract($content) : new SnapshotPayload('', ''); if (!$isExternal) { $this->bindingService->assertConsistentMapping($fileId, $padId, $accessMode); diff --git a/lib/Service/TrustedEmbedOriginsNormalizer.php b/lib/Service/TrustedEmbedOriginsNormalizer.php index 9e9f968..85ced87 100644 --- a/lib/Service/TrustedEmbedOriginsNormalizer.php +++ b/lib/Service/TrustedEmbedOriginsNormalizer.php @@ -50,25 +50,25 @@ private function normalizeOrigin(string $entry, bool $throwOnInvalid): string { if (!is_array($parts) || !isset($parts['scheme'], $parts['host'])) { return $this->invalid($throwOnInvalid, 'Trusted embed origins must be absolute origins: {origin}', $entry); } - if (isset($parts['path']) && trim((string)$parts['path'], '/') !== '') { + if (isset($parts['path']) && trim($parts['path'], '/') !== '') { return $this->invalid($throwOnInvalid, 'Trusted embed origins must not include a path: {origin}', $entry); } if (isset($parts['query']) || isset($parts['fragment']) || isset($parts['user']) || isset($parts['pass'])) { return $this->invalid($throwOnInvalid, 'Trusted embed origins must not include credentials, query, or fragment: {origin}', $entry); } - $scheme = strtolower((string)$parts['scheme']); + $scheme = strtolower($parts['scheme']); if ($scheme !== 'https') { return $this->invalid($throwOnInvalid, 'Trusted embed origins must use https: {origin}', $entry); } - $host = strtolower((string)$parts['host']); + $host = strtolower($parts['host']); if (str_contains($host, ':') && !str_starts_with($host, '[')) { $host = '[' . $host . ']'; } $origin = $scheme . '://' . $host; if (isset($parts['port'])) { - $port = (int)$parts['port']; + $port = $parts['port']; if ($port < 1 || $port > 65535) { return $this->invalid($throwOnInvalid, 'Trusted embed origins must use a valid TCP port: {origin}', $entry); } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 49d8cf1..90313a5 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,18 +1,9 @@ - - - - - - - - - - - - + + + @@ -22,13 +13,6 @@ - - - - - - - @@ -39,171 +23,53 @@ - - - getCode()]]> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - rootFolder]]> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - createTable('ep_pad_bindings')]]> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + @@ -213,43 +79,16 @@ - - - csrfTokenManager]]> - - - - - - - - - - now() - $startedAt) * 1000]]> - - l10n->t('Etherpad connection test failed: {detail}')]]> - - - - - - - - - - - - @@ -267,15 +106,6 @@ - - - - - - - rootFolder]]> - - @@ -330,22 +160,6 @@ - - rootFolder]]> - - - - - - - - - - - - - - @@ -353,35 +167,4 @@ - - - - - - - - - - - rootFolder]]> - rootFolder]]> - rootFolder]]> - - - - - - - - - - - - - - - - - - diff --git a/psalm.xml b/psalm.xml index 5e95d31..e2c8f16 100644 --- a/psalm.xml +++ b/psalm.xml @@ -16,4 +16,30 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/psalm/missing-class-stubs.php b/tests/psalm/missing-class-stubs.php new file mode 100644 index 0000000..d1dd89f --- /dev/null +++ b/tests/psalm/missing-class-stubs.php @@ -0,0 +1,37 @@ +