Merge pull request #3 from mittwald/chore/psalm-static-analysis

Add Psalm static analysis (composer analyse + CI job)

Author: Hermsi1337

.github/workflows/ci.yml

@@ -15,19 +15,15 @@ concurrency:
 
 jobs:
   unit-tests:
-    name: Unit tests (PHP ${{ matrix.php }})
+    name: Unit tests
     runs-on: ubuntu-latest
-    strategy:
-      fail-fast: false
-      matrix:
-        php: ['8.1', '8.2', '8.3', '8.4']
     steps:
       - uses: actions/checkout@v4
 
-      - name: Set up PHP ${{ matrix.php }}
+      - name: Set up PHP
         uses: shivammathur/setup-php@v2
         with:
-          php-version: ${{ matrix.php }}
+          php-version: '8.4'
           coverage: none
           tools: composer:v2
           extensions: imap, intl, mbstring
@@ -44,6 +40,29 @@ jobs:
       - name: Run unit tests
         run: composer test:unit -- --colors=never
 
+  static-analysis:
+    name: Static analysis (Psalm)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: '8.4'
+          coverage: none
+          tools: composer:v2
+          extensions: imap, intl, mbstring
+
+      - name: Install dependencies
+        uses: ramsey/composer-install@v3
+
+      - name: Fetch Roundcube source for type resolution
+        run: composer dist:fetch
+
+      - name: Run Psalm
+        run: composer analyse
+
   integration-tests:
     name: Integration tests (Dovecot via Docker)
     runs-on: ubuntu-latest
@@ -54,7 +73,7 @@ jobs:
       - name: Set up PHP
         uses: shivammathur/setup-php@v2
         with:
-          php-version: '8.3'
+          php-version: '8.4'
           coverage: none
           tools: composer:v2
           extensions: imap, intl, mbstring

AGENTS.md

@@ -57,6 +57,7 @@ future worker mode — must use an out-of-session store (file, DB, Roundcube cac
 ├── LICENSE                                   # MIT
 ├── README.md                                 # user-facing documentation
 ├── composer.json                             # plugin manifest (Roundcube plugin-installer type)
+├── psalm.xml                                 # Psalm static-analysis configuration (no baseline — every error must be fixed)
 ├── imapsync.php                              # plugin entry class (extends rcube_plugin)
 ├── imapsync.js                               # client-side: form handling, result rendering
 ├── config.inc.php.dist                       # default plugin config, copied by user to config.inc.php
@@ -91,7 +92,7 @@ future worker mode — must use an out-of-session store (file, DB, Roundcube cac
 ├── .gitattributes                            # export-ignore tests/, dist/, .github/, etc.
 ├── .github/
 │   └── workflows/
-│       ├── ci.yml                            # unit + integration tests, lint, docs-freshness
+│       ├── ci.yml                            # unit + integration tests, Psalm, lint, docs-freshness
 │       └── release.yml                       # tag → tarball + zip + checksums on GitHub Releases
 └── dist/                                     # gitignored — local Roundcube source for reference (see below)
 ```
@@ -203,6 +204,8 @@ global rules win.
   call.
 - Run with `composer test:unit` (unit only) or `composer test:integration` (Docker, real
   Dovecot) from the repo root. CI runs both.
+- Run `composer analyse` for Psalm static analysis over `imapsync.php`, `lib/`, and `tests/`;
+  it uses the Roundcube source in `dist/` for type resolution.
 
 ### Security expectations
 
@@ -332,9 +335,9 @@ pre-commit check.
 
 GitHub Actions (`.github/workflows/ci.yml`) runs the deterministic subset of these checks
 (config-key consistency, locale-key consistency) on every push and PR as the `docs-freshness`
-job, alongside `unit-tests`, `integration-tests`, and PHP linting. A failing freshness check
-blocks the PR. CI is a safety net, not a substitute — run the checks locally before pushing so
-you don't burn a round trip on something a 30-second grep would have caught.
+job, alongside `unit-tests`, `static-analysis`, `integration-tests`, and PHP linting. A failing
+freshness check blocks the PR. CI is a safety net, not a substitute — run the checks locally
+before pushing so you don't burn a round trip on something a 30-second grep would have caught.
 
 ---
 
@@ -397,6 +400,7 @@ Test suite:
 
 ```bash
 composer install
+composer analyse                 # Psalm static analysis (uses dist/ for Roundcube symbols)
 composer test:unit               # unit tests (fast, no network, no Docker)
 composer test:integration        # integration tests (requires a running Docker daemon)
 ```

README.md

@@ -187,6 +187,7 @@ composer install
 composer test:unit          # in-memory unit tests, no network, no Docker
 composer test:integration   # Dovecot integration suite, requires a running Docker daemon
 composer test               # alias for test:unit
+composer analyse            # Psalm static analysis over plugin code and tests
 ```
 
 The unit tests do not touch the network — the sync engine takes an injected IMAP-client

composer.json

@@ -29,18 +29,21 @@
     },
     "require-dev": {
         "phpunit/phpunit": "^10.5",
-        "testcontainers/testcontainers": "^1.0"
+        "testcontainers/testcontainers": "^1.0",
+        "vimeo/psalm": "^6.0"
     },
     "scripts": {
         "test": "@test:unit",
         "test:unit": "phpunit",
         "test:integration": "phpunit --configuration phpunit.integration.xml.dist",
+        "analyse": "psalm --no-progress",
         "dist:fetch": "sh -c 'if [ -d dist/roundcubemail-1.7.0 ]; then echo \"dist/roundcubemail-1.7.0 already present, skipping.\"; exit 0; fi; curl -fLsS -o dist/roundcubemail-1.7.0-complete.tar.gz https://github.com/roundcube/roundcubemail/releases/download/1.7.0/roundcubemail-1.7.0-complete.tar.gz && tar -xzf dist/roundcubemail-1.7.0-complete.tar.gz -C dist && rm dist/roundcubemail-1.7.0-complete.tar.gz && echo \"dist/roundcubemail-1.7.0 ready.\"'"
     },
     "scripts-descriptions": {
         "test": "Alias for test:unit.",
         "test:unit": "Run the in-memory unit-test suite (fast, no Docker).",
         "test:integration": "Run the Dovecot integration suite (requires a running Docker daemon).",
+        "analyse": "Run Psalm static analysis over imapsync.php, lib/, and tests/.",
         "dist:fetch": "Download and extract Roundcube 1.7.0 source into dist/ for local reference. Idempotent."
     },
     "autoload": {

imapsync.php

@@ -15,6 +15,7 @@ class imapsync extends rcube_plugin
 
     private rcmail $rc;
 
+    #[\Override]
     public function init(): void
     {
         $this->rc = rcmail::get_instance();
@@ -278,7 +279,7 @@ public function action_status(): void
     private function addFormRow(html_table $table, string $fieldId, string $labelKey, string $field): void
     {
         $table->add('title', html::label($fieldId, rcube::Q($this->gettext($labelKey))));
-        $table->add(null, $field);
+        $table->add('', $field);
     }
 
     private function createJobFromRequest(): RoundcubeImapSyncJob

lib/RoundcubeImapSyncClient.php

@@ -35,6 +35,7 @@ public function __construct(private readonly rcube_imap_generic $imap)
     {
     }
 
+    #[\Override]
     public function connect(string $host, int $port, string $user, string $password, array $options): void
     {
         $options['port'] = $port;
@@ -44,11 +45,13 @@ public function connect(string $host, int $port, string $user, string $password,
         }
     }
 
+    #[\Override]
     public function close(): void
     {
         $this->imap->closeConnection();
     }
 
+    #[\Override]
     public function listFolders(): array
     {
         $folders = $this->imap->listMailboxes('', '*');
@@ -59,18 +62,21 @@ public function listFolders(): array
         return array_values(array_map('strval', $folders));
     }
 
+    #[\Override]
     public function getHierarchyDelimiter(): string
     {
         $delimiter = $this->imap->getHierarchyDelimiter();
 
         return is_string($delimiter) && $delimiter !== '' ? $delimiter : '/';
     }
 
+    #[\Override]
     public function createFolder(string $folder): bool
     {
         return $this->imap->createFolder($folder);
     }
 
+    #[\Override]
     public function selectFolder(string $folder): int
     {
         $status = $this->imap->status($folder, ['MESSAGES']);
@@ -81,6 +87,7 @@ public function selectFolder(string $folder): int
         return (int) ($status['MESSAGES'] ?? 0);
     }
 
+    #[\Override]
     public function getFolderSize(string $folder): int
     {
         if ($this->supportsStatusSize()) {
@@ -108,6 +115,7 @@ public function getFolderSize(string $folder): int
         return $totalSize;
     }
 
+    #[\Override]
     public function getQuota(string $folder): ?array
     {
         $quota = $this->imap->getQuota($folder);
@@ -121,6 +129,7 @@ public function getQuota(string $folder): ?array
         ];
     }
 
+    #[\Override]
     public function supportsStatusSize(): bool
     {
         if ($this->statusSizeSupported === null) {
@@ -130,6 +139,7 @@ public function supportsStatusSize(): bool
         return $this->statusSizeSupported;
     }
 
+    #[\Override]
     public function fetchMessageIdentities(string $folder): array
     {
         $totalMessages = $this->selectFolder($folder);
@@ -160,6 +170,7 @@ public function fetchMessageIdentities(string $folder): array
         return $identities;
     }
 
+    #[\Override]
     public function fetchMessageRaw(string $folder, int $uid): ?array
     {
         $messages = $this->imap->fetch($folder, (string) $uid, true, ['UID', 'RFC822', 'FLAGS', 'INTERNALDATE']);
@@ -179,6 +190,7 @@ public function fetchMessageRaw(string $folder, int $uid): ?array
         ];
     }
 
+    #[\Override]
     public function appendMessage(string $folder, string $rawMessage, array $flags, ?string $internalDate): bool
     {
         $message = $rawMessage;
@@ -193,7 +205,11 @@ public function appendMessage(string $folder, string $rawMessage, array $flags,
             throw new RoundcubeImapSyncException($errorMessage);
         }
 
-        return $result;
+        // rcube_imap_generic::append returns mixed: false on failure, true on
+        // plain success, or the appended UID (string) when the server supports
+        // UIDPLUS. Our interface only contracts a bool, so collapse the
+        // success cases to true.
+        return (bool) $result;
     }
 
     /**

psalm.xml

@@ -0,0 +1,24 @@
+<?xml version="1.0"?>
+<psalm
+    errorLevel="6"
+    resolveFromConfigFile="true"
+    findUnusedBaselineEntry="true"
+    findUnusedCode="false"
+    cacheDirectory="/tmp/psalm-cache"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns="https://getpsalm.org/schema/config"
+    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
+>
+    <projectFiles>
+        <file name="imapsync.php"/>
+        <directory name="lib"/>
+        <directory name="tests"/>
+    </projectFiles>
+
+    <extraFiles>
+        <directory name="dist/roundcubemail-1.7.0/program/lib/Roundcube"/>
+    </extraFiles>
+
+    <!-- If errors in scope are unavoidable (Roundcube interop quirks),
+         add <issueHandlers> here. Prefer baselining over suppressing. -->
+</psalm>

tests/Fakes/FakeImapClient.php

@@ -23,27 +23,32 @@ public function seed(string $folder, array $messages): void
         $this->folders[$folder] = $messages;
     }
 
+    #[\Override]
     public function connect(string $host, int $port, string $user, string $password, array $options): void
     {
         if ($this->connectShouldFail) {
             throw new RoundcubeImapSyncException('Source connect failed.');
         }
     }
 
+    #[\Override]
     public function close(): void
     {
     }
 
+    #[\Override]
     public function listFolders(): array
     {
         return array_keys($this->folders);
     }
 
+    #[\Override]
     public function getHierarchyDelimiter(): string
     {
         return $this->delimiter;
     }
 
+    #[\Override]
     public function createFolder(string $folder): bool
     {
         if (isset($this->createFolderFailures[$folder])) {
@@ -59,6 +64,7 @@ public function createFolder(string $folder): bool
         return false;
     }
 
+    #[\Override]
     public function selectFolder(string $folder): int
     {
         if (!isset($this->folders[$folder])) {
@@ -68,6 +74,7 @@ public function selectFolder(string $folder): int
         return count($this->folders[$folder]);
     }
 
+    #[\Override]
     public function getFolderSize(string $folder): int
     {
         if ($this->getFolderSizeShouldFail) {
@@ -90,16 +97,19 @@ public function getFolderSize(string $folder): int
         return $total;
     }
 
+    #[\Override]
     public function getQuota(string $folder): ?array
     {
         return $this->quotaResult;
     }
 
+    #[\Override]
     public function supportsStatusSize(): bool
     {
         return $this->statusSizeSupported;
     }
 
+    #[\Override]
     public function fetchMessageIdentities(string $folder): array
     {
         if (!isset($this->folders[$folder])) {
@@ -114,6 +124,7 @@ public function fetchMessageIdentities(string $folder): array
         return $identities;
     }
 
+    #[\Override]
     public function fetchMessageRaw(string $folder, int $uid): ?array
     {
         if (!isset($this->folders[$folder][$uid])) {
@@ -127,6 +138,7 @@ public function fetchMessageRaw(string $folder, int $uid): ?array
         ];
     }
 
+    #[\Override]
     public function appendMessage(string $folder, string $rawMessage, array $flags, ?string $internalDate): bool
     {
         if (isset($this->appendQuotaFailures[$folder])) {
@@ -158,7 +170,10 @@ private function nextUid(string $folder): int
             return 1;
         }
 
-        return max(array_keys($this->folders[$folder])) + 1;
+        /** @var non-empty-array<int, mixed> $messages */
+        $messages = $this->folders[$folder];
+
+        return max(array_keys($messages)) + 1;
     }
 
     private function identityFromRaw(string $rawMessage): array

tests/Integration/PreflightAndQuotaIntegrationTest.php

@@ -15,6 +15,7 @@ final class PreflightAndQuotaIntegrationTest extends TestCase
     private static ?DovecotContainer $sourceContainer = null;
     private static ?DovecotContainer $destinationContainer = null;
 
+    #[\Override]
     public static function setUpBeforeClass(): void
     {
         if (getenv('DOVECOT_INTEGRATION_SKIP') === '1') {
@@ -38,6 +39,7 @@ public static function setUpBeforeClass(): void
         self::seedSourceContainer();
     }
 
+    #[\Override]
     public static function tearDownAfterClass(): void
     {
         try {