Repository: doctrine-extensions/DoctrineExtensions (language: php)

Issue:
The dependency on doctrine/annotations should become optional
## Feature Request

For projects that have migrated from annotations to attributes (as recommended by the Doctrine team), the `doctrine/annotations` package is unnecessary.
As the Doctrine team is planning to mark the `doctrine/annotations` package as abandoned (see https://github.com/doctrine/annotations/issues/485), it would be great if this package could stop being a mandatory dependency of the extensions.



Discussion / hints:
See #2554 as it has a more complete rundown of where the issues lie.
since this issue has been created, support for attributes has been implemented.
So for PHP 8+, attributes could be used to solve the issue:
- loggable and translatable provides mapping both using annotations and attributes now
- the helper methods could rely on those existing attributes to register an attribute driver instead of an annotations one
- ExtensionMetadataFactory could be creating a default attribute reader instead.

To me, the main blocker is actually the support for PHP 7.x. Maybe it is time to bump the min PHP version to 8.0 (or maybe 8.1 to be able to use enums in #2675 instead).

Note also that doctrine/orm 3.0 is dropping the annotation driver entirely, so you will have to do this migration to attributes before supporting ORM 3.0 (or making a conditional migration)
#2559 deals with the helpers.

As you pointed out, attributes exist now, and leaving the annotations in place won't hurt anything for the time being, so that one's adequately covered for now.

I think realistically the remaining hard dependencies are the non-nullable `$annotationReader` argument of the `ExtensionMetadataFactory` constructor (including the logic in `MappedEventSubscriber::getExtensionMetadataFactory()` which tries to create a default annotation reader to provide the factory if one doesn't exist) and inherently changing its `getDriver()` method to throw if it tries to use an annotation or attribute based driver without that dependency being passed in (as the way the method reads to me right now, if you're using a XML or YAML mapping driver, you should never reach the code paths where the reader would be required, so that's already a bit of an over-reaching constructor requirement).
Any update on this?  I don't think Symfony 7 will even support it.
Symfony won't have a native annotations reader but it won't block your project from upgrading.  If you're manually configuring services as your pre-edit comment suggests, you can use the `Gedmo\Mapping\Driver\AttributeReader` class in those `setAnnotationReader` method calls.
Is there an alias for that?  I can't seem to find it.  What should @annotation_reader be?

```yaml
services:
  gedmo.listener.timestampable:
    class: Gedmo\Timestampable\TimestampableListener
    tags:
      - { name: doctrine.event_subscriber, connection: default }
    calls:
      - [ setAnnotationReader, [ '@annotation_reader' ] ]

```
I'm hacking in services.yaml, but this is a bit of a mystery to me:

```yaml
    attr_reader: "@Gedmo\\Mapping\\Driver\\AttributeReader"
    Gedmo\Mapping\Driver\AttributeReader:
        alias: attribute_reader
```

Etc. 

It seems like this would be better as the default, and someone could set the annotation reader if they were using them.  There was a significant refactoring of this bundle a while ago to support attributes.  

I'm mostly trying to silence deprecation warnings, which are practically blocking the real issues.

This issue is that I'd like to get rid of the warning in 6.4 to turn off annotations in framework.yaml:

![image](https://github.com/doctrine-extensions/DoctrineExtensions/assets/619585/a1b87820-7dcb-4b82-9f79-744e4036ef81)


```yaml
framework:
    annotations: { enabled: false }
```


This package on its own doesn't provide a Symfony bundle, so that attribute reader won't be automatically set up in a Symfony app (if you're using [`StofDoctrineExtensionsBundle`](https://github.com/stof/StofDoctrineExtensionsBundle) you can check to see if that bundle's handling any of that already).  So defining a new service in your app for that class and updating your `setAnnotationReader` calls to use that is a step in helping to resolve that deprecation.

As for the Symfony deprecation, on top of updating your service configs for the classes from this package, you'll also need to check your other app services and third-party integrations to see if they have a hard requirement on the `doctrine/annotations` package in any way before you can turn off the framework's annotations integration.
My bundle does not yet handle that, because I just discovered the AttributeReader class in that discussion. But I will definitely handle that in the bundle directly ASAP.
@mbabker there is still one place with a hard dependency: `MappedEventSubscriber::getDefaultAnnotationReader` always creates an annotation reader and not the new attribute one (meaning that the dependency cannot become optional)
`MappedEventSubscriber::getDefaultAnnotationReader()` won't get called as long as you've called `MappedEventSubscriber::setAnnotationReader()` with an attribute reader first.  So even that shouldn't be a hard blocker to running a Symfony app without the `@annotation_reader` service, it's just the one spot that still forces `doctrine/annotations` to be installed.
@mbabker sure. but forcing the package to be installed is what will cause issues once the Doctrine team marks `doctrine/annotations` as abandoned as people will complain about getting a warning from Composer about the package being abandoned.
Supporting to run in a Symfony project that does not have the `annotation_reader` service defined is important to support Symfony 7. But this is not the original topic of this issue.
I'll PR this later, but https://github.com/doctrine-extensions/DoctrineExtensions/compare/main...mbabker:DoctrineExtensions:default-reader might be enough to solve that one, too (builds on the same logic as #2559 for deciding defaults).
@mbabker this is not the right approach IMO. This default reader will impact the kind of mapping that can be used in project-level entities for the Gedmo mapping. Passing an AttributeReader on PHP 8 would disable the support of annotations on PHP 8 by default, which is a BC break. When doctrine/annotations is installed, the default reader should support annotations. The ExtensionMetadataFactory will make sure to enable support for both annotations and attributes when the provided reader is an annotation reader.

This is different than the ORM mapping driver used for entities defined in the library. For those, we know that both attributes and annotations are provided and we can use either of them with the same result so preferring attributes on PHP 8 is fine.
Alternatively, we could make the default annotation reader nullable in the factory, letting it create an AttributeDriver when needed.
Providing the default annotation driver externally was important because of the cached reader to avoid reading the annotations several times. But AttributeDriver does not have this need (the parsing of attributes is done by PHP itself so there is no userland caching of it).
You would still have to default to an annotation reader in case the library exists, for BC reasons.
I can flip the order on the checks if that'll fix things.  Otherwise it turns into a much bigger review trying to make the reader nullable where it's currently not.  That was just a very quick idea to push up to see if it's even viable.
flipping the conditions should indeed be enough
#2713 to iterate on the fallback default behavior for the base subscriber class.
Unless I'm missing something, I think #2728 is the last PR needed before `doctrine/annotations` can be an optional production dependency.
May be, but `composer` still requires `doctrine/annotations`
https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/composer.json#L45
> May be, but `composer` still requires `doctrine/annotations`
> https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/composer.json#L45

Well aware.  It'll be updated in due time.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2a9702e5b..e61d11e37 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,9 @@ a release.
 - SoftDeleteable: `Gedmo\SoftDeleteable\Event\PreSoftDeleteEventArgs` and
   `Gedmo\SoftDeleteable\Event\PostSoftDeleteEventArgs` classes.
 
+### Changed
+- Make doctrine/annotations an optional dependency.
+
 ### Deprecated
 - Do not add type-hinted parameters `Gedmo\SoftDeleteable\Event\PreSoftDeleteEventArgs` and
   `Gedmo\SoftDeleteable\Event\PostSoftDeleteEventArgs` classes to `preSoftDelete` and `postSoftDelete` events.
diff --git a/composer.json b/composer.json
index 52c85a634..e122ec9cc 100644
--- a/composer.json
+++ b/composer.json
@@ -42,7 +42,6 @@
     "require": {
         "php": "^7.4 || ^8.0",
         "behat/transliterator": "^1.2",
-        "doctrine/annotations": "^1.13 || ^2.0",
         "doctrine/collections": "^1.2 || ^2.0",
         "doctrine/common": "^2.13 || ^3.0",
         "doctrine/event-manager": "^1.2 || ^2.0",
@@ -52,6 +51,7 @@
         "symfony/deprecation-contracts": "^2.1 || ^3.0"
     },
     "require-dev": {
+        "doctrine/annotations": "^1.13 || ^2.0",
         "doctrine/cache": "^1.11 || ^2.0",
         "doctrine/dbal": "^3.2",
         "doctrine/doctrine-bundle": "^2.3",
@@ -69,6 +69,7 @@
         "symfony/yaml": "^5.4 || ^6.0 || ^7.0"
     },
     "conflict": {
+        "doctrine/annotations": "<1.13 || >=3.0",
         "doctrine/dbal": "<3.2",
         "doctrine/mongodb-odm": "<2.3",
         "doctrine/orm": "<2.14.0 || 2.16.0 || 2.16.1",


Test patch: