Skip to content

fix: enforce interface declaration and merge rules - #59

Open
AlessioGiacobbe wants to merge 3 commits into
swoole:masterfrom
AlessioGiacobbe:split/interface-declaration-rules
Open

fix: enforce interface declaration and merge rules#59
AlessioGiacobbe wants to merge 3 commits into
swoole:masterfrom
AlessioGiacobbe:split/interface-declaration-rules

Conversation

@AlessioGiacobbe

Copy link
Copy Markdown
Contributor

Interface declaration and merge rules were unenforced:

  • Interface methods with bodies compiled (the body silently became dead code); final, private/protected, and explicit abstract modifiers were accepted; non-public interface constants and explicitly-abstract hooked properties too. All are Zend compile fatals (each probed for the exact rule and message).
  • interface I extends SomeClass either fataled with a misleading missing-symbol message or compiled when the class was declared later; both cases now fail with Zend's "cannot implement … it is not an interface".
  • Same-name methods arriving from several extended interfaces — or from several interfaces a class implements without defining the method — were never cross-checked: interface J extends I1, I2 compiled with mutually incompatible f() declarations. The first-seen declaration is now validated as an override of every later one, matching Zend's merge order; diamond inheritance of one original declaration never conflicts, and a method the class chain defines silences the pairwise check (Zend-probed: a never return satisfying both incompatible declarations compiles).

Part of the split of #39.

parseInterface accepted several declarations Zend rejects at compile
time (all wordings probed on 8.4.13, which renamed the modifier errors
to "must not be abstract/final"):

- interface method with a body ("Interface function I::f() cannot
  contain body")
- private/protected interface method ("Access type for interface
  method I::f() must be public")
- explicit `abstract` modifier on an interface method ("Interface
  method I::f() must not be abstract")
- `final` interface method ("Interface method I::f() must not be
  final")
- private/protected interface constant ("Access type for interface
  constant I::X must be public"); `final` interface constants remain
  legal per PHP 8.1
- explicit `abstract` on an interface hooked property ("Property in
  interface cannot be explicitly abstract...")
- `interface I extends A` where A is a known class, enum, or trait
  ("I cannot implement A - it is not an interface"); only checked when
  A's declaration has already been prepared - a parent declared later
  is left to the Translator (deferred to integrator)
- the same interface listed twice in extends ("Interface I cannot
  implement previously implemented interface A")

Zend's precedence for combined modifier violations (visibility, then
abstract, then final, then body) is preserved.
Two interfaces declaring the same method were never cross-checked:
`interface J extends I1, I2` and a class implementing both compiled
even when the declarations were mutually incompatible (Zend:
"Declaration of I1::f(): int must be compatible with I2::f(): string").
The first-seen declaration is now validated as an override of every
later one, mirroring Zend's merge order; diamond inheritance of one
original declaration never conflicts, and a method the class chain
defines silences the pairwise check (it is validated against each
interface individually instead) — all probed against Zend 8.4.
…ed methods

An interface can only extend other interfaces: naming a class either
fataled with a misleading missing-symbol message (declaration seen
earlier) or compiled silently (declaration appearing later). The
translation phase now rejects both with Zend's wording.

Same-name methods arriving from several extended interfaces (or from
several interfaces a class implements without defining the method) were
never cross-checked; the first-seen declaration is now validated as an
override of every later one, matching Zend's merge order, with diamond
inheritance of one original declaration exempt.
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.

1 participant