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.

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

Autoloading annotation classes #103

Closed
richard-ejem opened this Issue Nov 3, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@richard-ejem
Copy link

richard-ejem commented Nov 3, 2016

Why aren't annotation classes loaded by currently configured PHP autoload mechanisms?

It is called neither in https://github.com/doctrine/annotations/blob/master/lib/Doctrine/Common/Annotations/AnnotationRegistry.php#L123
nor in https://github.com/doctrine/annotations/blob/master/lib/Doctrine/Common/Annotations/DocParser.php#L457 (implicit autoload is explicitly turned off using class_exists($className, false) ).

It leads to this weird behavior:

spl_autoload_register(function() { eval('/** @Annotation */ class MyAnnotation {}'); });
$docParser = new DocParser();

// uncomment the following line to change the result
// class_exists('MyAnnotation');
try {
    $docParser->parse('/** @MyAnnotation */');
    echo "MyAnnotation exists";
} catch (AnnotationException $ex) {
    echo "MyAnnotation does not exist";
}

My suggestion would be to let the DocParser try to autoload the class.

P.S. I know that I can bypass it by adding custom loader to AnnotationRegistry that just executes class_exists(), but isn't it unnecessary (and non-intuitive)?

@stof

This comment has been minimized.

Copy link
Member

stof commented Nov 3, 2016

they can be, with a simple trick:

AnnotationRegistry::registerLoader('class_exists');

This works fine as long as your autoloaders are not breaking class_exists, i.e. they are not throwing an error when they fail to find a class.

The AnnotationRegistry system exists precisely because several big frameworks were failing this requirement in their autoloader at the time Doctrine Common 2.1 was written (including Doctrine Common itself and Zend Framework 1).
In the composer era, this is not an issue anymore generally.

@richard-ejem

This comment has been minimized.

Copy link

richard-ejem commented Nov 3, 2016

@stof Yes as I explained in P.S., I figured out this solution too. It works, but is one of potentially many things you have to figure out why they aren't working by default when experimenting with low-level configuration.
Wouldn't it be better to make calling the autoloader a default behavior and make use of AnnotationRegistry opt-in? Because in these days AFAIK major frameworks don't have this issue anymore (as it actually also breaks the class_exists() itself).

@stof

This comment has been minimized.

Copy link
Member

stof commented Nov 3, 2016

@richard-ejem changing it is planned for Doctrine 3. Changing it in 2.x might cause issues for some cases.

@Ocramius

This comment has been minimized.

Copy link
Member

Ocramius commented Feb 1, 2017

Cross-linking #61

@Majkl578

This comment has been minimized.

Copy link
Member

Majkl578 commented May 10, 2018

Handled in #205.

@Majkl578 Majkl578 closed this May 10, 2018

@Majkl578 Majkl578 self-assigned this May 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment