PHPStan level 9 on a large legacy codebase: a field guide to error classes and their fixes

★ ★ ★ ★ ★ Not rated

Starting point

The application is a modular CMS with a front end, an administration and an internal API layer, written over many years. PHPStan ran at level 9, but most errors lived in per-stage baseline files: 16,611 ignored errors when this work began. A baseline is a useful truce, not a fix: every ignored error is a place where the analyser stopped protecting us.

Goal

A zero baseline, reached gradually, while normal development continues on the same branch.

Constraint

No behaviour changes hidden inside type work. Every fix is either neutral or a deliberate bug fix that is named in the commit.

At the time of writing the count is 780, and the work found more than a dozen user-visible bugs along the way.

Rules of the game

PHPStan's own guidance is blunt and we adopted it as policy. Each rule exists because the shortcut hides the defect the error points at.

  • No @phpstan-ignore comments and no new baseline entries for new code.
  • No casts added only to silence an error. (int) $row['id'] turns an array into 1 without complaint.
  • No inline @var overrides and no assert() to force a type.
  • No widening of parameter or return types just to make an error go away.
  • Fix the cause: a missing interface, a nullable value nobody handled, a branch that can never run.

Baseline workflow: a ratchet, a pruner and a full-pass file

Three small tools make gradual progress safe.

The ratchet

A pre-commit hook counts every baseline entry, weighted by its count, compares the total with HEAD, and refuses the commit if it grew. Progress can only go one way.

php tests/Manual/System/phpstan_baseline_ratchet.php
# PHPStan baseline ratchet: HEAD=12449  now=11989  -460

The pruner

After a fix, matching baseline entries become stale. The pruner reads PHPStan's JSON output and removes or lowers entries that no longer match, across all baseline files.

vendor/bin/phpstan analyse --error-format=json > result.json
php tests/Manual/System/phpstan_baseline_prune.php --write --json=result.json

The full-pass file

Older stage baselines stay as they are. Everything else is collected in one regenerated file. The regeneration order matters: temporarily remove the full-pass include, prune the other files, regenerate the full-pass file, restore the include, and prune once more.

Always re-run PHPStan after pruning. An entry whose message wording changed looks stale to the pruner and is removed, but the error is still there. The pre-commit hook does not run PHPStan, so this once produced a commit with 44 open errors. The last step of every regeneration is a full analysis that must print [OK] No errors.
Entries that move between files. Traits are reported "in context of" each class that uses them, so the same error can surface under different paths. Set reportUnmatched: false for those baseline files and pin entries with paths: instead of a single path:.

mixed values from the database and requests

Symptom. Cannot cast mixed to int, Parameter #1 expects int, mixed given, on every fetchAssociative(), fetchOne() and request parameter.

Fix. One tiny helper that behaves exactly like a cast for scalars and null, and throws for anything that cannot belong there:

final class DbValue
{
    public static function int(mixed $value): int
    {
        if (\is_scalar($value) || $value === null) {
            return (int) $value;
        }
        throw new \UnexpectedValueException('Expected a number, got ' . \get_debug_type($value) . '.');
    }

    public static function nullableInt(mixed $value): ?int { return $value === null ? null : self::int($value); }
    public static function string(mixed $value): string { /* same idea, also accepts Stringable */ }
}

Existing behaviour is unchanged for valid data, so this can be applied mechanically. The difference is that a nested array no longer becomes 1 silently.

getId() on objects PHPStan cannot see

Symptom. Cannot call method getId() on object or on mixed. This was the largest cluster. Collections, generic managers and proxies hand back plain object, and code calls getId() on whatever arrives.

Fix. One interface that every entity with an identifier implements, and a helper for call sites where the static type is still loose:

interface IdentifiableInterface
{
    public function getId(): ?int;
}

final class EntityId
{
    public static function of(mixed $entity): ?int
    {
        return $entity instanceof IdentifiableInterface ? $entity->getId() : null;
    }
}

The important part is what happens next. The return type is now honestly ?int, and PHPStan follows that null to every place that assumed an integer. Each of those places needs a decision:

SituationDecision
Building a list or select optionsSkip items without an id (continue).
Deleting by idReturn a failure result: an unsaved entity cannot be deleted.
AuthenticationThrow: a user without an id must never be signed in.
LoggingPrint it as empty. Never interpolate a static call inside a double-quoted string (see section 12).
Why not just declare getId(): int?New entities genuinely have no id until they are flushed. Declaring otherwise moves the problem to a runtime TypeError.

