Skip to content
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.

Sign up for GitHub

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

Using generators to get a list of reflections #1476

Open
wants to merge 2 commits into
base: 6.52.x
Choose a base branch
from
Open

Using generators to get a list of reflections #1476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc0231c
Using generators to get a list of reflections
shcherbanich Jan 5, 2025
7fd413e
Fix source locator caching: only save cache after all iterations comp…
shcherbanich Jan 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion demo/parsing-whole-directory/example1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@

$reflector = new DefaultReflector($sourceLocator);

$classReflections = $reflector->reflectAllClasses();
$generator = $reflector->reflectAllClasses();

$classReflections = [];
foreach ($generator as $classReflection) {
$classReflections[] = $classReflection;
}

!empty($classReflections) && print 'success';
7 changes: 6 additions & 1 deletion demo/parsing-whole-directory/example2.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@

$reflector = new DefaultReflector($sourceLocator);

$classReflections = $reflector->reflectAllClasses();
$generator = $reflector->reflectAllClasses();

$classReflections = [];
foreach ($generator as $classReflection) {
$classReflections[] = $classReflection;
}

!empty($classReflections) && print 'success';
19 changes: 10 additions & 9 deletions src/Reflector/DefaultReflector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\Reflector;

use Generator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
use Roave\BetterReflection\Reflection\ReflectionClass;
Expand Down Expand Up @@ -43,11 +44,11 @@ public function reflectClass(string $identifierName): ReflectionClass
/**
* Get all the classes available in the scope specified by the SourceLocator.
*
* @return list<ReflectionClass>
* @return Generator<ReflectionClass>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should roll back to iterable<int, ReflectionClass> here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is the actual BC break, since list<T> is obviously an extremely small type

*/
public function reflectAllClasses(): iterable
public function reflectAllClasses(): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to change the native type here: iterable is a supertype of Generator, and works fine

