On 4/15/16 10:10 AM, Andrea Faulds wrote:
Hi Stas,
Stanislav Malyshev wrote:
I don't know what is complicated about "string|Stringable" or "Foo|Bar"
since it is super self-explanatory. However, I find myself checking the
It may be self-explanatory for you. It's much less self-explanatory for
somebody just starting to learn. It is also very dangerous - if it's
either Foo or Bar, can you call Foo::stuff on it or not? If it's string
or not string, can you call strlen on it? Etc., etc. It adds a lot of
cognitive load and complicates the whole picture. You may have a
specific use case where it is useful (which we have yet to see btw) but
please remember it's a language with literally millions of use cases and
users.
This is something that particularly concerns me about union types, in
that they reduce type safety. If you have a union type of Foo|Bar for
some variable, then the set of methods you can call on that variable
is actually the intersection, not the union, of the set of methods you
can call on Foo and Bar. Which, unless those two classes share some
interface, is probably an empty set. So there's nothing you can
actually do safely with it without doing checks within the body of the
function, and if you're doing that, then why do we have a type
declaration? It's only barely more useful than omitting a type
declaration at all; type declarations are supposed to prevent you
needing to check. On the other hand, if the two classes share some
methods, then either there's an interface you can already use here, or
you can create one. Either way, you don't need a union type.
There are some cases where you can't create an interface, but if
that's the case, I think it is more worthwhile to look at how we can
fix those cases, rather than add what amounts to a hacky workaround.
Thanks!
I think there's 2 general use cases for union types that would be "good
things", which are different for & and |, and have very little...
intersection. (*yaaaaaaa!*)
The OR case is for cases where the language doesn't support a unified
case at all. The most obvious example here is array|Traversable. If I
want "a thing I can foreach()", then PHP right now has no way of saying
that syntactically. You have to type on array, or Traversable, or not
type at all. array|Traversable is what you really want, because those
DO have an overlap (foreach-ablility), PHP is just incapable of
representing that otherwise.
A similar example would be callable|SomeInterface. An interface can
specify a signature for __invoke(), which gives you documentation on the
format that is expected for a callable. However, you can't strictly
enforce that because then you don't allow for a function or closure that
fits the same method signature. That means you have to leave it
untyped. This, I argue, would be better *and* reasonably type safe:
interface MiddlewareInterface {
function __invoke(RequestInterface $req, ResponseInterface $res);
}
function middleware_builder(callable|MiddlewareInterface $m) {
// ...
}
As that self-documents that MiddlewareInterface is the callable
signature we need, but still allows an arbitrary callable to be passed.
It's not perfect (I could pass a string of a function name that doesn't
have that interface and it would still explode), but it is an
improvement over middleware_builder() having no type specification at
all, as is the case today.
(Hat tip to Matthew O'Phinney for cueing me into this neat feature of
interfaces and __invoke().)
On the flipside, the & is mostly useful for where you need multiple
interfaces for something. For instance, there's the PSR-7
ResponseInterface. Drupal also has a number of interfaces for value
objects to indicate their cacheability metadata, such as
CacheableMetadataInterface. But that applies to more than just
Responses, of course, so having it extend ResponseInterface is not
good. So how can I specify that I need an object that is BOTH
ResponseInterface AND CacheableMetdataInterface? That's an entirely
reasonable thing to do, but currently PHP doesn't allow for it at all.
Even having a custom interface that extends both of those doesn't help,
because then my class needs to implement the child interface, not both
parents.
Being able to type on function foo(ResponseInterface &
CacheableMetadatInterface $r) seems entirely reasonable to me, and
definitely more type safe than leaving it untyped and relying on
documentation.
The danger zone is, as many people have noted, OR-ing interfaces
together. That does undermine type safety in most cases, I'd argue.
However, it's also rather obvious when it does so, because you end up
with a giant if-statement inside your function. So while it's a risk, I
don't think it's a subtle one, especially if well documented.
Actually, there is a use case for ORing interfaces, and that's for BC
reasons. Take, for instance, Symfony 2's routing system. It started
off with a RouteMatcherInterface::match(RequestContext $r) method, where
RequestContext is a "junior version" of a Request. After a while it
became evident that having the full for-reals Request object was useful,
so after prompting from Drupal they build an alternate version that used
a full Request, but overlaped in ugly ways with the original. Making
RequestContext extend Request (or vice versa) was completely impractical
for various reasons. However, as a BC transitional approach it would
have been reasonable to simply update the interface to
RouteMatcherInterface::(RequestContext|Request $r), as a signaling
mechanism that the old object still worked for a while, and despite
there not being a common type most of the methods *are* common between
those two objects. That would have allowed that service to fully
internalize the BC layer.
So while I agree there are risks to misuse of Class|Class type hints, I
think the overall benefits (detailed above) outweigh the risks. Even
then, hinting on A|B is better than hinting on nothing and still
accepting A or B. At least it's explicit that you're doing something risky.
--
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
