The doctrine/annotations dependency should be optional

[stof]

Currently, the package has a mandatory dependency on doctrine/annotations.

As the Doctrine team is planning to mark that package as abandoned (see doctrine/annotations#485) by deprecating it in favor of using PHP attributes in projects instead, it would be great if php-cs-fixer could avoid bringing it as a dependency in all projects using the tool.

A solution could be to add a check for the presence of doctrine/annotations in the fixers relying on it, relying on projects to provide it. Anyway, a project has no reason to enable the annotation fixers if they don't use annotations.

If this is done, the annotation fixers should also be removed from rule sets, providing a dedicated annotation rule set instead, to make it easier to configure projects without annotations.

[Wirone]

Thanks @stof for raising this.

If I see correctly, then doctrine/annotations package is used only in @DoctrineAnnotation ruleset. Technically, Doctrine\Common\Annotations\DocLexer is used in PhpCsFixer\Doctrine\Annotation\Tokens::createFromDocComment() which in theory is a part of public API, but Tokens is marked as @internal so theoretically we could drop it without BC break. That method is only used in AbstractDoctrineAnnotationFixer, so if we move it away from Tokens, we're set to drop that dependency.

A solution could be to add a check for the presence of doctrine/annotations in the fixers relying on it

Maybe these fixers should have such check in the implementation of FixerInterface::supports() or FixerInterface::isCandidate()? So even if configured, but package was not available, they wouldn't apply fix (or would throw an exception?). I believe it could be done in AbstractDoctrineAnnotationFixer since all 4 fixers extend it.

If this is done, the annotation fixers should also be removed from rule sets, providing a dedicated annotation rule set instead, to make it easier to configure projects without annotations.

I don't know if we can remove these fixers just like that from the rule sets, because it would be a BC break. The good thing is these fixers are only part of @DoctrineAnnotation rule set, so they can still be a part of it, but we could mark that rule set as deprecated. Considering check for the package existence, annotation-based fixers would still work if people had package installed (but by their own, not as a Fixer's dependency).

I don't know if dedicated fixers for attributes are required, I believe universal fixers for attributes would be enough. But correct me if I'm wrong 😉.

[stof]

If they are only part of the @DoctrineAnnotation rule set, it means that my suggestion of having a dedicated rule set is actually already implemented.

regarding the PhpCsFixer\Doctrine\Annotation\Tokens class, it is an internal class used only for AbstractDoctrineAnnotationFixer. I don't see any need to remove it.

[Wirone]

You're right, somehow I thought it's the main PhpCsFixer\Tokenizer\Tokens 😅. That makes it even easier, we could add the check directly to PhpCsFixer\Doctrine\Annotation\Tokens::createFromDocComment() and fail the fixing process if dependency is not met. We should just:

• move doctrine/annotations to require-dev, so tests pass for these fixers just like it's now
• handle missing package somehow and print proper message to the user (or maybe exception with good message would be enough)

Maybe adding this package to suggests would be good too, with proper information about the need of having it if @DoctrineAnnotations rule set is used.

[stof]

I think having a clear error message could be enough there.
Or maybe we need an exception during configuration loading instead of during fixing to fail early (the createFromDocComment method should still implement the exception during fixing in case some project reuses fixers without going through the configuration check)