[Routing] Psr4DirectoryLoader issue with abstract classes

[MatTheCat]

Symfony version(s) affected

6.2-beta

Description

The new Psr4DirectoryLoader only supports attribute or annotation type so it will use the AnnotationClassLoader which don’t want to deal with abstract classes:

symfony/src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php

Lines 114 to 116 in b02a689

	if ($class->isAbstract()) {
	throw new \InvalidArgumentException(sprintf('Annotations from class "%s" cannot be read as it is abstract.', $class->getName()));
	}

(See 946ccb6 for rationale.)

If AnnotationDirectoryLoader skips them, Psr4DirectoryLoader does not meaning it will crash on abstract classes.

How to reproduce

Configure your routes to use the PSR-4 route loader, eg.

<?php

use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;

return static function (RoutingConfigurator $routes)
{
    $routes->import(['path' => '../src/Controller', 'namespace' => 'App\Controller'], 'annotation');
};

and make one of your controllers extends an abstract one (it does not need to define any method).

Refresh the cache to see the error.

Possible Solution

Psr4DirectoryLoader should skip abstract classes. As it means it will need to create an instance of ReflectionClass it could be useful to make AnnotationClassLoader::load accept these.

Additional Context

Spotted from #48161

[derrabus]

This is a very good catch. I indeed missed that case in my test suite. Thank you for working on a fix in #48169! However, I believe there's a simpler way to address the issue, see #48170.

[MatTheCat]

As every loader using AnnotationClassLoader will need to instantiate a ReflectionClass that means two are created for each controller; my PR addressed this point and the potentially wrong $type passed in

symfony/src/Symfony/Component/Routing/Loader/Psr4DirectoryLoader.php

Line 90 in b02a689

$collection->addCollection($this->import($className, 'attribute'));

Don’t really mind though, do what you think is best!

[derrabus]

As every loader using AnnotationClassLoader will need to instantiate a ReflectionClass that means two are created for each controller;

I believe we can live with that.

my PR addressed this point and the potentially wrong $type passed in

Why do you think a wrong $type is passed and what does it have to do with the issue of not being able to load from directories with abstract classes?

[MatTheCat]

Why do you think a wrong $type is passed

Because attribute is hardcoded in

symfony/src/Symfony/Component/Routing/Loader/Psr4DirectoryLoader.php

Line 90 in b02a689

$collection->addCollection($this->import($className, 'attribute'));

whereas $type could be annotation. When working on my PR this led to wrong error messages.

what does it have to do with the issue of not being able to load from directories with abstract classes?

Nothing, just spotted while working on it. It is such a small change I included it in this issue and the associated PR.

[derrabus]

Because attribute is hardcoded in […] whereas $type could be annotation.

Functionality-wise this does not make much of a difference. Both types are equivalent.

When working on my PR this led to wrong error messages.

Okay, for DX it's probably a good idea to pass the actual type then. Can you open a new PR just for that change? And please include a test that reproduces the issue with the wrong error message.

[MatTheCat]

Hm I cannot produce a failing test case so it must be linked to the changes I was making.
Sorry for the mixed issues 😅

[derrabus]

All right. Thanks for having a look and thank you very much for this bug report!