Untyped response arrays

Symptom. Cannot access offset 'message' on mixed, repeated in every repository that reads the internal API's response envelope.

Fix. A small reader with one method per field, each with a safe default:

ApiEnvelope::isSuccess($response);            // true only for success === true
ApiEnvelope::message($response, 'Save failed.');
ApiEnvelope::code($response);                 // always a ResultCode
ApiEnvelope::arrayData($response);            // array or null
Do not put @phpstan-assert-if-true on isSuccess().It looks attractive, but on inputs that already have the asserted shape PHPStan reports the call as "always true", which creates new errors in exactly the code you were cleaning up.

Nullable context objects

Symptom. A request-scoped context, in our case the current language, stored its value as object. Every consumer called getLanguage()->getId() and passed the result to methods expecting int.

Fix. Type the stored value with the real entity class and give consumers two explicit accessors. Each caller then states its intent:

public function getLanguageId(): ?int
{
    return $this->language?->getId();
}

public function requireLanguageId(): int
{
    return $this->getLanguageId() ?? throw new \LogicException('Language is not set in the context.');
}

Front-end presenters, which always run after the language has been resolved, use requireLanguageId(). Background services such as e-mail sending accept null and fall back to the default language explicitly.

Bug this exposed. One path passed the raw URL alias 'cs' where a numeric language id was expected, so an activation e-mail could be sent in the wrong language. Typing the context made it visible.

Duck typing and the dead branches it hides

Symptom. Call to an undefined method on a generic manager or mapper, usually guarded by method_exists().

Fix. Introduce the interface the code was already assuming (for us: a mapper interface, a tree mapper interface and a tree manager interface) and declare it on the classes. The method_exists() guards disappear.

Once types are honest, PHPStan begins to report instanceof ... will always evaluate to true and Result of && is always false. Treat these as findings, not noise: they mark code that can never run. A typical case is unwrapping a lazy proxy for a value that is now guaranteed to be the real entity. Delete such branches instead of baselining them.

Results are not a transport for typed data

A helper that returned table, columns and scope inside Result::success(data: [...]) lost every type on the way out, because getData() is mixed. Seven tree operations then passed mixed into typed repository methods. A small readonly value object, returned as TreeStructure|Result (where Result means failure), removed the casts and the errors together.

Traits that ended up in the wrong class

Errors like "call to an undefined method CategoryPresenter::initUser()" often mean a trait was added to a class that never needed it. In our case a category overview presenter used the advert-detail trait, and with it exposed a confirmation signal and two form components on a page that has no advert. The template used none of it, so the fix was to remove the trait.

Doctrine ORM 3 association mappings

Symptom. Offset access on association mappings, for example $mapping['joinColumns'][0]['name'], fails analysis because ORM 3 returns mapping objects, not arrays.

Fix. One helper that narrows to the right mapping class and fails loudly otherwise:

public static function joinColumnName(AssociationMapping $mapping): string
{
    if ($mapping instanceof ToOneOwningSideMapping) {
        return $mapping->joinColumns[0]->name;
    }
    throw new \LogicException('Association ' . $mapping->fieldName . ' has no join column.');
}

public static function mappedBy(AssociationMapping $mapping): ?string
{
    return $mapping instanceof InverseSideMapping ? $mapping->mappedBy : null;
}

Class names: class-string or a check

Symptom. expects class-string<object>, string given at getClassMetadata(), getRepository() and the DI container's getByType().

Fix. It depends on where the name comes from:

  • Passed in by the caller: annotate the parameter @param class-string $entityClass and let PHPStan walk the chain upwards. A small script repeated the step until no new sites appeared.
  • Built at run time (module and component names, a factory class stored in the database, a payment callback): check it with class_exists(). A helper such as hasServiceByType() can carry @phpstan-assert-if-true class-string $className, so the check also narrows the type.
  • Not always a class: a hydrator's "property type" can also be int. Here the right fix is an isEntity() check with an assertion, not a class-string annotation.

A related trap: array_map('intval', $ids) on a mixed array silently turns an array element into 1. A DbValue::ints() helper casts scalars exactly like (int), drops elements that cannot be numbers, and keeps a list a list.

Generics and template bounds

Base managers, services and mappers are shared by dozens of modules. Making them generic in one step would touch every module at once. Two PHPStan features avoid that.

Template defaults let unconverted modules keep working as before. When every template has a default equal to its bound, an unparametrised Advert in a signature simply means "the bound", and missingType.generics is not reported. Adding = bound to 539 entity templates and to the service and manager layers removed about 2,400 baseline entries without touching runtime code:

