-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PropertyInfo] Extract property type from property declaration #31798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
4baeb0c
e903eeb
e71852d
550c4d2
afaea21
d11d8f0
f2b5c1a
72ee401
2a273c1
4e5bfe1
2f8b2fe
7f1095d
92e8be6
d6c3e5a
bc8d70c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
self
and parent
type
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,13 +209,8 @@ private function extractFromDeclaredType(string $class, string $property) | |
} | ||
|
||
$reflectionType = $reflectionProperty->getType(); | ||
$type = $reflectionType->getName(); | ||
|
||
if ($reflectionType->isBuiltIn()) { | ||
return [new Type(static::MAP_TYPES[$type] ?? $type, $reflectionType->allowsNull())]; | ||
} | ||
|
||
return [new Type(Type::BUILTIN_TYPE_OBJECT, $reflectionType->allowsNull(), $type)]; | ||
return [$this->extractFromReflectionType($reflectionType, null, $reflectionProperty)]; | ||
} | ||
|
||
/** | ||
|
@@ -234,7 +229,7 @@ private function extractFromMutator(string $class, string $property): ?array | |
if (!$reflectionType = $reflectionParameter->getType()) { | ||
return null; | ||
} | ||
$type = $this->extractFromReflectionType($reflectionType, $reflectionMethod); | ||
$type = $this->extractFromReflectionType($reflectionType, $reflectionMethod, null); | ||
|
||
if (\in_array($prefix, $this->arrayMutatorPrefixes)) { | ||
$type = new Type(Type::BUILTIN_TYPE_ARRAY, false, null, true, new Type(Type::BUILTIN_TYPE_INT), $type); | ||
|
@@ -256,7 +251,7 @@ private function extractFromAccessor(string $class, string $property): ?array | |
} | ||
|
||
if ($reflectionType = $reflectionMethod->getReturnType()) { | ||
return [$this->extractFromReflectionType($reflectionType, $reflectionMethod)]; | ||
return [$this->extractFromReflectionType($reflectionType, $reflectionMethod, null)]; | ||
} | ||
|
||
if (\in_array($prefix, ['is', 'can', 'has'])) { | ||
|
@@ -291,7 +286,7 @@ private function extractFromConstructor(string $class, string $property): ?array | |
} | ||
$reflectionType = $parameter->getType(); | ||
|
||
return $reflectionType ? [$this->extractFromReflectionType($reflectionType, $constructor)] : null; | ||
return $reflectionType ? [$this->extractFromReflectionType($reflectionType, $constructor, null)] : null; | ||
} | ||
|
||
if ($parentClass = $reflectionClass->getParentClass()) { | ||
|
@@ -320,7 +315,7 @@ private function extractFromDefaultValue(string $class, string $property) | |
return [new Type(static::MAP_TYPES[$type] ?? $type)]; | ||
} | ||
|
||
private function extractFromReflectionType(\ReflectionType $reflectionType, \ReflectionMethod $reflectionMethod): Type | ||
private function extractFromReflectionType(\ReflectionType $reflectionType, ?\ReflectionMethod $reflectionMethod, ?\ReflectionProperty $reflectionProperty): Type | ||
{ | ||
$phpTypeOrClass = $reflectionType->getName(); | ||
$nullable = $reflectionType->allowsNull(); | ||
|
@@ -331,19 +326,27 @@ private function extractFromReflectionType(\ReflectionType $reflectionType, \Ref | |
$type = new Type(Type::BUILTIN_TYPE_NULL, $nullable); | ||
} elseif ($reflectionType->isBuiltin()) { | ||
$type = new Type($phpTypeOrClass, $nullable); | ||
} elseif (null !== $reflectionMethod) { | ||
$type = new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, $this->resolveTypeName($phpTypeOrClass, $reflectionMethod, null)); | ||
} elseif (null !== $reflectionProperty) { | ||
$type = new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, $this->resolveTypeName($phpTypeOrClass, null, $reflectionProperty)); | ||
} else { | ||
$type = new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, $this->resolveTypeName($phpTypeOrClass, $reflectionMethod)); | ||
throw new \BadMethodCallException('You should provide method or property reflection'); | ||
} | ||
|
||
return $type; | ||
} | ||
|
||
private function resolveTypeName(string $name, \ReflectionMethod $reflectionMethod): string | ||
private function resolveTypeName(string $name, ?\ReflectionMethod $reflectionMethod, ?\ReflectionProperty $reflectionProperty): string | ||
{ | ||
if (null === ($reflection = $reflectionMethod ?? $reflectionProperty)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. calling the variable |
||
throw new \BadMethodCallException('You should provide method or property reflection'); | ||
} | ||
|
||
if ('self' === $lcName = strtolower($name)) { | ||
return $reflectionMethod->getDeclaringClass()->name; | ||
return $reflection->getDeclaringClass()->name; | ||
} | ||
if ('parent' === $lcName && $parent = $reflectionMethod->getDeclaringClass()->getParentClass()) { | ||
if ('parent' === $lcName && $parent = $reflection->getDeclaringClass()->getParentClass()) { | ||
return $parent->name; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,11 +16,13 @@ | |
* | ||
* @author Tales Santos <tales.augusto.santos@gmail.com> | ||
*/ | ||
class Php74Dummy | ||
class Php74Dummy extends ParentDummy | ||
{ | ||
public int $int; | ||
public ?string $string; | ||
public Dummy $dummy; | ||
public parent $parent; | ||
public self $self; | ||
public ?Dummy $optionalDummy; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like you forgot about "callable" property, which is mention in test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this test case should be removed because declaring |
||
public iterable $iterable; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as we only need
->getDeclaringClass()
, which is common on both class, I suggest using a single argument accepting\ReflectionMethod|\ReflectionProperty
(and same inextractFromReflectionType
). This will keep the code simplerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better than contribute with something is learn something contributing.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this doesn't seems to be a valid syntax. Is your suggestion something like this?
Or you are suggesting to not declare this argument with type:
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both
\ReflectionMethod
and\ReflectionProperty
implementsReflector
interface, so you can declare it like this:Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, both implements
\Reflector
but\Reflector
doesn't have the necessarygetDeclaringClass()
method. In this case, I don't think that we should use this interface as a typehint.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In any case It is far better than just
object
. For more specific typehint@param
declaration will be enough.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As both
extractFromReflectionType
andresolveTypeName
are private methods, I'm good to remove any typehint for this argument and declare its type on@param
docblock. What do you think?