Using generators to get a list of reflections

[shcherbanich]

Part of PR https://github.com/Roave/BetterReflection/pull/1475 related to using generators instead of arrays to get a list of reflections

What has been done?

1. locateIdentifiersByType now returns a generator to process each individual class sequentially

2. reflectAllClasses / reflectAllFunctions / reflectAllConstants methods of DefaultReflector class now return generators instead of arrays

3. FindReflectionOnLine now takes reflections one by one using generators

[Ocramius]

@asgrim @kukulich I think the BC break is worth it here: what do you think? Is it OK to move the source locators and API from list<T> to iterable<int, T>?

[asgrim]

@asgrim @kukulich I think the BC break is worth it here: what do you think? Is it OK to move the source locators and API from list<T> to iterable<int, T>?

Reasonable change yes; any impact to our own projects we can handle but definitely keen for @kukulich to check here; depending how phpstan consumes these APIs, may or may not be impactful

[kukulich]

@ondrejmirtes will this change be a problem for PHPStan?

[ondrejmirtes]

@kukulich Yes, it's a BC break. For example I have custom source locator wrappers which do this:

	public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
	{
		return $this->sourceLocator->locateIdentifiersByType($reflector, $identifierType);
	}

If these changes really lead to better performance (so that BR does not do work that would be thrown away), it might be a better idea to introduce new methods or interfaces that return generators, and keep the old classes and interfaces there.

After that we can keep the old methods that will just do iterator_to_array on the new methods. Optionally we could deprecated the "old" methods.

Unfortunately if these changes go through and BetterReflection releases a new major, I could not upgrade to it in PHPStan with a clear conscience until 3.0 because these source locators are part of public PHPStan API. I know that for example Rector depends on them and also creates its own implementations.

[shcherbanich]

@Ocramius @asgrim @kukulich Given all the complexities, let me go back to the first implementation, creating new methods (e.g. iterateIdentifiersByType / iterateAllClasses / ... ) and marking the old ones as deprecated, as noted in the comment above. This won't be difficult, and the change can be released safely as soon as possible.

[Ocramius]

@shcherbanich let it rest a bit before jumping at it: as you can see, discussion is underway, at least :D

I could not upgrade to it in PHPStan with a clear conscience until 3.0 because these source locators are part of public PHPStan API.

This is a real bummer: stuff like source locators in BetterReflection are marked internal for good reason (their API is squishy), so this is unfortunate.

That said, I fully understand the problem, and will think about it today/tomorrow, before we decide whether/how to proceed.

[ondrejmirtes]

Are they really internal? I'm also implementing my own and I'd say it's often useful for projects to write their own. So it might be better to un-internal them.