Skip to content

chore(Types): clear phpstan level 7 outside classic - #2203

Merged
DerManoMann merged 34 commits into
zircote:masterfrom
DerManoMann:chore/phpstan-level-7
Sep 17, 2026
Merged

DerManoMann merged 34 commits into
zircote:masterfrom
DerManoMann:chore/phpstan-level-7

Conversation

@DerManoMann

Copy link
Copy Markdown
Collaborator

Overview

Level 5 is what CI analyses today, and it is enough to miss a whole class of
problem: it reports missing annotations, but not mixed arriving somewhere a
type was already declared. #2161 annotated the spec namespaces for level 6 and
stopped there, because nothing enforces a level per path and the tree was too
far from a global bump to be worth one.

Level 7 turns out to be the cheaper and more useful target. It asks a different
question — is this argument, return, offset or property write actually the type
it claims — and the answers are things that can be wrong rather than merely
undeclared. It also skips docs/examples almost entirely, which is the single
most expensive chunk of level 6 and the one that would change generated schemas.

This clears level 7 across tools/, tests/, and src/ outside the classic
annotation and attribute classes. Most of it is annotation, but the type checker
found real things on the way, and a few of them were declared types that were
optimistic rather than imprecise.

phpstan.neon.dist stays at level: 5. Raising it needs the remaining classic
errors gone, and those go when classic does; baselining them instead would take
the baseline from ten entries to well over a hundred, which is worse than the
guard is worth. This is the code side of a future bump, not the bump.

Three errors are deliberately left reported, each with the reason in a comment
beside it. In all three the obvious silencing fix — a guard or a cast — would
settle a behaviour question by accident, so they stay visible until the
behaviour is decided.

Changes

  • AttributeTranslatorInterface::translate() takes and returns array<object>
    rather than array<AttributeInterface>, matching the filter AttributeFactory
    already applies to the result
  • Builder::$sources drops the unreachable scalar and object arms of its native
    type, leaving the array its docblock always claimed
  • Analysis gains shapes for the class, interface, trait and enum definitions,
    imported by the analyser and the processors that read them
  • Context loses two @property tags for data it never held, and $other is
    typed for what the doc block parser actually returns
  • DocBlockDescriptions::summaryAndDescription() names the annotations that
    reach it, retiring a phpstan-baseline.neon entry
  • PathItems::findGoverningPathItem() uses ClassReflector::tryReflect() in
    place of a catch (\ReflectionException) aimed at the wrong throwable
  • Augmenter\Shortcuts::processMediaType() assigns each shortcut property
    explicitly instead of looping over a list of names
  • Utils\Pipeline and TypedList carry their element and iterator types;
    Generator and Builder carry Pipeline<Analysis>
  • Eight tests gained an assertInstanceOf for the annotation they parse, having
    read subclass properties off the base type without asserting it
  • CompilerTest splits the array-indexing callers of compileSchema() onto a
    wrapper, leaving the one caller that depends on the \stdClass case
  • HybridBridge, ExpandEnums and Augmenter\Enums normalise converted maps
    to lists where a list is declared

Adds a list<string> type to ScopedTrait::$scopes and its scope()
parameter, plus a static return type on scope(). Clears all 9
level-7 errors in the file.
Adds return/param types across DocGenerator: array<string, string>
on the abstract generate(), \ReflectionClass<object> generics on
configurableParameters()/collectOptions()/resolveDefault(), and
handles the false cases realpath()/file_get_contents()/preg_split()
can statically return, each with a behavior-preserving fallback.
Clears all 13 level-7 errors in the file.
Adds list/shape return types and class-string<AbstractAnnotation>
parameter types across AttributeGenerator's collection methods, plus
one inline @var cast where a class name is built from a string
concatenation phpstan cannot itself prove is a class-string.

Also tightens AbstractAnnotation::$_parents from array<class-string<...>>
to list<class-string<...>> — docblock-only, no native type or behavior
change, so it carries none of the downstream-compatibility risk flagged
for classic; it was needed for collectParents()'s return
type to type-check.

Verified against the real docgen output (`php tools/docgen.php ref`):
no diff. Clears 21 of the file's level-7 errors.
Pipeline<T> parametrizes process()'s payload, not the pipe item
type, which was declared PipeInterface|callable — broad enough to
admit a bare string/array callable, which every ReflectionClass-based
introspection in this file (getConfig(), configure()) already assumed
could not happen. Narrows the item type to PipeInterface|(callable&object):
still callable (process() invokes pipes directly), but always an
object, matching what every default pipe and every closure pipe in
the test suite actually is. Docblock-only — no native signature change.

Also specifies the deprecated top-level Pipeline shim's inherited
generic as <mixed>, and adds the routine missing array/generic types
elsewhere in the file (configure()/normaliseConfig()'s loosely-shaped
config array, configurableParameters()'s ReflectionClass<object>).

Full suite green (2259 tests). Clears 14 level-7 errors across the
two files, and the callable/ReflectionClass errors this shape carried
into every generator that walks a pipeline.
Adds array<string, string> on generate(), the shared shape/generics
docblocks on collectAugmenterData()/collectProcessorData(), and
guards the two calls that return false on failure in
ProcessorGenerator (ReflectionClass::getFileName(), glob()) — the
former throws (a processor's own class always has a file), the
latter falls back to an empty list.

Verified against the real docgen output (`php tools/docgen.php proc
aug`): no diff. Clears 12 level-7 errors across the two files.
Adds list<string> on ExampleGenerator::$examples, array<string, string>
on generate(), a false-guard on the readme's file_get_contents(), and
the matching array shape on Renderer::exampleSection()'s $files param.

Verified against the real docgen output (`php tools/docgen.php
example`): no diff. Clears 4 level-7 errors.
TypedList<T> implemented \IteratorAggregate without an @implements
annotation, so iterator_to_array() on any TypedList (getTranslators(),
withResolvers()'s list) lost T and fell back to array<mixed> for every
caller — including AttributeFactory and Resolver, whose own docblocks
already specified the real element type correctly. Adding
@implements \IteratorAggregate<int, T> and a matching @return on
getIterator() fixes the inference at the source instead of casting
around it at each call site.

Also fixes remove()'s list<T> invariant: unset() on an array-typed
property phpstan considers list-shaped breaks that guarantee at the
unset site, not the reindex; moved the mutation to a local variable
so the property is only ever written back as an actual list.

ExtensionPointGenerator's translators()/resolvers() now get their
real element types (AttributeTranslatorInterface/ResolverInterface)
via array_values() on the now-correctly-typed iterators, instead of
list<object>.

Full suite green (2259 tests); docgen output unchanged. Clears 9
level-7 errors across the two files.
Adds array<string, string> on generate(), a class-string<AbstractAttribute>
cast for the fqdn built from the Spec directory scan (proven a
subclass by the isSubclassOf() check just above), and the shape/generics
docblocks on collectClassData()/collectParameters() matching
parseDocblock()'s return shape.

Verified against the real docgen output (`php tools/docgen.php
spec`): no diff. Clears 6 level-7 errors.
provideFixCases() yields either a one-element (already-correct, no
change expected) or two-element (expected, input) array per case;
types both shapes instead of leaving the iterable untyped. Finishes
tools/ for the level-7 push.
PSR-3's LoggerInterface already types $message as string|\Stringable;
the concrete log() overrides here declared it untyped, so phpstan saw
mixed inside the method body. Adds the matching docblock (native
signature already widens correctly per PSR-3) and casts to string
before trigger_error(), which only accepts one. Clears 2 level-7
errors.
Propagates class-string<AbstractAnnotation> for $sourceClass across
TypeResolverInterface::mapNativeType()/augmentSchemaType(),
AbstractTypeResolver::type2ref()/doAugment(), and their
TypeInfoTypeResolver/LegacyTypeResolver overrides — all previously
plain string, the same shape that already flagged Schema::$type
et al. as classic-adjacent debt. Adds the Symfony TypeInfo generics
(BuiltinType<TypeIdentifier>, CollectionType<...>, etc.) TypeResolver
needs at level 7, and types LegacyTypeResolver::normaliseTypeResult()'s
loosely-shaped params.

Schema::$type is `non-empty-array<string>|string` (classic docblock,
untouched) but TypeMapper::toSpecTypes() and SchemaType::$type are
each used from two call sites with different, incompatible shape
guarantees (list vs non-empty-array) — no single signature satisfies
both without either loosening past what the property accepts or
tightening past what a caller can prove. Left the two library
functions at their natural loose type (array<string>) and cast locally
at each call site instead, with a one-line comment saying which
operation (array_map, toSpecTypes) is relied on not to change the
array's shape.

**Left 9 errors unresolved in TypeInfoTypeResolver.php, on purpose.**
`applyToAnnotation()`'s additionalProperties branch dereferences
`$schema->additionalProperties->type/oneOf/allOf/anyOf` once the
property is already set — but `additionalProperties` is `bool|
AdditionalProperties`, and `Undefined::isDefault()` only guards the
sentinel, not `false`. A schema with `@OA\Schema(additionalProperties:
false)` whose PHP type also carries nested type info would hit this
branch with a plain bool and fatal on `->type`. That's a real
correctness bug, not a typing gap, and fixing behavior isn't this
PR's job — recorded for its own entry rather than patched here or
cast away to silence phpstan.

Full suite green (2259 tests). CS clean.
Context is a #[AllowDynamicProperties] bag documented entirely through
@Property tags; most of this batch is giving those tags, and the
methods that read/write them in bulk (__construct(), __unserialize(),
isVersion()), real value types instead of bare `array`.

Two of the tags were wrong, not just imprecise, once checked against
every reader: $other holds whatever Doctrine's DocParser returns for
an annotation that isn't one of ours (confirmed via
DocBlockAnnotationFactory/AttributeAnnotationFactory) — never
AbstractAnnotation, the declared type's whole point. $extends and
$implements are never set or read anywhere as Context properties at
all — that data lives in the plain `$definition` array ReflectionAnalyser
builds instead; removed the two dead tags rather than inventing a type
for something that doesn't exist.

fullyQualifiedName() built genuinely unverifiable class-strings by
concatenation at five return points; funneled them through one
asClassString() helper instead of casting each site individually.

ClassReflector::tryReflect()'s own `@param class-string $fqdn` (added
in zircote#2201) worked against its documented purpose — it
exists specifically to accept a name that might not resolve to
anything. Loosened to plain `string`, which is what let
ReflectionAnalyser/DocBlockParser's callers stop needing their own
casts for a name they can't have verified either.

One CS trap worth remembering: `phpdoc_to_comment` silently downgrades
an inline `/** @var */` cast to a plain (phpstan-invisible) comment
unless it immediately precedes an assignment — a cast before a bare
method call or a `return` gets demoted with no error, so the "fix"
silently stops working. `composer cs` after every phpstan-clean check
would have caught this immediately; two of these casts had to be
rewritten through a throwaway local variable once it did.

Full suite green (2259 tests). Clears 18 level-7 errors.
LocallyCalledStaticMethodToNonStaticRector flags a private static
method called only from within its own class; composer lint runs
rector --dry-run and caught it. Missed because I'd only been running
composer cs, not the full lint (cs + rector) or analyse (phpstan at
the project's actual configured level) — both now checked every batch
going forward.
Reuses the file's own @phpstan-type ScannerDetails alias across the
collect_* methods instead of leaving them at plain array, types the
$resolve closure's return and the internal $uses map as class-string
to match, and guards three false/null cases PhpParser's API allows
but this code didn't check: file_get_contents() failing, parse()
returning null on an empty file, and a promoted constructor
parameter's $var being an Expr\Error (parse-recovery placeholder)
rather than a real Variable.

Full suite green (2259 tests). composer lint and analyse both clean.
Clears 13 level-7 errors.
Mechanical: \ReflectionClass<object> on every plain \ReflectionClass
param/return across fromReflector(), membersOf(), hasAttributes(),
getDirect{Methods,Interfaces,Traits}(), readAttributes(). Also fixes
getDirectTraits()'s declared list<> return: array_filter() over
array_map() preserves the original keys, so without array_values()
the result could carry gaps and wasn't actually a list.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 10 level-7 errors.
$VALID_ANNOTATIONS gets its list<class-string<AbstractAnnotation>>
type; doDeserializeProperty()/doDeserializeBaseProperty() get the
mixed/array<string> types their callers already assume. Guards
file_get_contents() and json_encode() failing in deserializeFile()
(both throw OpenApiException, matching isValidAnnotationClass()'s
existing failure style in the same methods), and narrows is_object()
to instanceof \stdClass at the one call site that forwards into
doDeserialize()'s \stdClass-typed $c.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 9 level-7 errors.
setTags()/setPaths() already documented array<string>; the backing
properties, constructor and getters did not. Clears 6 level-7
errors.
The four definition arrays were plain `array` everywhere, so nothing
downstream (ExpandClasses/Interfaces/Traits, MergePropertiesTrait,
the Augmenter inheritance pipes) could see what keys they carry.
Adds four @phpstan-type aliases on Analysis and imports them where
the shapes are consumed.

They are spelled out separately rather than as one shape with optional
keys because `extends` genuinely differs: a single parent for a class
or trait, a *list* of parents for an interface. Writing that honestly
is what surfaced two latent problems:

- getSuperClasses() read `$classDefinition['extends']` and used it as
  an array key without checking. For a class that is a string, but the
  method is also called (via getTraitsOfClass/getInterfacesOfClass)
  with interface names, where `extends` is a list — which would be an
  illegal offset. Now returns early instead.
- getTraitsOfClass() appends its nullable $source to $sources and then
  indexes $this->classes with it; null is skipped explicitly now.

Neither is reachable through the current call paths as far as I can
tell, so these are guards, not bug fixes — but the untyped array was
hiding the question entirely.

ReflectionAnalyser built one definition array and dispatched on
'add' . ucfirst($contextType) . 'Definition'; phpstan cannot narrow a
dynamic method name, so that is now an explicit switch. Same four
calls, no behavior change, and the shape mismatch becomes visible if
the builder and any add*Definition() ever drift apart.

Full suite green (2259 tests). composer lint and analyse clean.
Clears ~40 level-7 errors across src/.
Applies the same Pipeline<Analysis> generic and object-typed pipe
walker established for Utils\Pipeline, reuses Builder's existing
BuilderSource alias for generate()/scanSources() instead of bare
iterable, and types getConfig()/getDefaultConfig()/normaliseConfig().

Two things fell out of typing rather than being sought:

- addNamespace() fed array_unique() straight into setNamespaces(),
  but array_unique() preserves keys, so removing a duplicate left a
  gapped array where a list was declared. array_values() restores it.
- Typing the walker's $pipe as object (it is always one; callable
  admitted string/array callables that every ReflectionClass call in
  the walker would have rejected) let Rector replace two is_a() calls
  with instanceof — the check is_a() was only needed for.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 18 level-7 errors.
array_map() preserves keys, so every classic keyed map converted into a
Spec list came out as array<X> against a declared list<X>: examples on
Parameter/MediaType/Header/Components, and allOf/anyOf/oneOf/prefixItems
on Schema. array_values() is safe for all of them — the compiler re-keys
examples off Spec\Example::$example (compileKeyedMap(..., 'example', ...)),
and the four Schema keywords are positional in OpenAPI. patternProperties
and dependentSchemas are genuinely keyed and are left alone.

Also types convertSecurityRequirements()/convertCallbacks()/filterType().

**One error is left standing on purpose**, and it is not a typing gap:
Spec\ServerVariable::$enum is list<string> but classic's is
list<string|int|float|bool|UnitEnum>|class-string. Whether a server
variable enum may hold non-strings is an OpenAPI compliance question
(the spec says string), not something to cast away here.

Two more stood in convertHeader()'s $content array_map until zircote#2204,
which is the behaviour fix they were waiting on: a Header whose content
is a JsonContent was silently dropped in hybrid, and an is_array() guard
here would have cleared the errors while making the drop look deliberate.
resolveContent() now accepts Header and does the unwrapping, so both go
without a guard. It keeps the guard it already had, for the non-array
shapes the declared type still permits.

Full suite green (2265 tests). composer lint and analyse clean.
Takes HybridBridge from 15 level-7 errors on master to 1.
processMediaType() looped over a list of property names and did
$mediaType->schema->{$prop} = $mediaType->{$prop}. The shortcut
properties mirror OA\Schema's one for one, so this is sound — but
a dynamic property name makes that invisible, and phpstan can only
see the union of all five types on each side, so every assignment
read as a type error (five of them, on one line).

Spelled out per property instead. Semantics are preserved exactly,
including the part easy to lose: in the else branch only a shortcut
the schema has no value for is moved across, and one the schema
already sets stays on the media type rather than being nulled.
MEDIA_TYPE_SCHEMA_PROPERTIES had no other use and is gone.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 5 level-7 errors.
Mostly narrowing at the source rather than at each call site:

- Spec\AbstractAttribute::getClassName() returns a reflector's name,
  which is always a class-string; typing it there fixed five errors
  across PathItems and Inheritance\Operations.
- SchemaType::isRef() already guarantees is_string($this->type); a
  @phpstan-assert-if-true says so, fixing both Augmenter\Types sites.
- wrapList() declared list<T> but returned its input array unchanged,
  which preserves keys; array_values() makes that true.
- Enums/HybridBridge-style array_values() on the enum case map.

Two `Schema\Ref|string` narrowings in Refs::dedupAllOfRefs() and
MediaTypes are defensive, not bug fixes. Both looked reachable —
dedupAllOfRefs() runs unconditionally while resolveRefRefs(), which
collapses Ref objects to strings, only runs when the ref map is
non-empty, so a Ref object as an array key would fatal. I could not
build a repro: a schema in $payload->schemas with a reflector is
itself a component, so the ref map is never empty when that loop has
anything to key on. Narrowed rather than claimed.

PathItems::findGoverningPathItem() now uses ClassReflector::tryReflect()
instead of `new \ReflectionClass()` in a try/catch. phpstan called that
catch dead once the param was class-string, and it is right that
ReflectionException is not the risk — but the catch was not pointless,
it was aimed at the wrong throwable. A class-string is a name, not a
promise the class loads, which is exactly what zircote#2201 built
tryReflect() for. So this also widens what that guard actually catches.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 16 level-7 errors.
Mostly mechanical array/generic types: AugmentTags' whitelist and tag
lists, CleanUnusedComponents' ref accumulator and SplObjectStorage,
and DocBlockParser::parseDocblock()'s by-ref $tags — the last typed at
the source, which also cleared it in the three Processors\Concerns\
DocblockTrait contexts that forward to it.

Two real narrowings behind the mechanical ones:

- Components::$_nested values are string|array<string>, so the
  `2 == count($nested)` guard distinguishing the [property, key] form
  was calling count() on a possible string — already a TypeError in
  PHP 8 if it ever happened. is_array() makes the guard say what it
  means, and Rector then tightened both to === since count() is now
  provably int.
- MergeIntoOpenApi's array_filter() over $merge preserves keys, so the
  list<> that mergeAnnotations() declares was not guaranteed;
  array_values() restores it.

extractCommentSummary() also guards preg_split() returning false.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 12 level-7 errors.
…seline entry

summaryAndDescription()'s docblock claimed Operation|Property|Parameter|
Schema. Three of those four are wrong: Parameter and Property are
`continue`d before the call ("they have their dedicated processor"),
and Schema has no $summary at all. The annotations that actually carry
both summary and description are Examples, Info, Operation, PathItem
and Tag.

The stale union had a phpstan-baseline entry holding it in place, so
correcting it made that entry obsolete — removed, and composer analyse
is clean without it. One fewer baselined error at the project's own
level, from level-7 work.

Also in this batch:

- ExpandEnums: class-string<UnitEnum> for the context enum name, and
  `$enum->value ?? $enum->name` spelled out as an explicit BackedEnum
  check — equivalent (?? suppresses the undefined-property notice on a
  pure enum) but no longer relying on that suppression.
- MergeJsonContent/MergeXmlContent: $parent->content is declared
  array|JsonContent|XmlContent|MediaType|Attachable, so indexing it
  needed is_array(). Behaviour-preserving: a nested JsonContent arrives
  in _unmerged, never on content itself, so the branch only ever
  replaces the UNDEFINED sentinel.
- AbstractAnnotation::$_unmerged typed list<AbstractAnnotation> —
  docblock only, and what makes array_search()/array_splice() over it
  provably int-keyed.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 11 level-7 errors.
GenerateInput's six array options are list<string> (argv values);
SourceFinder, Utils\Config and Specification\Walker get the routine
array/generic types.

Components::componentTypes() returns array_keys($_nested), which is
already typed array<class-string<AbstractAnnotation>, ...> — saying so
(plus array_values(), since array_filter() preserves keys) resolved
all four AugmentRefs errors, three of which were unresolved template
types rather than plain missing annotations. Its ref() also gets the
is_array() guard on the $_nested [property, key] form, same as the two
processors in the previous batch.

Fixes a regression from that batch: typing $_unmerged as a list made
CleanUnmerged's unset()-in-loop invalid, since unset() leaves gaps.
Rewritten to build the kept list instead — same result, and arguably
what it meant.

Two things left as-is and commented rather than changed:

- GenerateCommand's -a handler: if neither class_exists() branch
  matches, $processor stays the raw CLI string and is added to the
  pipeline, which cannot invoke it. Pre-existing; validating it is
  behaviour, not typing.
- Annotations\Components::$callbacks keeps its untyped @var array —
  classic's level-6 backlog, which this work leaves alone.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 16 level-7 errors.
Two of these are corrections, not annotations:

- Builder::$sources declared `string|SplFileInfo|Reflector|iterable`
  natively while its docblock said list<BuilderSource|...>. It is only
  ever [], appended to, or set from setSources(array); the scalar and
  object arms of that union were never reachable. Narrowed to `array`,
  which is what the docblock already claimed.
- AttributeTranslatorInterface::translate() declared both $attributes
  and its return as array<AttributeInterface>, but AttributeFactory
  ends with array_filter(..., $item instanceof AttributeInterface) —
  it filters precisely because translators can pass other objects
  through, and $created is array<object> by the interface's own
  docblock. Widened both to array<object> with a note pointing at that
  final filter; the narrow type was describing an invariant the code
  deliberately does not hold.

Also: compileKeyedMap()'s $items only ever has its values read, so
list<object> was needlessly strict against a name-keyed encoding map;
AugmentMediaType guards classic's $ref (string|class-string|object)
with is_string() before handing it to OpenApi::ref(), which rejects
anything but a '#/' pointer regardless; routine types on Resolver and
Builder::getDefaultAugmenters()'s PipeInterface<Specification>.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 14 level-7 errors; src/ is now down to classic plus the three
errors the Header content drop and the ServerVariable enum question hold open.
The three traits under tests/Concerns/ are used by most of the suite,
so phpstan reports their errors once per using class — typing
GeneratesTestMatrix, AssertsSpecEquals and UsesFixtures cleared 35
reported errors from nine.

assertSpecEquals() now accepts `false` alongside array|stdClass|string|
null. Six callers pass file_get_contents() straight in, and the point
of those assertions is to fail on the comparison with a readable diff,
not on the argument type — so widening the signature matches what the
callers intend rather than making each of them guard.

discoverFixtures() also guards glob() returning false.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 35 level-7 errors in tests/.
Typing OpenApiTestCase's helpers cascades through most of the suite —
its analysisFromFixtures()/processorPipeline()/getContext() signatures
account for a large share of what tests/ reported.

Giving annotationsFromDocBlockParser() its real return type,
list<OA\AbstractAnnotation>, surfaced eight places where a test reads
a *subclass* property (Contact::$url, Contact::$email, Schema::$examples,
MediaType::$examples) off the base type without ever asserting which
annotation it parsed. Each of those tests knows exactly what it fed the
parser, so each gained an assertInstanceOf: the type narrows and the
test now actually checks the thing it was assuming. Assertion count
goes 20235974 -> 20235982 accordingly.

That also means composer analyse — level 5, the project's own gate —
was passing those eight only because the untyped `array` return made
every element mixed. No baseline entry involved; they were simply
invisible.

Two recurring test-side shapes fixed alongside: PipelineTest::pipe()
declared `callable` while returning a Closure (the object-ness is what
Pipeline's item type needs), and the MergeJson/MergeXmlContent tests
cast $response->content before assertCount(), matching the (array) cast
already used a few lines above in the same methods.

One trap worth recording: a docblock placed *after* a #[DataProvider]
attribute does not attach to the method. Four of these were written
that way at first and silently did nothing.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 54 level-7 errors in tests/.
Bulk pass over ~50 data-provider returns and test-method array
parameters across tests/. Mostly array<mixed>/iterable<mixed>: weak,
but accurate, and these are fixture tables whose shapes vary per case.
Generated by a script driven off phpstan's own output, then checked by
hand for the two things a script gets wrong — a duplicate @PARAM where
one already existed, and a @PARAM emitted after @return.

CompilerTest's 27 offset/argument errors had one root: compileSchema()
returns array|\stdClass and 14 callers index into it.

I first assumed the \stdClass arm was dead — nothing accessed the
result with -> — narrowed the return to array, and the suite caught it:
test30OmitsIfThenElse asserts the result *equals* `new \stdClass()`,
because an empty compiled schema has to serialise as `{}` rather than
`[]`. The union is load-bearing.

So it stays, and the indexing callers now go through a
compileSchemaArray() wrapper that asserts the array case once. That
test keeps calling compileSchema() directly, and the reason the union
exists is now written above it.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 83 level-7 errors in tests/.
DocsAccuracyTest's 18 errors were almost all one shape: file_get_contents()
and preg_match() results flowing on unchecked. Both now go through small
helpers — readFile() and captureGroup() — that fail with a useful message
instead of indexing into `false` or an empty match array.

captureGroup() exists because PHPUnit's assertions do not narrow types
for static analysis: assertNotEmpty($m) then $m[1] still reads as
possibly-missing, and so does assertArrayHasKey(1, $m). fail() returns
never, so a helper built on it does narrow — and reports the failing
pattern rather than an undefined-offset notice.

AttributesSyncTest: static exclusion lists get list<string>,
prepDocComment() accepts the string|false getDocComment() actually
returns, and the two class-strings built by concatenating a namespace
onto getShortName() are annotated at the point they are built.

parameterType() reused one $var for both the array and string phases of
its work, which is why its declared ?string return could not hold.
Split into $parts for the array phase; the early return for the
non-string case replaces a truthiness check that also treated '' as
absent, which it still does.

Rector then tightened a != to !== once the ReflectionNamedType guard
made getName() provably string — same follow-on as the earlier
is_array()/count() batch.

Full suite green (2259 tests). composer lint and analyse clean.
Clears 31 level-7 errors in tests/.
The long tail: glob()/file_get_contents()/getFileName() false guards,
class-string annotations where a provider supplies one, array_values()
where a list<> was declared, and assertInstanceOf where a test reads a
property off a union it already knows the shape of.

Two that were not mechanical:

- The anonymous PSR-3 loggers took a `@param string|\Stringable
  $message` docblock and phpstan kept reporting the parameter as
  untyped — unlike the named classes in src/Loggers/, where the same
  docblock worked. Gave them the native `string|\Stringable` type
  LoggerInterface already declares, which is better anyway.
- AssertsSchemaStructure's allOf-ref extraction: Schema\Ref extends
  Schema, so Ref::$ref inherits the parent's Schema\Ref|string|null
  type even though it only ever holds the string. An instanceof ternary
  therefore does not narrow, and neither does casting. Resolved with an
  explicit is_string() on the unwrapped value. That unwrap now appears
  in four places and is worth one helper once something else touches
  ref handling.

Also weakened a few of the previous batch's array<mixed> annotations
back to their real types where a constructor or factory needed it
(PathFilter's list<string>, Result's log shape).

Worth recording: phpstan's result cache survived `clear-result-cache`
here and reported several of these as unfixed after they were fixed.
TMPDIR=... for a fresh cache dir was the reliable way to confirm.

tests/ is now clean at level 7. Full suite green (2259 tests).
composer lint and analyse clean. Clears 34 errors.
Each has a reason that only existed in a commit message until now, which
is the wrong place for it: the next person to run phpstan at level 7 sees
an unexplained error and the obvious fix — a guard or a cast — buries the
thing the error is pointing at.

- HybridBridge::convertServer(): Spec\ServerVariable::$enum is
  list<string> per the spec while classic's accepts more.
- TypeInfoTypeResolver::applyToAnnotation(): an explicit
  additionalProperties: false is dereferenced as an object.

A third sat in HybridBridge::convertHeader(), where a Header whose content
is a JsonContent was silently dropped. zircote#2204 has since fixed that, so the
error and the comment explaining it both go with it.
The earlier batch gave three anonymous test loggers a native
string|\Stringable $message, because phpstan does not honour an
@PARAM docblock on an anonymous class the way it does on a named one.
That types the parameter — and breaks psr/log 1.1, where
LoggerInterface::log() declares $message untyped, so the override is a
signature incompatibility and PHP fatals before any test runs.

composer.json allows ^1.1 || ^2.0 || ^3.0 and CI's lowest-dependency
cell locks 1.1.0, which is where this surfaced; locally only 3.0.2 was
ever installed. Raising the floor would fix it too, but that is a BC
break for consumers and not something to decide inside a typing change.

Extracted the three into named doubles under tests/Doubles/ instead —
RecordingLogger and ForwardingLogger. Named classes get the docblock
honoured, so $message stays untyped and 1.x keeps working.

ForwardingLogger holds its entry list by reference rather than the
recorder closure the anonymous version used. Context::__serialize()
drops anonymous classes but keeps named ones, so a closure property
made any Context carrying the tracking logger unserializable —
ContextTest::testSerialize caught it.

Verified by installing psr/log 1.1.0 locally: the previous commit
fatals, this one runs the full suite green. Lockfile restored after.
…and definition

Rebasing onto master pulled in zircote#2196, which rewrote the two Console classes this
branch had annotated, and zircote#2202, which added a data provider after the pass that
typed them. Three errors came back; none is a conflict resolution, all three are
the level 7 work applied to code that did not exist when the branch started.

GenerateInput's array properties: zircote#2196 documented them as array<string>, which
is true but weaker than the call site needs — SourceFinder's $directory and
$exclude take list<string>|string, so both reported argument.type. Restored to
list<string>, which is also what the console hands over: array arguments and
multi-value options arrive sequentially. Nothing new is reported at the
assignment side, because fromInput() reads InputInterface::getArgument() and
getOption(), and mixed is a level 9 concern rather than a level 7 one.

CompilerTest::exampleValueProvider(), added by zircote#2202, already carries a @return
but leaves a bare array inside one closure signature, so the iterable value type
is still unspecified. Typed as the compiled document it receives.

Also types invalidEnumOptionCases(), the provider zircote#2196 added, to match every
other provider in the file.

126 deliberate errors as before: 117 classic, 8 held by PR 57, 1 ServerVariable.
Suite 2267 green, composer cs clean.
@DerManoMann
DerManoMann merged commit 8bf0e03 into zircote:master Sep 17, 2026
18 checks passed
@DerManoMann
DerManoMann deleted the chore/phpstan-level-7 branch September 17, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant