Markus On 19 October 2014 20:31, Markus Fischer <mar...@fischer.name> wrote: > On 16.10.14 06:39, Levi Morrison wrote: >> - The design and accompanying section of reflection[3] has been >> rewritten entirely. >> >> [3]: https://wiki.php.net/rfc/returntypehinting#reflection > > I've some comments about the Reflection API addition/changes: > > 1. > "Note that getReturnType will always return a ReflectionType > object, even if there is no return type declared." > > That feels weird, TBH. Other parts of the Reflection-API don't do that, > they simply return false if I would call e.g. getParentClass() when > there's none.
That's a different situation though. A function always returns something, even if no type was declared and there was no explicit return statement. There's currently no concept of "void" in PHP in this context. > > 2. > "This API was designed so you could use ReflectionType::getKind() > in a switch statement to cover all cases, rather than be forced to use > an if/else structure that calls isArray(), isCallable, etc like the > ReflectionParameter API does." > > I don't like the deviation from the existing Reflection API. I'm not > saying it's perfect, but I fear a "haystack,needle vs. needle,haystack" > thing here and would prefer the existing convention for is*() methods. > > I wouldn't see a conflict it providing both as I see the usefulness. I understand where you are coming from here, but if we are going to take this approach we should provide both in the context of parameters - i.e. transition to the new RelfectionType API everywhere - rather than providing a leaky approach in the context of new features for the sake of consistency. The ReflectionType was specifically designed to be suitable for both parameters and return values because it represents only the type, whereas the ReflectionParameter is only suitable for, well, parameters, because it represents more. I think that providing access to a ReflectionType instance from a ReflectionParameter is going to be proposed before release, but it is outside the scope of, and dependent on the acceptance of, this RFC. > > 3. The new class is called "ReflectionType", shouldn't it be > "ReflectionReturnType" ? No, see above. > > 4. Other reflection classes, e.g. ReflectionParameter provides "missing > link" methods, specifically I'm missing: > > public ReflectionClass getDeclaringClass ( void ); > public ReflectionFunctionAbstract getDeclaringFunction ( void ); This is a valid point, although remember a RelectionType represents a *type* and not a *return type declaration*. I'm not sure if there's value in adding a ReflectionReturnType layer between the type and the function it is retrieved from (I suspect not, this level of granularity seems unnecessary to me, you'd just pass the ReflectionFunction around). A parameter is slightly different as it has other properties as well as just a type declaration (name, required, default value etc). The only other property a return type could have is whether it is "nullable" - which is explicitly not part of this RFC (see https://wiki.php.net/rfc/nullable_typehints) and could be indicated using a bitmask with the getKind(), which I *think* is part of the reason the RFC specifically states that the values of the ReflectionType::IS_* constants should not be relied upon. TL;DR I agree this is inconsistent, but I also don't think it matters because I don't think anyone would need/use it. I probably wouldn't complain if someone wanted to add it, though. > > 5. I think, also like in ReflectionParameter, there should a direct > shortcut to the class, if provided (and not just the string name): > > public ReflectionClass getClass ( void ); > Agreed (not 100% clear cut because there are special typehint cases that do not have a corresponding class but I think this is somewhat nitpicking) Thanks, Chris -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
