[DISCUSSION] ReflectionType::accepts

4 years ago by Brent Roose — view source — reply

unread

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

4 years ago by Pierre R. — view source — reply

unread

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

4 years ago by Sebastian Bergmann — view source — reply

unread

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.

4 years ago by Nikita Popov — view source — reply

unread

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

4 years ago by Brent Roose — view source — reply

unread

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

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