diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f05598..c604867 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/AGENTS.md b/AGENTS.md index 8b5afb4..62540c9 100644 --- a/AGENTS.md +++ b/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) ``` diff --git a/README.md b/README.md index 038aa43..1c83578 100644 --- a/README.md +++ b/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 diff --git a/composer.json b/composer.json index 5831a3a..1266ab6 100644 --- a/composer.json +++ b/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": { diff --git a/imapsync.php b/imapsync.php index 0e9da90..b57a6bc 100644 --- a/imapsync.php +++ b/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 diff --git a/lib/RoundcubeImapSyncClient.php b/lib/RoundcubeImapSyncClient.php index e13ae66..63c267a 100644 --- a/lib/RoundcubeImapSyncClient.php +++ b/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,6 +62,7 @@ public function listFolders(): array return array_values(array_map('strval', $folders)); } + #[\Override] public function getHierarchyDelimiter(): string { $delimiter = $this->imap->getHierarchyDelimiter(); @@ -66,11 +70,13 @@ public function getHierarchyDelimiter(): string 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; } /** diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 0000000..aae5ea7 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + diff --git a/tests/Fakes/FakeImapClient.php b/tests/Fakes/FakeImapClient.php index 1f3cfd6..dfb355c 100644 --- a/tests/Fakes/FakeImapClient.php +++ b/tests/Fakes/FakeImapClient.php @@ -23,6 +23,7 @@ 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) { @@ -30,20 +31,24 @@ public function connect(string $host, int $port, string $user, string $password, } } + #[\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 $messages */ + $messages = $this->folders[$folder]; + + return max(array_keys($messages)) + 1; } private function identityFromRaw(string $rawMessage): array diff --git a/tests/Integration/PreflightAndQuotaIntegrationTest.php b/tests/Integration/PreflightAndQuotaIntegrationTest.php index b8f9a17..1be9be0 100644 --- a/tests/Integration/PreflightAndQuotaIntegrationTest.php +++ b/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 { diff --git a/tests/Integration/SyncEngineIntegrationTest.php b/tests/Integration/SyncEngineIntegrationTest.php index 46335ff..08d0b50 100644 --- a/tests/Integration/SyncEngineIntegrationTest.php +++ b/tests/Integration/SyncEngineIntegrationTest.php @@ -17,6 +17,7 @@ final class SyncEngineIntegrationTest extends TestCase private static ?DovecotContainer $destinationContainer = null; private static string $sourceSubfolder = ''; + #[\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 {