test(oop): cover the constructor_owner cases #796 left untested - #994
Merged
Conversation
Closes #871. #796 introduced `types::constructor_owner` — the walk that finds the class whose `__construct` PHP runs when a descendant of a class with a non-public constructor is instantiated — and the existing coverage exercised only the 0-argument private-ancestor success path. The listed gaps are behaviours that are already CORRECT on main; what was missing is the pins that keep them so. A new `tests/codegen/oop/constructor_owner.rs` groups the member-visibility half, where private and protected deliberately diverge: - a PROTECTED ancestor constructor IS inherited: it runs through the descendant, and `method_exists()` is `true` on both classes; - it is still not public, so the descendant is not instantiable and the reflected declaring class is the ancestor; - a PRIVATE ancestor constructor is NOT inherited: `method_exists()` is `false` on the descendant while staying `true` on the declaring class — the half that makes the owner walk necessary at all; - `get_class_methods()` agrees, with an inherited public method alongside so the assertion is a contrast rather than a tautology (the function is scope-aware, so a private constructor is omitted from its own declaring class too); - a descendant's own PUBLIC constructor replaces a private ancestor's. Two error tests cover the hiding direction, which the walk must NOT take: a descendant that declares its own PRIVATE constructor hides an inherited public one, so `new static()` from the ancestor's scope and a named `new Child()` both name the DESCENDANT's constructor, exactly as php-src does (`Call to private HideChild::__construct() from scope HideOwner`). Every fixture's output was diffed against reference PHP 8.5 and matches. The remaining items on the issue — omitted defaults on the owner path, a descendant replacing the constructor, and `method_exists`/`getMethods` staying false — are covered by the tests in PRs #990 and #991. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Greptile SummaryThis PR adds regression coverage for PHP constructor ownership and visibility behavior without changing production code.
Confidence Score: 5/5The PR appears safe to merge because it only adds coherent regression tests and no outstanding issue was identified. The changes since the previous review introduce no additional modifications, and the current test additions are properly registered, documented, and consistent with the repository’s native compatibility-test structure.
|
| Filename | Overview |
|---|---|
| tests/codegen/oop.rs | Registers the new constructor-owner codegen test module. |
| tests/codegen/oop/constructor_owner.rs | Adds focused end-to-end tests for private and protected constructor ownership, inheritance, reflection, and replacement semantics. |
| tests/error_tests/classes_traits.rs | Adds compile-error coverage for descendant private constructors hiding inherited public constructors. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
PHP[PHP constructor fixture] --> Compile[Compile and type-check]
Compile --> Execute[Run generated program]
Execute --> Assert[Compare output or expected error]
Reviews (2): Last reviewed commit: "test(oop): cover the constructor_owner c..." | Re-trigger Greptile
Guikingone
force-pushed
the
test/871-constructor-owner-coverage
branch
from
September 13, 2026 19:39
2006987 to
f4d10b3
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.
Closes #871.
#796 introduced
types::constructor_owner— the walk that finds the class whose__constructPHP runs when a descendant of a class with a non-public constructor is instantiated — and the existing coverage exercised only the 0-argument private-ancestor success path.Every gap the issue lists is a behaviour that is already correct on
main. What was missing is the pins that keep it so, which is what this PR adds; no production code changes.What is covered
A new
tests/codegen/oop/constructor_owner.rsgroups the member-visibility half, where private and protected deliberately diverge:test_protected_ancestor_constructor_is_inherited_and_runsmethod_exists()istrueon both classestest_protected_ancestor_constructor_leaves_the_descendant_uninstantiableisInstantiable()isfalseand the reflected declaring class is the ancestortest_private_ancestor_constructor_is_not_a_member_of_the_descendantmethod_exists()isfalseon the descendant,trueon the declaring class — the half that makes the owner walk necessary at alltest_get_class_methods_omits_an_inherited_private_constructorget_class_methods()agrees, with an inherited public method alongsidetest_a_descendants_own_public_constructor_replaces_a_private_ancestorsTwo error tests in
classes_traits.rscover the direction the walk must not take — a descendant that declares its own private constructor hides an inherited public one, sonew static()from the ancestor's scope and a namednew Child()both name the descendant's constructor, as php-src does (Call to private HideChild::__construct() from scope HideOwner). Resolving to the ancestor would have accepted those calls, since the ancestor's constructor is public and the call site is its own scope.The
get_class_methodsassertion is deliberately a contrast rather than a count of zero: the function is scope-aware, so from global scope a private constructor is omitted from its own declaring class too (php-src returns0for both classes when that is the only method). The inherited public method alongside it is what proves the list is populated at all.Verification
Every fixture's output was diffed against reference PHP 8.5 and matches byte for byte.
--test codegen_tests oop(810) and--test error_tests(1520) pass.Overlap
The remaining items on the issue — omitted defaults on the owner path, a descendant replacing the constructor, and
method_exists/getMethodsstaying false — are covered by the tests in #990 and #991, so they are not duplicated here.🤖 Generated with Claude Code
https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr