Skip to content

fix(checker): a covariant return may name a class the build walk has not reached yet - #2296

Draft
phcorp wants to merge 2 commits into
illegalstudio:mainfrom
phcorp:fix/covariant-return-to-unbuilt-subclass
Draft

phcorp wants to merge 2 commits into
illegalstudio:mainfrom
phcorp:fix/covariant-return-to-unbuilt-subclass

Conversation

@phcorp

@phcorp phcorp commented Nov 8, 2026

Copy link
Copy Markdown

Override covariance is validated during the class-build walk, which visits classes in name order (for reproducible class ids). When the override's return type names a class that comes later in that order, that class has no ClassInfo yet. is_subclass_of / class_implements_interface then answer "unrelated", and valid PHP is rejected:

class Scalar {}
class EnumNode extends Scalar {}
class ScalarDef { protected function make(): Scalar { return new Scalar(); } }
class EnumDef extends ScalarDef { protected function make(): EnumNode { return new EnumNode(); } }
// error: Cannot override method EnumDef::make with incompatible return type EnumNode (parent returns Scalar)

Renaming EnumDef so it sorts after EnumNode makes the error disappear. symfony/config's EnumNodeDefinition::instantiateNode(): EnumNode hits exactly this.

pending_class_parents already bridges this gap for the class being built. This PR extends the same idea to classes that are only declared so far:

  • The table: Checker::declared_class_ancestry holds each class's declared parent and implemented interfaces, taken from the flattened declarations. It is filled right before the class walk and cleared after it, so afterwards classes alone answers as before.
  • Subclass check: is_subclass_of resolves parents through parent_of, which looks at the built ClassInfo, then pending_class_parents, then the declaration.
  • Interface check: class_implements_interface answers for a class that is not built yet from its declared interfaces and those of its ancestors, including interface inheritance (interfaces are built before classes).

Tests:

  • test_covariant_return_to_a_class_built_later (tests/codegen/oop/inheritance.rs) covers a subclass of the parent's return class and a class implementing the parent's return interface through an ancestor, both declared after the overriding class. It prints N\ZEnumNode|N\ZEnumNode|N\ZEnumNode, as php 8.5 does, and fails on main.
  • test_error_override_return_to_unrelated_later_class (tests/error_tests/type_system.rs): an unrelated later class is still rejected.

Runs:

  • ELEPHC_PHP_CHECK=1 cargo test --test codegen_tests inheritance: 33 passed, 1 failed. The failure is dom_html_review_generic_inheritance_injects_prelude, a local link error: the _elephc_libxml2_v1_* symbols are missing because the libxml2 native dependency isn't built here.
  • cargo test --test codegen_tests interface: 217 passed.
  • cargo test --test error_tests return: 68 passed.
  • cargo test --test error_tests override: 25 passed.

@github-actions github-actions Bot added area:types Touches type checking, inference, or compatibility. size:s Small pull request. type:fix Corrects broken or incompatible behavior. labels Nov 8, 2026
@phcorp

phcorp commented Nov 8, 2026

Copy link
Copy Markdown
Author

Extended to enums: they are built after every class, so a class method returning an enum that implements the parent's return interface hit the same order dependence (tempest/highlight's Pattern::getTokenType(): TokenType implemented as : TokenTypeEnum). The declared ancestry now includes enum declarations; new test test_covariant_return_to_an_enum_implementing_the_interface (fails without it, prints keyword|keyword like php 8.5). ELEPHC_PHP_CHECK=1 cargo test --test codegen_tests enum: all green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:types Touches type checking, inference, or compatibility. size:s Small pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant