TLDR:
I propose to introduce a new class \ReflectionContext (or perhaps
\ReflectionScope?), to give information about imported aliases and the
namespace.
Background / motivation
Some libraries that parse doc comment annotations, e.g. phpDocumentor,
need to know the class aliases and the namespace that are active in
the place (scope) where the class, function or method is declared.
E.g. phpDocumentor has this class:
https://github.com/phpDocumentor/TypeResolver/blob/552bf401d264a443819a66233932be6a23f59d78/src/Types/Context.php
To get this information, they parse the PHP file where the class is defined:
https://github.com/phpDocumentor/TypeResolver/blob/552bf401d264a443819a66233932be6a23f59d78/src/Types/ContextFactory.php
It would be much more pleasant if PHP would provide this information
through the reflection API.
A custom library should not need to do an expensive and fragile
parsing job for information that is already known.
Also, this external parsing might not cover all cases, depending how
it is implemented. E.g. local namespace scopes in curly brackets. (Who
uses those, btw?)
Proposal
Make information about class aliases (imported through use statements)
and the active namespace available through the reflection API.
$reflClass = new \ReflectionClass(C::class);
$reflContext = $reflClass->getContext();
$aliases = $reflContext->getAliases();
Details
class ReflectionContext {
function getNamespace() : string {..}
/**
- @return string[]
- Format: $[$alias] = $qcn
*/
function getAliases() : string[] {..}
/**
- @return string|null
- The qcn of the class, or null if it is a built-in type like "string".
*/
function resolveAlias($alias) : ?string {..}
}
The $reflClass->getContext()->getNamespace() is the same as
$reflClass->getNamespace().
But having it all in the $context object allows this object to be
passed around to components that need it.
Open questions
In a method doc comment, we also want to resolve the "self" keyword.
For this, the class name is needed as well, not just external imports
and namespace.
Maybe also introduce \RefllectionMethod::getContext()? Not sure.
Should it be "context" or "scope"? It is not the same. Maybe "scope"
leads to the wrong expectations.
Andreas Hennings andreas@dqxtech.net schrieb am Mo., 11. Dez. 2017, 01:39:
TLDR:
I propose to introduce a new class \ReflectionContext (or perhaps
\ReflectionScope?), to give information about imported aliases and the
namespace.Background / motivation
Some libraries that parse doc comment annotations, e.g. phpDocumentor,
need to know the class aliases and the namespace that are active in
the place (scope) where the class, function or method is declared.E.g. phpDocumentor has this class:
To get this information, they parse the PHP file where the class is
defined:It would be much more pleasant if PHP would provide this information
through the reflection API.
A custom library should not need to do an expensive and fragile
parsing job for information that is already known.Also, this external parsing might not cover all cases, depending how
it is implemented. E.g. local namespace scopes in curly brackets. (Who
uses those, btw?)Proposal
Make information about class aliases (imported through use statements)
and the active namespace available through the reflection API.$reflClass = new \ReflectionClass(C::class);
$reflContext = $reflClass->getContext();
$aliases = $reflContext->getAliases();Details
class ReflectionContext {
function getNamespace() : string {..}
/**
- @return string[]
- Format: $[$alias] = $qcn
*/
function getAliases() : string[] {..}/**
- @return string|null
- The qcn of the class, or null if it is a built-in type like
"string".
*/
function resolveAlias($alias) : ?string {..}
}The $reflClass->getContext()->getNamespace() is the same as
$reflClass->getNamespace().But having it all in the $context object allows this object to be
passed around to components that need it.Open questions
In a method doc comment, we also want to resolve the "self" keyword.
For this, the class name is needed as well, not just external imports
and namespace.Maybe also introduce \RefllectionMethod::getContext()? Not sure.
Should it be "context" or "scope"? It is not the same. Maybe "scope"
leads to the wrong expectations.
Documentation tools shouldn't run the code IMO, that means they won't have
access to that feature.
Nikic's PHP parser already contains a tool that resolves all namespaces,
you can probably write a similar visitor for the Ast that also works for
PHPdoc blocks.
Regards, Niklas
Documentation tools shouldn't run the code IMO, that means they won't have
access to that feature.
By "documentation tools" do you mean libraries like phpDocumentor?
You are right, those need to parse anyway, they cannot use reflection API.
But tools for annotation discovery may want to use native reflection
instead of parsing, it is an implementation choice.
Nikic's PHP parser already contains a tool that resolves all namespaces, you
can probably write a similar visitor for the Ast that also works for PHPdoc
blocks.Regards, Niklas
Indeed that already exists at
https://github.com/Roave/BetterReflection/blob/2.0.1/docs/features.md#analysing-types-from-docblocks
- relatively new lib, so it probably didn't get noticed upfront in here.
Yes, parser / userland solutions exist for this purpose.
(I have seen BetterReflection)
I just thought since this information is already available, a library
that uses reflection API should not need a userland parser to get it.
Indeed that already exists at
https://github.com/Roave/BetterReflection/blob/2.0.1/
docs/features.md#analysing-types-from-docblocks
- relatively new lib, so it probably didn't get noticed upfront in here.
Yes, parser / userland solutions exist for this purpose.
(I have seen BetterReflection)
I just thought since this information is already available, a library
that uses reflection API should not need a userland parser to get it.
Unless the codebase being analyzed is trusted and not legacy
(wordpress-style) any tool based on the current reflection API is basically
a potential security issue or a set of potentially harmful side-effects.
The reason for me and James building BetterReflection was essentially that,
since the current API is flawed and not really fixable without BC breaks
(removing the side-effect), so I strongly encourage any code analysis tool
to just use the userland adapters we wrote, and only switch to core
reflection when performance is more critical than security.
This also means that if your addition makes it into the language it will be
implemented also by those adapters BTW, so push on ?
Indeed that already exists at
https://github.com/Roave/BetterReflection/blob/2.0.1/docs/
features.md#analysing-types-from-docblocks
- relatively new lib, so it probably didn't get noticed upfront in here.
Yes, parser / userland solutions exist for this purpose.
(I have seen BetterReflection)I just thought since this information is already available, a library
that uses reflection API should not need a userland parser to get it.Unless the codebase being analyzed is trusted and not legacy
(wordpress-style) any tool based on the current reflection API is basically
a potential security issue or a set of potentially harmful side-effects.
The reason for me and James building BetterReflection was essentially that,
since the current API is flawed and not really fixable without BC breaks
(removing the side-effect), so I strongly encourage any code analysis tool
to just use the userland adapters we wrote, and only switch to core
reflection when performance is more critical than security.
These side effects would be that the class loader loads files which can
break things?
This also means that if your addition makes it into the language it will
be implemented also by those adapters BTW, so push on ?
Indeed that already exists at
https://github.com/Roave/BetterReflection/blob/2.0.1/docs/fe
atures.md#analysing-types-from-docblocks
- relatively new lib, so it probably didn't get noticed upfront in here.
Yes, parser / userland solutions exist for this purpose.
(I have seen BetterReflection)I just thought since this information is already available, a library
that uses reflection API should not need a userland parser to get it.Unless the codebase being analyzed is trusted and not legacy
(wordpress-style) any tool based on the current reflection API is basically
a potential security issue or a set of potentially harmful side-effects.
The reason for me and James building BetterReflection was essentially that,
since the current API is flawed and not really fixable without BC breaks
(removing the side-effect), so I strongly encourage any code analysis tool
to just use the userland adapters we wrote, and only switch to core
reflection when performance is more critical than security.
These side effects would be that the class loader loads files which can
break things?
Yes. Reflecting over a codebase which contains even just polyfills
(duplicate classes) can already lead to unexpected crashes. Reflecting over
non-PSR-2 code can even lead to worse things such as starting a DB
connection and performing unwanted operations.
These side effects would be that the class loader loads files which can
break things?Yes. Reflecting over a codebase which contains even just polyfills
(duplicate classes) can already lead to unexpected crashes. Reflecting over
non-PSR-2 code can even lead to worse things such as starting a DB
connection and performing unwanted operations.
My own use case avoids this problem.
Instead of randomly scanning anything, you need to tell the annotation
parser explicitly which namespace directory you want to scan.
It is the caller's responsibility that all class files in this
directory are nicely shaped, and can be safely autoloaded.
In fact I did use a userland parser in the past, but then decided that
native reflection is safe with the above assumption.
These side effects would be that the class loader loads files which can
break things?Yes. Reflecting over a codebase which contains even just polyfills
(duplicate classes) can already lead to unexpected crashes. Reflecting over
non-PSR-2 code can even lead to worse things such as starting a DB
connection and performing unwanted operations.My own use case avoids this problem.
Instead of randomly scanning anything, you need to tell the annotation
parser explicitly which namespace directory you want to scan.
It is the caller's responsibility that all class files in this
directory are nicely shaped, and can be safely autoloaded.In fact I did use a userland parser in the past, but then decided that
native reflection is safe with the above assumption.
Maybe one fragile edge case would be if one of the files causes the
autoloader to include a different file, outside of the library, due to
ambiguous namespace mapping.
But this problem would also occur during everyday use of the library,
if this class is used.
So the condition is: Only scan namespace directories where you know
that every class can be safely autoloaded.
In fact I do not explicitly include the class file, but let the class
loader do this, to be sure that the same version of the class is used
that would be used in a regular request / process.