Fix the turbo-ext review findings - #6550
Merged
Merged
Conversation
ondrejmirtes
force-pushed
the
turbo-review-fixes
branch
from
September 23, 2026 08:16
69a8e96 to
1828704
Compare
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
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
force-pushed
the
turbo-review-fixes
branch
from
September 23, 2026 08:42
1828704 to
1d6bd94
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/
propAtWritecleanups, dead code and comment fixes are left out.Bugs with a concrete trigger
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.phar://scan paths find their symbols. Result keys use symtable semantics.protected array $visitorsused to hit a fatal error.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.opcache.file_cacheis configured, because the stripped op_arrays would persist into later, unarmed runs.IntersectionType::getFiniteTypes()matches by value, not bydescribe():(1|2)&(1|2|3)used to have[2].setOffsetValueType()no longer overflows atPHP_INT_MAX.CallableTypeHelperkeeps the lazy reasons of a maybe parameter comparison.Declaration parity
signature-parity.phpnow 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;ScopeOpsdefaults (a named-argument call used to throw ArgumentCountError);Misuse parity
Values that violate the
@apiType signatures now raise the twin's TypeError/Error instead of segfaulting:Also covered:
reorderArgs()key sorting;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
smoke.phpwith opcache off and on.turbo-ext/tests, includingparser-corpusandparser-upstream-corpus.signature-parityandside-by-side.walk-trace.php: identical, 615k lines.make phpstan.🤖 Generated with Claude Code
https://claude.ai/code/session_018Fdg8n3eEi4ZoXa4dA2k4A