Both sides previous revisionPrevious revisionNext revision | Previous revisionNext revisionBoth sides next revision |
rfc:magic-methods-signature [2020/04/15 23:16] – Hide link carusogabriel | rfc:magic-methods-signature [2020/05/23 04:01] – Convert mixed annotations to actual types carusogabriel |
---|
====== PHP RFC: Ensure correct magic methods' signatures when typed ====== | ====== PHP RFC: Ensure correct signatures of magic methods ====== |
* Version: 1.0 | * Version: 1.0 |
* Date: 2020-04-05 | * Date: 2020-04-05 |
===== Introduction ===== | ===== Introduction ===== |
| |
This RFC is inspired by [[https://bugs.php.net/69718|a bug report]] that lists magic methods in PHP that do not have any checks at all to their signature when typed. | It is currently possible to write magic methods that have signatures that don't match the signature expected, such as //%%__%%clone(): float// or //%%__%%isset(): Closure//. |
| |
However, this implementation is not as simple as just check for the methods' signatures, as this is a Backward Incompatible Change for those who have incorrect signatures over their codebases. | This behavior of allowing incorrect signatures was reported as [[https://bugs.php.net/69718|a bug]]. |
| |
Nowadays, PHP already checks the signature for the following methods: | As ensuring magic methods have correct signatures is a backward compatible break for code that currently has incorrect signatures, this change would only be appropriate in a major PHP release. |
| |
* clone return type: https://3v4l.org/Ub54p | |
* construct return type: https://3v4l.org/CCL11 | |
* destruct return type: https://3v4l.org/HNkgW | |
| |
===== Motivation ===== | ===== Motivation ===== |
| |
Since the introduction of types in PHP 7.0, only the 3 checks listed above were introduced to make sure that developers are using PHP's magic methods correctly. | PHP's Magic Methods is something that PHP provides allowing developers to track and act on specific changes of behavior of a certain class. Given that fact, the same should ensure that the end-users are using these methods consistently across different codebases. |
| |
For PHP 8, this RFC aims to expand these checks, making their usage standard across different projects and applications. | Since the introduction of types in PHP 7.0, only the checks list below were introduced to make sure that developers are using PHP's magic methods correctly: |
| |
===== Proposal ===== | * //%%__%%clone()// return type: https://3v4l.org/Ub54p |
| * //%%__%%construct()// return type: https://3v4l.org/CCL11 |
| * //%%__%%destruct()// return type: https://3v4l.org/HNkgW |
| * //%%__%%toString()// return type: https://3v4l.org/jIg7b/rfc#git-php-master |
| |
This RFC proposes to introduce the following signatures checks when magic methods are typed. | For PHP 8, this RFC aims to expand these checks. |
| |
**Important:** only, and only if, any of the listed magic methods use type hints and/or return types, these checks will be performed. In case they don't have types declared, nothing specific will happen. | ===== Proposal ===== |
| |
| This RFC proposes to add parameter and return types checks per the following details. Other magic methods will be not modified. |
| |
The proposal follows the [[https://php.net/oop5.magic|Magic Methods documentation]]. | The proposal follows the [[https://php.net/oop5.magic|Magic Methods documentation]]. |
| |
<code php> | <code php> |
/** @return mixed */ | Foo::__call(string $name, array $arguments): mixed; |
Foo::__call(string $name, array $arguments); | |
| |
/** @return mixed */ | Foo::__callStatic(string $name, array $arguments): mixed; |
Foo::__callStatic(string $name, array $arguments); | |
| |
Foo::__clone(): void; | Foo::__clone(): void; |
| |
/** @return mixed */ | Foo::__debugInfo(): ?array; |
Foo::__get(string $name); | |
| Foo::__get(string $name): mixed; |
| |
Foo::__isset(string $name): bool; | Foo::__isset(string $name): bool; |
| |
/** @param mixed $value */ | Foo::__serialize(): array; |
Foo::__set(string $name, $value): void; | |
| Foo::__set(string $name, mixed $value): void; |
| |
| Foo::__set_state(array $properties): object; |
| |
| Foo::__sleep(): array; |
| |
| Foo::__unserialize(array $data): void; |
| |
Foo::__unset(string $name): void; | Foo::__unset(string $name): void; |
| |
| Foo::__wakeup(): void; |
</code> | </code> |
| |
**Note:** The //%%__%%construct// and //%%__%%destruct// methods won't suffer any change. They won't allow //void// as a return type given the fact that (almost) no language, including PHP, has the concept of Constructors and Destructors "returning" something after their execution. | **Note:** The //%%__%%construct()// and //%%__%%destruct()// methods won't suffer any changes. They won't allow //void// as a return type given the fact that (almost) all languages, including PHP, don't have the concept of Constructors and Destructors "returning" something after their execution. |
| |
===== Backward Incompatible Changes ===== | ===== Backward Incompatible Changes ===== |
| |
An important note here is that if one of the listed magic methods is directly called, and it has the wrong signatures, the error will be thrown regardless. | |
| |
For example https://3v4l.org/CuTNm, after this RFC, the errors about incorrect signatures will be thrown. | |
| |
==== To Magic Methods without types declared ==== | ==== To Magic Methods without types declared ==== |
===== RFC Impact ===== | ===== RFC Impact ===== |
| |
Scraping the top 1000 Composer packages (using Nikita's [[https://gist.github.com/nikic/a2bfa3e2f604f66115c3e4b8963a6c72|script]]), [[https://gist.github.com/carusogabriel/e0b36e7cd9e6846e04f79008cb7e35d6|the results show]] only 7 occurrences, which has shown that none they are a problem, as //%%__%%call//, //%%__%%callStatic// and //%%__%%get// do not have checks at this time. | Scraping the top 1000 Composer packages (using Nikita's [[https://gist.github.com/nikic/a2bfa3e2f604f66115c3e4b8963a6c72|script]]), [[https://gist.github.com/carusogabriel/e0b36e7cd9e6846e04f79008cb7e35d6|the results]] show only 7 occurrences of not matching signatures. |
| |
| Luckily, none of them is a problem as //%%__%%call()//, //%%__%%callStatic()// and //%%__%%get()// do not have checks at this time. |
| |
Even with a //mixed// RFC that wouldn't be a problem, as [[https://github.com/php/php-src/blob/ad7e93a023a9/Zend/tests/type_declarations/mixed/inheritance/mixed_return_inheritance_success2.phpt|a specific type can override it]], by the Liskov Substitution Principle. | Even with a //mixed// RFC that wouldn't be a problem, as [[https://github.com/php/php-src/blob/ad7e93a023a9/Zend/tests/type_declarations/mixed/inheritance/mixed_return_inheritance_success2.phpt|a specific type can override it]], by the Liskov Substitution Principle. |
===== Future Scope ===== | ===== Future Scope ===== |
| |
This RFC only aims to add checks for the methods' signatures but as a Future Scope, a runtime check of what is been returning in the methods could be added, same as //%%__%%serialize// (https://3v4l.org/HLiTj) and //%%__%%toString// (https://3v4l.org/Dbe6G). | This RFC only aims to add checks for the methods' signatures but as a Future Scope, a runtime check of what is been returning in the methods could be added, same as |
| |
| * //%%__%%serialize()//: https://3v4l.org/HLiTj |
| * //%%__%%toString()//: https://3v4l.org/Dbe6G |
| * //%%__%%debugInfo()//: https://3v4l.org/0EEPh |
| * //%%__%%sleep()//: https://3v4l.org/dH96A |
| |
===== Proposed Voting Choices ===== | ===== Proposed Voting Choices ===== |
| |
Yes/No. | Yes/No. |
| |
| ===== External resources ===== |
| |
| - Discussion thread: https://externals.io/message/109542 |
| |