Re: [PHP-DEV] Unified ReflectionType methods

2021-10-04 Thread Nikita Popov
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

2021-10-03 Thread Larry Garfield
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

2021-10-03 Thread G. P. B.
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

2021-10-02 Thread Andreas Hennings
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

2021-10-02 Thread Andreas Hennings
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

2021-10-02 Thread Andreas Hennings
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

2021-10-02 Thread Sebastian Bergmann

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

2021-10-02 Thread tyson andre
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

2021-10-01 Thread Andreas Hennings
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