Re: [PHP-DEV] Unified ReflectionType methods
On Sun, Oct 3, 2021 at 3:42 PM G. P. B. wrote: > On Sat, 2 Oct 2021 at 00:54, Andreas Hennings wrote: > > > Hello list, > > I would like to propose new methods for ReflectionType, that would > > allow treating ReflectionNamedType and ReflectionUnionType in a > > unified way. > > This would eliminate the need for if (.. instanceof) in many use cases. > > > > Some details can still be discussed, e.g. whether 'null' should be > > included in builtin type names, whether there should be a canonical > > ordering of type names, whether we should use class names as array > > keys, etc. > > > > [...] > > > > What do you think? > > > > -- Andreas > > > I don't think this is a good idea, at least in a form like this. > I understand that the ReflectionType API is cumbersome but I don't think > merging everything together makes a lot of sense. > Want does need to know if one is dealing with a Union, Intersection, or > single type and conflating the APIs is IMHO a bad idea as it doesn't > consider future extensions. > And I am not talking about more complex composite types such as > (A&B)|C|(X&Y), as those will be handled within the existing design, but > potential additions to the type system like function types, literal types, > generics, etc. > > I also don't really understand what the argument for removing instanceof > checks is other than more generic code which works, but if this breaks at > the next type system addition, this seems rather pointless. > Agree, I don't think this suggestion makes sense. The current ReflectionType hierarchy is specifically designed so you have to do those instanceof checks and deal with the different kinds of types we support. The ReflectionType hierarchy isn't complex for fun, it directly reflects the complexity of the type system. As the type system becomes more complicated, there will be more kinds of types that are exposed through reflection and need to be handled appropriately. I am fine with additions as Tyson suggests them: Methods that work across *all* ReflectionTypes. The proposed methods do not fall in this category, as they don't hold up for intersection types in PHP 8.1 already, let alone future type system additions. Regards, Nikita
Re: [PHP-DEV] Unified ReflectionType methods
On Sat, Oct 2, 2021, at 4:37 PM, tyson andre wrote: > Hi Andreas, > >> Hello list, >> I would like to propose new methods for ReflectionType, that would >> allow treating ReflectionNamedType and ReflectionUnionType in a >> unified way. >> This would eliminate the need for if (.. instanceof) in many use cases. >> >> Some details can still be discussed, e.g. whether 'null' should be >> included in builtin type names, whether there should be a canonical >> ordering of type names, whether we should use class names as array >> keys, etc. >> ... >> What do you think? > > Relatedly, I also had different ideas lately about new methods for > ReflectionType, though of a different form. > > 1. To simplify code that would check `instanceof` for all current and > future types such as `never` and `mixed` and intersection types > `ReflectionType->allowsValue(mixed $value, bool $strict = true): > bool` > >Maybe also `allowsClass(string $className, bool $strict = true): > bool` to avoid needing to instantiate values (weak casting allows > Stringable->string). > 2. To simplify code generation, e.g. in mocking libraries for unit > testing: `ReflectionType->toFullyQualifiedString(): string` (e.g. > `\A|\B`) (may need to throw ReflectionType for types that can't be > resolved, e.g. `parent` in reflection of traits, keep `static` as is) > > (The raw output of `__toString()` isn't prefixed with `\` (e.g. > `A&B`) and can't be used in namespaces > > The fact that both intersection and union types (and possibility of > union types of full intersection types) > make it hard for me to believe that getBuiltinTypes and > getBuiltinClasses would be used correctly when used. > > Thanks, > Tyson I think at this point I prefer this approach, for the reasons George mentioned. "Here's a value, is it going to work if I pass it to this type check?" is a nice and generic operation that would extend gracefully as the type system expands. It also dovetails with the pattern matching that Ilija and I had been working on, although that's now stalled as he is unavailable for personal reasons. That is also largely a "here's a type def, does this value match it?" check, but with a few extra bits. (If someone wants to take over the code side of that, please let me know. It's over my head to implement but we have a fairly-complete design and the code is partially done.) --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Unified ReflectionType methods
On Sat, 2 Oct 2021 at 00:54, Andreas Hennings wrote: > Hello list, > I would like to propose new methods for ReflectionType, that would > allow treating ReflectionNamedType and ReflectionUnionType in a > unified way. > This would eliminate the need for if (.. instanceof) in many use cases. > > Some details can still be discussed, e.g. whether 'null' should be > included in builtin type names, whether there should be a canonical > ordering of type names, whether we should use class names as array > keys, etc. > > [...] > > What do you think? > > -- Andreas I don't think this is a good idea, at least in a form like this. I understand that the ReflectionType API is cumbersome but I don't think merging everything together makes a lot of sense. Want does need to know if one is dealing with a Union, Intersection, or single type and conflating the APIs is IMHO a bad idea as it doesn't consider future extensions. And I am not talking about more complex composite types such as (A&B)|C|(X&Y), as those will be handled within the existing design, but potential additions to the type system like function types, literal types, generics, etc. I also don't really understand what the argument for removing instanceof checks is other than more generic code which works, but if this breaks at the next type system addition, this seems rather pointless. Best regards, George P. Banyard
Re: [PHP-DEV] Unified ReflectionType methods
Another challenge would be array types like array or C[], or array{i: int, s: string}. And template/generic types like \Iterator. Following the normalization idea, we could add a method getArrayTypes(). But I am not sure how far we get with this, I think there will be a wall somewhere. On Sun, 3 Oct 2021 at 08:18, Andreas Hennings wrote: > > On Sat, 2 Oct 2021 at 16:37, tyson andre wrote: > > > > Hi Andreas, > > > > > Hello list, > > > I would like to propose new methods for ReflectionType, that would > > > allow treating ReflectionNamedType and ReflectionUnionType in a > > > unified way. > > > This would eliminate the need for if (.. instanceof) in many use cases. > > > > > > Some details can still be discussed, e.g. whether 'null' should be > > > included in builtin type names, whether there should be a canonical > > > ordering of type names, whether we should use class names as array > > > keys, etc. > > > ... > > > What do you think? > > > > Relatedly, I also had different ideas lately about new methods for > > ReflectionType, though of a different form. > > > > 1. To simplify code that would check `instanceof` for all current and > > future types such as `never` and `mixed` and intersection types > > `ReflectionType->allowsValue(mixed $value, bool $strict = true): bool` > > > >Maybe also `allowsClass(string $className, bool $strict = true): bool` > > to avoid needing to instantiate values (weak casting allows > > Stringable->string). > > Ok for me, why not. > I would see it as a distinct PR, separate from my original proposal. > The problem or limitation of these methods is that they don't reflect > the full expressiveness of the type system. > That is, the return values of these methods would not be sufficient to > recreate an equivalent type. > > > 2. To simplify code generation, e.g. in mocking libraries for unit testing: > > `ReflectionType->toFullyQualifiedString(): string` (e.g. `\A|\B`) (may need > > to throw ReflectionType for types that can't be resolved, e.g. `parent` in > > reflection of traits, keep `static` as is) > > > > (The raw output of `__toString()` isn't prefixed with `\` (e.g. `A&B`) > > and can't be used in namespaces > > Again, if we iron out the details, it can be a good idea as a distinct > standalone PR. > > > > > The fact that both intersection and union types (and possibility of union > > types of full intersection types) > > make it hard for me to believe that getBuiltinTypes and getBuiltinClasses > > would be used correctly when used. > > Just to mention it, my proposed names were getBuiltinNames() and > getClassNames() to be super clear that the return value will be > string[]. > > For future intersection types, there could be additional methods > getBuiltinIntersectionTypes() and getClassIntersectionTypes(), or it > would be a single method getIntersectionTypes(). Not sure yet which > combos make sense. E.g. array&callable would combine two builtin > types, Countable&callable would combine a class-like type and a > builtin type. > Every type would be broken into a normal form, where the top-level is > treated as a union type, and each item can either be a pure builtin > type, a pure class type, or an intersection of pure types. > > int|string|(A&(B|(C&D))) === int | string | (A&B) | (A&C&D) > > If we always use this normal form, then the ReflectionIntersectionType > only needs to have getBuiltinNames() and getClassNames(), not > getUnionTypes(). > > Not sure yet what to do with 'static' and '$this'. > > > > > > Thanks, > > Tyson > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: https://www.php.net/unsub.php > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Unified ReflectionType methods
On Sun, 3 Oct 2021 at 07:40, Sebastian Bergmann wrote: > > Am 02.10.2021 um 16:37 schrieb tyson andre: > > `ReflectionType->allowsValue(mixed $value, bool $strict = true): bool` > > Not having to implement and maintain that functionality in userland would > be brilliant and much appreciated. Right now we have > https://github.com/sebastianbergmann/type for this for use cases related > to test doubles in PHPUnit. Wow. this package looks powerful. But it also looks like it would require a lot of if/else, switch, or dynamic dispatch, when dealing with those types. E.g. if ($type->isUnion()) ... > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Unified ReflectionType methods
On Sat, 2 Oct 2021 at 16:37, tyson andre wrote: > > Hi Andreas, > > > Hello list, > > I would like to propose new methods for ReflectionType, that would > > allow treating ReflectionNamedType and ReflectionUnionType in a > > unified way. > > This would eliminate the need for if (.. instanceof) in many use cases. > > > > Some details can still be discussed, e.g. whether 'null' should be > > included in builtin type names, whether there should be a canonical > > ordering of type names, whether we should use class names as array > > keys, etc. > > ... > > What do you think? > > Relatedly, I also had different ideas lately about new methods for > ReflectionType, though of a different form. > > 1. To simplify code that would check `instanceof` for all current and future > types such as `never` and `mixed` and intersection types > `ReflectionType->allowsValue(mixed $value, bool $strict = true): bool` > >Maybe also `allowsClass(string $className, bool $strict = true): bool` to > avoid needing to instantiate values (weak casting allows Stringable->string). Ok for me, why not. I would see it as a distinct PR, separate from my original proposal. The problem or limitation of these methods is that they don't reflect the full expressiveness of the type system. That is, the return values of these methods would not be sufficient to recreate an equivalent type. > 2. To simplify code generation, e.g. in mocking libraries for unit testing: > `ReflectionType->toFullyQualifiedString(): string` (e.g. `\A|\B`) (may need > to throw ReflectionType for types that can't be resolved, e.g. `parent` in > reflection of traits, keep `static` as is) > > (The raw output of `__toString()` isn't prefixed with `\` (e.g. `A&B`) > and can't be used in namespaces Again, if we iron out the details, it can be a good idea as a distinct standalone PR. > > The fact that both intersection and union types (and possibility of union > types of full intersection types) > make it hard for me to believe that getBuiltinTypes and getBuiltinClasses > would be used correctly when used. Just to mention it, my proposed names were getBuiltinNames() and getClassNames() to be super clear that the return value will be string[]. For future intersection types, there could be additional methods getBuiltinIntersectionTypes() and getClassIntersectionTypes(), or it would be a single method getIntersectionTypes(). Not sure yet which combos make sense. E.g. array&callable would combine two builtin types, Countable&callable would combine a class-like type and a builtin type. Every type would be broken into a normal form, where the top-level is treated as a union type, and each item can either be a pure builtin type, a pure class type, or an intersection of pure types. int|string|(A&(B|(C&D))) === int | string | (A&B) | (A&C&D) If we always use this normal form, then the ReflectionIntersectionType only needs to have getBuiltinNames() and getClassNames(), not getUnionTypes(). Not sure yet what to do with 'static' and '$this'. > > Thanks, > Tyson > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Unified ReflectionType methods
Am 02.10.2021 um 16:37 schrieb tyson andre: `ReflectionType->allowsValue(mixed $value, bool $strict = true): bool` Not having to implement and maintain that functionality in userland would be brilliant and much appreciated. Right now we have https://github.com/sebastianbergmann/type for this for use cases related to test doubles in PHPUnit. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Unified ReflectionType methods
Hi Andreas, > Hello list, > I would like to propose new methods for ReflectionType, that would > allow treating ReflectionNamedType and ReflectionUnionType in a > unified way. > This would eliminate the need for if (.. instanceof) in many use cases. > > Some details can still be discussed, e.g. whether 'null' should be > included in builtin type names, whether there should be a canonical > ordering of type names, whether we should use class names as array > keys, etc. > ... > What do you think? Relatedly, I also had different ideas lately about new methods for ReflectionType, though of a different form. 1. To simplify code that would check `instanceof` for all current and future types such as `never` and `mixed` and intersection types `ReflectionType->allowsValue(mixed $value, bool $strict = true): bool` Maybe also `allowsClass(string $className, bool $strict = true): bool` to avoid needing to instantiate values (weak casting allows Stringable->string). 2. To simplify code generation, e.g. in mocking libraries for unit testing: `ReflectionType->toFullyQualifiedString(): string` (e.g. `\A|\B`) (may need to throw ReflectionType for types that can't be resolved, e.g. `parent` in reflection of traits, keep `static` as is) (The raw output of `__toString()` isn't prefixed with `\` (e.g. `A&B`) and can't be used in namespaces The fact that both intersection and union types (and possibility of union types of full intersection types) make it hard for me to believe that getBuiltinTypes and getBuiltinClasses would be used correctly when used. Thanks, Tyson -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Unified ReflectionType methods
Hello list, I would like to propose new methods for ReflectionType, that would allow treating ReflectionNamedType and ReflectionUnionType in a unified way. This would eliminate the need for if (.. instanceof) in many use cases. Some details can still be discussed, e.g. whether 'null' should be included in builtin type names, whether there should be a canonical ordering of type names, whether we should use class names as array keys, etc. abstract class ReflectionType { [..] // existing methods. /** * @return string[] */ public function getClassNames(): array; /** * @return \ReflectionClass[] */ public function getClasses(): array { $classes = []; foreach ($this->getClassNames() as $className) { $classes[] = new ReflectionClass($className); } return $classes; } /** * @return string[] */ public function getBuiltinNames(): array; } class ReflectionNamedType extends ReflectionType { [..] // existing methods. public function getClassNames(): array { return $this->isBuiltin() ? [] : $this->getName(); } public function getBuiltinNames(): array { $names = $this->isBuiltin() ? [] : $this->getName(); if ($this->allowsNull()) { $names[] = 'null'; } return $names; } } class ReflectionUnionType extends ReflectionType { [..] // existing methods. public function getClassNames(): array { $namess = []; foreach ($this->getTypes() as $type) { $namess[] = $type->getClassNames(); } return array_unique(array_merge(...$types)); } public function getBuiltinNames(): array { $namess = []; foreach ($this->getTypes() as $type) { $namess[] = $type->getBuiltinNames(); } return array_unique(array_merge(...$types)); } } What do you think? -- Andreas -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php