[PHP-INTERNALS] [RFC] ReflectionParameter::hasTypehint() / getTypehintText()

Why 5.7? Looks like the sort of self-contained function addition you could get into 5.6.1.

Personally I think we shouldn't add features in 5.6.1. Those should be
reserved for bug and security fixes.

Agreed. Point releases have traditionally been for bugfixes only.
Sometimes we sneak new functionality in, but there's not enough of an
urgent, earth-shattering need for these functions to justify it. With
our pace of yearly releases and the fact that 5.6 is already in alpha,
it makes sense to me to be conservative and only add to 5.next.

-Sara

Sara,

I'm hoping to improve the way types are handled in reflection but I've
been waiting until the return types and nullable types RFCs have been
voted on as that will most likely impact my decisions. I'd like to
introduce getType and hasType functions that holds various bits of
information such as isArray(), isCallable() and could also contain the
text representation of the type. What are your thoughts on that?

As in something like this?

$rf = new ReflectionFunction('foo');
list($rp) = $rf->getParameters();
$hint = $rp->getType(); // returns ReflectionTypeHint
if ($hint->isArray()) { ... }

I thought about that (and would like to see it), which is why I
specifically went for the name getTypehintText(), which leaves the
"get me an object version" (i.e. getTypehint()) left available. See
also: https://wiki.php.net/rfc/reflectionparameter.typehint#on_the_naming
  If you think it's a bit too busy to have getTypehint(): object, and
getTypehintText(): string, I can hold off on this RFC for awhile.
There's certainly no rush.


Or did you mean something else?

-Sara

Yeah, I mean something like that. Maybe we could collaborate in IRC
somewhere to work out details? We can move much faster that way. I'm
regularly in #hhvm on freenode and #php.doc and #php.pecl on efnet as
LeviM or LeviM_t.

Per our conversation in IRC, I've updated the RFC and implementation

I don't think it should return an empty string when hasTypehint has
failed (the 4th "test") in your test case (for $d). IMO, it should
return NULL.

cheers,
Derick

If it doesn't have type information I think an empty string should be
returned. Since you can't have an empty type hint there really isn't a
conflict there. This means the function can always return a string.

That's my $0.02.

I'd prefer NULL to indicate the complete absence of a type hint too —
it ends up as an empty string if it's used in that context regardless,
but the changed type is useful. I don't feel strongly enough about
this that it would affect my vote, though.
Adam, providing 2.17 Canadian cents.

The current version of the RFC makes this point moot.

hasTypeAnnotation() would return false, while
getTypeAnnotation() would return NULL (as opposed to a
ReflectionTypeAnnotation instance)

The new equivalent to $rp->getTypehintText() is now
(string)$rp->getTypeAnnotation()

-Sara

We might want to rename `ReflectionTypeAnnotation::isNullable` to
`ReflectionTypeAnnotation::allowsNull`. I think it is a bit more
understandable; it also is used by `ReflectionParmeter` so it would be
a bit more consistent. What do you think?

Absent pre-existing uses, I like following the is*() pattern of
isArray/isCallable for consistency. But good catch, it's probably
best to avoid inconsistency with the parent class.

-Sara

On your updated proposal, there is a comment on how class names should
be returned. Right now, the test case only tests for "StdClass", but I
also think you should have a look at namespaced classes, and full
qualified class names in tests. I think that might show class names
should be returned with a \ in front of it.

cheers,
Derick

Nice catch, I'll integrate that after Levi has finished with his
return type hinting rfc (we agreed to get his bit done first, then
I'll rebase my reflection stuff onto it)

-Sara