/**
 * @template TEntity of BaseEntity = BaseEntity
 */
abstract class Manager { /* ... */ }

Call-site variance lets shared code accept any specialisation without invariance errors:

/** @param Manager<covariant ServiceInterface<covariant BaseEntity>, covariant BaseEntity> $manager */

Modules then opt in one by one by adding @extends Manager<Product>, and each step removes a slice of the baseline.

Bound templates to what the code really calls

Entities with translations had @template TTranslation of object, so every $translation->getLanguageId() inside the shared trait was an "undefined method" error, repeated for each of 54 entities. The fix is an interface for what the code calls, and an intersection bound, because the trait's native types still say BaseEntity:

interface TranslationInterface
{
    public function getLanguageId(): ?int;
}

/** @template TTranslation of BaseEntity&TranslationInterface */
trait TranslationCollectionTrait { /* ... */ }

A global type alias keeps the docblocks readable, and in parameter types it should be covariant so that Status<StatusText> is accepted:

# phpstan.neon
parameters:
    typeAliases:
        TranslationEntity: 'App\Core\Base\BaseEntity&App\...\TranslationInterface'

/** @param OrderStatus<covariant TranslationEntity> $status */

Tightening the bound turns every X<object> in a docblock into an error, which is useful. The 131 sites were rewritten with a script that reads each class's @template order and replaces only the argument at the translation position. The same tightening exposed an entity declared as @extends BasePage<Route, PageText> with its two arguments swapped. With object bounds this had passed silently, and one caller carried an inline @var to work around it. This one step removed 528 baseline entries.

Arrays: value types and shapes

Symptom. should return array<int, string> but returns array<int|string, string>. A generic list helper cannot know that every key is an integer.

Value types for plain array parameters

A parameter typed only as array produces missingType.iterableValue. We filled about a thousand of them by parameter name, following the convention that the mapper interface already used: *Ids becomes array<int>, rows, params and values become array<string, mixed>, and anything else becomes array<array-key, mixed>.

Expect new errors, and read them. PHPStan treats a bare array as implicit mixed and does not check what is done with its values. Once the value type is written out, the mixed becomes explicit and every unchecked use is reported. About 480 such places appeared. Among them was a query builder whose include('*') violated its own interface, and an array_merge() that would fail on a string.

Let PHPStan tell you the return type

For a method that returns array without a value type, or has no return type at all, you do not have to guess. Temporarily add a deliberately wrong tag: @return array<never, never> for arrays, @return void for untyped methods. PHPStan then reports "should return … but returns X" for every return, and X is the type it inferred. Merge the X values per method and write them back. Keep only concrete results: skip anything with literal values, anything very long, and template expressions like T of object = object. Prefix class names with \, because PHPStan prints them without it. Do not write vague results like array<array-key, mixed>: they reveal follow-up mixed errors without adding any precision.

Array shapes from generic helpers. If the loop already exists, build the map in that loop with keys you have already checked. It is shorter than the helper call and fully typed:

$options = [];
foreach ($items as $item) {
    $id = EntityId::of($item);
    if ($id === null) {
        continue;
    }
    $options[$id] = $item->getTranslation($langId)?->getName() ?? '';
}

The same pattern fixes array_map(fn ($x): int => EntityId::of($x), ...): map to ?int, then array_filter and array_values.

Tests that tighten with you

Unit tests often stub entities with anonymous classes that merely have a getId() method. When a parameter becomes ?Language, or EntityId::of() checks for the interface, those stubs either raise a TypeError or silently turn into null, which is worse.

  • Prefer a real entity instance: (new Language())->setId(1).
  • If an anonymous class is unavoidable, let it implement the interface.
  • Return a real context object from a stubbed presenter instead of a nested anonymous class.

Mass edits that bite

Most of these fixes were applied with scripts across hundreds of files. These traps showed up more than once:

TrapResultGuard
Regex stops at ( or ?->DbValue::int($conn->executeQuery) without its argumentsRun PHPStan after every batch. It caught both cases.
Helper call inside a double-quoted string"user={EntityId::of($u)}" fails at runtime, because PHP does not interpolate static callsA tokenizer scan for helper names inside string tokens
Leading backslash carried over\ApiEnvelope:: resolved to the global namespaceLint plus PHPStan
Mixed indentationA pattern written for tabs does not match a file indented with spacesAssert the match count and stop the script
Inserting return; as a guardFatal errors in non-void methodsphp -l on every changed file

Each script asserts that every pattern matched exactly the expected number of times. A script that fails loudly halfway is far better than one that silently skips a file.

Where the types got lost

Halfway through, most of the remaining errors no longer pointed at bad code. They pointed at a value whose real type was known to the developer but had been lost on the way to PHPStan. Every place below turned a concrete entity back into BaseEntity or object. Fixing the source removed dozens of errors downstream at once.

WhereSymptomFix
A leaf entity without @extends Base<…>Every template of the shared base falls back to its defaultInfer the arguments: for each template, find the base property typed with it, then read targetEntity from the leaf's Doctrine mapping
A shared class that uses a collection trait without @use Trait<T>The trait's methods return Collection<int, BaseEntity>Add @use directly above the use line
Fluent setters declared : self(new Message())->setA()->setB() becomes the shared base class@return $this on every setter whose body only returns $this
A fluent query object whose include() returns plain selffindAll()->include([…])->execute() yields BaseEntity[]@return self<T> on every fluent method
getEntityClass(): class-stringA generic hydrator cannot return the manager's entityclass-string<TEntity> along the whole service → manager chain
A /** @var … */ placed between two attributesThe property keeps its bare native type (Collection without element types); PHP reads the comment, PHPStan does notMove the comment above the first attribute. The parser attaches a comment to the first token of the declaration, which is the attribute. We found 43 of these with a tokenizer scan
A response helper declared : mixedEvery caller wraps it in is_array() "just in case"Declare the real array shape; the guards turn into "always true" and go away

Two operational lessons came with it. First, after changing generics on a base class, the PHPStan result cache can keep reporting the old state; clear it before trusting a surprising result. Second, PHPStan 2.2 prints its own advice on stderr: do not override an inferred type with an inline @var or assert(). For mixed coming from Doctrine (query parts, scalar results), narrow it at runtime with is_array() or instanceof instead.

One more trap for mass edits: a script that adds /** @return $this */ above every setter will also find setters inside a commented-out /* … */ block. The inserted */ closes that comment early and breaks the file. Skip lines that the tokenizer reports as comments.

Two framework traps cost us a regression each. In Nette, the type of an action*() parameter is also read by the link generator: an untyped or nullable parameter is optional, while int $order without a default makes every link to that action fail with "Missing parameter". Type action parameters as nullable. And $form['x'] returns the generic component interface, so methods like setRequired() or addConditionOn() are "undefined". An inline @var above it does not narrow anything; a small typed accessor on your form class ($form->typedComponent('x', BaseControl::class), throwing on mismatch) does.

One last discipline point: after a mass removal of "unused" properties, re-run analysis without the baseline and look for property.notFound. Our own cleanup had removed constructor dependencies that a factory still read, and the crawler did not notice because the affected page sat behind a password form.

Not every error points at the code. Nette calls a form handler with values in whatever type the handler declares: array, ArrayHash or a DTO class. The phpstan-nette stub still says callable(static, mixed), so every correctly typed handler was rejected. A second stub for the same class is not allowed, because stub validation reports it as declared twice. We redeclared the property on the application's own Form subclass with callable(static, never). That left one honest mismatch with the vendor stub instead of about fifty false errors.

Late lessons from the last thousand

  • Some types are best left implicit until there is an accessor. Writing @return array<int, array<string, mixed>> on the settings and configuration getters turned about 140 unchecked uses such as all()[0]['uploadDir']['abs'] into explicit mixed errors. The honest fix is a typed accessor, not a docblock: Config::path(1, 'uploadDir', 'abs') returns a string (empty when missing) and Config::value(1, 'mail') returns the raw value for the caller to check. With the eleven offset chains rewritten, the getters could be typed without a single new error; values read through the settings getter go through DbValue::int() instead of (int).
  • By-reference parameters overwrite the caller's type. A helper prepareCategoryFilter(array &$filters) documented as array<array-key, mixed> turned the caller's carefully typed array<int, Filter> into array<mixed> after the call. Type the by-reference parameter as precisely as the caller, or use @param-out.
  • A template bound cannot point to a template declared after it. @template TService of ServiceInterface<TEntity> above @template TEntity produced 309 "not subtype" errors, because the bound does not resolve the later template. The real fix means reordering the templates and every @extends; we left it for a separate step.
  • Stubs need autoloadable parents. A stub that corrected one wrong @return object in a payment SDK failed stub validation ("extends unknown class"), because the SDK is loaded only through a classmap. An instanceof check at our call site was the simpler fix.
  • The typed accessor works for components too. The same typedComponent() used for form controls, added to the shared component trait, replaced every inline @var above $this['x'] in presenters and components. One of those comments named a class that did not exist.
  • Source-text tests resist "harmless" edits. A test asserted the exact text of a call, so adding ?? '' at the call site broke it. Accepting ?string in the called helper fixed the same error without touching the call.

Real bugs found along the way

Type work is not cosmetic. Among the defects fixed while removing more than 10,000 baseline entries:

  • A VAT rate select that always rendered empty.
  • A missing theme that produced HTTP 503 instead of falling back.
  • A findAllBy() wrapper that returned at most one entity.
  • Pagination arithmetic broken by operator precedence.
  • Tree deletion that could delete nodes before validating the subtree data.
  • Numeric category URLs returning HTTP 500 instead of resolving or returning 404.
  • Soft 404 pages answering with status 200.
  • An activation e-mail that could be sent in the wrong language.
  • A login error message built from a null result code.
  • An advert detail page that called a method that did not exist on its meta-data object. It never crashed only because the image it depended on was always empty, so Open Graph images had silently never been generated. Making a nullable property non-nullable exposed both problems.
  • Article detail pages that never loaded their main image, so they had no Open Graph image either. A second entity had its generic arguments swapped (user and image), just like the page entity earlier.
  • A category overview that inherited the advert-detail trait, and with it a signal and two forms it never needed.
  • An advert edit form that always loaded the default parameter set. The code iterated the advert's category relations and kept items that were instanceof Category, which a relation never is, so the category list was always empty.
  • A company detail page that deduplicated category objects with array_unique(), which converts values to strings and throws for objects. It only survived because the categories were not loaded yet.
  • A discussion post counter that called a method on a comment thread that may be null.
  • A service that reported the wrong entity class (a copy-paste from a sibling service), so cache invalidation and permission checks targeted another table.
  • Two clear…() collection methods that called clear() on a property that did not exist.
  • A query builder check that compared an alias string with Select objects, never matched, and added the root alias to SELECT twice.
  • A path built as $root . $row['path'] ?? '': concatenation binds first, so a missing key produced a warning instead of the fallback.
  • A query-string array (?curr[]=x, ?date[]=x) reached a string-typed property or new DateTime() and turned every page into HTTP 500. Values read from the URL are mixed in PHPStan for a reason.
  • A Result::fromArray([]) for a non-JSON API answer crashed on ResultCode::fromValue(null).
  • A whole request-time decorator mechanism that looped over presenter properties on every request and changed nothing: every setter it looked for had been commented out long ago. One decorator even checked for setCurrency and then called setCurr.
  • An API request with an unknown locale crashed with a TypeError: the loader was declared to return a localization while the lookup could return null, which the caller already handled.
  • A comment list component declared a return type with a namespace that did not exist, so creating its form could only end in a TypeError.
  • Two entities used an "identifiable" trait without having an $id property; any getId() call would read an undefined property.
  • A legacy serialized image-variant format bypassed the normaliser that the JSON format went through, so old variants came back with untyped values.
  • A newsletter activation e-mail with no address and a pending-publication notice for a component without a translation domain both ended in a TypeError instead of a handled failure.

Checklist

  • A ratchet in the pre-commit hook: the baseline count may only go down.
  • After every prune or regeneration, a full PHPStan run prints [OK] No errors.
  • mixed from storage goes through DbValue, not through casts.
  • Identifiers come from an interface; ?int is handled where it appears, with a stated decision.
  • Context objects store real types and offer an explicit get…Id() and require…Id() pair.
  • "Always true" and "always false" reports are treated as dead code to delete.
  • Shared base classes become generic with template defaults and call-site variance, one module at a time; template bounds name the interface the code actually calls.
  • Mass-edit scripts assert match counts; every batch ends with php -l, PHPStan and the unit tests.
  • Test stubs use real entities or implement the interfaces.
  • Every leaf entity has @extends, every shared class has @use for its generic traits, and fluent methods return $this or self<T>.
  • After changing generics on a base class, clear the PHPStan result cache before trusting the numbers.
0.0
★ ★ ★ ★ ★
0 reviews
5 ★
0 %
4 ★
0 %
3 ★
0 %
2 ★
0 %
1 ★
0 %

Add a comment

There's nothing here.

No comments have been posted here yet.