AI Technical Debt Analysis

Summary

A technical analysis of the PHP code base (lib/, ~503 files excluding lib/Vendor/) identified the remaining technical debt now that the app requires PHP 8.1+ and Nextcloud 32+. The good news first: the classic legacy patterns are already gone — no \OC::$server, no OCP\ILogger, no deprecated query builder ->execute(), no legacy hooks, events go through IEventDispatcher. The remaining debt falls into three buckets: PHP 7.0-era code style at scale, two generations of the same concept coexisting, and inconsistent conventions.

This is a tracking ticket; each item below is a candidate for its own focused PR.

Deprecated API usage

• Controller annotations → attributes: 29 of 36 controllers still use deprecated docblock annotations (@NoAdminRequired, @NoCSRFRequired, …), only 8 use PHP attributes (#[NoAdminRequired]). Mechanical migration, should be one PR.
• IConfig app values → IAppConfig: 49 getAppValue/setAppValue call sites vs. 14 IAppConfig usages — both APIs in active use for the same purpose. Standardize on IAppConfig for app-level values.

PHP 7.0-era style (vs. PHP 8.1 floor)

• Constructor property promotion: ~355 files with constructors, only ~10 use promoted properties. Rector-assisted sweep; land before other refactors to avoid rebase pain. https://github.com/nextcloud/mail/pull/13124
• Native property types: hundreds of docblock-only /** @var */ properties, concentrated in lib/Db/ entities (all 44 use untyped protected + @method docblocks) and older services. Prerequisite for a stricter Psalm level (currently errorLevel="3").
• Native enums: zero enum declarations. Candidates: LocalMessage type/status constants, Recipient TYPE_*, Address type constants, OutOfOfficeParser/FilterParser state constants, Model\Classifier. Start with pure in-memory ones; DB-backed need backed enums + mapper care.
• Leftover string idioms: 4 remaining strpos() !== false patterns → str_contains(); a handful of switch statements that would read better as match. https://github.com/nextcloud/mail/pull/13125

Same thing, different concepts — alignment needed

• Two JSON response classes: custom OCA\Mail\Http\JsonResponse (19 controllers) vs. OCP JSONResponse (15 controllers); inconsistent status code conventions. Converge on the custom one.
• #[TrapError] coverage: applied in 26 of 36 controllers; the rest hand-roll try/catch or leak 500s. Apply uniformly and standardize services on throwing typed OCA\Mail\Exception\* exceptions instead of returning sentinel nulls.
• Two sending pipelines: lib/Send/ handler chain vs. the older MailTransmission monolith with overlapping responsibilities (sent-folder copy, flagging). Decide the chain is the target and shrink MailTransmission to SMTP transport.
• Sync layer ownership: MailboxSync → SyncService → ImapToDbSynchronizer → IMAP\Sync\Synchronizer — responsibilities leak between the middle two (both know about locking and mailbox state). Should be written down/cleaned before further sync work lands.
• MessageMapper name collision: Db\MessageMapper vs. IMAP\MessageMapper forces import aliases in ~10 files with per-file alias spellings. Rename the IMAP one (e.g. ImapMessageFetcher). Same for FolderMapper.
• Serialization: 43 × jsonSerialize(), one bespoke toJson() (Db\MailAccount), plus hand-built arrays in some controllers. Converge on JsonSerializable.
• AI APIs span two generations: TextProcessing (sync: smart replies, follow-up, translation, event extraction) vs. TaskProcessing (async: summaries); TaskProcessingListener distinguishes task origins by sniffing for 'max_tokens' in the prompt. Migrate to TaskProcessing behind one internal abstraction.

Smaller items

• Config keys as scattered string literals ('allow_new_mail_accounts', 'layout_message_view', 'llm_processing', …) — introduce a constants class.
• IMAP client lifecycle: ~60% of call sites use try/finally { logout() }; introduce a withClient(callable) wrapper to make the safe pattern the only pattern.
• N+1 in SnoozeService (per-message getSrcMailboxId() lookup).
• MessageMapper::updateBulk() binds boolean flags with PARAM_INT instead of PARAM_BOOL.
• 2–3 direct new DateTime('now') calls bypassing ITimeFactory (ProxyDownloadResponse, FollowUpClassifierListener).
• Provisioning checks (getProvisioningId() !== null) repeated in 4+ places — extract Account::isProvisioned().
• Move the two global functions in lib/functions.php (array_flat_map, chunk_uid_sequence) into a utility class.
• Only MessagesController paginates; other list endpoints return unbounded result sets.

Suggested order

1. Controller attributes migration (deprecation with a server-side deadline)
2. One JSON response class + #[TrapError] everywhere
3. Constructor property promotion sweep (before any typing work)
4. IAppConfig migration + config key constants
5. Native property types → stricter Psalm level
6. Enums, naming collisions, lifecycle wrapper, remaining items

This analysis was produced with AI assistance (Claude Code, claude-fable-5); headline counts were verified manually with grep. Full report: TECHNICAL_DEBT_ANALYSIS.md (local).

Start with lib/ and the local TECHNICAL_DEBT_ANALYSIS.md report, then select one checkbox rather than treating this tracking issue as a single task. Use the suggested order to choose a focused area; done means a separate, scoped PR addresses that item and verifies its stated migration or cleanup.