fix(checker): an inherited constructor's arity error names its declaring class - #993
Merged
Conversation
…ing class Closes #870. A private constructor is not inherited, so `new Child()` on a descendant of a class that declares one runs the ANCESTOR's constructor. php-src names that ancestor in the diagnostic — `Too few arguments to function Owner::__construct()` — and #796 made elephc's VISIBILITY error say so. The arity error still read `Constructor 'Child::__construct' expects 1 arguments, got 0`, pointing at a class whose source has no constructor to look at. `infer_new_object_type` already resolves the declaring class for the visibility check; it now computes that label once and uses it for the arity and named-argument diagnostics too. For a class that declares its own constructor the declaring class and the instantiated class coincide, so every other message is unchanged. Tests cover the two arity directions, an unknown named argument, a grandparent-declared constructor (mirroring the visibility half's existing chain test), and the two unchanged shapes: a class with its own constructor, and an inherited PUBLIC one. Wrong-arg-count on the resolved owner constructor was untested before. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
github-actions
Bot
added
area:types
Greptile SummaryUpdates constructor diagnostics to identify the class that declares an inherited constructor rather than the class being instantiated.
Confidence Score: 5/5The PR appears safe to merge with no outstanding findings. The earlier documentation concern was corrected and its thread was resolved; no subsequent changes introduced new issues, and the current implementation consistently labels inherited constructor diagnostics with the declaring class.
|
| Filename | Overview |
|---|---|
| src/types/checker/inference/objects/constructors.rs | Derives a shared constructor label from existing declaring-class metadata and uses it consistently across visibility, arity, and named-argument diagnostics. |
| tests/error_tests/classes_traits.rs | Adds focused regression tests covering inherited constructor diagnostics and unchanged own-constructor behavior. |
Reviews (3): Last reviewed commit: "docs(checker): say that the name moves f..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Review follow-up on #993. The comment claimed "every other diagnostic is unchanged", which understates the change: an inherited PUBLIC or PROTECTED constructor IS in the descendant's own method map, so the owner walk stops at the descendant — but `method_declaring_classes` still points at the ancestor that wrote it, and the label therefore moves for those too. Measured on `class Base { public function __construct(int $n) {} } class Sub extends Base {} new Sub();`: before the fix `Constructor 'Sub::__construct'`, after it `Constructor 'Base::__construct'`. php-src names `Base` as well, so the new message is the correct one — but a future maintainer reading the old comment would not have expected the public path to move at all. Only a class that declares its OWN constructor is unchanged, because there the two names are the same class. The public-inheritance test's docblock is corrected the same way: it was labelled "already correct and stays so", when it is in fact one of the messages this fix changes. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Guikingone
added a commit
that referenced
this pull request
Sep 13, 2026
Review follow-up on #993. The comment claimed "every other diagnostic is unchanged", which understates the change: an inherited PUBLIC or PROTECTED constructor IS in the descendant's own method map, so the owner walk stops at the descendant — but `method_declaring_classes` still points at the ancestor that wrote it, and the label therefore moves for those too. Measured on `class Base { public function __construct(int $n) {} } class Sub extends Base {} new Sub();`: before the fix `Constructor 'Sub::__construct'`, after it `Constructor 'Base::__construct'`. php-src names `Base` as well, so the new message is the correct one — but a future maintainer reading the old comment would not have expected the public path to move at all. Only a class that declares its OWN constructor is unchanged, because there the two names are the same class. The public-inheritance test's docblock is corrected the same way: it was labelled "already correct and stays so", when it is in fact one of the messages this fix changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Guikingone
force-pushed
the
fix/870-inherited-ctor-arity-owner
branch
from
September 13, 2026 19:39
691704d to
99249af
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
2 participants
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 #870.
What was wrong
A private constructor is not inherited, so
new Child()on a descendant of a class that declares one runs the ancestor's constructor. php-src names that ancestor in the diagnostic:#796 made elephc's visibility error say so (
Cannot access private constructor: Owner::__construct). The arity error still read:— pointing at a class whose source has no constructor to look at.
Fix
infer_new_object_typealready resolves the declaring class for the visibility check. It now computes that label once and uses it for the arity and named-argument diagnostics too.For a class that declares its own constructor the declaring class and the instantiated class coincide, so every other message is unchanged. The reflection-owner constructor path (
ReflectionClassand friends) is untouched — those are builtins that always declare their own.Tests
The issue notes wrong-arg-count on the resolved owner constructor was untested. Six tests in
tests/error_tests/classes_traits.rs, next to the existing visibility ones:Ownernormalize_named_call_args--test error_tests(1524) and--test codegen_tests oop(805) pass;cargo buildis warning-free.🤖 Generated with Claude Code
https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr