Skip to content

fix(preprocessor): validate compound type declarations and class-scope type keywords - #65

Open
AlessioGiacobbe wants to merge 4 commits into
swoole:masterfrom
AlessioGiacobbe:split/compound-type-declarations
Open

fix(preprocessor): validate compound type declarations and class-scope type keywords#65
AlessioGiacobbe wants to merge 4 commits into
swoole:masterfrom
AlessioGiacobbe:split/compound-type-declarations

Conversation

@AlessioGiacobbe

Copy link
Copy Markdown
Contributor

Compound type declarations were not validated for well-formedness — all of these compiled, each a Zend compile fatal (probed individually): duplicate union members after alias/namespace resolution (int|string|int, Foo|\Foo, iterable expanded so iterable|array names the overlapping component); bool/true/false overlaps (bool|false, true|false → "bool must be used instead"); mixed/void/never inside unions; ?mixed, ?null, ?void, ?never; non-class standard types and duplicates inside intersections; self/static return types outside class scope (closures exempt, as Zend compiles them); duplicate implements entries for classes and enums.

One validation helper runs from a resolveTypeDecl override, so parameters, returns, properties, constants, and closures share the same pass without double-firing. use T, T is deliberately still accepted — Zend silently dedupes trait use (probed).

Part of the split of #39.

@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 focused tests pass (14/14), but three Zend type rules still compile successfully:

  1. object absorbs every class type:
class Foo {}
function f(object|Foo $value): void {}

Zend: Type Foo|object contains both object and a class type, which is redundant. Track object versus resolved class members in both member orders.

  1. Whole DNF groups cannot be skipped:
interface A {}
interface B {}
function f((A&B)|(B&A) $value): void {}

Zend rejects the second intersection as redundant. It also rejects (A&B)|A because the intersection is more restrictive than A. Canonicalize intersection member sets (order-insensitive) and check exact/restrictive DNF redundancy.

  1. Class-scope keywords are only checked for a bare return node. They also occur in parameters and nested union/DNF returns:
function f(self $value): void {}
function g(): self|stdClass {}

Both must fail with Cannot use "self" when no class scope is active; this PR compiles them. Validate self/static recursively for every declaration context. static must additionally remain return-only.

Please add negative tests for these forms. The current comment explicitly saying whole-DNF redundancy is not checked documents a known PHP incompatibility, so this is not ready to merge as the compound-type validator.

@AlessioGiacobbe
AlessioGiacobbe force-pushed the split/compound-type-declarations branch from 7a50c2d to 5caac2f Compare September 2, 2026 08:14
@AlessioGiacobbe

Copy link
Copy Markdown
Contributor Author

All three rule sets implemented, rebased on current master (~40 fresh Zend 8.4.13 probes drove the exact messages and orderings):

  1. object + class-type redundancy — fires in both member orders, with Zend's type rendering (class types in source order, builtins in Zend's canonical order); iterable|object and null|object stay legal as in Zend.
  2. Whole-DNF redundancy — intersection member sets canonicalized order-insensitively: equal sets reject (Type `B&A` is redundant with type `A&B` ), and a strict superset rejects as more restrictive in both orders ((A&B)|A, A|(A&B), (A&B)|(A&B&C)); (A&B)|(A&C) stays legal. The stale "not checked" comment is gone.
  3. Class-scope keywords everywhereself/parent validated recursively through nullable/union/intersection/DNF in parameters, returns, properties, and class/interface constants; closures stay exempt (Zend allows self there — verified). Two probe findings worth noting: static in a parameter or property type is a parse error in both Zend and php-parser (it can never reach the compiler), and Zend accepts static in class-constant types — so the only reachable static check is the no-class-scope return case, which was already exact. Documented in the commit body.

17 new fixtures (every negative one executed under real Zend and producing exactly the asserted error), 31/31 focused tests green, full suite and sweep identical to base.

@AlessioGiacobbe
AlessioGiacobbe force-pushed the split/compound-type-declarations branch from 5caac2f to 2b08773 Compare September 2, 2026 09:20

@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.

Please rebase this branch onto the latest master; #45, #52, #59, #63, and #66 have now been merged.

The handling of self/parent/static inside intersections is not correct for DNF types. validateIntersectionTypeDecl() skips these names on the assumption that buildTypeCheckFromNode() rejects them later, but that rejection only runs when the top-level node is an IntersectionType; a DNF intersection nested inside a UnionType goes through buildTypeCheckClause() and is accepted.

For example, inside a class this branch accepts:

class C {
    public function f((self&Countable)|stdClass $value): void {}
}

Zend rejects it with Type self cannot be part of an intersection type. The same needs coverage for parent in a class that actually has a parent. Note the PHP distinction: a bare self&Countable intersection is accepted, while self inside a DNF intersection is rejected, so this cannot be implemented as an unconditional ban in every intersection.

Please validate this at the compound-type layer with the necessary top-level/DNF context and add both positive bare-intersection and negative DNF tests.

…e type keywords

resolveTypeDecl now runs a shared well-formedness pass before resolving,
so parameters, returns, properties, class/interface constants, and
closure signatures all obey Zend's compile-time compound-type rules
(each probed on 8.4.13):

