Loading files through autoloading - a possible solution

[oprypkhantc]

Hey @alekitto.

We stumbled upon your lib and wanted to use it to auto-discover classes in graphqlite. It generally works as expected, but isn't using autoloading. I understand why it's not using autoloading, but there are cases where autoloading is required for the class to function properly.

For example, we have a pre-8.0 enum implementation that uses a custom autoloader to call ::initialize() on enum classes being loaded, which initializes static properties of an enum.

So I dug a little bit deeper to see if there's anything that can be done on your end without Composer actually fixing composer/composer#6987.

What I thought was class-finder could check if the file that it's trying to load is one of the files from files section in composer.json. That's relatively trivial to do, because composer generates a vendor/composer/autoload_files.php file that just returns a list of all files to be loaded. Something like this:

$vendorDir = dirname(
    (new ReflectionClass(\Composer\Autoload\ClassLoader::class))->getFilename()
);
$autoloadedFiles = array_flip(
    require $vendorDir . '/autoload_files.php'
);

Then, when actually trying to load a class, instead of blindly using include_once, it could rely on autoloading to do so, given we first check if the file is contained in $autoloadedFiles:

if (isset($autoloadedFiles[$path]) {
    continue;
}

ErrorHandler::register();
try {
    if (!$this->exists($class)) {
        continue;
    } 
} catch (Throwable) { /** @phpstan-ignore-line */
    continue;
} finally {
    ErrorHandler::unregister();
}

yield $class => $this->reflectorFactory->reflect($class);

I understand this feels hacky. However, this solves a real world problem for us, and possibly more for others that are related to not using autoloading; and to be fair, this entire package is just a big hack around PHP's weird autoloading state.

I can PR these changes if that's something you'd accept.

[alekitto]

Hi @oprypkhantc!
Thanks for your explanation and research for a solution; I think this is a very good idea, and I'm sure that can be implemented in the iterators, but I also need the current behaviour to be preserved in some way.

I'm currently thinking about a flag to enable/disable the use of the autoloader instead of the include_once call. Will this be enough for your use case?

Eg:

$finder = (new ComposerFinder())
  ->useAutoloading();
foreach ($finder as $className => $reflector) {
  // ...
}

[oprypkhantc]

Sure, that should work. That flag can also push some kind of ExceptComposerAutoloadFilesIterator to the list to exclude these files from the start, and the flag would only control whether the try { $include(); } catch {} block needs to be executed or just skipped.

I'll PR the changes, thanks :)

[alekitto]

Sure, that should work. That flag can also push some kind of ExceptComposerAutoloadFilesIterator to the list to exclude these files from the start

Exactly!

and the flag would only control whether the try { $include(); } catch {} block needs to be executed or just skipped.

Probably the $include closure should be changed to trigger the autoloader. The exists method only does simple checks and does not invoke the autoloaders.

I'll PR the changes, thanks :)

Great! Thanks, I'll review it ASAP.

[alekitto]

Hi @oprypkhantc,
I've implemented the solution per your suggestions, and exposed useAutoloading and skipNonInstantiable methods in ComposerFinder. I'll write some tests tomorrow, but if you want to try the master version, I will be glad to listen your feedback.

[oprypkhantc]

That's awesome @alekitto. Thank you, we'll definitely try that in the meantime. Sorry for not submitting a PR myself - didn't have time to do so :)

[alekitto]

@oprypkhantc now the finder uses autoloading by default.
I'll wait for your feedback on the master version to tag a new release.

[oprypkhantc]

@alekitto I've tested it on the dev-master in our project and it works exactly as anticipated :) Thank you!