Skip to content

Fix the turbo-ext review findings - #6550

Merged
ondrejmirtes merged 46 commits into
2.3.xfrom
turbo-review-fixes
Sep 23, 2026
Merged

ondrejmirtes merged 46 commits into
2.3.xfrom
turbo-review-fixes

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Fixes the findings of a full review of turbo-ext/src, which read every C++ file against its PHP twin. Each finding is one commit. Where the bug could be reached, the commit adds a regression test that was seen failing before the fix. The CI fix the review found (CombinationsHelper::combinations([]) addref'ing the read-only empty array) already landed on 2.3.x in 9574d5f.

This is the reduced version of the branch. It keeps the fixes with a concrete trigger, the stronger declaration parity check with the drifts it forced, and the crash-to-error parity fixes. The unreachable hardening, the helper-consolidation refactor, the slot-constant/propAtWrite cleanups, dead code and comment fixes are left out.

Bugs with a concrete trigger

  • Symbol scan: the native scan reproduces php_strip_whitespace() byte for byte. Previously a string like "{$a["#"]}" or a heredoc end could hide or invent classes and functions in scanned files.
  • Scanned files: they are read through PHP's stream layer, so phar:// scan paths find their symbols. Result keys use symtable semantics.
  • NodeTraverser: properties are declared like php-parser's. A subclass redeclaring protected array $visitors used to hit a fatal error.
  • ScopeOps: ScopeOps::scopeWith() resets every memo property of a natively cloned scope. A rule deriving a scope from its NodeCallbackScope used to get stale types for nodes it had already asked about.
  • Deep chains: native recursions over deep chains continue on a fresh C stack. They used to segfault at around 40k levels.
  • Pipe operator: the parser holds the parenthesized arrow functions it tracks. A reused object handle used to drop "Arrow functions on the right hand side of |> must be parenthesized".
  • TrustedTypes: the pass no longer arms when opcache.file_cache is configured, because the stripped op_arrays would persist into later, unarmed runs.
  • Inference marker: after a throwing inference, the in-process marker is kept as the twin keeps it.
  • PHP twin fixes, ported identically:
    • IntersectionType::getFiniteTypes() matches by value, not by describe(): (1|2)&(1|2|3) used to have [2].
    • setOffsetValueType() no longer overflows at PHP_INT_MAX.
    • CallableTypeHelper keeps the lazy reasons of a maybe parameter comparison.
  • TypeCombinatorCache: a memo hit returns the current call's own member of an operand, never another call's object.

Declaration parity

signature-parity.php now checks every shadowed class under real-name activation: class shape, parameter types and defaults, return types, properties and constants. This PR fixes every drift it found, including:

  • PHPStanTurbo\… names in arginfo;
  • missing return types;
  • ScopeOps defaults (a named-argument call used to throw ArgumentCountError);
  • missing private constants and methods.

Misuse parity

Values that violate the @api Type signatures now raise the twin's TypeError/Error instead of segfaulting:

  • non-Type callback results, members and variadic arguments;
  • wrong argument classes on the ExpressionResultStorage, StatementsHandler, TypeSpecifier and PropertyHooksProcessor entry points;
  • paths that returned "exception pending" with nothing pending.

Also covered:

  • NodeTraverser splice and copy semantics and its LogicException messages, which now match php-parser;
  • signed reorderArgs() key sorting;
  • binary-safe member keys;
  • Nette's RegexpException in isValidIdentifier().

One deliberate divergence: the native IntersectionType/UnionType constructors reject a non-object member with a TypeError, while the twin only fails at first use. It only affects code that already violates Type[], and it replaces segfaults.

Verification

  • Clean strict build and clang-tidy.
  • smoke.php with opcache off and on.
  • Every differential suite in turbo-ext/tests, including parser-corpus and parser-upstream-corpus.
  • signature-parity and side-by-side.
  • walk-trace.php: identical, 615k lines.
  • Full PHPUnit suite with and without the extension (22,233 tests).
  • make phpstan.

🤖 Generated with Claude Code

https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A

ondrejmirtes and others added 29 commits September 23, 2026 10:34
The set of parenthesized arrow functions was keyed by object handle
without keeping the node alive, while php-parser's SplObjectStorage
holds it. Error recovery can drop a parenthesized ArrowFunction; a later
unparenthesized one reusing its handle then passed for parenthesized and
"Arrow functions on the right hand side of |> must be parenthesized" was
lost. Store an owned copy with a ZVAL_PTR_DTOR destructor, like the
createdArrays tracker does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native stripper in front of SymbolFinderInFiles' cleaner removed
comments with a hand scanner and copied everything else through, on the
claim that whitespace does not matter downstream. Both halves were wrong:

- inside {$...} / ${...} interpolation the lexer is in script mode, so a
  nested "..." is a separate string; the scanner ended the outer string
  at it, lost `class HashInterp` after "{$a["#"]}" and invented a class
  inside "{$a["/*"]}";
- zend_strip() collapses whitespace tokens, newlines included, and after
  a heredoc's closing label writes the next token verbatim - a comment
  too - followed by "\n". The cleaner's // handling skips to the next
  newline and its quote pairing is naive, so which symbols survive
  depends on exactly those bytes.

Port the lexer rules zend_strip() can observe instead (states and state
stack for strings, interpolation, heredoc/nowdoc, variable offsets and
property lookups, brace nesting; the tokens that keep whitespace or
comments inside - casts, `yield from`; the skipped shebang line) and
mirror zend_strip() on top. The output is byte-identical to
php_strip_whitespace() over the repository and vendor/ and over ~40,000
fuzzed and mutated inputs, with short_open_tag on and off.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…nder

The twin reads each file with php_strip_whitespace(), i.e. the engine's
include-style stream open: stream wrappers (phar:// is a supported scan
case), the include path, Windows' UTF-8 paths. The native finder used a
raw open(), so a phar:// or file:// path found no symbols, and on Windows
_open() took the UTF-8 path as ANSI. Open through
php_stream_open_wrapper_ex() with the same flags instead, and stop on an
exception a user stream wrapper throws.

Also mirror the rest of the twin's loop: key the result with
zend_symtable_update() like `$result[$file] = ...` (a numeric-string path
becomes an int key), throw findSymbolsInFile()'s TypeError for a
non-string entry instead of skipping it, and php_strip_whitespace()'s
ValueError for a path with a NUL byte.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native class declared $visitors and $stopTraversal by hand, untyped,
with stopTraversal defaulting to false, while php-parser has
`protected array $visitors = []` and `protected bool $stopTraversal`
(typed, uninitialized until traverse()). A PHP subclass redeclaring
`protected array $visitors` was a fatal error natively, and reading
stopTraversal before traverse() returned false instead of throwing.

Declare them through the generated declareProperties(), clear
IS_PROP_UNINIT when traverse() writes stopTraversal, and let
traverse()/removeVisitor() on an unset $visitors throw the engine's
uninitialized-property Error like the twin's read does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…p-parser

The native NodeTraverser wrote replacement nodes straight into the
property's table (shared with its own guard reference, never separated),
and assigned the property only when splices were recorded. A visitor
holding the parent's array saw it change mid-traversal, and a visitor
reassigning the property kept its array where php-parser overwrites it
with the traversal result. It also separated every subnode array up
front, so each empty list - the shared [] - became a fresh heap table
retained in cached ASTs (25 KB over NodeScopeResolver.php's AST).

traverseArray() now holds its own reference to the array it iterates,
writes replacements by key into a copy made on the first write, and
returns the resulting array (the input's table when nothing changed);
traverseNode() assigns it to the property through the engine write path
unless the property already holds exactly that table.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
php-parser applies the recorded removals and replacement arrays as
array_splice($nodes, $i, 1, $replace), last first, with the element's
key $i as the offset. On a list that is the element's position, which
is all the native rebuild handled: for other keys the offset is clamped
(keys past the end remove nothing, a negative key counts from the end),
every splice renumbers the integer keys and keeps the string keys, and a
string key is array_splice()'s TypeError. The native traverser spliced
by position and dropped the keys.

Record the key next to the position and, when the array is not a list,
replay the splices one by one with array_splice()'s clamping and
renumbering; a list keeps the single rebuild pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The invalid-return messages named the returned type through
zend_zval_value_name() ("int", "float", "true", the class name) where
php-parser uses gettype() ("integer", "double", "boolean", "object"),
and the unreasonable-replacement messages left out the node types php-parser
interpolates ("Trying to replace statement (Stmt_Echo) with expression
(Expr_Variable)..."). Use zend_zval_get_legacy_type() and call getType()
on both nodes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The trusted-types pass rewrites op_arrays inside opcache's optimizer,
before the script is persisted. With opcache.file_cache set the stripped
op_arrays are written to disk, and a later run that never armed the pass
- a --debug run, or one without the extension loaded - executes them
without their argument and return type checks. Refuse to arm in
pt_trusted_types_set_prefix() when a file cache is configured.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The twin matches through Nette's Strings::match(), which turns a
run-time PCRE failure into a RegexpException carrying
RegexpException::MESSAGES[preg_last_error()] and the pattern as the
message and the error as the code; the port threw "preg_match() failed"
with code 0. Build the same message and code.

The identifier pattern was also interned at request time into a static
nothing reset; create it as a permanent interned string at MINIT.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…peWith()

The native scopeWith() clones the scope instead of building a new one
through the factory like the twin's duplicateWith(), and reset a
hand-kept list of memo properties on the clone. NodeCallbackScope's
askedTypes/askedNativeTypes memos were missing from it, so a scope a rule
derived from its callback scope (invalidateExpression(), mergeWith(), ...)
answered a synthetic node from the callback scope's memo. The list also
named truthyScopes/falseyScopes, which no longer exist.

The reset list is now derived from the class: every typed instance
property with a declared default is reset to that default — exactly the
properties a freshly constructed scope starts with (promoted properties
have no default, and the untyped promoted $nodeCallback is excluded by
the typed condition).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
ExpressionResult's collectReadVariableNames(), IssetabilityResolution's
isSet()/isSetUndefined() down the inner chain and IssetabilityDescriptor's
inner resolution recursed on the C stack, where their PHP twins recurse on
the VM stack: a deep concatenation chain segfaulted, and a deep
resolution chain threw "Maximum call stack size reached" where the twin
answers. Each recursive step now goes through pt_engine_with_stack(),
which continues on a fresh C stack segment when the current one runs low.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native ExpressionResultStorage and ExpressionResultStorageStack glue
parsed its object arguments without a class check: mergeResults() with a
foreign object read its property slots as the storage's tables and
crashed, and storeExpressionResult() / findExpressionResult() / push()
silently accepted what the twin's parameter types reject.

mergeResults() now requires the native class (its slots are read
directly); the Expr parameters are checked against the class map's Expr,
and the ExpressionResult / ExpressionResultStorage parameters through the
new pt_shadow_instanceof(), which also accepts the real-named PHP twin
under the prefixed activation of the differential tests. The methods are
registered by their generated signatures.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…rts them

The native ArgumentsNormalizer::reorderArgs() sorted the reordered
arguments comparing integer keys as zend_ulong, so a negative key sorted
last, where the twin's ksort() puts it first. Compare them as zend_long.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native ClassReflection built the scope-qualified member cache key
("%s-%s"), the display name with template types and the missing enum
case message with zend_strpprintf(), whose %s stops at a NUL byte. A
member name with a NUL byte in a class scope then shared the cache key
of its prefix (getMethod("area\0x") answered area()), and the message
lost the rest of the name. Build them length-aware, like the twin's
sprintf() and concatenation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The twin's inferPrivatePropertyType() removes the class's entry in
$inferClassConstructorPropertyTypesInProcess only when the inference
returns normally; one that throws leaves it set, so later asks for that
class infer nothing. The native port removed it unconditionally.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…ementStep()

The native processStatementStep() used the elements of the caller's
statement list as objects without a check and crashed on anything else,
where the twin's `Node\Stmt $stmt` parameter throws a TypeError. Throw
the same TypeError.

The misuse-parity section of smoke.php gains a child-process harness for
the engine classes (they cannot be declared next to their twins): the
same probes run once over the PHP classes and once over the native ones
under their real names, and their outcomes are compared.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…ooksProcessor entry points

The @api TypeSpecifier::specifyTypesInCondition(), create(),
specifyDefaultTypes() and handleDefaultTruthyOrFalseyContext() and
PropertyHooksProcessor::processPropertyHooks() parsed their object
parameters without a class check (and the callable $nodeCallback as any
value), so wrong arguments ran into the bodies — answering, or failing
with an internal error — where the twin throws its TypeError. Check them
against the class map's interfaces and the native class entries, the
native type node against Identifier|Name|ComplexType|null and the node
callback for callability, with the twin's messages.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…hing pending

Several native paths returned their "exception pending" value without an
exception, so the glue returned with nothing thrown (a caller then got
null): IfHandler after warning about a foreign elseif or else,
SwitchHandler and TryCatchHandler after warning about a non-array case,
catch or catch-type list, TryCatchHandler after warning about a foreign
finally, SimpleImpurePoint on a non-object scope, TypeSpecifier on a
non-object nullsafe result, and PhpClassReflectionExtension on a
non-array result of a call declared to return an array.

Each now does what the twin does: warns and iterates nothing over a
non-array list, throws the TypeError of the call the twin makes next
(processExprNode(), processStmtNodesInternal(), callNodeCallback()),
the "Call to a member function" Error, or the engine's return-type
TypeError for a callee that broke its signature. SwitchHandler reads a
foreign case's properties with the engine's warning instead of crashing,
and IfHandler prints the undefined array key with ZEND_LONG_FMT.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…aticType

The native traverse() handed whatever the callback returned to the
constructor, which reads the class entry of the value, so a callback
returning a string or an int crashed the process. The twins pass the
result into a typed `?Type $subtractedType` parameter and throw a
TypeError instead; the natives now raise the same TypeError for any
value that is neither null nor a Type.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
traverse() and traverseSimultaneously() stored whatever the callback
returned in the typed keyType/itemType slots, and the next call reading
them crashed. The twin's `new self($keyType, $itemType)` throws a
TypeError from its typed constructor parameters; the native now checks
both values the same way, key type first, before constructing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The twin's constructor does not check its array<string, Type>
$properties, so a non-Type value only fails at the first method called
on it, with the engine's "Call to a member function X() on string"
Error (or, in accepts(), the TypeError of
VerbosityLevel::getRecommendedLevelByType()). The native read the value's
object pointer unchecked at every such site and crashed. A helper now
returns the object or raises that Error, accepts() checks the typed
parameter the twin passes it to first, and traverse() compares the
callback's result with `!==` semantics like the twin does, so an
unchanged non-object value keeps the original instance.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The twins' IntersectionType and UnionType constructors store their
members unchecked, and a member that is not a Type fails at its first
use (an Error, or a TypeError from a Type-typed closure). The native
code dereferenced such a member as an object in nearly every method of
IntersectionType, in several of UnionType and BenevolentUnionType, and
in every port iterating getTypes(), crashing the process. Instead of
checking each of those sites, the native constructors now reject a
non-object member with a TypeError — the invariant the native code
already relies on — which UnionType's constructor gets for free in the
loop it already runs over the members. BenevolentUnionType's
getOffsetValueType() additionally checks the members it gets from a
possibly overridden getTypes() and raises the twin's Error.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…points

union(), doUnion(), intersect() and doIntersect() parsed their variadic
arguments as any values and handed them straight to the implementation,
which crashed on a scalar (TypeCombinator::doUnion('x')) and failed
later with an unrelated Error on a non-Type object. The twins' `Type
...$types` parameter rejects both with a TypeError naming the argument;
the glue now raises the same TypeError before calling the
implementation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native TypeTraverser::map() callbacks of TypeCombinator and of the
called-on-type member prototypes reported a non-object $type argument
as an ArgumentCountError, which the twins' `Type $type` closure
parameters raise as a TypeError; the argument count and the type are now
checked separately. countConstantArrayValueTypes() also checks each
element the way the twin's TypeTraverser::map(Type $type, ...) call does,
so a scalar or a non-Type object element is that TypeError instead of
reaching the callback (a scalar) or failing with an unrelated Error (an
object).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
A memoized result that was a member of a union or intersection operand
(remove(int|string, string) returning the union's `int`, removeNull() of
`T|null` returning `T`) was handed to every later structurally equal
call, which then got the first call's member instead of its own — an
object the PHP implementation, which does not memoize, never returns
there. Like operand results, such results are now marked in the memo
slot's alignment bits, and a hit returns the member of the new call's
operands with the recorded member's structural hash; a result whose
hash is not unique among the operands' members, or that is also an
operand, is not memoized. The header comment claimed the memo only
shares what TypeCombinator itself shares, which did not hold for
members; it now says what is shared and what is not.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…setValueType()

When setting an offset on a list turns the result into a non-list,
IntersectionType::setOffsetValueType() keeps the list accessory if the
offset lies in int<0, N + 1> for a known offset N of the list. With N =
PHP_INT_MAX, `N + 1` became a float and IntegerRangeType::fromInterval()
threw a TypeError; the native port computed it in signed arithmetic,
which overflowed. No offset exists past PHP_INT_MAX, so the range is now
open-ended there (int<0, max>) in the twin and the port alike.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
IntersectionType::getFiniteTypes() keyed each member's finite types by
describe(VerbosityLevel::typeOnly()), which is the same 'int', 'string'
or 'float' for every value of a kind: (1|2)&(1|2|3) had the finite types
[2] instead of [1, 2], and disjoint members could share one. The values
are now keyed by FiniteTypeSet::key(), whose keys are equal exactly when
the types are, and the values it does not key (floats, constant arrays)
are matched with equals(). Ported to the native class the same way.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The twins declare `private const TRUNCATE_ACCESSORIES_LIMIT` (ArrayType)
and `private const EXTRA_OFFSET_CLASSES` (ObjectType); the native classes
only had a #define and a local copy of the list, so reflection saw
fewer constants on them than on the twins. Both are registered now with
the twins' visibility and values, and ObjectType's
isExtraOffsetAccessibleClass() walks the one list the constant is built
from.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
CallableTypeHelper::isParametersAcceptorSuperTypeOf() rebuilds a maybe
parameter result to append its "needs to be same or wider" reason, and
passed on only the eager reasons. A maybe result can carry lazy reasons
- a union parameter type's or() keeps those of its no members, like
ConstantArrayType's reason for sealed shapes that cannot be intersected
- so a reason that would have been listed had it been built eagerly was
lost from getReasons(). The rebuilt result now keeps them; the native
port does the same. (The accepts() path converts AcceptsResults, which
have no lazy reasons, so nothing is lost there.)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
ondrejmirtes and others added 17 commits September 23, 2026 10:34
signature-parity.php compared the natives only as the PHPStanTurbo\
prefixed declarations, mapped prefixed class names back to the twins,
skipped erased and absent types and ignored properties, constants and
non-public methods — so arginfo naming PHPStanTurbo\ classes, methods
without return types, missing defaults (a named-argument call skipping
the parameter throws natively), undeclared or untyped properties and
missing class constants all passed.

The check now activates the native classes under the real names, the way
TurboExtensionEnabler does, and compares them against a dump of the twins
from a child process that never activates: class shape, every method of
any visibility (a private one is optional, but a class that declares any
of its twin's private methods declares all of them) with parameter types,
optionality and default values and return types, every property with its
type, default, visibility, static and readonly flags, and every class
constant with its value. The classes that drift today are listed as
pending; the following commits fix them and shrink the list.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…ures

TrinaryLogic, AcceptsResult and IsSuperTypeOfResult spelled their
arginfo by hand: parameter types named the PHPStanTurbo\ prefixed
classes, no method declared a return type, and the properties were an
untyped `value` and `object`-typed `result` slots instead of the twins'
typed ones. TrinaryLogic also lacked the private YES/MAYBE/NO constants,
its private create() and the static flyweight properties.

The methods now register by the generated sig:: declarations, the
properties come from declareProperties() and TrinaryLogic declares its
constants and create(). The singletons clear IS_PROP_UNINIT when they
initialize the now typed `int $value`, like the engine's write does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…gnatures

ScopeContext, ExpressionTypeHolder and ConditionalExpressionHolder
declared untyped null-default properties and spelled their arginfo by
hand, with erased or PHPStanTurbo\ prefixed parameter types and no
return types.

They now register by the generated sig:: declarations and declare the
twins' typed (and for ExpressionTypeHolder readonly) properties through
declareProperties(); the slots keep their order, so the native readers
are unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
ScopeOps spelled its arginfo by hand: class-typed parameters were erased
to object, no method declared a return type, and the optional
parameters of invalidateExpressionEntries() and
shouldInvalidateExpression() carried no default value — so a
named-argument call skipping one (`keepPropertyFetches: true`) failed
with an ArgumentCountError natively while the twin accepted it.

The methods now register by the generated sig:: declarations, which
carry the twin's types and defaults.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
AssignHandler, OutputBufferHelper, ExpressionResult,
IssetabilityDescriptor, IssetabilityLinkInfo, MutatingScope, ScopeOps,
ForeachHandler and TypeSpecifier did not declare their twins' private
constants, so reflection (and a closure bound to the class) saw classes
without them.

They are declared now; the array-valued ones are persistent lists built
once at module startup by pt_persistent_string_list(), which
VolatileExpressionHelper's private list builder becomes. ScopeOps'
keyMayHideSubExpressions() and MutatingScope's serialization check read
the same tables the constants are built from.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native class declared its promoted optional properties with their
parameter defaults (null, false, []), while the twin's promoted
properties have none — reflection reported default values the twin does
not have. construct() writes every slot anyway, so the generated
declareProperties() replaces the hand-written list, and the slot writers
clear IS_PROP_UNINIT the way the engine's first write of a typed property
does.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
Natives that declare their twins' private methods declared all but a
few: InitializerExprTypeResolver's getIntegerBounds(),
getMaxModuloMagnitude(), shiftLeftOverflows() and toIntBound(),
InitializerExprContext's parseNamespace(), IntegerRangeType's
isSubTypeOfUnionWithReason() and the private static create() of the
TypeSpecifierContext, PassedByReference and TemplateTypeVariance
flyweights. Each is registered now over the C++ member that already
implements it, and the differential tests call them in the class scope
on both sides.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…atures

The PHPStan\Parser visitors, ParserRunner, NodeScanner, ExprPrinter,
ClassStatementsGatherer and CombinationsHelper spelled their arginfo by
hand without the twins' return types (`?Node` enterNode(), `?array`
beforeTraverse(), ...) and with some class-typed parameters erased to
object. They now register by the generated sig:: declarations.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…Type's hand-spelled methods by signature

ConstantArrayTypeBuilder's mutators, TypeCombinatorCache's methods and
ObjectType::resetCaches() still spelled their arginfo by hand, without
the twins' `void` / `Type` return types, although generated sig::
declarations exist for them. They register by those now.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
ArenaCache's static methods spelled their arginfo by hand without the
twin's return types; they register by the generated sig:: declarations
now.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native kept its own PT_PCRE_PROP_* slot enum, a hand-written copy of
the twin's property list and hand-spelled arginfo in which the
ClassReflection, ClassMemberAccessAnswerer and adapter parameters were
erased to object and no method declared a return type. It now uses the
generated slots:: constants, declareProperties() and the sig::
declarations (the constructor's among them, which name the twin's
parameter classes for the container just as the hand list did).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…atures

PhpFileCleaner declared none of its twin's five private properties —
the generator could not express the `string $contents = ''` default, so
no declareProperties() was generated — and SymbolFinderInFiles neither
declared nor kept the promoted $cleaner. Both registered their one
working method by hand, without its return type.

reg::Class learns typed properties with a string default
(typedStringProperty()), which the generator now emits, so both classes
declare their properties by the generated declarations; the
SymbolFinderInFiles constructor stores $cleaner, and clean() and
findSymbols() register by their sig:: declarations.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
The native class declared none of the twin's 25 private properties (the
per-resolution state resolve() keeps natively). It declares them through
the generated declareProperties() now, so reflection sees the twin's
class.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…urn type

The native storage declared $fallback untyped instead of the twin's
`?self` and registered duplicate() by hand without its return type. The
property is declared as the twin's now and duplicate() registers by its
generated signature.

Its id-keyed result arrays deliberately replace the twin's
SplObjectStorage; signature-parity.php learns to list such single
differences ($deliberateDrift) with the reason, and fails once one no
longer occurs.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
Every class it listed now matches its twin. NodeTraverser's protected
traverseNode()/traverseArray() stay undeclared on purpose — traverse()
walks the tree in one native call — and are listed as a deliberate
difference with the reason instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
…ives

The natives accepted a non-Type object where the twins declare a Type:
the constructor glue of the Type classes and every equals() parsed a
`Type` / `?Type` parameter with Z_PARAM_OBJECT (StringType::equals()
answered false for a stdClass, new IterableType(new stdClass, ...) built
an object that failed later with an Error), and traverse callbacks
returning a non-Type object passed through CallableType, ClosureType,
GenericObjectType, GenericStaticType, UnionType, BenevolentUnionType,
IntersectionType and pt_type_traverse_call(). A method missing on a
non-Type receiver raised "phpstan_turbo: method X::y not found" instead
of the engine's "Call to undefined method X::y()".

The zp::TypeObj / zp::TypeObjOrNull parameter kinds check the argument
against the Type interface (the TemplateType constructors' $default
too); pt_call_type_fci() checks a traverse callback's result — only for
callables that are not native, the native ones return Types — where the
twin hands it on to a Type parameter; pt_throw_undefined_method() raises
the engine's Error with the method spelled as the Type interface declares
it. CombinationsHelper's array-of-arrays TypeError names the declared
class instead of PHPStanTurbo\CombinationsHelper.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A
@ondrejmirtes
ondrejmirtes merged commit efaa82a into 2.3.x Sep 23, 2026
521 of 531 checks passed
@ondrejmirtes
ondrejmirtes deleted the turbo-review-fixes branch September 23, 2026 08:46
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