- duplicate union members, case-insensitive and after alias/namespace
  resolution ("Duplicate type int is redundant", "Duplicate type
  App\Sub\Thing is redundant"); iterable is expanded to
  array|Traversable first, so iterable|array and iterable|\Traversable
  report the overlapping component exactly like Zend, while a
  namespace-local Traversable stays legal
- bool with false/true names the literal as the duplicate in either
  order; true|false demands bool ("Type contains both true and false,
  bool must be used instead")
- mixed/void/never inside a union ("... can only be used as a
  standalone type"), ?mixed ("Type mixed cannot be marked as nullable
  since mixed already includes null"), ?null, ?void, ?never
- intersection members must be class types ("Type int cannot be part
  of an intersection type"); duplicate intersection members are
  redundant; self/parent/static keep the established TypeCheckGenerator
  diagnostic; redundancy between whole DNF groups is not checked (Zend
  uses a distinct "Type X&Y is redundant with type X&Y" pass)
- self/static return types on free functions ("Cannot use \"static\"
  when no class scope is active"); closures keep accepting them since
  they may be bound to a scope later, matching Zend
- duplicate interfaces in an implements list, for classes and enums
  ("Class A cannot implement previously implemented interface I");
  duplicate trait use stays legal - Zend deduplicates it silently
…word rules

Three Zend compile-time type rules were still accepted, all probed
against PHP 8.4.13:

- object absorbing class types: a union naming object beside any class
  type (a class, interface, or enum name, self/parent/static, or a DNF
  group) is rejected in either member order with Zend's message and
  type rendering — class types first in source order, then the
  standard types in Zend's canonical order, e.g.
  "Type Foo|object|null contains both object and a class type, which
  is redundant". iterable beside object stays legal, matching Zend.

- whole-DNF redundancy: intersection groups and plain class members are
  compared as canonical, order-insensitive member sets. An equal set is
  "Type B&A is redundant with type A&B"; a strict superset is
  rejected as more restrictive, in both orders: (A&B)|A, A|(A&B), and
  (A&B)|(A&B&C2) all fail like Zend. The stale comment claiming
  whole-DNF redundancy is not checked is gone.

- class-scope type keywords: self/parent/static are validated
  recursively through nullable, union, intersection, and DNF nodes in
  parameters, returns, properties, and class or interface constants.
  A free function has no class scope (Zend errors no matter where it
  is declared), while closures keep their runtime binding and stay
  exempt. parent additionally requires the scope to have a parent
  class ("Cannot use \"parent\" when current class scope has no
  parent"), with traits exempt because parent stays late-bound until
  the consuming class is known. static outside a return type never
  reaches the compiler: PHP's grammar rejects it in parameter and
  property types, and Zend accepts it in class-constant types, which
  always have a class scope.
…e compound layer

validateIntersectionTypeDecl skipped self/parent/static on the
assumption that buildTypeCheckFromNode rejects them later, but that
rejection only runs when the top-level node is an IntersectionType: a
DNF group nested inside a union goes through buildTypeCheckClause,
which flattens the intersection and accepted the keyword as a
late-bound class type. Inside a class,
`(self&Countable)|stdClass $value` compiled while Zend fatals.

Probed against PHP 8.4.13: a class-scope keyword can never be part of
an intersection, bare or as a DNF member, in any declaration context.
A bare `self&Ix` parameter, a `(self&Ix)|Other` parameter, promoted
parameter, or property, a `(parent&Ix)|Other` parameter or class
constant in a class with a parent, and `static&Ix` or
`(static&Ix)|Other` return types all fail with
"Type self cannot be part of an intersection type" in the matching
spelling. The scope errors keep their Zend precedence: with no class
scope, or no parent class, the "Cannot use ..." fatals from
validateClassScopeTypeKeywords fire first, exactly as Zend orders
them. A keyword as a plain union member beside a DNF group, e.g.
`(Ia&Ib)|self`, stays legal.

The compound layer now rejects the keyword directly, so bare and DNF
shapes report the same text; the buildTypeCheckFromNode backstop and
the ClassTest expectations adopt the same backtick rendering.
@AlessioGiacobbe
AlessioGiacobbe force-pushed the split/compound-type-declarations branch from 2b08773 to 626a289 Compare September 2, 2026 13:18
@AlessioGiacobbe

Copy link
Copy Markdown
Contributor Author

Rebased onto current master and fixed at the compound layer — with one finding from probing PHP 8.4.13 directly: Zend rejects class-scope keywords in every intersection, bare ones included, so the bare/DNF distinction described in the review does not hold on 8.4:

class C { public function f(self&Countable $v): void {} }
// Fatal error: Type self cannot be part of an intersection type

The shape Zend does accept is the keyword as a plain union member beside a DNF group — (Ia&Ib)|self, (Ia&Ib)|parent, (Ia&Ib)|static all compile. parent/static behave the same way in their intersections, and the scope-precedence messages win where applicable (Cannot use "self" when no class scope is active, Cannot use "parent" when current class scope has no parent).

Implementation follows the probed behavior: validateIntersectionTypeDecl() now rejects self/parent/static unconditionally with Zend's message (no context flag needed), the backstop in buildTypeCheckFromNode() reports identical text, and scope-precedence is preserved because validateClassScopeTypeKeywords() runs before the compound validator in every declaration context.

Tests: negatives for bare and DNF intersections across parameters, returns, properties, constants and promoted parameters (self, parent with a real parent, static in returns); positive coverage that (Ia&Ib)|self|parent|static union members still compile. Every fixture was first executed under real PHP 8.4.13. If 8.5 relaxes bare intersections here I'm happy to gate it, but 8.4 is unambiguous.

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