chore(quality): migrate to PHPStan 2 and clear the 27 residual errors

[rubenvdlinde]

Supersedes #363 (the bare dependabot bump), which turns PHPStan 2 on without fixing what it then reports.

Result

phpstan analyse: 27 errors → 0. 1801 PHPUnit tests green (5676 assertions, 0 warnings — down from 1). phpcs: 0 errors.

Bumps phpstan/phpstan to ^2.0 and conduction/hydra-gates to ^1.8.2, which carries the shared phpstan-base.neon fixes (treatPhpDocTypesAsCertain: false and the ignore for PHPMD's @SuppressWarnings tripping phpDoc.parseError). Those were verified on both majors before release, so the bump does not silently disarm the gate.

Dead method_exists() probes (5)

Four controllers carry an identical resolveActiveOrganisation() probing for getActiveOrganisationWithFallback() on OpenRegister's OrganisationMapper. Three independent sources declare it — this app's stub, openregister@development, and openregister@main — so the probe could never be false and its fallback was unreachable.

I checked all three deliberately. The app's own stub proves nothing on its own: it is hand-maintained here, so verifying against it would just be confirming what we wrote. The two canonical branches are what make this safe.

The fifth is generateStreamOfText() on OpenAIChat|OllamaChat — both branches of the declared union declare it. A provider that advertises streaming but cannot deliver still throws MissingFeatureException, and that catch is untouched.

Dead type guards (5)

• ToolOversightController — two $updated instanceof ObjectEntity guards (each also compared === true). Their comment said saveObject()'s return type "is not guaranteed across OpenRegister versions". It is: both main and development declare saveObject(): ObjectEntity as a hard return type, which PHP itself enforces. The TypeError the comment feared was not reachable.
• TalkTurnDispatcher — is_callable() on a callable already narrowed by the is_a() check directly above it. The comment claimed ITriggerableProvider was absent from the pinned OCP; the pinned OCP now ships it. The is_a() guard stays.
• ProviderFactory — four is_string($response) checks on a value the line above casts with (string).

Redundant expressions (7)

• BudgetService — isset(...) && ... !== null; isset() is already false for null.
• DelegationService / DelegationContext — six ?-> uses on the left side of ??, which PHP already null-suppresses. Purely syntactic; current() really can return null and still can.
• ToolGrantResolver — two array_values(array_keys(...)); array_keys() already returns a list.

Real type-contract fix (1)

SkillBundleSerializer::toBundle() takes $dropped/$droppedAgents by reference and initialises both to [] on entry, but its signature says ?array, so every caller inferred array|null. That leaked into SkillController::bundlePublish() and made the entire JSONResponse payload an unresolvable type.

Both now carry @param-out — documenting what the method has always actually guaranteed, rather than widening or asserting anything. SkillMarketplaceService::updateExisting() gets the same treatment for &$outcome.

Optional runtime dependency (1 scoped ignore)

MailReadService probes for OCA\Mail service classes with class_exists() before resolving them from the container. The Mail app is not in static analysis and MAIL_CLASSES holds literal class-name strings, so PHPStan 2 proves the probe always false.

That verdict is correct about the analysis environment and wrong about production. The probe is precisely what makes the soft dependency safe; deleting it would turn "Mail is not installed" from a null return into a container exception. So it gets an ignore scoped to that one file and the function.impossibleType identifier — the same shape as the existing OCA\Talk ignores. It is the only class_exists() call in the file, so the scope cannot hide a second one.

This is the one entry that suppresses rather than fixes, and it is deliberate. If you would rather the Mail classes were stubbed into tests/Stubs like the OpenRegister/Talk contracts, say so and I will do that instead.

Pre-existing test warning, fixed

ToolGrantCodec::grantStringFor() opened its loop with an unconditional (string)$value and only then checked for arrays, so every array-valued constraint raised "Array to string conversion" before the correct value overwrote it — 3 tests triggered it. The output was already correct; the warning was noise. The array case is now handled first, and the suite reports 0 warnings.

[github-actions]

Quality Report — ConductionNL/hermiq @ 8309fa5

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
check-manifest		✅
test-l10n		✅
format		✅
composer			✅	✅ 121/121
npm			✅	✅ 719/719
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					✅
Newman					✅
Playwright					✅
Hydra gates					✅

---

Quality workflow — 2026-08-20 21:59 UTC

Download the full PDF report from the workflow artifacts.