Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [DISCUSSION] ReflectionType::accepts »

[DISCUSSION] ReflectionType::accepts

php.internals

Brent

5 years ago
Hi Internals Since the addition of union types, it has become a little more cumbersome to determine whether a given parameter type accepts specific input. Personally I've been reusing this code blob in several places because of it: ``` /** @var \ReflectionNamedType[] $types */ $types = match ($type::class) { ReflectionUnionType::class => $type->getTypes(), ReflectionNamedType::class => [$type], }; foreach ($types as $type) { if (! is_subclass_of($type->getName(), ShouldBeStored::class)) { continue; } // … } ``` I wonder whether we would discuss adding a method on ReflectionType that accepts any given input, and tells you whether that input is valid for that type or not. I was thinking about `ReflectionType::accepts(string|object $input): bool` but we could discuss another name. With it, the above example could be refactored like so: ``` if (! $type->accepts(ShouldBeStored::class)) { return; } // … ``` I believe this method should accept both class names and objects to be consistent with the existing reflection API. It would also work with intersection types, if they are to be added some day. What do you think? Kind regards Brent

Pierre

5 years ago
Le 29/01/2021 à 09:14, Brent Roose a écrit :
> Hi Internals > > Since the addition of union types, it has become a little more cumbersome to determine whether a given parameter type accepts specific input. Personally I've been reusing this code blob in several places because of it: > > ``` > /** @var \ReflectionNamedType[] $types */ > $types = match ($type::class) { > ReflectionUnionType::class => $type->getTypes(), > ReflectionNamedType::class => [$type], > }; > > foreach ($types as $type) { > if (! is_subclass_of($type->getName(), ShouldBeStored::class)) { > continue; > } > > // … > } > ``` > > I wonder whether we would discuss adding a method on ReflectionType that accepts any given input, and tells you whether that input is valid for that type or not. I was thinking about `ReflectionType::accepts(string|object $input): bool` but we could discuss another name. With it, the above example could be refactored like so: > > ``` > if (! $type->accepts(ShouldBeStored::class)) { > return; > } > > // … > ``` > > I believe this method should accept both class names and objects to be consistent with the existing reflection API. It would also work with intersection types, if they are to be added some day. > > What do you think? > > Kind regards > Brent
Hello, I think this would be a good addition. I personally had to write the exact same algorithm to solve this exact same problem yesterday night. Cheers,
-- Pierre

Sebastian Bergmann

5 years ago
Am 29.01.2021 um 09:18 schrieb Pierre R.:
> I think this would be a good addition. I personally had to write the exact > same algorithm to solve this exact same problem yesterday night.
https://github.com/sebastianbergmann/type is the implementation that PHPUnit uses.

Nikita Popov

5 years ago
On Fri, Jan 29, 2021 at 9:15 AM Brent Roose <brendt@stitcher.io> wrote:
> Hi Internals > > Since the addition of union types, it has become a little more cumbersome > to determine whether a given parameter type accepts specific input. > Personally I've been reusing this code blob in several places because of it: > > ``` > /** @var \ReflectionNamedType[] $types */ > $types = match ($type::class) { > ReflectionUnionType::class => $type->getTypes(), > ReflectionNamedType::class => [$type], > }; > > foreach ($types as $type) { > if (! is_subclass_of($type->getName(), ShouldBeStored::class)) { > continue; > } > > // … > } > ``` > > I wonder whether we would discuss adding a method on ReflectionType that > accepts any given input, and tells you whether that input is valid for that > type or not. I was thinking about `ReflectionType::accepts(string|object > $input): bool` but we could discuss another name. With it, the above > example could be refactored like so: > > ``` > if (! $type->accepts(ShouldBeStored::class)) { > return; > } > > // … > ``` > > I believe this method should accept both class names and objects to be > consistent with the existing reflection API. It would also work with > intersection types, if they are to be added some day. > > What do you think? > > Kind regards > Brent >
There's two possible notions of what "accepts" does: * acceptsValue(), whether a given value satisfies the type. Probably needs a flag to determine whether strict_types semantics should be used or not. * isSubTypeOf(), whether a given type satisfies the type in a subtyping relationship. You seem to be mixing these two up into one concept, even though they are quite different. Which is the one you actually need? (I am much more open to providing the former than the latter.) Nikita

Brent

5 years ago
Hi Nikita I was indeed thinking about the former: acceptsValue; but I would excpect that if the concrete value is a subtype of the current type, the fuction would also return true. What I'm basically interested in: will this method throw a type error when I pass this value? In other words: "will this method accept this value?" Is that in line with your view on `acceptsValue`? Kind regards Brent

Nikita Popov

5 years ago
On Fri, Jan 29, 2021 at 11:16 AM Brent Roose <brendt@stitcher.io> wrote:
> Hi Nikita > > I was indeed thinking about the former: acceptsValue; but I would excpect > that if the concrete value is a subtype of the current type, the fuction > would also return true. What I'm basically interested in: will this method > throw a type error when I pass this value? In other words: "will this > method accept this value?" > > Is that in line with your view on `acceptsValue`? > > Kind regards > Brent >
I just took a quick look at this, and apart from the strict_types distinction, another annoying edge case here is that our type checks aren't side-effect free. As of PHP 8.1, we have at least the deprecation warning for precision loss in weak float -> int casts, and more generally, arbitrary side-effects during __toString() calls on objects. So just performing a normal type check may not be suitable for this use case. Regards, Nikita