>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3619893. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !32 >>> <p>audit_trail has shipped its own <code>phpstan.neon</code> at <code>level: 3</code> since <span class="drupalorg-gitlab-issue-link drupalorg-gitlab-link-wrapper"><a href="https://git.drupalcode.org/project/audit_trail/-/work_items/3618632" class="drupalorg-gitlab-link">https://git.drupalcode.org/project/audit_trail/-/work_items/3618632</a></span>. That issue measured the next two bands and deliberately left them out of scope: level 4 reported 55 errors and level 5 reported 67. Measured again on today's 1.x head, the numbers are 54 and 66.</p> <p>Level 4 is the dead-code band: PHPStan starts reporting a condition whose outcome it can already prove. Level 5 adds argument types on every call. Both are worth having in a module whose value proposition is that its integrity checks actually run, because a guard that can never fire reads exactly like one that works, and no test catches it: the branch behind it is unreachable, so coverage never goes near it.</p> <h3>Proposed resolution</h3> <p>Raise the shipped configuration to <code>level: 5</code> and clear all 66 findings. No baseline and no new <code>ignoreErrors</code> entry, on the same reasoning the file already carries: a baseline is a level nobody meets, written down.</p> <h3>Three runtime defects, two of them level 4 found</h3> <p><code>ContextContributorBase::getContributorPluginDefinition()</code> and <code>AuditTrailFilterBase::getFilterPluginDefinition()</code> each call <code>assert($definition instanceof SomeAttribute)</code> on what <code>getPluginDefinition()</code> returns, and each declares the attribute class as its return type. PHPStan reports both instanceof checks as always false, and it is right: <code>AttributeClassDiscovery</code> stores <code>$attribute-&gt;get()</code> as the definition, and <code>AttributeBase::get()</code> returns an array built from the attribute's own properties, not the attribute object. Only an attribute class that overrides <code>get()</code> to return itself, the way <code>EntityType</code> does, reaches a plugin as an object. Neither <code>ContextContributor</code> nor <code>AuditTrailFilter</code> overrides it.</p> <p>Confirmed on a running site rather than inferred: <code>createInstance($id)-&gt;getLabel()</code> throws <code>AssertionError</code> for every context contributor plugin and every filter plugin. With <code>zend.assertions</code> off the assert is skipped and the return type declaration throws a <code>TypeError</code> instead, so it fails either way. Nothing inside the module calls <code>getLabel()</code> or <code>getDescription()</code> on those two base classes, which is why the suite is green, but both methods are declared on the public plugin interfaces, so any implementation calling them hits this.</p> <p>Chasing that same wrong belief through the rest of the module turned up a third one, and this one operators see. <code>AuditTrailChainForm::getAvailableContributorOptions()</code> and <code>getAvailableFilterOptions()</code> read <code>$definition-&gt;label</code> off the same array, got NULL, and fell through to their <code>?? $id</code> fallback, so the Add-contributor and Add-filter selects on the chain edit form offered a list of machine names. The enabled-row label came from a different helper that happened to handle both shapes, which is why nobody noticed. PHPStan does not report this one at any level: property access on a loosely typed definition is invisible to it, and the fallback swallows the NULL. The analyser found the belief; grepping for every other site holding it found the defect that mattered.</p> <h3>Level 4: 54 findings</h3> <ul> <li><strong>44 checks that can never fail.</strong> 14 are <code>??</code> against an array offset the shape already guarantees. 12 are <code>assertInstanceOf()</code> in the service-wiring tests, against a service whose class the container map already pins. 9 are <code>instanceof AuditTrailTsaProviderInterface</code> on a value already typed as <code>AuditTrailTsaProvider</code>. The rest are <code>is_array()</code> and <code>method_exists()</code> on settled types, a nullsafe call on a non-nullable container, and two comparisons in <code>AuditTrailSegmentsController</code> that re-test <code>$file_purged_at === 0</code> after an early return has already established it.</li> <li><strong>9 checks that can never succeed.</strong> The two plugin-definition asserts above, plus a negated boolean in the TSA cron hook, three test assertions, and one docblock defect of exactly the kind level 3 was raised for: <code>AuditTrailCronArchiveHooks</code>'s retention resolver returns <code>compact_after_us</code>, but its <code>@return</code> shape omits the key, so both PHPStan and a reader of that docblock see the whole compaction stage as unreachable.</li> <li><strong>1 dead constant.</strong> <code>EntityAuditSettingsForm::OPERATIONS</code> has no reader anywhere in the module.</li> </ul> <p>The three test assertions in the second group need individual triage rather than one blanket answer. The suite is green, so either PHPStan infers a narrower return type from an implementation than that implementation really produces, or an assertion is pinned to the wrong value and the test is passing for another reason. Replacing the wider <code>@return</code> tags on <code>AuditTrailVerifier</code> with the class's existing <code>@phpstan-type</code> aliases was tried and changes nothing: PHPStan keeps the type it infers from the body, so the answer is in the bodies.</p> <h3>Level 5: 12 more</h3> <ul> <li><strong>1 mistyped argument, and it is a real one.</strong> <code>EntityAuditSettingsForm::__construct()</code> type-hints <code>TypedDataManagerInterface</code> for the argument it forwards to <code>ConfigFormBase::__construct()</code>, which requires <code>TypedConfigManagerInterface</code>. The form's own <code>create()</code> passes <code>config.typed</code>, which satisfies both, so the site works; the declared hint is simply wider than the contract it feeds, and anything constructing the form directly with a plain typed data manager gets a <code>TypeError</code> out of the parent.</li> <li><strong>6 named arguments PHPUnit does not allow.</strong> <code>AuditTrailFilterShortCircuitTest</code> calls <code>InvocationMocker::with()</code> with named arguments, and that method is marked <code>@no-named-arguments</code>: the names bind to the variadic, not to the parameters they look like they name.</li> <li><strong>5 no-op <code>array_values()</code> calls</strong> on values that are already lists, in <code>AuditTrailChain</code> and <code>AuditTrailChainForm</code>.</li> </ul> <p>AI-Generated: Yes (Claude Code was used to help draft this issue summary, to run the PHPStan measurements it quotes, and to confirm the two plugin-definition failures against a running site. I reviewed the work before posting; there is no merge request on this issue yet.)</p>