Skip to content

fix(preprocessor): enforce property-hook placement rules for class properties - #63

Open
AlessioGiacobbe wants to merge 2 commits into
swoole:masterfrom
AlessioGiacobbe:split/property-hook-rules
Open

fix(preprocessor): enforce property-hook placement rules for class properties#63
AlessioGiacobbe wants to merge 2 commits into
swoole:masterfrom
AlessioGiacobbe:split/property-hook-rules

Conversation

@AlessioGiacobbe

Copy link
Copy Markdown
Contributor

Property-hook placement rules were unenforced on the class path (the interface path already had them): hooks on static properties ("Cannot declare hooks for static property"), hooks on readonly properties incl. readonly classes ("Hooked properties cannot be readonly"), abstract hooked properties in non-abstract classes, abstract properties without hooks, abstract hooked properties with a default or with all hooks bodied, and bodiless hooks on non-abstract properties (previously the lowering silently fabricated a concrete accessor).

Each rule and its precedence (static → readonly → abstract) probed against Zend 8.4.13; traits keep abstract hooked properties.

Part of the split of #39.

…operties

The interface path already validated hook placement; class and trait
properties accepted every combination. parseClassPropertyDef now
mirrors Zend's compile-time rules (probed on 8.4.13, including the
precedence order static -> readonly -> abstract rules):

- hooks on a static property ("Cannot declare hooks for static
  property")
- hooks on a readonly property, including properties made readonly by
  a `readonly class` ("Hooked properties cannot be readonly")
- `abstract` on a hook-less property ("Only hooked properties may be
  declared abstract")
- abstract hooked property with a default value ("Cannot specify
  default value for virtual hooked property A::$x")
- abstract hooked property whose hooks all have bodies ("Abstract
  property A::$x must specify at least one abstract hook")
- abstract hooked property in a non-abstract class; traits stay exempt
  (the consuming class satisfies the hook) and enums are already
  rejected by the property ban
- bodiless hook on a non-abstract property, in classes and traits
  ("Non-abstract property hook must have a body"); previously the
  lowering fabricated a concrete backing-store accessor for it
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