Skip to content

fix(preprocessor): reject abstract method bodies and abstract private class methods - #62

Merged
matyhtf merged 2 commits into
swoole:masterfrom
AlessioGiacobbe:split/abstract-declaration-rules
Sep 2, 2026
Merged

fix(preprocessor): reject abstract method bodies and abstract private class methods#62
matyhtf merged 2 commits into
swoole:masterfrom
AlessioGiacobbe:split/abstract-declaration-rules

Conversation

@AlessioGiacobbe

Copy link
Copy Markdown
Contributor

Abstract-method declaration rules were unenforced: an abstract method WITH a body compiled (the body silently dropped — Zend: "Abstract function A::f() cannot contain body", also fatal in traits, probed), and abstract private function in a class compiled (Zend fatal; legal in traits since PHP 8.0 — the trait exemption is preserved and tested).

Zend's diagnostic precedence (private before body) probed and matched.

Part of the split of #39.

… class methods

Two abstract-method rules Zend enforces at compile time were missing:

- an abstract method with a body was accepted and the body silently
  dropped; Zend fatals with "Abstract function A::f() cannot contain
  body" (applies to classes and traits alike, probed on 8.4.13)
- `abstract private function` in a class can never be implemented,
  since private methods do not participate in overriding; Zend fatals
  with "Abstract function A::f() cannot be declared private". Traits
  keep accepting it (allowed since PHP 8.0: the consuming class
  supplies the private implementation)

Zend reports the private-modifier error before the body error when both
apply; the checks are ordered to match.

@matyhtf matyhtf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The declaration checks match Zend precedence for class and trait abstract methods. Focused tests pass (3/3). No blocking issue found.

@matyhtf
matyhtf merged commit 58c3bb6 into swoole:master Sep 2, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants