Everything

php

[RFC][DISCUSSION] ReflectionParameter::getClassName()

php.internals

Philip Sturgeon

11 years ago

This RFC adds a new method to ReflectionParameter to allow easy access
to a class name in a type hint, avoiding the need to actually load the
class and use `get_class()` or `::class`.

https://wiki.php.net/rfc/reflectionparameter-getclassname

Cheers.

Matteo Beccati

11 years ago

Hi Phil,

On 31/01/2015 05:14, Philip Sturgeon wrote:

> This RFC adds a new method to ReflectionParameter to allow easy access
> to a class name in a type hint, avoiding the need to actually load the
> class and use `get_class()` or `::class`.
>
> https://wiki.php.net/rfc/reflectionparameter-getclassname

Looks good to me!

Just a couple of minor things.

1) The RFC does not mention what happens if the parameter is not type
hinted, or what happens when hints are namespaced and/or use clauses are
used.

2) There's a tiny bit of overlap with "scalar type hints", if it is
accepted (in any form) and reflection support added to it. Depending on
the way such reflection support is implemented, the method proposed here
could be a perfect match. Or perhaps it could be in the way.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Philip Sturgeon

11 years ago

On Sat, Jan 31, 2015 at 3:12 AM, Matteo Beccati <php@beccati.com> wrote:

> Hi Phil,
>
>
> On 31/01/2015 05:14, Philip Sturgeon wrote:
>>
>> This RFC adds a new method to ReflectionParameter to allow easy access
>> to a class name in a type hint, avoiding the need to actually load the
>> class and use `get_class()` or `::class`.
>>
>> https://wiki.php.net/rfc/reflectionparameter-getclassname
>
>
> Looks good to me!
>
> Just a couple of minor things.
>
> 1) The RFC does not mention what happens if the parameter is not type
> hinted, or what happens when hints are namespaced and/or use clauses are
> used.
>
> 2) There's a tiny bit of overlap with "scalar type hints", if it is accepted
> (in any form) and reflection support added to it. Depending on the way such
> reflection support is implemented, the method proposed here could be a
> perfect match. Or perhaps it could be in the way.
>
>
> Cheers
> --
> Matteo Beccati
>
> Development & Consulting - http://www.beccati.com/

Thanks! Good questions.

1) Thanks for your help on GitHub in making tests for this. Implemented nicely.

2) There might be some overlap in the scalar type hint stuff kinda,
but I'd like to think there isn't. getClassName() is just
getClass()->name without loading the actual class. Getting "string' or
"int" back is likely to have its own logic in the scalar type hint RFC
so I'd rather stay clear of it altogether.

Dan Ackroyd

11 years ago

On 31 January 2015 at 17:31, Philip Sturgeon <pjsturgeon@gmail.com> wrote:

> On Sat, Jan 31, 2015 at 3:12 AM, Matteo Beccati <php@beccati.com> wrote:
>>
>> 2) There's a tiny bit of overlap with "scalar type hints",
>
> 2) There might be some overlap in the scalar type hint stuff kinda,
> but I'd like to think there isn't. getClassName() is just
> getClass()->name

I think Matteo's point is that if any scalar type RFC passes, a
type-hint for a parameter would not always be the name of a class, so
the method name 'getClassName()' would be misleading.

It would be good to choose a better name to avoid that problem.

cheers
Dan

Andrew Faulds

11 years ago

> On 1 Feb 2015, at 01:23, Dan Ackroyd <danack@basereality.com> wrote:
>
> On 31 January 2015 at 17:31, Philip Sturgeon <pjsturgeon@gmail.com> wrote:
>> On Sat, Jan 31, 2015 at 3:12 AM, Matteo Beccati <php@beccati.com> wrote:
>>>
>>> 2) There's a tiny bit of overlap with "scalar type hints",
>>
>> 2) There might be some overlap in the scalar type hint stuff kinda,
>> but I'd like to think there isn't. getClassName() is just
>> getClass()->name
>
>
> I think Matteo's point is that if any scalar type RFC passes, a
> type-hint for a parameter would not always be the name of a class, so
> the method name 'getClassName()' would be misleading.
>
> It would be good to choose a better name to avoid that problem.

We already have that problem (array, callable).

I think the more important issue is the conflict with the ReflectionTypeAnnotation RFC, which proposes something similar to what was originally part of the Return Types RFC:

https://wiki.php.net/rfc/reflectionparameter.typehint

Thanks.

--
Andrea Faulds
http://ajf.me/

Philip Sturgeon

11 years ago

On Sat, Jan 31, 2015 at 8:27 PM, Andrea Faulds <ajf@ajf.me> wrote:

>
>> On 1 Feb 2015, at 01:23, Dan Ackroyd <danack@basereality.com> wrote:
>>
>> On 31 January 2015 at 17:31, Philip Sturgeon <pjsturgeon@gmail.com> wrote:
>>> On Sat, Jan 31, 2015 at 3:12 AM, Matteo Beccati <php@beccati.com> wrote:
>>>>
>>>> 2) There's a tiny bit of overlap with "scalar type hints",
>>>
>>> 2) There might be some overlap in the scalar type hint stuff kinda,
>>> but I'd like to think there isn't. getClassName() is just
>>> getClass()->name
>>
>>
>> I think Matteo's point is that if any scalar type RFC passes, a
>> type-hint for a parameter would not always be the name of a class, so
>> the method name 'getClassName()' would be misleading.
>>
>> It would be good to choose a better name to avoid that problem.
>
> We already have that problem (array, callable).
>
> I think the more important issue is the conflict with the ReflectionTypeAnnotation RFC, which proposes something similar to what was originally part of the Return Types RFC:
>
> https://wiki.php.net/rfc/reflectionparameter.typehint
>
> Thanks.
>
> --
> Andrea Faulds
> http://ajf.me/
>
>
>
>

Hey, missed a few replies.

1. There is no conflict with scalar types at all. The class name is
not a scalar type :) We already have getClass(), this is just getting
the name of that class without loading it.

> a type-hint for a parameter would not always be the name of a class, so

the method name 'getClassName()' would be misleading.

This wont be a problem Dan. getClass() wouldn't work if you call it on
a `foo(string $bar)`, and this will fall over too. The test and RFC
both show that.

So, are we good to vote?

Matteo Beccati

11 years ago

On 14/02/2015 16:34, Philip Sturgeon wrote:

> On Sat, Jan 31, 2015 at 8:27 PM, Andrea Faulds <ajf@ajf.me> wrote:
>> I think the more important issue is the conflict with the
>> ReflectionTypeAnnotation RFC, which proposes something similar to
>> what was originally part of the Return Types RFC:
>>
>> https://wiki.php.net/rfc/reflectionparameter.typehint

That's the "reflection support" I was talking about! I knew there was
something, but I just couldn't find it in the RFCs themself.

I've been playing a little bit with Sara's work (as you might have
noticed if you follow php-cvs - sorry again!) and got it working with
the scalar type hints branch:

https://github.com/mbeccati/php-src/tree/scalar_hints_with_reflection

It's still missing the scalar part and possibly an easy way to get the
class name as Phil suggests.

> So, are we good to vote?

Not sure. Maybe it's worth to update and expand the RFC above?

Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Matteo Beccati

11 years ago

Hi,

On 14/02/2015 21:23, Matteo Beccati wrote:

> On 14/02/2015 16:34, Philip Sturgeon wrote:
>> On Sat, Jan 31, 2015 at 8:27 PM, Andrea Faulds <ajf@ajf.me> wrote:
>>> I think the more important issue is the conflict with the
>>> ReflectionTypeAnnotation RFC, which proposes something similar to
>>> what was originally part of the Return Types RFC:
>>>
>>> https://wiki.php.net/rfc/reflectionparameter.typehint
>
> That's the "reflection support" I was talking about! I knew there was
> something, but I just couldn't find it in the RFCs themself.
>
> I've been playing a little bit with Sara's work (as you might have
> noticed if you follow php-cvs - sorry again!) and got it working with
> the scalar type hints branch:
>
> https://github.com/mbeccati/php-src/tree/scalar_hints_with_reflection
>
> It's still missing the scalar part and possibly an easy way to get the
> class name as Phil suggests.

Thanks to my cats waking me up too early on a sunday morning, I've
rebased Sara's patch to current master and updated it to support return
types:

https://github.com/mbeccati/php-src/commits/reflection.typehint

The new methods are:

* ReflectionFunctionAbstract::hasReturnTypeAnnotation()
* ReflectionFunctionAbstract::getReturnTypeAnnotation()
* ReflectionTypeAnnotation::isInstance()

