fix(translator): validate overrides of built-in class methods - #55
Open
AlessioGiacobbe wants to merge 1 commit into
Open
fix(translator): validate overrides of built-in class methods#55AlessioGiacobbe wants to merge 1 commit into
AlessioGiacobbe wants to merge 1 commit into
Conversation
checkParentMethodCanBeOverridden()'s internal-parent branch only checked
the PRIVATE and FINAL modifiers via reflection and then stopped, so an
override of any Zend built-in method was never signature-checked:
narrowed parameters, static/instance mismatches, narrowed visibility and
incompatible real return types were all accepted (all fatal in Zend,
e.g. "Declaration of C::offsetGet(int $key): string must be compatible
with ArrayObject::offsetGet(mixed $key): mixed").
Add validateInternalMethodOverrideSignature(), mirroring Zend's
zend_do_perform_implementation_check on host ReflectionMethod data:
- visibility may widen but not narrow; staticness must match;
- a by-ref return may be added but not dropped;
- the child may not require more arguments; extra parameters must be
optional or variadic;
- parameters are contravariant with invariant by-ref-ness, and a
trailing child variadic absorbs remaining parent positions (a
variadic parent requires a variadic child);
- the return type is covariant, enforced ONLY for real return types:
ReflectionMethod::getReturnType() is null for TENTATIVE return
types, which Zend merely deprecates on mismatch, never fatals.
ReflectionType data (named/nullable/union/intersection, incl. self,
parent and static) is mapped into the existing accepted-types DNF so the
comparison reuses isReturnTypeCoveredBy()/isAcceptedTypeSubset().
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
1 participant
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.
Overrides of methods inherited from Zend built-in classes were never signature-checked — only PRIVATE/FINAL flags were consulted:
class C extends ArrayObject { public function offsetGet(int $key): string {...} }andpublic static function count(): intboth compiled (Zend: fatal). Compiled classes register as internal classes, where ZendVM does not re-run these checks, so the invalid vtables reached runtime.A reflection-based validator now enforces the Zend ruleset against built-in parents: visibility may widen but not narrow, staticness must match, a by-ref return may be added but not dropped, required-argument counts, per-position by-ref-ness with variadic absorption, parameter contravariance, and return covariance — mapping ReflectionType (named/nullable/union/intersection, self/parent/static) into the existing DNF comparison machinery. TENTATIVE return types are exempt, matching Zend, which only deprecates a tentative mismatch (
ArrayObject::count(): stringstays accepted).Verified against Zend 8.4.13, 13-case probe matrix.
Part of the split of #39.