From df2c23233131b05eb24aa668965458a214fffef8 Mon Sep 17 00:00:00 2001 From: Jaggob <37583151+Jaggob@users.noreply.github.com> Date: Tue, 9 Jun 2026 10:53:11 +0200 Subject: [PATCH] chore(#122): fix real Psalm type issues + enable findUnusedCode PR B of the Psalm baseline burndown. Fixes all 34 real type findings so the type-issue baseline drops to zero, then turns on findUnusedCode. Type fixes: - 8 IEventListener listeners get @template-implements IEventListener - LifecycleService restore-result @return shapes reconciled (add pad_id?) - PadCreationService::createFromUrl @return gains snapshot_warning_code?; withCreateRollback action closure return hint dropped so the templated return type carries the precise shape - ExternalPadExportFetcher::getPublicTextFromResolvedExternalPad @param widened to the full resolved shape (origin/pad_id were rejected) - Viewer/PublicViewer ErrorMapper: parenthesize callable return union - EtherpadHealthCheckService latency math uses float literal - ConsistencyCheckService / AdminController: drop redundant is_array guards - PublicShareUrlBuilder: remove genuinely-dead '' branches (trimmed path) - Preview/Application/PadFileCreator/ExternalPadExportFetcher: targeted @psalm-suppress with rationale where Psalm can't model runtime (OCP\Image not in ocp stubs, Files-app event, curl write-callback mutation, basename('/') === '') findUnusedCode: - Enabled in psalm.xml - Entry points (controllers, listeners, migrations, settings, jobs, hooks, preview, Application) annotated @psalm-api so Psalm roots from them - Removed real dead code surfaced: 5 unused local inits, 1 unused param (buildProtectedRestorePadName), 1 unnecessary @var - Remaining findings are NC DI/entry-point false positives (constructors with no literal `new`, repair-step internals reached only via run()); captured in psalm-baseline.xml so new dead code is still caught composer psalm green; 409 PHPUnit tests pass. --- lib/AppInfo/Application.php | 7 + .../AbstractPendingDeleteRetryJob.php | 3 + .../ColdPendingDeleteRetryJob.php | 3 + .../HotPendingDeleteRetryJob.php | 3 + .../WarmPendingDeleteRetryJob.php | 3 + lib/Controller/AbstractPadController.php | 1 + lib/Controller/AdminController.php | 6 +- lib/Controller/EmbedController.php | 3 + lib/Controller/PadCreateController.php | 1 + lib/Controller/PadLifecycleController.php | 1 + lib/Controller/PadSessionController.php | 1 + lib/Controller/PublicViewerController.php | 3 + .../PublicViewerControllerErrorMapper.php | 2 +- lib/Controller/ViewerController.php | 3 + .../ViewerControllerErrorMapper.php | 2 +- lib/Hooks/TrashbinHookHandler.php | 3 + lib/Listeners/CSPListener.php | 4 + .../FileCreatedFromTemplateListener.php | 3 + lib/Listeners/LoadFilesScriptsListener.php | 4 + .../LoadPublicShareScriptsListener.php | 4 + lib/Listeners/LoadViewerListener.php | 4 + lib/Listeners/MoveToTrashListener.php | 4 + .../RegisterTemplateCreatorListener.php | 4 + lib/Listeners/RestoreFromTrashListener.php | 4 + lib/Migration/BackfillPadMimeType.php | 3 + lib/Migration/MarkApiKeySensitive.php | 1 + lib/Migration/RegisterMimeType.php | 3 + .../Version000001Date20260304222000.php | 3 + .../Version000002Date20260512170000.php | 3 + .../Version000003Date20260512230000.php | 4 +- lib/Preview/PadPreviewProvider.php | 8 + lib/Service/ConsistencyCheckService.php | 2 +- lib/Service/EtherpadHealthCheckService.php | 2 +- lib/Service/ExternalPadExportFetcher.php | 7 +- lib/Service/LifecycleService.php | 11 +- lib/Service/PadBootstrapService.php | 2 - lib/Service/PadCreationService.php | 4 +- lib/Service/PadFileCreator.php | 4 + lib/Service/PadMetadataService.php | 1 - lib/Service/PadOpenService.php | 1 - lib/Service/PublicShareUrlBuilder.php | 4 +- lib/Settings/AdminSection.php | 3 + lib/Settings/AdminSettings.php | 3 + psalm-baseline.xml | 361 +++++++++++------- psalm.xml | 2 +- 45 files changed, 337 insertions(+), 171 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 21fb157..24c07b6 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -20,6 +20,9 @@ use OCP\Files\Template\RegisterTemplateCreatorEvent; use OCP\Security\CSP\AddContentSecurityPolicyEvent; +/** + * @psalm-api + */ class Application extends App implements IBootstrap { public const APP_ID = 'etherpad_nextcloud'; @@ -42,6 +45,10 @@ public function register(IRegistrationContext $context): void { ); $context->registerEventListener(AddContentSecurityPolicyEvent::class, \OCA\EtherpadNextcloud\Listeners\CSPListener::class); + // LoadAdditionalScriptsEvent is provided by the Files app, not by + // nextcloud/ocp, so Psalm can't prove it is-a OCP\EventDispatcher\Event + // and rejects the generic IEventListener listener here. + /** @psalm-suppress InvalidArgument */ $context->registerEventListener( LoadAdditionalScriptsEvent::class, \OCA\EtherpadNextcloud\Listeners\LoadFilesScriptsListener::class, diff --git a/lib/BackgroundJob/AbstractPendingDeleteRetryJob.php b/lib/BackgroundJob/AbstractPendingDeleteRetryJob.php index a59450d..7e29610 100644 --- a/lib/BackgroundJob/AbstractPendingDeleteRetryJob.php +++ b/lib/BackgroundJob/AbstractPendingDeleteRetryJob.php @@ -13,6 +13,9 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; +/** + * @psalm-api + */ abstract class AbstractPendingDeleteRetryJob extends TimedJob { protected const INTERVAL_SECONDS = 24 * 60 * 60; protected const MIN_AGE_SECONDS = 0; diff --git a/lib/BackgroundJob/ColdPendingDeleteRetryJob.php b/lib/BackgroundJob/ColdPendingDeleteRetryJob.php index d065d0d..250447d 100644 --- a/lib/BackgroundJob/ColdPendingDeleteRetryJob.php +++ b/lib/BackgroundJob/ColdPendingDeleteRetryJob.php @@ -9,6 +9,9 @@ namespace OCA\EtherpadNextcloud\BackgroundJob; +/** + * @psalm-api + */ class ColdPendingDeleteRetryJob extends AbstractPendingDeleteRetryJob { protected const INTERVAL_SECONDS = 24 * 60 * 60; protected const MIN_AGE_SECONDS = 24 * 60 * 60; diff --git a/lib/BackgroundJob/HotPendingDeleteRetryJob.php b/lib/BackgroundJob/HotPendingDeleteRetryJob.php index 7526e28..02b73fc 100644 --- a/lib/BackgroundJob/HotPendingDeleteRetryJob.php +++ b/lib/BackgroundJob/HotPendingDeleteRetryJob.php @@ -9,6 +9,9 @@ namespace OCA\EtherpadNextcloud\BackgroundJob; +/** + * @psalm-api + */ class HotPendingDeleteRetryJob extends AbstractPendingDeleteRetryJob { protected const INTERVAL_SECONDS = 5 * 60; protected const MIN_AGE_SECONDS = 0; diff --git a/lib/BackgroundJob/WarmPendingDeleteRetryJob.php b/lib/BackgroundJob/WarmPendingDeleteRetryJob.php index be53b8a..8dbd45f 100644 --- a/lib/BackgroundJob/WarmPendingDeleteRetryJob.php +++ b/lib/BackgroundJob/WarmPendingDeleteRetryJob.php @@ -9,6 +9,9 @@ namespace OCA\EtherpadNextcloud\BackgroundJob; +/** + * @psalm-api + */ class WarmPendingDeleteRetryJob extends AbstractPendingDeleteRetryJob { protected const INTERVAL_SECONDS = 60 * 60; protected const MIN_AGE_SECONDS = 60 * 60; diff --git a/lib/Controller/AbstractPadController.php b/lib/Controller/AbstractPadController.php index 54f4b0b..957441a 100644 --- a/lib/Controller/AbstractPadController.php +++ b/lib/Controller/AbstractPadController.php @@ -32,6 +32,7 @@ * * Each concrete controller keeps its constructor narrow — it only * declares the services it actually uses on top of the base deps. + * @psalm-api */ abstract class AbstractPadController extends Controller { public function __construct( diff --git a/lib/Controller/AdminController.php b/lib/Controller/AdminController.php index 81aebb6..26f079d 100644 --- a/lib/Controller/AdminController.php +++ b/lib/Controller/AdminController.php @@ -26,6 +26,9 @@ use OCP\IRequest; use OCP\IUserSession; +/** + * @psalm-api + */ class AdminController extends Controller { private const CONSISTENCY_SAMPLE_LIMIT = 25; private const PENDING_DELETE_RETRY_BATCH_SIZE = 500; @@ -159,8 +162,7 @@ function (): string { /** @return array */ private function readJsonPayload(): array { - $params = $this->request->getParams(); - return is_array($params) ? $params : []; + return $this->request->getParams(); } private function requireAdmin(): void { diff --git a/lib/Controller/EmbedController.php b/lib/Controller/EmbedController.php index 4793f58..542d4cd 100644 --- a/lib/Controller/EmbedController.php +++ b/lib/Controller/EmbedController.php @@ -22,6 +22,9 @@ use OCP\IUser; use OCP\IUserSession; +/** + * @psalm-api + */ class EmbedController extends Controller { public function __construct( string $appName, diff --git a/lib/Controller/PadCreateController.php b/lib/Controller/PadCreateController.php index a16f8a6..29e625c 100644 --- a/lib/Controller/PadCreateController.php +++ b/lib/Controller/PadCreateController.php @@ -23,6 +23,7 @@ /** * Create-side endpoints for `.pad` files. Spans empty creates, copies in * a parent folder, template-based creates, and external-URL imports. + * @psalm-api */ class PadCreateController extends AbstractPadController { public function __construct( diff --git a/lib/Controller/PadLifecycleController.php b/lib/Controller/PadLifecycleController.php index dea34b5..2b44fc0 100644 --- a/lib/Controller/PadLifecycleController.php +++ b/lib/Controller/PadLifecycleController.php @@ -27,6 +27,7 @@ * Lifecycle + sync endpoints — trash, restore, recover-from-snapshot, * sync state, and the find-original lookup that the copy-of-a-pad * recovery affordance hangs off. + * @psalm-api */ class PadLifecycleController extends AbstractPadController { public function __construct( diff --git a/lib/Controller/PadSessionController.php b/lib/Controller/PadSessionController.php index f08eb04..96d3a32 100644 --- a/lib/Controller/PadSessionController.php +++ b/lib/Controller/PadSessionController.php @@ -29,6 +29,7 @@ * pad session state for a viewer: actual open flow, lazy frontmatter * init on first open, and the metadata + resolve readouts used by * embed-side surfaces. + * @psalm-api */ class PadSessionController extends AbstractPadController { public function __construct( diff --git a/lib/Controller/PublicViewerController.php b/lib/Controller/PublicViewerController.php index 9271e69..af098cf 100644 --- a/lib/Controller/PublicViewerController.php +++ b/lib/Controller/PublicViewerController.php @@ -23,6 +23,9 @@ use OCP\ISession; use OCP\Share\IShare; +/** + * @psalm-api + */ class PublicViewerController extends PublicShareController { private ?IShare $share = null; diff --git a/lib/Controller/PublicViewerControllerErrorMapper.php b/lib/Controller/PublicViewerControllerErrorMapper.php index d3d0066..a9f3134 100644 --- a/lib/Controller/PublicViewerControllerErrorMapper.php +++ b/lib/Controller/PublicViewerControllerErrorMapper.php @@ -50,7 +50,7 @@ public function runForData(callable $action, callable $success): DataResponse { /** * @param callable(): mixed $action - * @param callable(mixed): RedirectResponse|TemplateResponse $success + * @param callable(mixed): (RedirectResponse|TemplateResponse) $success */ public function runForTemplate(callable $action, callable $success, string $token): RedirectResponse|TemplateResponse { try { diff --git a/lib/Controller/ViewerController.php b/lib/Controller/ViewerController.php index c14cda2..5ed91ae 100644 --- a/lib/Controller/ViewerController.php +++ b/lib/Controller/ViewerController.php @@ -23,6 +23,9 @@ use OCP\IUser; use OCP\IUserSession; +/** + * @psalm-api + */ class ViewerController extends Controller { public function __construct( string $appName, diff --git a/lib/Controller/ViewerControllerErrorMapper.php b/lib/Controller/ViewerControllerErrorMapper.php index f2102a1..76c0da1 100644 --- a/lib/Controller/ViewerControllerErrorMapper.php +++ b/lib/Controller/ViewerControllerErrorMapper.php @@ -32,7 +32,7 @@ public function __construct( /** * @param callable(): mixed $action - * @param callable(mixed): RedirectResponse|TemplateResponse $success + * @param callable(mixed): (RedirectResponse|TemplateResponse) $success * @param string|null $notFoundMessage context-specific message for `NotFoundException`; defaults to the open-pad wording */ public function runForTemplate( diff --git a/lib/Hooks/TrashbinHookHandler.php b/lib/Hooks/TrashbinHookHandler.php index f55f7cd..63deddb 100644 --- a/lib/Hooks/TrashbinHookHandler.php +++ b/lib/Hooks/TrashbinHookHandler.php @@ -13,6 +13,9 @@ use OCA\EtherpadNextcloud\Listeners\RestoreFromTrashListener; use Psr\Log\LoggerInterface; +/** + * @psalm-api + */ class TrashbinHookHandler { /** * @param array $params diff --git a/lib/Listeners/CSPListener.php b/lib/Listeners/CSPListener.php index 8352dce..16f4678 100644 --- a/lib/Listeners/CSPListener.php +++ b/lib/Listeners/CSPListener.php @@ -14,6 +14,10 @@ use OCP\IConfig; use OCP\Security\CSP\AddContentSecurityPolicyEvent; +/** + * @template-implements IEventListener + * @psalm-api + */ class CSPListener implements IEventListener { public function __construct( private IConfig $config, diff --git a/lib/Listeners/FileCreatedFromTemplateListener.php b/lib/Listeners/FileCreatedFromTemplateListener.php index 4613006..179961c 100644 --- a/lib/Listeners/FileCreatedFromTemplateListener.php +++ b/lib/Listeners/FileCreatedFromTemplateListener.php @@ -37,6 +37,9 @@ * On any skip / failure inside the source-template branch the target is * reset to empty *and* re-initialised so a fresh blank pad still opens * cleanly on the first call. + * + * @template-implements IEventListener + * @psalm-api */ class FileCreatedFromTemplateListener implements IEventListener { public function __construct( diff --git a/lib/Listeners/LoadFilesScriptsListener.php b/lib/Listeners/LoadFilesScriptsListener.php index 2e73cd0..c8f9058 100644 --- a/lib/Listeners/LoadFilesScriptsListener.php +++ b/lib/Listeners/LoadFilesScriptsListener.php @@ -14,6 +14,10 @@ use OCP\EventDispatcher\IEventListener; use OCP\Util; +/** + * @template-implements IEventListener + * @psalm-api + */ class LoadFilesScriptsListener implements IEventListener { public function handle(Event $event): void { if (!$event instanceof LoadAdditionalScriptsEvent) { diff --git a/lib/Listeners/LoadPublicShareScriptsListener.php b/lib/Listeners/LoadPublicShareScriptsListener.php index 9ec57e6..ab4224c 100644 --- a/lib/Listeners/LoadPublicShareScriptsListener.php +++ b/lib/Listeners/LoadPublicShareScriptsListener.php @@ -14,6 +14,10 @@ use OCP\EventDispatcher\IEventListener; use OCP\Util; +/** + * @template-implements IEventListener + * @psalm-api + */ class LoadPublicShareScriptsListener implements IEventListener { public function handle(Event $event): void { if (!$event instanceof BeforeTemplateRenderedEvent) { diff --git a/lib/Listeners/LoadViewerListener.php b/lib/Listeners/LoadViewerListener.php index eca7aff..eaccf5f 100644 --- a/lib/Listeners/LoadViewerListener.php +++ b/lib/Listeners/LoadViewerListener.php @@ -14,6 +14,10 @@ use OCP\EventDispatcher\IEventListener; use OCP\Util; +/** + * @template-implements IEventListener + * @psalm-api + */ class LoadViewerListener implements IEventListener { public function handle(Event $event): void { if (!$event instanceof LoadViewer) { diff --git a/lib/Listeners/MoveToTrashListener.php b/lib/Listeners/MoveToTrashListener.php index 424cdc8..d9da34d 100644 --- a/lib/Listeners/MoveToTrashListener.php +++ b/lib/Listeners/MoveToTrashListener.php @@ -16,6 +16,10 @@ use OCP\Files\File; use Psr\Log\LoggerInterface; +/** + * @template-implements IEventListener + * @psalm-api + */ class MoveToTrashListener implements IEventListener { public function __construct( private LifecycleService $lifecycleService, diff --git a/lib/Listeners/RegisterTemplateCreatorListener.php b/lib/Listeners/RegisterTemplateCreatorListener.php index ae5fac2..26b34c2 100644 --- a/lib/Listeners/RegisterTemplateCreatorListener.php +++ b/lib/Listeners/RegisterTemplateCreatorListener.php @@ -15,6 +15,10 @@ use OCP\Files\Template\TemplateFileCreator; use OCP\IL10N; +/** + * @template-implements IEventListener + * @psalm-api + */ class RegisterTemplateCreatorListener implements IEventListener { public function __construct( private IL10N $l10n, diff --git a/lib/Listeners/RestoreFromTrashListener.php b/lib/Listeners/RestoreFromTrashListener.php index ff3f501..ecdef8b 100644 --- a/lib/Listeners/RestoreFromTrashListener.php +++ b/lib/Listeners/RestoreFromTrashListener.php @@ -19,6 +19,10 @@ use OCP\IUserSession; use Psr\Log\LoggerInterface; +/** + * @template-implements IEventListener + * @psalm-api + */ class RestoreFromTrashListener implements IEventListener { public function __construct( private LifecycleService $lifecycleService, diff --git a/lib/Migration/BackfillPadMimeType.php b/lib/Migration/BackfillPadMimeType.php index bfae864..70543bf 100644 --- a/lib/Migration/BackfillPadMimeType.php +++ b/lib/Migration/BackfillPadMimeType.php @@ -12,6 +12,9 @@ use OCP\Migration\IRepairStep; use OCP\IDBConnection; +/** + * @psalm-api + */ class BackfillPadMimeType implements IRepairStep { private const MIME_PAD = 'application/x-etherpad-nextcloud'; private const MIME_PART = 'application'; diff --git a/lib/Migration/MarkApiKeySensitive.php b/lib/Migration/MarkApiKeySensitive.php index fc55dff..935a61d 100644 --- a/lib/Migration/MarkApiKeySensitive.php +++ b/lib/Migration/MarkApiKeySensitive.php @@ -23,6 +23,7 @@ * {@see AdminSettingsRepository::persist()}; this only backfills the flag * onto the value installs stored before that change. Idempotent: writing * the same value with `sensitive: true` is a no-op once the flag is set. + * @psalm-api */ class MarkApiKeySensitive implements IRepairStep { public function __construct( diff --git a/lib/Migration/RegisterMimeType.php b/lib/Migration/RegisterMimeType.php index ba4fddc..bf70eca 100644 --- a/lib/Migration/RegisterMimeType.php +++ b/lib/Migration/RegisterMimeType.php @@ -14,6 +14,9 @@ use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; +/** + * @psalm-api + */ class RegisterMimeType implements IRepairStep { private const MIME = 'application/x-etherpad-nextcloud'; private const MIME_ALIAS = 'etherpad-nextcloud-pad'; diff --git a/lib/Migration/Version000001Date20260304222000.php b/lib/Migration/Version000001Date20260304222000.php index d13817a..c9b0e72 100644 --- a/lib/Migration/Version000001Date20260304222000.php +++ b/lib/Migration/Version000001Date20260304222000.php @@ -14,6 +14,9 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; +/** + * @psalm-api + */ class Version000001Date20260304222000 extends SimpleMigrationStep { /** * @param Closure(): ISchemaWrapper $schemaClosure diff --git a/lib/Migration/Version000002Date20260512170000.php b/lib/Migration/Version000002Date20260512170000.php index 3884eb8..88bff7b 100644 --- a/lib/Migration/Version000002Date20260512170000.php +++ b/lib/Migration/Version000002Date20260512170000.php @@ -17,6 +17,9 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; +/** + * @psalm-api + */ class Version000002Date20260512170000 extends SimpleMigrationStep { public function __construct( private IDBConnection $db, diff --git a/lib/Migration/Version000003Date20260512230000.php b/lib/Migration/Version000003Date20260512230000.php index 7167df7..d5945af 100644 --- a/lib/Migration/Version000003Date20260512230000.php +++ b/lib/Migration/Version000003Date20260512230000.php @@ -17,6 +17,9 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; +/** + * @psalm-api + */ class Version000003Date20260512230000 extends SimpleMigrationStep { public function __construct( private IDBConnection $db, @@ -24,7 +27,6 @@ public function __construct( } public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { - /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); if (!$schema->hasTable(BindingService::TABLE)) { return; diff --git a/lib/Preview/PadPreviewProvider.php b/lib/Preview/PadPreviewProvider.php index 02760a9..d617529 100644 --- a/lib/Preview/PadPreviewProvider.php +++ b/lib/Preview/PadPreviewProvider.php @@ -28,6 +28,7 @@ * A future iteration could render the actual snapshot text into the * preview (similar to how the Text app previews Markdown), but that's * a separate feature — this exists to silence the 4xx noise. + * @psalm-api */ class PadPreviewProvider implements IProviderV2 { private const ASSET_PATH = __DIR__ . '/../../img/preview-fallback.png'; @@ -44,6 +45,13 @@ public function isAvailable(FileInfo $file): bool { return is_readable(self::ASSET_PATH); } + /** + * `OCP\Image` is the concrete implementation of `OCP\IImage` at runtime, + * but it isn't shipped in the `nextcloud/ocp` stubs Psalm analyses against, + * so Psalm can't see that it satisfies the `?IImage` return type. + * + * @psalm-suppress InvalidReturnType, InvalidReturnStatement + */ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { if (!is_readable(self::ASSET_PATH)) { return null; diff --git a/lib/Service/ConsistencyCheckService.php b/lib/Service/ConsistencyCheckService.php index 9367648..e93ca56 100644 --- a/lib/Service/ConsistencyCheckService.php +++ b/lib/Service/ConsistencyCheckService.php @@ -63,6 +63,6 @@ private function sampleBindingsWithoutFile(int $limit): array { $result = $qb->executeQuery(); $rows = $result->fetchAll(); $result->closeCursor(); - return is_array($rows) ? $rows : []; + return $rows; } } diff --git a/lib/Service/EtherpadHealthCheckService.php b/lib/Service/EtherpadHealthCheckService.php index 56a71cf..c9f85cf 100644 --- a/lib/Service/EtherpadHealthCheckService.php +++ b/lib/Service/EtherpadHealthCheckService.php @@ -64,7 +64,7 @@ public function check(ValidatedAdminSettings $settings): HealthCheckResult { $settings->etherpadApiHost, $settings->etherpadApiVersion, (int)($result['pad_count'] ?? 0), - (int)round(($this->now() - $startedAt) * 1000), + (int)round(($this->now() - $startedAt) * 1000.0), rtrim($settings->etherpadApiHost, '/') . '/api/' . $settings->etherpadApiVersion . '/listAllPads', $this->pendingDeleteRetryService->countPendingDeletes(), ); diff --git a/lib/Service/ExternalPadExportFetcher.php b/lib/Service/ExternalPadExportFetcher.php index 61428db..f43137b 100644 --- a/lib/Service/ExternalPadExportFetcher.php +++ b/lib/Service/ExternalPadExportFetcher.php @@ -70,7 +70,7 @@ private function buildPublicExportUrl(string $padUrl, string $format): string { } /** - * @param array{pad_url:string,host:string,port:int,resolved_ips:list} $resolved + * @param array{origin:string,pad_id:string,pad_url:string,host:string,port:int,resolved_ips:list} $resolved */ private function getPublicTextFromResolvedExternalPad(array $resolved): string { $url = $this->buildPublicExportUrl($resolved['pad_url'], 'txt'); @@ -156,6 +156,11 @@ private function sendPinnedPublicGetRequest(string $url, string $host, int $port curl_close($curl); if ($success === false) { + // $sizeExceeded is flipped to true inside the CURLOPT_WRITEFUNCTION + // closure above when the response exceeds the cap; Psalm doesn't + // model curl invoking that callback, so it wrongly infers the flag + // stays false here. + /** @psalm-suppress TypeDoesNotContainType */ if ($sizeExceeded) { throw new EtherpadClientException( 'External public pad export exceeds maximum size (' . self::EXTERNAL_EXPORT_MAX_BYTES . ' bytes).' diff --git a/lib/Service/LifecycleService.php b/lib/Service/LifecycleService.php index 39e0266..1966a49 100644 --- a/lib/Service/LifecycleService.php +++ b/lib/Service/LifecycleService.php @@ -279,7 +279,7 @@ public function handleTrash(File $file): array { } } - /** @return array{status: string, reason?: string, file_id: int, old_pad_id?: string, new_pad_id?: string} */ + /** @return array{status: string, reason?: string, file_id: int, pad_id?: string, old_pad_id?: string, new_pad_id?: string} */ public function handleRestore(File $file): array { $fileId = (int)$file->getId(); if (!$this->isPadFile($file)) { @@ -396,7 +396,7 @@ public function handleRestore(File $file): array { * caller has already verified the user owns the file, and the security * model demands we never reuse the `pad_id` from frontmatter. * - * @return array{status: string, reason?: string, file_id: int, old_pad_id?: string, new_pad_id?: string} + * @return array{status: string, reason?: string, file_id: int, pad_id?: string, old_pad_id?: string, new_pad_id?: string} */ public function recoverFromSnapshot(File $file): array { $fileId = (int)$file->getId(); @@ -418,11 +418,10 @@ public function recoverFromSnapshot(File $file): array { return $result; } - /** @return array{status: string, reason?: string, file_id: int, old_pad_id?: string, new_pad_id?: string} */ + /** @return array{status: string, reason?: string, file_id: int, pad_id?: string, old_pad_id?: string, new_pad_id?: string} */ private function restoreWithoutBinding(File $file, int $fileId): array { $oldPadId = ''; $newPadId = ''; - $currentContent = ''; $fileContentUpdated = false; $managedPadCreated = false; $bindingCreated = false; @@ -547,7 +546,7 @@ private function isExternalPadFile(File $file, ?string $content = null): bool { private function provisionRestorePadId(string $accessMode, string $oldPadId): string { if ($accessMode === BindingService::ACCESS_PROTECTED) { $groupId = $this->etherpadClient->createGroup(); - $padName = $this->buildProtectedRestorePadName($oldPadId); + $padName = $this->buildProtectedRestorePadName(); return $this->etherpadClient->createGroupPad($groupId, $padName); } @@ -591,7 +590,7 @@ private function buildPublicRestorePadId(string $oldPadId): string { return 'r-' . trim($normalized, '-') . '-' . $suffix; } - private function buildProtectedRestorePadName(string $oldPadId): string { + private function buildProtectedRestorePadName(): string { $suffix = $this->secureRandom->generate(14, ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_DIGITS); return 'restored-' . $suffix; } diff --git a/lib/Service/PadBootstrapService.php b/lib/Service/PadBootstrapService.php index 7df358a..6c8754f 100644 --- a/lib/Service/PadBootstrapService.php +++ b/lib/Service/PadBootstrapService.php @@ -91,8 +91,6 @@ public function initializeMissingFrontmatter(string $uid, File $file, string $ex $binding = $this->bindingService->findByFileId($fileId); $createdNewBinding = false; $createdNewPad = false; - $padId = ''; - $accessMode = BindingService::ACCESS_PROTECTED; if ($binding !== null) { $padId = (string)$binding['pad_id']; diff --git a/lib/Service/PadCreationService.php b/lib/Service/PadCreationService.php index e461cf8..a1f520e 100644 --- a/lib/Service/PadCreationService.php +++ b/lib/Service/PadCreationService.php @@ -183,14 +183,14 @@ function () use ($parentFolderId, $name, $accessMode, &$path, &$padId): array { } /** - * @return array{file:string,file_id:int,pad_id:string,access_mode:string,pad_url:string} + * @return array{file:string,file_id:int,pad_id:string,access_mode:string,pad_url:string,snapshot_warning_code?:string} */ public function createFromUrl(string $uid, string $file, string $padUrl): array { $path = $this->padPaths->normalizeCreatePath($file); $fileCreated = false; return $this->withCreateRollback( - function () use ($uid, $path, $padUrl, &$fileCreated): array { + function () use ($uid, $path, $padUrl, &$fileCreated) { $fileNode = $this->padFileCreator->createUserFile($uid, $path); $fileCreated = true; $fileId = (int)$fileNode->getId(); diff --git a/lib/Service/PadFileCreator.php b/lib/Service/PadFileCreator.php index e6c9e78..6b5de70 100644 --- a/lib/Service/PadFileCreator.php +++ b/lib/Service/PadFileCreator.php @@ -32,6 +32,10 @@ public function createUserFile(string $uid, string $absolutePath): File { $parentPath = dirname($relativePath); $fileName = basename($relativePath); + // Psalm infers basename() as non-empty-string, but it returns '' for + // slash-only inputs (e.g. basename('/')), so the empty-string guard is + // a real defensive check, not dead code. + /** @psalm-suppress TypeDoesNotContainType */ if ($fileName === '' || $fileName === '.' || $fileName === '..') { throw new \RuntimeException('Invalid target filename.'); } diff --git a/lib/Service/PadMetadataService.php b/lib/Service/PadMetadataService.php index efd1d53..8b527a3 100644 --- a/lib/Service/PadMetadataService.php +++ b/lib/Service/PadMetadataService.php @@ -51,7 +51,6 @@ public function findOriginalForCopy(string $uid, int $fileId): PadOriginalLookup return new PadOriginalLookup(found: false); } - $padId = ''; try { $padId = $this->padFileService->readPad((string)$node->getContent())->padId; } catch (\Throwable) { diff --git a/lib/Service/PadOpenService.php b/lib/Service/PadOpenService.php index 64d6e55..5a3ca32 100644 --- a/lib/Service/PadOpenService.php +++ b/lib/Service/PadOpenService.php @@ -120,7 +120,6 @@ private function buildOpenContext( throw new EtherpadClientException('External pad metadata requires public access_mode.'); } - $effectivePadUrl = ''; $originalPadUrl = ''; if ($isExternal) { diff --git a/lib/Service/PublicShareUrlBuilder.php b/lib/Service/PublicShareUrlBuilder.php index 143fc46..ab8f1c0 100644 --- a/lib/Service/PublicShareUrlBuilder.php +++ b/lib/Service/PublicShareUrlBuilder.php @@ -58,13 +58,13 @@ public function buildShareRedirectUrl(string $token, mixed $fileParam): string { } $dir = dirname($path); - if ($dir === '.' || $dir === '') { + if ($dir === '.') { $dir = '/'; } else { $dir = '/' . $dir; } $fileName = basename($path); - if ($fileName === '' || !str_ends_with(strtolower($fileName), '.pad')) { + if (!str_ends_with(strtolower($fileName), '.pad')) { throw new NotAPadFileException('The selected file is not a .pad document.'); } diff --git a/lib/Settings/AdminSection.php b/lib/Settings/AdminSection.php index 1ec2b0a..0164cd5 100644 --- a/lib/Settings/AdminSection.php +++ b/lib/Settings/AdminSection.php @@ -12,6 +12,9 @@ use OCP\IURLGenerator; use OCP\Settings\IIconSection; +/** + * @psalm-api + */ class AdminSection implements IIconSection { public function __construct( private IL10N $l10n, diff --git a/lib/Settings/AdminSettings.php b/lib/Settings/AdminSettings.php index 99766a2..c2ad094 100644 --- a/lib/Settings/AdminSettings.php +++ b/lib/Settings/AdminSettings.php @@ -19,6 +19,9 @@ use OCP\Settings\ISettings; use OCP\Util; +/** + * @psalm-api + */ class AdminSettings implements ISettings { public function __construct( private IConfig $config, diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 90313a5..490c979 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,170 +1,235 @@ - - - - - - - - - - - - + + + + + + + + + + + + + + - - - + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - + + + + + + + + + + + + + - - now() - $startedAt) * 1000]]> - + + + - - - - - - - - + + + + + + + + - - buildSkippedResult('binding_not_pending_delete', $fileId, $oldPadId)]]> - buildSkippedResult('binding_state_transition_conflict', $fileId, $oldPadId)]]> - buildSkippedResult('delete_on_trash_disabled', $fileId)]]> - buildSkippedResult('external_pad', $fileId, $oldPadId)]]> - buildSkippedResult('not_pad_file', $fileId)]]> - - - - - + + + + + + + + + + + + + - - withCreateRollback( - function () use ($uid, $path, $padUrl, &$fileCreated): array { - $fileNode = $this->padFileCreator->createUserFile($uid, $path); - $fileCreated = true; - $fileId = (int)$fileNode->getId(); - if ($fileId <= 0) { - throw new \RuntimeException('Could not resolve new file ID.'); - } - - $seeded = $this->externalPadSeeder->seed($fileNode, $fileId, $padUrl); - // Preserve the historical key ordering for the external-create - // response: `file` is the first key so tests asserting via - // `assertSame` keep matching after the refactor. - $result = ['file' => $path] + $seeded; - return $result; - }, - function () use ($uid, $path, &$fileCreated): void { - $this->rollbackService->rollbackExternalCreate($uid, $path, $fileCreated); - }, - function (\Throwable $e) use ($path, $padUrl): ?array { - if ($e instanceof EtherpadClientException) { - return [ - 'message' => 'External pad URL validation failed', - 'context' => [ - 'file' => $path, - 'padUrl' => $padUrl, - ], - ]; - } - - return null; - }, - function () use ($path, $padUrl): array { - return [ - 'message' => 'External pad create failed', - 'context' => [ - 'file' => $path, - 'padUrl' => $padUrl, - ], - ]; - }, - )]]> - - - - + + + - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/psalm.xml b/psalm.xml index e2c8f16..fd42434 100644 --- a/psalm.xml +++ b/psalm.xml @@ -3,7 +3,7 @@ errorLevel="4" phpVersion="8.1" resolveFromConfigFile="true" - findUnusedCode="false" + findUnusedCode="true" autoloader="tests/psalm/ocp-autoload.php" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config"