The latter should fulfil Phil's getClassName() purpose, albeit it's much
more verbose:

function get_name (ReflectionParameter $rp) {
if ($rp->hasTypeAnnotation()) {
$ra = $rp->getTypeAnnotation();
if ($ra->isInstance()) {
return (string) $ra;
}
}
}

I've also added scalar support in:

https://github.com/mbeccati/php-src/commits/scalar_hints_with_reflection

which has:

* ReflectionTypeAnnotation::isScalar()

I believe that both isInstance/isScalar would probably apply to
ReflectionParameter too, unless the "old way" is supposed to be deprecated.

@Phil I apologise it I hijacked your RFC, but I think that having just
ReflectionParameter::getClassName() falls a bit short IMHO, especially
with return types.

@Andrea, @Sara I hope you don't mind ;)


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

François Laupretre

11 years ago

> De : Matteo Beccati [mailto:php@beccati.com]
>
> The new methods are:
>
> * ReflectionFunctionAbstract::hasReturnTypeAnnotation()
> * ReflectionFunctionAbstract::getReturnTypeAnnotation()
> * ReflectionTypeAnnotation::isInstance()

I agree Reflection must support hinting but annotations, int the PHP world, are a different concept and don't have anything to do here. OK for ReflectionTypeHint, but not for ReflectionTypeAnnotation.

Regards

François

Matteo Beccati

11 years ago

Hi everyone,

I know it's late for the RFC party, but it looks like some form of STH
is going to land in PHP7. Without reflection support, which is also
missing from return types as far as I know.

That's why I'm resuming this thread, as I still think this is the best
approach. Many do not like ReflectionTypeAnnotation, but that's a
trivial change, if we can agree that the behaviour is correct.


Cheers

On 15/02/2015 10:28, Matteo Beccati wrote:

> Hi,
>
> On 14/02/2015 21:23, Matteo Beccati wrote:
>> On 14/02/2015 16:34, Philip Sturgeon wrote:
>>> On Sat, Jan 31, 2015 at 8:27 PM, Andrea Faulds <ajf@ajf.me> wrote:
>>>> I think the more important issue is the conflict with the
>>>> ReflectionTypeAnnotation RFC, which proposes something similar to
>>>> what was originally part of the Return Types RFC:
>>>>
>>>> https://wiki.php.net/rfc/reflectionparameter.typehint
>>
>> That's the "reflection support" I was talking about! I knew there was
>> something, but I just couldn't find it in the RFCs themself.
>>
>> I've been playing a little bit with Sara's work (as you might have
>> noticed if you follow php-cvs - sorry again!) and got it working with
>> the scalar type hints branch:
>>
>> https://github.com/mbeccati/php-src/tree/scalar_hints_with_reflection
>>
>> It's still missing the scalar part and possibly an easy way to get the
>> class name as Phil suggests.
>
> Thanks to my cats waking me up too early on a sunday morning, I've
> rebased Sara's patch to current master and updated it to support return
> types:
>
> https://github.com/mbeccati/php-src/commits/reflection.typehint
>
> The new methods are:
>
> * ReflectionFunctionAbstract::hasReturnTypeAnnotation()
> * ReflectionFunctionAbstract::getReturnTypeAnnotation()
> * ReflectionTypeAnnotation::isInstance()
>
> The latter should fulfil Phil's getClassName() purpose, albeit it's much
> more verbose:
>
> function get_name (ReflectionParameter $rp) {
>    if ($rp->hasTypeAnnotation()) {
>      $ra = $rp->getTypeAnnotation();
>      if ($ra->isInstance()) {
>        return (string) $ra;
>      }
>    }
> }
>
> I've also added scalar support in:
>
> https://github.com/mbeccati/php-src/commits/scalar_hints_with_reflection
>
> which has:
>
> * ReflectionTypeAnnotation::isScalar()
>
> I believe that both isInstance/isScalar would probably apply to
> ReflectionParameter too, unless the "old way" is supposed to be deprecated.
>
> @Phil I apologise it I hijacked your RFC, but I think that having just
> ReflectionParameter::getClassName() falls a bit short IMHO, especially
> with return types.
>
> @Andrea, @Sara I hope you don't mind ;)
>
>
> Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/