{
/** @var list<ReflectionClass> $allClasses */
/** @var Generator<ReflectionClass> $allClasses */
$allClasses = $this->sourceLocator->locateIdentifiersByType(
$this,
new IdentifierType(IdentifierType::IDENTIFIER_CLASS),
Expand Down Expand Up @@ -79,11 +80,11 @@ public function reflectFunction(string $identifierName): ReflectionFunction
/**
* Get all the functions available in the scope specified by the SourceLocator.
*
* @return list<ReflectionFunction>
* @return Generator<ReflectionFunction>
*/
public function reflectAllFunctions(): iterable
public function reflectAllFunctions(): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment above: the native type can stay iterable, and the return type can be iterable<int, ReflectionFunction>

{
/** @var list<ReflectionFunction> $allFunctions */
/** @var Generator<ReflectionFunction> $allFunctions */
$allFunctions = $this->sourceLocator->locateIdentifiersByType(
$this,
new IdentifierType(IdentifierType::IDENTIFIER_FUNCTION),
Expand Down Expand Up @@ -115,11 +116,11 @@ public function reflectConstant(string $identifierName): ReflectionConstant
/**
* Get all the constants available in the scope specified by the SourceLocator.
*
* @return list<ReflectionConstant>
* @return Generator<ReflectionConstant>
*/
public function reflectAllConstants(): iterable
public function reflectAllConstants(): Generator
Comment on lines +119 to +121
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment above: the native type can stay iterable, and the return type can be iterable<int, ReflectionConstant>

{
/** @var list<ReflectionConstant> $allConstants */
/** @var Generator<ReflectionConstant> $allConstants */
$allConstants = $this->sourceLocator->locateIdentifiersByType(
$this,
new IdentifierType(IdentifierType::IDENTIFIER_CONSTANT),
Expand Down
13 changes: 7 additions & 6 deletions src/Reflector/Reflector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\Reflector;

use Generator;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionConstant;
use Roave\BetterReflection\Reflection\ReflectionFunction;
Expand All @@ -21,9 +22,9 @@ public function reflectClass(string $identifierName): ReflectionClass;
/**
* Get all the classes available in the scope specified by the SourceLocator.
*
* @return list<ReflectionClass>
* @return Generator<ReflectionClass>
*/
public function reflectAllClasses(): iterable;
public function reflectAllClasses(): Generator;
Comment on lines -24 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface should be rolled back, and the @return types should switch from list to iterable


/**
* Create a ReflectionFunction for the specified $functionName.
Expand All @@ -35,9 +36,9 @@ public function reflectFunction(string $identifierName): ReflectionFunction;
/**
* Get all the functions available in the scope specified by the SourceLocator.
*
* @return list<ReflectionFunction>
* @return Generator<ReflectionFunction>
*/
public function reflectAllFunctions(): iterable;
public function reflectAllFunctions(): Generator;

/**
* Create a ReflectionConstant for the specified $constantName.
Expand All @@ -49,7 +50,7 @@ public function reflectConstant(string $identifierName): ReflectionConstant;
/**
* Get all the constants available in the scope specified by the SourceLocator.
*
* @return list<ReflectionConstant>
* @return Generator<ReflectionConstant>
*/
public function reflectAllConstants(): iterable;
public function reflectAllConstants(): Generator;
}
7 changes: 4 additions & 3 deletions src/SourceLocator/Type/AbstractSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
use Roave\BetterReflection\Reflection\Reflection;
Expand Down Expand Up @@ -54,15 +55,15 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
*
* @throws ParseToAstFailure
*/
final public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
final public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest declaring iterable here too: Generator is a very concrete type.

We can yield from any iterable anyway, and we don't need to force every implementation into a Generator.

See https://3v4l.org/j4S0l

{
$locatedSource = $this->createLocatedSource(new Identifier(Identifier::WILDCARD, $identifierType));

if (! $locatedSource) {
return [];
return;
}

return $this->astLocator->findReflectionsOfType(
yield from $this->astLocator->findReflectionsOfType(
$reflector,
$locatedSource,
$identifierType,
Expand Down
16 changes: 5 additions & 11 deletions src/SourceLocator/Type/AggregateSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
use Roave\BetterReflection\Reflection\Reflection;
use Roave\BetterReflection\Reflector\Reflector;

use function array_map;
use function array_merge;

class AggregateSourceLocator implements SourceLocator
{
/** @param list<SourceLocator> $sourceLocators */
Expand All @@ -32,14 +30,10 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
return null;
}

/**
* {@inheritDoc}
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks good, but the return type should probably be iterable

{
return array_merge(
[],
...array_map(static fn (SourceLocator $sourceLocator): array => $sourceLocator->locateIdentifiersByType($reflector, $identifierType), $this->sourceLocators),
);
foreach ($this->sourceLocators as $sourceLocator) {
yield from $sourceLocator->locateIdentifiersByType($reflector, $identifierType);
}
}
}
11 changes: 8 additions & 3 deletions src/SourceLocator/Type/AnonymousClassObjectSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PhpParser\NodeTraverser;
Expand All @@ -25,7 +26,6 @@
use Roave\BetterReflection\SourceLocator\Located\AnonymousLocatedSource;
use Roave\BetterReflection\Util\FileHelper;

use function array_filter;
use function assert;
use function file_get_contents;
use function str_contains;
Expand Down Expand Up @@ -55,9 +55,14 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
*
* @throws ParseToAstFailure
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of this class can be rolled back

public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
{
return array_filter([$this->getReflectionClass($reflector, $identifierType)]);
$reflection = $this->getReflectionClass($reflector, $identifierType);
if (! $reflection) {
return;
}

yield $reflection;
}

private function getReflectionClass(Reflector $reflector, IdentifierType $identifierType): ReflectionClass|null
Expand Down
11 changes: 8 additions & 3 deletions src/SourceLocator/Type/ClosureSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Roave\BetterReflection\SourceLocator\Type;

use Closure;
use Generator;
use PhpParser\Node;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\NodeTraverser;
Expand All @@ -26,7 +27,6 @@
use Roave\BetterReflection\SourceLocator\Located\AnonymousLocatedSource;
use Roave\BetterReflection\Util\FileHelper;

use function array_filter;
use function assert;
use function file_get_contents;
use function str_contains;
Expand Down Expand Up @@ -56,9 +56,14 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
*
* @throws ParseToAstFailure
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of this class can be rolled back

{
return array_filter([$this->getReflectionFunction($reflector, $identifierType)]);
$reflection = $this->getReflectionFunction($reflector, $identifierType);
if (! $reflection) {
return;
}

yield $reflection;
}

private function getReflectionFunction(Reflector $reflector, IdentifierType $identifierType): ReflectionFunction|null
Expand Down
7 changes: 4 additions & 3 deletions src/SourceLocator/Type/Composer/PsrAutoloaderLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type\Composer;

use Generator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
use Roave\BetterReflection\Reflection\Reflection;
Expand Down Expand Up @@ -54,11 +55,11 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
/**
* Find all identifiers of a type
*
* @return list<Reflection>
* @return Generator<Reflection>
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of this class can be rolled back, except for the return type being iterable

{
return (new DirectoriesSourceLocator(
yield from (new DirectoriesSourceLocator(
$this->mapping->directories(),
$this->astLocator,
))->locateIdentifiersByType($reflector, $identifierType);
Expand Down
8 changes: 3 additions & 5 deletions src/SourceLocator/Type/DirectoriesSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use Roave\BetterReflection\Identifier\Identifier;
Expand Down Expand Up @@ -55,11 +56,8 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
return $this->aggregateSourceLocator->locateIdentifier($reflector, $identifier);
}

/**
* {@inheritDoc}
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of this class can be rolled back, except for the return type being iterable

{
return $this->aggregateSourceLocator->locateIdentifiersByType($reflector, $identifierType);
yield from $this->aggregateSourceLocator->locateIdentifiersByType($reflector, $identifierType);
}
}
5 changes: 3 additions & 2 deletions src/SourceLocator/Type/FileIteratorSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use Iterator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
Expand Down Expand Up @@ -83,8 +84,8 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
*
* @throws InvalidFileLocation
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of this class can be rolled back, except for the return type being iterable

{
return $this->getAggregatedSourceLocator()->locateIdentifiersByType($reflector, $identifierType);
yield from $this->getAggregatedSourceLocator()->locateIdentifiersByType($reflector, $identifierType);
}
}
17 changes: 12 additions & 5 deletions src/SourceLocator/Type/MemoizingSourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
use Roave\BetterReflection\Reflection\Reflection;
Expand Down Expand Up @@ -37,17 +38,23 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
= $this->wrappedSourceLocator->locateIdentifier($reflector, $identifier);
}

/** @return list<Reflection> */
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
/** @return Generator<Reflection> */
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator
{
$cacheKey = sprintf('%s_%s', $this->reflectorCacheKey($reflector), $this->identifierTypeToCacheKey($identifierType));

if (array_key_exists($cacheKey, $this->cacheByIdentifierTypeKeyAndOid)) {
return $this->cacheByIdentifierTypeKeyAndOid[$cacheKey];
yield from $this->cacheByIdentifierTypeKeyAndOid[$cacheKey];

return;
}

$items = [];
foreach ($this->wrappedSourceLocator->locateIdentifiersByType($reflector, $identifierType) as $item) {
yield $items[] = $item;
Comment on lines +52 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original implementation caches items before returning them: the new implementation caches items after returning them.

I'm unsure about approach here, but it is a subtle change that can lead to uncached results if the consumer of this iterator crashes

}

return $this->cacheByIdentifierTypeKeyAndOid[$cacheKey]
= $this->wrappedSourceLocator->locateIdentifiersByType($reflector, $identifierType);
$this->cacheByIdentifierTypeKeyAndOid[$cacheKey] = $items;
}

private function reflectorCacheKey(Reflector $reflector): string
Expand Down
5 changes: 3 additions & 2 deletions src/SourceLocator/Type/SourceLocator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Roave\BetterReflection\SourceLocator\Type;

use Generator;
use Roave\BetterReflection\Identifier\Identifier;
use Roave\BetterReflection\Identifier\IdentifierType;
use Roave\BetterReflection\Reflection\Reflection;
Expand All @@ -26,7 +27,7 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
/**
* Find all identifiers of a type
*
* @return list<Reflection>
* @return Generator<Reflection>
*/
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array;
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): Generator;
Comment on lines +30 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move to iterable rather than Generator

}
Loading
Loading