Infer return type from reflection if no phpdoc given #906
Conversation
| $type = $this->getReturnTypeFromDocBlock($reflection); | ||
| if ($type) { | ||
| return $type; | ||
| } |
There was a problem hiding this comment.
As mentioned, same behaviour as before: prefer docblock
| public function getAttributeReturnsImportedClass(): DateTime | ||
| public function getAttributeReturnsImportedClassAttribute(): DateTime | ||
| { | ||
| } | ||
|
|
||
| public function getAttributeReturnsFqnClass(): \Illuminate\Support\Facades\Date | ||
| public function getAttributeReturnsFqnClassAttribute(): \Illuminate\Support\Facades\Date |
There was a problem hiding this comment.
Bugfix of my own tests 😅
| * @property-read mixed $attribute_return_type_int_or_null | ||
| * @property-read int|null $attribute_return_type_int_or_null |
There was a problem hiding this comment.
This shows the improvement…
| * @property-read mixed $attribute_with_int_return_type | ||
| * @property-read int $attribute_with_int_return_type |
|
Not sure why travis doesn't show up integrated here anymore, PR tests are executed => https://travis-ci.org/github/barryvdh/laravel-ide-helper/builds/669882120 |
|
ping @barryvdh thanks for considering 🙇 |
|
I don't think that documentation should be the authoritative source of truth and we should always prioritize code itself and fallback to documentation if no return type provided |
Unfortunately this will not work in practice because PHPs type system is inferior in terms of what types and combinations you can express. Thereof it's my opinion the only sensible thing to do it to make the phpdoc authoritative! |
|
i guess your talking about something like this /**
* @return \App\Models\Post[]
*/
public function allPosts(): array
{
// ...
}didn't think of this, it makes sense in this example |
|
Yes, exactly! |
You can add an explicit function phpdoc But yeah, having this PR merged would be great 😄 |
|
Friendly ping @barryvdh 🤗 |
|
thanks @mfn , this will be nice! |
|
It is! Already deleted lots of now-redundant phpdocs in my projects |
Up until now, to have the
get…Attributereturn a useful type information in the class phpdoc, it was necessary to specify the@returnannotation; otherwise the result was alwaysmixed.This now adds the capability to derive the type from the reflection itself.
* property-read mixed $foo* property-read string $fooNote that the docblock is still the authoritative source if provided. The reason is that phpdoc expressiveness / supporting tooling still exceeds PHPs type capabilities.
But this also means if the docblock is wrong, so will the type be and this PR won't fix that.
OTOH it removes the need for a docblock for the simple cases (e.g. no iterables of a type).
Tests with lots-o-variations included.
This is in fact already implemented for the
elseif (!method_exists('Illuminate\Database\Eloquent\Model', $method)case, but this part in fact prefers the native type over the docblock; not sure why and didn't want to change that for now.